Re: [PATCHv2 3/9] conf: new network bridge device attribute fdb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/02/2014 12:08 PM, Laine Stump wrote:
> The fdb attribute of a network's bridge subelement tells libvirt how
> the bridge's forwarding database (fdb) is managed. In the default
> mode, "learningWithFlood", management of fdb entries is left to the
> kernel, which determines entries in part by turning on promiscuous
> mode on all ports of the bridge, flooding packets to all ports when
> the correct destination is unknown, and adding/removing entries to the
> fdb as it sees incoming traffic from particular MAC addresses.  In
> "managed" mode, libvirt turns off learning and floowing on all the

s/floowing/flooding

unless of course it was COW's involved in the flood, then perhaps
floowing is more apropos ;-)

> bridge ports connected to guest domain interfaces, and adds/removes
> fdb entries according to the MAC addresses in the domain interface
> configurations. A side effect of turning off learning and
> unicast_flood on the ports of a bridge is that (with Linux kernel 3.17
> and newer), the kernel can automatically turn off promiscuous mode one
> or more of the bridge's ports (usually only the one interface that is
> used to connect the bridge to the physical network). The result is
> better performance (because packets aren't being flooded to all ports,
> and can be dropped earlier when they are of no interest) and better
> security (since a guest will only receive traffic intended for the
> guest interface's configured MAC address).
> 
> The attribute looks like this in the configuration:
> 
>   <network>
>     <name>test</name>
>     <bridge name='br0' fdb='managed'/>
>     ...
> 
> This patch only adds the config knob, documentation, and test
> cases. The functionality behind this knob is added in later patches.
> ---
> 
> Changes from V1: Changed name and values of attribute, as outlined in
> the patchset cover letter.
> 
>  docs/formatnetwork.html.in                         | 42 +++++++++++++++---
>  docs/schemas/network.rng                           |  9 ++++
>  src/conf/network_conf.c                            | 50 +++++++++++++++++-----
>  src/conf/network_conf.h                            | 11 +++++
>  src/libvirt_private.syms                           |  2 +
>  tests/networkxml2xmlin/host-bridge-no-flood.xml    |  6 +++
>  .../nat-network-explicit-flood.xml                 | 21 +++++++++
>  tests/networkxml2xmlout/host-bridge-no-flood.xml   |  6 +++
>  .../nat-network-explicit-flood.xml                 | 23 ++++++++++
>  tests/networkxml2xmltest.c                         |  2 +
>  10 files changed, 155 insertions(+), 17 deletions(-)
>  create mode 100644 tests/networkxml2xmlin/host-bridge-no-flood.xml
>  create mode 100644 tests/networkxml2xmlin/nat-network-explicit-flood.xml
>  create mode 100644 tests/networkxml2xmlout/host-bridge-no-flood.xml
>  create mode 100644 tests/networkxml2xmlout/nat-network-explicit-flood.xml
> 

ACK - although there's a grammar nit for the html page and a couple of
naming comments (go with your gut on resolution - the names seem fine
just thinking consistency between *_AUTO and learningWithFlood and using
"none"/NONE instead of "absent"/ABSENT).

John

> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
> index dc438ae..e624e1d 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in
> @@ -81,7 +81,7 @@
>  
>      <pre>
>          ...
> -        &lt;bridge name="virbr0" stp="on" delay="5"/&gt;
> +        &lt;bridge name="virbr0" stp="on" delay="5" fdb="managed"/&gt;
>          &lt;domain name="example.com"/&gt;
>          &lt;forward mode="nat" dev="eth0"/&gt;
>          ...</pre>
> @@ -92,18 +92,48 @@
>          defines the name of a bridge device which will be used to construct
>          the virtual network. The virtual machines will be connected to this
>          bridge device allowing them to talk to each other. The bridge device
> -        may also be connected to the LAN. It is recommended that bridge
> -        device names started with the prefix <code>vir</code>, but the name
> -        <code>virbr0</code> is reserved for the "default" virtual
> -        network.  This element should always be provided when defining
> +        may also be connected to the LAN. When defining
>          a new network with a <code>&lt;forward&gt;</code> mode of
> +
>          "nat" or "route" (or an isolated network with
> -        no <code>&lt;forward&gt;</code> element).
> +        no <code>&lt;forward&gt;</code> element), libvirt will
> +        automatically generate a unique name for the bridge device if
> +        none is given, and this name will be permanently stored in the
> +        network configuration so that that the same name will be used
> +        every time the network is started. For these types of networks
> +        (nat, routed, and isolated), a bridge name beginning with the
> +        prefix "virbr" is recommended (and that is what is
> +        auto-generated), but not enforced.
>          Attribute <code>stp</code> specifies if Spanning Tree Protocol
>          is 'on' or 'off' (default is
>          'on'). Attribute <code>delay</code> sets the bridge's forward
>          delay value in seconds (default is 0).
>          <span class="since">Since 0.3.0</span>
> +
> +        <p>
> +          The <code>fdb</code> attribute of the bridge element is used

s/The /The optional /

> +          to tell libvirt how the bridge's forwarding database (fdb)
> +          will be updated. In the default

s/In the default/If not provided or if using the/

(or something like that to indicate the default action)

> +          mode <code>learningWithFlood</code>, the kernel
> +          automatically adds and removes entries, using bridge
> +          learning, flooding, and promiscuous mode on the bridge's
> +          ports in order to determine the proper destination for
> +          packets.  In <code>managed</code> mode, libvirt disables
> +          learning and unicast_flooding on all bridge ports connected
> +          to guest domain interfaces, and explicitly adds entries to
> +          the fdb based on the MAC addresses in the domain interface
> +          configurations. Turning off learning and unicast_flood can
> +          also permit the kernel to turn off promiscuous mode on some
> +          ports of the bridge, and the combination of turning off
> +          these three settings can improve performance and provide
> +          better security, but can also cause some networking setups
> +          to stop working (e.g. vlan tagging, multicast) and is not
> +          supported by older kernels.
> +          <span class="since">Since 1.2.11, requires kernel 3.17 or
> +          newer</span>
> +        </p>
> +
> +
>        </dd>
>        <dt><code>domain</code></dt>
>        <dd>
> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
> index 4546f80..cc0095b 100644
> --- a/docs/schemas/network.rng
> +++ b/docs/schemas/network.rng
> @@ -65,6 +65,15 @@
>                </attribute>
>              </optional>
>  
> +            <optional>
> +              <attribute name="fdb">
> +                <choice>
> +                  <value>learningWithFlood</value>

I actually like your other name "auto", but this is fine, too.

> +                  <value>managed</value>
> +                </choice>
> +              </attribute>
> +            </optional>
> +
>            </element>
>          </optional>
>  
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index a249e32..4c5809d 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -55,6 +55,10 @@ VIR_ENUM_IMPL(virNetworkForward,
>                VIR_NETWORK_FORWARD_LAST,
>                "none", "nat", "route", "bridge", "private", "vepa", "passthrough", "hostdev")
>  
> +VIR_ENUM_IMPL(virNetworkBridgeFDB,
> +              VIR_NETWORK_BRIDGE_FDB_LAST,
> +              "absent", "learningWithFlood", "managed")

"none" is more common than "absent"...

> +
>  VIR_ENUM_DECL(virNetworkForwardHostdevDevice)
>  VIR_ENUM_IMPL(virNetworkForwardHostdevDevice,
>                VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_LAST,
> @@ -2108,6 +2112,17 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>      }
>      VIR_FREE(tmp);
>  
> +    tmp = virXPathString("string(./bridge[1]/@fdb)", ctxt);
> +    if (tmp) {
> +        if ((def->fdb = virNetworkBridgeFDBTypeFromString(tmp)) <= 0) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("Invalid fdb setting '%s' "
> +                             "in network '%s'"), tmp, def->name);
> +            goto error;
> +        }
> +        VIR_FREE(tmp);
> +    }
> +
>      tmp = virXPathString("string(./mac[1]/@address)", ctxt);
>      if (tmp) {
>          if (virMacAddrParse(tmp, &def->mac) < 0) {
> @@ -2290,6 +2305,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>                             def->name);
>              goto error;
>          }
> +        if (def->fdb) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("bridge fdb setting not allowed "
> +                             "in %s mode (network '%s')"),
> +                           virNetworkForwardTypeToString(def->forward.type),
> +                           def->name);
> +            goto error;
> +        }
>          /* fall through to next case */
>      case VIR_NETWORK_FORWARD_BRIDGE:
>          if (def->delay || stp) {
> @@ -2783,22 +2806,27 @@ virNetworkDefFormatBuf(virBufferPtr buf,
>              virBufferAddLit(buf, "</forward>\n");
>      }
>  
> +
>      if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
> -         def->forward.type == VIR_NETWORK_FORWARD_NAT ||
> -         def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
> +        def->forward.type == VIR_NETWORK_FORWARD_NAT ||
> +        def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
> +        def->bridge || def->fdb) {
>  
>          virBufferAddLit(buf, "<bridge");
> -        if (def->bridge)
> -            virBufferEscapeString(buf, " name='%s'", def->bridge);
> -        virBufferAsprintf(buf, " stp='%s' delay='%ld'/>\n",
> -                          def->stp ? "on" : "off",
> -                          def->delay);
> -    } else if (def->forward.type == VIR_NETWORK_FORWARD_BRIDGE &&
> -               def->bridge) {
> -        virBufferEscapeString(buf, "<bridge name='%s'/>\n", def->bridge);
> +        virBufferEscapeString(buf, " name='%s'", def->bridge);
> +        if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
> +            def->forward.type == VIR_NETWORK_FORWARD_NAT ||
> +            def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
> +            virBufferAsprintf(buf, " stp='%s' delay='%ld'",
> +                              def->stp ? "on" : "off", def->delay);
> +        }
> +        if (def->fdb) {
> +            virBufferAsprintf(buf, " fdb='%s'",
> +                             virNetworkBridgeFDBTypeToString(def->fdb));
> +        }
> +        virBufferAddLit(buf, "/>\n");
>      }
>  
> -
>      if (def->mac_specified) {
>          char macaddr[VIR_MAC_STRING_BUFLEN];
>          virMacAddrFormat(&def->mac, macaddr);
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index 660cd2d..cf9b743 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -54,6 +54,16 @@ typedef enum {
>  } virNetworkForwardType;
>  
>  typedef enum {
> +   VIR_NETWORK_BRIDGE_FDB_ABSENT = 0,

again NONE is more common...

> +   VIR_NETWORK_BRIDGE_FDB_AUTO,

Since it's really never used otherwise - LEARNING_WITH_FLOOD keeps names
in synch rather than AUTO

> +   VIR_NETWORK_BRIDGE_FDB_MANAGED,
> +
> +   VIR_NETWORK_BRIDGE_FDB_LAST,
> +} virNetworkBridgeFDBType;
> +
> +VIR_ENUM_DECL(virNetworkBridgeFDB)
> +
> +typedef enum {
>      VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NONE = 0,
>      VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI,
>      VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV,
> @@ -231,6 +241,7 @@ struct _virNetworkDef {
>      int   connections; /* # of guest interfaces connected to this network */
>  
>      char *bridge;       /* Name of bridge device */
> +    int  fdb; /* enum virNetworkBridgeFDBType - default is learningWithFlood */
>      char *domain;
>      unsigned long delay;   /* Bridge forward delay (ms) */
>      bool stp; /* Spanning tree protocol */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 6b6c51b..703cba8 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -529,6 +529,8 @@ virNetDevVPortTypeToString;
>  
>  # conf/network_conf.h
>  virNetworkAssignDef;
> +virNetworkBridgeFDBTypeFromString;
> +virNetworkBridgeFDBTypeToString;
>  virNetworkConfigChangeSetup;
>  virNetworkConfigFile;
>  virNetworkDefCopy;
> diff --git a/tests/networkxml2xmlin/host-bridge-no-flood.xml b/tests/networkxml2xmlin/host-bridge-no-flood.xml
> new file mode 100644
> index 0000000..d5fced0
> --- /dev/null
> +++ b/tests/networkxml2xmlin/host-bridge-no-flood.xml
> @@ -0,0 +1,6 @@
> +<network>
> +  <name>host-bridge-net</name>
> +  <uuid>81ff0d90-c91e-6742-64da-4a736edb9a8e</uuid>
> +  <forward mode="bridge"/>
> +  <bridge name="br0" fdb='managed'/>
> +</network>
> diff --git a/tests/networkxml2xmlin/nat-network-explicit-flood.xml b/tests/networkxml2xmlin/nat-network-explicit-flood.xml
> new file mode 100644
> index 0000000..d6fa1b7
> --- /dev/null
> +++ b/tests/networkxml2xmlin/nat-network-explicit-flood.xml
> @@ -0,0 +1,21 @@
> +<network>
> +  <name>default</name>
> +  <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> +  <bridge name="virbr0" fdb="learningWithFlood"/>
> +  <forward mode="nat" dev="eth1"/>
> +  <ip address="192.168.122.1" netmask="255.255.255.0">
> +    <dhcp>
> +      <range start="192.168.122.2" end="192.168.122.254"/>
> +      <host mac="00:16:3e:77:e2:ed" name="a.example.com" ip="192.168.122.10"/>
> +      <host mac="00:16:3e:3e:a9:1a" name="b.example.com" ip="192.168.122.11"/>
> +    </dhcp>
> +  </ip>
> +  <ip family="ipv4" address="192.168.123.1" netmask="255.255.255.0">
> +  </ip>
> +  <ip family="ipv6" address="2001:db8:ac10:fe01::1" prefix="64">
> +  </ip>
> +  <ip family="ipv6" address="2001:db8:ac10:fd01::1" prefix="64">
> +  </ip>
> +  <ip family="ipv4" address="10.24.10.1">
> +  </ip>
> +</network>
> diff --git a/tests/networkxml2xmlout/host-bridge-no-flood.xml b/tests/networkxml2xmlout/host-bridge-no-flood.xml
> new file mode 100644
> index 0000000..ff07943
> --- /dev/null
> +++ b/tests/networkxml2xmlout/host-bridge-no-flood.xml
> @@ -0,0 +1,6 @@
> +<network>
> +  <name>host-bridge-net</name>
> +  <uuid>81ff0d90-c91e-6742-64da-4a736edb9a8e</uuid>
> +  <forward mode='bridge'/>
> +  <bridge name='br0' fdb='managed'/>
> +</network>
> diff --git a/tests/networkxml2xmlout/nat-network-explicit-flood.xml b/tests/networkxml2xmlout/nat-network-explicit-flood.xml
> new file mode 100644
> index 0000000..340f6bb
> --- /dev/null
> +++ b/tests/networkxml2xmlout/nat-network-explicit-flood.xml
> @@ -0,0 +1,23 @@
> +<network>
> +  <name>default</name>
> +  <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> +  <forward dev='eth1' mode='nat'>
> +    <interface dev='eth1'/>
> +  </forward>
> +  <bridge name='virbr0' stp='on' delay='0' fdb='learningWithFlood'/>
> +  <ip address='192.168.122.1' netmask='255.255.255.0'>
> +    <dhcp>
> +      <range start='192.168.122.2' end='192.168.122.254'/>
> +      <host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10'/>
> +      <host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11'/>
> +    </dhcp>
> +  </ip>
> +  <ip family='ipv4' address='192.168.123.1' netmask='255.255.255.0'>
> +  </ip>
> +  <ip family='ipv6' address='2001:db8:ac10:fe01::1' prefix='64'>
> +  </ip>
> +  <ip family='ipv6' address='2001:db8:ac10:fd01::1' prefix='64'>
> +  </ip>
> +  <ip family='ipv4' address='10.24.10.1'>
> +  </ip>
> +</network>
> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c
> index 65ac591..34a5211 100644
> --- a/tests/networkxml2xmltest.c
> +++ b/tests/networkxml2xmltest.c
> @@ -120,6 +120,8 @@ mymain(void)
>      DO_TEST("hostdev");
>      DO_TEST_FULL("hostdev-pf", VIR_NETWORK_XML_INACTIVE);
>      DO_TEST("passthrough-address-crash");
> +    DO_TEST("nat-network-explicit-flood");
> +    DO_TEST("host-bridge-no-flood");
>  
>      return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>  }
> 

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]