Re: [libvirt] [PATCH] network: restrict usage of port management APIs

On 08/09/2018 03:58 AM, Daniel P. Berrangé wrote: > On Wed, Aug 08, 2018 at 11:41:23PM -0400, Laine Stump wrote: >> On 08/08/2018 11:46 AM, Daniel P. Berrangé wrote: >>> The port allocation APIs are currently called unconditionally for all >>> types of NIC, but (mostly) only do anything for NICs with type=network. >>> >>> The exception is the port allocate API which does some validation even >>> for NICs with type!=network. Relying on this validation is flawed, >>> however, since the network driver may not even be installed, so virt >>> drivers must not delegation validation to it for NICs with >>> type!=network. >> Although I never thought through all the minute details to the end (and >> didn't need to because ,,,AllocateActualDevice() wasn't in a public >> API), I had always figured that these calls into the network driver >> would be where, eventually, we would do all of the plumbing for a >> network device, like creating the tap/macvtap device during startup, and >> disconnecting/deleting devices during domain shutdown. (it also kind of >> makes sense for nwfilters to be added/removed by the network driver >> during these APIs, since the filterref is in the <interface> definition). >> >> That's the reason for the unconditional calls. >> >> (one of the "minute details" that I hadn't thought about was the fact >> that we probably can't do *all* of the plumbing at one time - at the >> very least we need to have one API call for creating the devices and >> hooking them up, and another call that happens right before the guest >> CPUs are started (to bring everything online). Still, if we limit the >> netAllocate API to only being called for type='network' then we >> definitely will need an additional API that will likely be called >> unconditionally just after netAllocation is called just for type='network'.) > Yep, this is a bigger problem than I first considered. We have four > hypervisors using tap devices (qemu, lxc, libxl & bhyve) and all of > them have different requirements for the way the tap devices are > created. This is going to make it hard to delegate everything to the > network driver, as the work is hypervisor specific. The differences between qemu and lxc were on my mind but, without looking, I had naively assumed that "all the others" (including libxl and bhve) were just doing something similar to qemu :-/

>> BTW, once it is the responsibility of the network driver to create tap >> devices and connect them to bridges, the network driver will no longer >> be optional (unless we want to duplicate the code in the hypervisor >> drivers), but that is the price we have to pay in order to give >> unprivileged libvirt the ability to use any type of network device. > Even if the network driver can create tap devices, I'd still want to > deal with type=network vs type=bridge separately for access control > purposes. It is desirable to be able to have ACLs on the opening operation > and the natural place to attach the ACL is against the virNetworkPtr > object. We have no object to represent standalone bridge devices outside > of a virNetworkPtr, so nothing to attach an ACL to for creating tap > devices. So are you saying that the network driver simply wouldn't be able to create the tap devices for type=bridge (or type=direct)? I'm not exactly following you (although I don't doubt what you say).