Re: [libvirt] [PATCH 1/3] network: new network forward mode 'open'

On 08/12/2016 03:52 AM, Daniel P. Berrange wrote: Sigh. If only that was the case :-/ (Seriously, I wish we had the last 5 years' experiences as a guide when we discussed the addition of forward mode='bridge' (and vepa and private) back in 2011. <forward mode='bridge'> can mean one of two types of connections, depending on a couple other settings: 1) if there is a <bridge> element giving a device name, then guests are connected to the named bridge (which must already exist) with a tap device. No dnsmasq process is started, and no iptables rules are added. <ip> elements are forbidden. 2) if there is no <bridge> element, then guests are connected to the physical device named in <forward dev='xyz'/> using macvtap bridge mode. Again, no dnsmasq, no iptables, and <ip> elements are forbidden. At the time I'd suggested adding a higher level "type" attribute to the network to support these very different types, but was discouraged from that because existing management applications would be confused by these new networks that appeared to be identical to existing networks (because the management app was unaware of the newly added "type" attribute). The forward mode was chosen because it was an existing attribute that all management apps already checked; the idea was that at the very least they would see an unrecognized value and claim ignorance, and at best they might display the bridge name or forwarding device name, giving some sort of clue about the network. (This morning I had been feeling guilty about creating such a complicated mess, but then I looked back at the design discussions on the mailing list (it went through several versions before I wrote any code) and see that it really was a group fail.. er, effort :-P.) There would still be the issue of dns/dhcp servers - default (and only) behavior for mode='bridge' is to never set these up, but the aim of this patch is provide a way to turn off iptables rules without affecting anything else. (Hmm, but if no IP addresses are given, which is *always* the case for the existing bridge modes, then it's not possible to create a dns server anyway) Also there is the problem that a network with mode='nat|route' auto-generates a bridge device name if none is given, but mode='bridge' with no bridge name given is taken to mean "use macvtap bridge mode". Of course that mode requires that there be a forward dev manually specified, so maybe a further complication of the rules for identification could do it... Let's see... If there is <forward mode='bridge'> and: 1) if there are any <ip> elements in the forward: then we assume that it is a libvirt-managed network (i.e. exactly what this patch does for <forward mode='open'/> - bridge device created/destroyed, dnsmasq run as needed, but no iptables rules)\> 2) if no <ip> elements, and <bridge create='yes'> (the first time it's started bridge name may or may not be specified) same as 1 (except that there couldn't be any dnsmasq due to lack of <ip>.) 3) if no <ip> elements, and <bridge name='xyz'> is defined: then it uses an existing bridge, no iptables, no dnsmasq (what is currently done) 4) <forward mode='bridge' dev='xyz'/>, no <bridge> element: macvtap bridge mode Okay. I think that is unambiguous (although a bit complicated, but it already was complicated anyway). Unless my description makes you nauseous, I'll redo the patch that way and see how ugly it is. Two questions though: 1) Currently if someone has <forward mode='bridge'>, <bridge name='xyz'/>, and tries to add any <ip> elements, that is an error; should we continue to make this an error unless they also specify <bridge create='yes'>? Or should we assume/automatically add create='yes' in that case? 2) should we begin automatically adding "create='yes'" to the <bridge> element of <forward mode='nat|route'> networks? Or leave them as-is? (internally, it would be nice to have the create attribute there, as it makes a lot of decisions easier to code; you could argue it's redundant in almost all cases though)