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

Laine Stump laine at laine.org
Fri Aug 12 15:19:00 UTC 2016

On 08/12/2016 03:52 AM, Daniel P. Berrange wrote:
> On Thu, Aug 11, 2016 at 10:41:45PM -0400, Laine Stump wrote:
>> The new forward mode 'open' is just like mode='route', except that no
>> firewall rules are added to assure that any traffic does or doesn't
>> pass. It is assumed that either they aren't necessary, or they will be
>> setup outside the scope of libvirt.
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=846810
>> ---
>>   docs/formatnetwork.html.in                         | 22 ++++++++++++
>>   docs/schemas/network.rng                           |  1 +
>>   src/conf/network_conf.c                            | 25 +++++++++++--
>>   src/conf/network_conf.h                            |  1 +
>>   src/network/bridge_driver.c                        | 41 +++++++++++++++-------
>>   tests/networkxml2confdata/open-network.conf        | 11 ++++++
>>   tests/networkxml2confdata/open-network.xml         |  9 +++++
>>   tests/networkxml2conftest.c                        |  1 +
>>   .../open-network-with-forward-dev.xml              |  9 +++++
>>   tests/networkxml2xmlin/open-network.xml            |  9 +++++
>>   tests/networkxml2xmlout/open-network.xml           |  9 +++++
>>   tests/networkxml2xmltest.c                         |  2 ++
>>   12 files changed, 125 insertions(+), 15 deletions(-)
>>   create mode 100644 tests/networkxml2confdata/open-network.conf
>>   create mode 100644 tests/networkxml2confdata/open-network.xml
>>   create mode 100644 tests/networkxml2xmlin/open-network-with-forward-dev.xml
>>   create mode 100644 tests/networkxml2xmlin/open-network.xml
>>   create mode 100644 tests/networkxml2xmlout/open-network.xml
>> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
>> index a9226e5..12d1bed 100644
>> --- a/docs/formatnetwork.html.in
>> +++ b/docs/formatnetwork.html.in
>> @@ -260,6 +260,28 @@
>>               <span class="since">Since 0.4.2</span>
>>             </dd>
>> +          <dt><code>open</code></dt>
>> +          <dd>
>> +            As with mode='route', guest network traffic will be
>> +            forwarded to the physical network via the host's IP
>> +            routing stack, but there will be no firewall rules added
>> +            to either enable or prevent any of this traffic. When
>> +            forward='open' is set, the <code>dev</code> attribute
>> +            cannot be set (because the forward dev is enforced with
>> +            firewall rules, and the purpose of forward='open' is to
>> +            have a forwarding mode where libvirt doesn't add any
>> +            firewall rules).  This mode presumes that the local LAN
>> +            router has suitable routing table entries to return
>> +            traffic to this host, and that some other management
>> +            system has been used to put in place any necessary
>> +            firewall rules. Although no firewall rules will be added
>> +            for the network, it is of course still possible to add
>> +            restrictions for specific guests using
>> +            <a href="formatnwfilter.html">nwfilter rules</a> on the
>> +            guests' interfaces.)
>> +            <span class="since">Since 2.2.0</span>
>> +          </dd>
>> +
> Isn't this basically the same as forward mode="bridge", except that
> we still create the bridge ourselves, instead of requiring it to be
> pre-created ?

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.)

> If so, I wonder if its better add a attribute 'create=yes|no' to
> the <bridge> element instead ?

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)

More information about the libvir-list mailing list