[netcf-devel] [PATCH 4/4] Allow enslaving a bond to a bridge

David Lutterkort lutter at redhat.com
Tue Sep 15 23:44:39 UTC 2009


  * src/drv_initscripts.c (drv_xml_desc, rm_interface): also match bonds
    inside bridges; (drv_define, drv_undefine): make sure we alias/unalias
    bonds inside bridges in modprobe config
  * data/xml/interface.rng: allow adding a bond to a bridge; split bonding
    schema to differentiate between nested bond in a bridge and a toplevel
    bond
  * data/xml/initscripts-get.xsl: produce ifcfg for bonds inside a bridge
  * data/xml/initscripts-put.xsl: transform a bond with its slaves inside a
    bridge into a nested <bond/> element
  * tests/test-initscripts.c: test bond-in-a-bridge transformations
---
 data/xml/initscripts-get.xsl      |   45 +++++++---
 data/xml/initscripts-put.xsl      |   46 ++++++----
 data/xml/interface.rng            |  177 ++++++++++++++++++++----------------
 src/drv_initscripts.c             |   59 ++++++++++---
 tests/initscripts/bridge-bond.xml |   37 ++++++++
 tests/interface/bridge-bond.xml   |   14 +++
 tests/test-initscripts.c          |    1 +
 7 files changed, 260 insertions(+), 119 deletions(-)
 create mode 100644 tests/initscripts/bridge-bond.xml
 create mode 100644 tests/interface/bridge-bond.xml

diff --git a/data/xml/initscripts-get.xsl b/data/xml/initscripts-get.xsl
index 46e39b1..f144092 100644
--- a/data/xml/initscripts-get.xsl
+++ b/data/xml/initscripts-get.xsl
@@ -84,26 +84,21 @@
         <xsl:if test="@type = 'vlan'">
           <xsl:call-template name="bare-vlan-interface"/>
         </xsl:if>
+        <xsl:if test="@type = 'bond'">
+          <xsl:call-template name="bare-bond-interface"/>
+        </xsl:if>
         <node label="BRIDGE" value="{../../@name}"/>
       </tree>
+      <xsl:if test="@type = 'bond'">
+        <xsl:call-template name="bond-slaves"/>
+      </xsl:if>
     </xsl:for-each>
   </xsl:template>
 
   <!--
       Bond
   -->
-  <xsl:template match="/interface[@type = 'bond']">
-    <tree>
-      <xsl:call-template name="name-attr"/>
-      <xsl:call-template name="startmode"/>
-      <xsl:call-template name="mtu"/>
-      <xsl:call-template name="interface-addressing"/>
-      <node label="BONDING_OPTS">
-        <xsl:attribute name="value">
-          <xsl:call-template name="bonding-opts"/>
-        </xsl:attribute>
-      </node>
-    </tree>
+  <xsl:template name="bond-slaves">
     <xsl:for-each select='bond/interface'>
       <tree>
         <xsl:call-template name="bare-ethernet-interface"/>
@@ -113,6 +108,32 @@
     </xsl:for-each>
   </xsl:template>
 
+  <xsl:template name="bonding-opts-node">
+    <node label="BONDING_OPTS">
+      <xsl:attribute name="value">
+        <xsl:call-template name="bonding-opts"/>
+      </xsl:attribute>
+    </node>
+  </xsl:template>
+
+  <xsl:template name="bare-bond-interface">
+    <xsl:call-template name="name-attr"/>
+    <xsl:call-template name="startmode"/>
+    <xsl:call-template name="mtu"/>
+    <xsl:call-template name="bonding-opts-node"/>
+  </xsl:template>
+
+  <xsl:template match="/interface[@type = 'bond']">
+    <tree>
+      <xsl:call-template name="name-attr"/>
+      <xsl:call-template name="startmode"/>
+      <xsl:call-template name="mtu"/>
+      <xsl:call-template name="interface-addressing"/>
+      <xsl:call-template name="bonding-opts-node"/>
+    </tree>
+    <xsl:call-template name="bond-slaves"/>
+  </xsl:template>
+
   <!--
        Named templates, following the Relax NG syntax
   -->
diff --git a/data/xml/initscripts-put.xsl b/data/xml/initscripts-put.xsl
index 0f73f50..0bd5dbc 100644
--- a/data/xml/initscripts-put.xsl
+++ b/data/xml/initscripts-put.xsl
@@ -82,9 +82,12 @@
           <xsl:attribute name="delay"><xsl:value-of select="node[@label = 'DELAY']/@value"/></xsl:attribute>
         </xsl:if>
         <xsl:for-each select="/descendant-or-self::*[node[@label = 'BRIDGE' and @value = $iface]]">
-          <xsl:if test="count(node[@label = 'VLAN']) = 0">
+          <xsl:if test="count(node[@label = 'VLAN' or @label = 'BONDING_OPTS']) = 0">
             <xsl:call-template name="bare-ethernet-interface"/>
           </xsl:if>
+          <xsl:if test="count(node[@label = 'BONDING_OPTS']) > 0">
+            <xsl:call-template name="bare-bond-interface"/>
+          </xsl:if>
           <xsl:if test="count(node[@label = 'VLAN']) > 0">
             <xsl:call-template name="bare-vlan-interface"/>
           </xsl:if>
@@ -96,27 +99,38 @@
   <!--
       Bond
   -->
+  <xsl:template name="bond-element">
+    <xsl:variable name="iface" select="node[@label= 'DEVICE']/@value"/>
+    <bond>
+      <xsl:variable name="opts" select="node[@label = 'BONDING_OPTS']/@value"/>
+      <xsl:call-template name="bonding-opts">
+        <xsl:with-param name="opts" select="$opts"/>
+      </xsl:call-template>
+      <xsl:variable name="primary" select="bond:option($opts, 'primary')"/>
+      <xsl:for-each select="/descendant-or-self::*[node[@label = 'MASTER' and @value = $iface]][node[@label = 'DEVICE' and @value = $primary]]">
+        <xsl:call-template name='bare-ethernet-interface'/>
+      </xsl:for-each>
+      <xsl:for-each select="/descendant-or-self::*[node[@label = 'MASTER' and @value = $iface]][node[@label = 'DEVICE' and @value != $primary]]">
+        <xsl:call-template name='bare-ethernet-interface'/>
+      </xsl:for-each>
+    </bond>
+  </xsl:template>
+
+  <xsl:template name="bare-bond-interface">
+    <interface type="bond">
+      <xsl:call-template name="name-attr"/>
+      <xsl:call-template name="bond-element"/>
+    </interface>
+  </xsl:template>
+
   <xsl:template name="bond-interface"
-                match="tree[node[@label = 'DEVICE'][@value = //tree/node[@label = 'MASTER']/@value]]">
+                match="tree[node[@label = 'DEVICE'][@value = //tree/node[@label = 'MASTER']/@value]][count(node[@label = 'BRIDGE']) = 0]">
     <interface type="bond">
-      <xsl:variable name="iface" select="node[@label= 'DEVICE']/@value"/>
       <xsl:call-template name="name-attr"/>
       <xsl:call-template name="startmode"/>
       <xsl:call-template name="mtu"/>
       <xsl:call-template name="interface-addressing"/>
-      <bond>
-        <xsl:variable name="opts" select="node[@label = 'BONDING_OPTS']/@value"/>
-        <xsl:call-template name="bonding-opts">
-          <xsl:with-param name="opts" select="$opts"/>
-        </xsl:call-template>
-        <xsl:variable name="primary" select="bond:option($opts, 'primary')"/>
-        <xsl:for-each select="/descendant-or-self::*[node[@label = 'MASTER' and @value = $iface]][node[@label = 'DEVICE' and @value = $primary]]">
-          <xsl:call-template name='bare-ethernet-interface'/>
-        </xsl:for-each>
-        <xsl:for-each select="/descendant-or-self::*[node[@label = 'MASTER' and @value = $iface]][node[@label = 'DEVICE' and @value != $primary]]">
-          <xsl:call-template name='bare-ethernet-interface'/>
-        </xsl:for-each>
-      </bond>
+      <xsl:call-template name="bond-element"/>
     </interface>
   </xsl:template>
 
diff --git a/data/xml/interface.rng b/data/xml/interface.rng
index e5e25fe..b4f6326 100644
--- a/data/xml/interface.rng
+++ b/data/xml/interface.rng
@@ -123,6 +123,7 @@
           <choice>
             <ref name="bare-ethernet-interface"/>
             <ref name="bare-vlan-interface"/>
+            <ref name="bare-bond-interface"/>
           </choice>
         </zeroOrMore>
       </element>
@@ -135,90 +136,108 @@
   <!--
       Bonds
   -->
-  <define name="bond-interface">
-    <element name="interface">
-      <attribute name="type">
-        <value>bond</value>
-      </attribute>
-      <ref name="name-attr"/>
-      <ref name="startmode"/>
-      <ref name="mtu"/>
-      <ref name="interface-addressing"/>
-      <element name="bond">
-        <optional>
-          <attribute name="mode">
-            <choice>
-              <value>balance-rr</value>
-              <!-- The primary interface is the first interface child
-                   of the bond element -->
-              <value>active-backup</value>
-              <value>balance-xor</value>
-              <value>broadcast</value>
-              <value>802.3ad</value>
-              <value>balance-tlb</value>
-              <value>balance-alb</value>
-            </choice>
-          </attribute>
-        </optional>
+  <define name="bond-interface-common">
+    <attribute name="type">
+      <value>bond</value>
+    </attribute>
+    <!-- The name attribute is only useful for reporting back and is always
+         of the form DEVICE.VLAN
+    -->
+    <optional><ref name="name-attr"/></optional>
+  </define>
 
-        <!-- FIXME: add more attributes
+  <define name="bond-element">
+    <element name="bond">
+      <optional>
+        <attribute name="mode">
+          <choice>
+            <value>balance-rr</value>
+            <!-- The primary interface is the first interface child
+                 of the bond element -->
+            <value>active-backup</value>
+            <value>balance-xor</value>
+            <value>broadcast</value>
+            <value>802.3ad</value>
+            <value>balance-tlb</value>
+            <value>balance-alb</value>
+          </choice>
+        </attribute>
+      </optional>
 
-             mode == 802.3ad
-               ad_select
-               lacp_rate
-               xmit_hash_policy
+      <!-- FIXME: add more attributes
 
-             mode == active-backup
-               fail_over_mac
-               num_grat_arp when mode == active-backup (since 3.3.0)
-               num_unsol_na when mode == active-backup (ipv6, since 3.4.0)
+           mode == 802.3ad
+             ad_select
+             lacp_rate
+             xmit_hash_policy
 
-             mode == balance-xor
-               xmit_hash_policy       (since 2.6.3/3.2.2)
-        -->
+           mode == active-backup
+             fail_over_mac
+             num_grat_arp when mode == active-backup (since 3.3.0)
+             num_unsol_na when mode == active-backup (ipv6, since 3.4.0)
 
-        <choice>
-          <element name="miimon">
-            <!-- miimon frequency in ms -->
-            <attribute name="freq"><ref name="uint"/></attribute>
-            <optional>
-              <attribute name="downdelay"><ref name="uint"/></attribute>
-            </optional>
-            <optional>
-              <attribute name="updelay"><ref name="uint"/></attribute>
-            </optional>
-            <optional>
-              <!-- use_carrier -->
-              <attribute name="carrier">
-                <choice>
-                  <!-- use MII/ETHTOOL ioctl -->
-                  <value>ioctl</value>
-                  <!-- use netif_carrier_ok() -->
-                  <value>netif</value>
-                </choice>
-              </attribute>
-            </optional>
-          </element>
-          <element name="arpmon">
-            <attribute name="interval"><ref name="uint"/></attribute>
-            <attribute name="target"><ref name="ipv4-addr"/></attribute>
-            <optional>
-              <attribute name="validate">
-                <choice>
-                  <value>none</value>
-                  <value>active</value>
-                  <value>backup</value>
-                  <value>all</value>
-                </choice>
-              </attribute>
-            </optional>
-          </element>
-        </choice>
-        <oneOrMore>
-          <!-- The slave interfaces -->
-          <ref name="bare-ethernet-interface"/>
-        </oneOrMore>
-      </element>
+           mode == balance-xor
+             xmit_hash_policy       (since 2.6.3/3.2.2)
+      -->
+
+      <choice>
+        <element name="miimon">
+          <!-- miimon frequency in ms -->
+          <attribute name="freq"><ref name="uint"/></attribute>
+          <optional>
+            <attribute name="downdelay"><ref name="uint"/></attribute>
+          </optional>
+          <optional>
+            <attribute name="updelay"><ref name="uint"/></attribute>
+          </optional>
+          <optional>
+            <!-- use_carrier -->
+            <attribute name="carrier">
+              <choice>
+                <!-- use MII/ETHTOOL ioctl -->
+                <value>ioctl</value>
+                <!-- use netif_carrier_ok() -->
+                <value>netif</value>
+              </choice>
+            </attribute>
+          </optional>
+        </element>
+        <element name="arpmon">
+          <attribute name="interval"><ref name="uint"/></attribute>
+          <attribute name="target"><ref name="ipv4-addr"/></attribute>
+          <optional>
+            <attribute name="validate">
+              <choice>
+                <value>none</value>
+                <value>active</value>
+                <value>backup</value>
+                <value>all</value>
+              </choice>
+            </attribute>
+          </optional>
+        </element>
+      </choice>
+      <oneOrMore>
+        <!-- The slave interfaces -->
+        <ref name="bare-ethernet-interface"/>
+      </oneOrMore>
+    </element>
+  </define>
+
+  <define name="bare-bond-interface">
+    <element name="interface">
+      <ref name="bond-interface-common"/>
+      <ref name="bond-element"/>
+    </element>
+  </define>
+
+  <define name="bond-interface">
+    <element name="interface">
+      <ref name="bond-interface-common"/>
+      <ref name="startmode"/>
+      <ref name="mtu"/>
+      <ref name="interface-addressing"/>
+      <ref name="bond-element"/>
     </element>
   </define>
 
diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index 8be1be8..299949b 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -715,8 +715,9 @@ char *drv_xml_desc(struct netcf_if *nif) {
     ERR_BAIL(ncf);
 
     ndevs = aug_fmt_match(ncf, &devs,
-              "%s[ DEVICE = '%s' or BRIDGE = '%s' or MASTER = '%s']/DEVICE",
-              ifcfg_path, nif->name, nif->name, nif->name);
+              "%s[ DEVICE = '%s' or BRIDGE = '%s' or MASTER = '%s'"
+              "    or MASTER = ../*[BRIDGE = '%s']/DEVICE ]/DEVICE",
+              ifcfg_path, nif->name, nif->name, nif->name, nif->name);
     ERR_BAIL(ncf);
 
     nint = uniq_ifcfg_paths(ncf, ndevs, devs, &intf);
@@ -808,9 +809,12 @@ static void rm_interface(struct netcf *ncf, const char *name) {
     aug = get_augeas(ncf);
     ERR_BAIL(ncf);
 
+    /* The last or clause catches slaves of a bond that are enslaved to
+     * a bridge NAME */
     r = xasprintf(&path,
-          "%s[ DEVICE = '%s' or BRIDGE = '%s' or MASTER = '%s']",
-          ifcfg_path, name, name, name);
+          "%s[ DEVICE = '%s' or BRIDGE = '%s' or MASTER = '%s' "
+          "    or MASTER = ../*[BRIDGE = '%s']/DEVICE ]",
+                  ifcfg_path, name, name, name, name);
     ERR_NOMEM(r < 0, ncf);
 
     r = aug_rm(aug, path);
@@ -849,6 +853,41 @@ static void rm_all_interfaces(struct netcf *ncf, xmlDocPtr ncf_xml) {
     xmlXPathFreeContext(context);
 }
 
+/* Dig through interface NAME and all its subinterfaces for bonds
+ * and either add aliases in modprobe.conf for it (ALIAS == true), or
+ * remove such aliases (ALIAS == false)
+ */
+static void bond_setup(struct netcf *ncf, const char *name, bool alias) {
+    void (*setup)(struct netcf *ncf, const char *name);
+    int nslaves = 0;
+    char **slaves = NULL;
+
+    if (alias)
+        setup = modprobed_alias_bond;
+    else
+        setup = modprobed_unalias_bond;
+
+    if (is_bond(ncf, name)) {
+        setup(ncf, name);
+        ERR_BAIL(ncf);
+    }
+
+    if (is_bridge(ncf, name)) {
+        nslaves = bridge_slaves(ncf, name, &slaves);
+        ERR_BAIL(ncf);
+        for (int i=0; i < nslaves; i++) {
+            if (is_bond(ncf, slaves[i])) {
+                setup(ncf, slaves[i]);
+                ERR_BAIL(ncf);
+            }
+        }
+    }
+
+ error:
+    free_matches(nslaves, &slaves);
+    return;
+}
+
 struct netcf_if *drv_define(struct netcf *ncf, const char *xml_str) {
     xmlDocPtr ncf_xml = NULL, aug_xml = NULL;
     char *name = NULL;
@@ -874,10 +913,8 @@ struct netcf_if *drv_define(struct netcf *ncf, const char *xml_str) {
     aug_put_xml(ncf, aug_xml);
     ERR_BAIL(ncf);
 
-    if (is_bond(ncf, name)) {
-        modprobed_alias_bond(ncf, name);
-        ERR_BAIL(ncf);
-    }
+    bond_setup(ncf, name, true);
+    ERR_BAIL(ncf);
 
     r = aug_save(aug);
     ERR_THROW(r < 0, ncf, EOTHER, "aug_save failed");
@@ -902,10 +939,8 @@ int drv_undefine(struct netcf_if *nif) {
     aug = get_augeas(ncf);
     ERR_BAIL(ncf);
 
-    if (is_bond(ncf, nif->name)) {
-        modprobed_unalias_bond(ncf, nif->name);
-        ERR_BAIL(ncf);
-    }
+    bond_setup(ncf, nif->name, false);
+    ERR_BAIL(ncf);
 
     rm_interface(ncf, nif->name);
     ERR_BAIL(ncf);
diff --git a/tests/initscripts/bridge-bond.xml b/tests/initscripts/bridge-bond.xml
new file mode 100644
index 0000000..c72f8d3
--- /dev/null
+++ b/tests/initscripts/bridge-bond.xml
@@ -0,0 +1,37 @@
+<?xml version="1.0"?>
+<forest>
+  <tree path="/files/etc/sysconfig/network-scripts/ifcfg-br0">
+    <node label="DEVICE" value="br0"/>
+    <node label="ONBOOT" value="yes"/>
+    <node label="MTU" value="1500"/>
+    <node label="TYPE" value="Bridge"/>
+    <node label="STP" value="off"/>
+  </tree>
+  <tree path="/files/etc/sysconfig/network-scripts/ifcfg-eth2">
+    <node label="DEVICE" value="eth2"/>
+    <node label="ONBOOT" value="yes"/>
+    <node label="MTU" value="1500"/>
+    <node label="BRIDGE" value="br0"/>
+  </tree>
+  <tree path="/files/etc/sysconfig/network-scripts/ifcfg-bond0">
+    <node label="DEVICE" value="bond0"/>
+    <node label="ONBOOT" value="yes"/>
+    <node label="MTU" value="1500"/>
+    <node label="BONDING_OPTS" value="'mode=active-backup primary=eth1 miimon=100 updelay=10 use_carrier=0'"/>
+    <node label="BRIDGE" value="br0"/>
+  </tree>
+  <tree path="/files/etc/sysconfig/network-scripts/ifcfg-eth1">
+    <node label="DEVICE" value="eth1"/>
+    <node label="ONBOOT" value="yes"/>
+    <node label="MTU" value="1500"/>
+    <node label="MASTER" value="bond0"/>
+    <node label="SLAVE" value="yes"/>
+  </tree>
+  <tree path="/files/etc/sysconfig/network-scripts/ifcfg-eth0">
+    <node label="DEVICE" value="eth0"/>
+    <node label="ONBOOT" value="yes"/>
+    <node label="MTU" value="1500"/>
+    <node label="MASTER" value="bond0"/>
+    <node label="SLAVE" value="yes"/>
+  </tree>
+</forest>
diff --git a/tests/interface/bridge-bond.xml b/tests/interface/bridge-bond.xml
new file mode 100644
index 0000000..5337f03
--- /dev/null
+++ b/tests/interface/bridge-bond.xml
@@ -0,0 +1,14 @@
+<interface type="bridge" name="br0">
+  <start mode="onboot"/>
+  <mtu size="1500"/>
+  <bridge stp="off">
+    <interface type="ethernet" name="eth2"/>
+    <interface type="bond" name="bond0">
+      <bond mode="active-backup">
+        <miimon freq="100" updelay="10" carrier="ioctl"/>
+        <interface type="ethernet" name="eth1"/>
+        <interface type="ethernet" name="eth0"/>
+      </bond>
+    </interface>
+  </bridge>
+</interface>
diff --git a/tests/test-initscripts.c b/tests/test-initscripts.c
index 39d0865..56d430e 100644
--- a/tests/test-initscripts.c
+++ b/tests/test-initscripts.c
@@ -178,6 +178,7 @@ static void testTransforms(CuTest *tc) {
     assert_transforms(tc, "bridge-no-address");
     assert_transforms(tc, "bridge-vlan");
     assert_transforms(tc, "bridge-empty");
+    assert_transforms(tc, "bridge-bond");
     assert_transforms(tc, "ethernet-static");
     assert_transforms(tc, "ethernet-static-no-prefix");
     assert_transforms(tc, "ethernet-dhcp");
-- 
1.6.2.5



More information about the netcf-devel mailing list