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)


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to