Add the ability to set any mode/lb on bonds

This requires a schema change, adding 'mode' and 'load-balance' fields,
a semantic invariant that 'load-balance' can only be set in the case of
LACP and XOR bonds, a mapper from the mode/lb strings, ie.
"round-robin" to their VPP numeric counterparts, a bunch of unit tests.

Then in the reconciler, changing bonds (__bond_has_diff()) will
invalidate any LCP or sub-interfaces built on them, so those will have
to be pruned. create_bondethernet() will now create (or re-create)
the bond with the correct flags.

Unit-tests, YAML tests and the integration test all pass.

Updated config-guide.

config/bondethernet.py

@@ -52,6 +52,70 @@ def is_bond_member(yaml, ifname):
     return False
 
 
+def get_mode(yaml, ifname):
+    """ Return the mode of the BondEthernet as a string, defaulting to 'lacp'
+        if no mode is given. Return None if the bond interface doesn't exist.
+
+        Return values: 'round-robin','active-backup','broadcast','lacp','xor'
+        """
+    ifname, iface = get_by_name(yaml, ifname)
+    if not iface:
+        return None
+
+    if not 'mode' in iface:
+        return 'lacp'
+    return iface['mode']
+
+
+def mode_to_int(mode):
+    """ Returns the integer representation in VPP of a given bondethernet mode,
+        or -1 if 'mode' is not a valid string.
+
+        See src/vnet/bonding/bond.api and schema.yaml for valid pairs. """
+
+    ret = { 'round-robin': 1, 'active-backup': 2, 'xor': 3, 'broadcast': 4, 'lacp': 5 }
+    try:
+        return ret[mode]
+    except:
+        pass
+    return -1
+
+
+def get_lb(yaml, ifname):
+    """ Return the loadbalance strategy of the BondEthernet as a string. Only
+        'xor' and 'lacp' modes have loadbalance strategies, so return None if
+        those modes are not used.
+
+        Return values: 'l2', 'l23', 'l34', with 'l34' being the default if
+        the bond is in xor/lacp mode without a load-balance strategy set
+        explicitly."""
+    ifname, iface = get_by_name(yaml, ifname)
+    if not iface:
+        return None
+    mode = get_mode(yaml, ifname)
+    if not mode in ['xor','lacp']:
+        return None
+
+    if not 'load-balance' in iface:
+        return 'l34'
+    return iface['load-balance']
+
+
+def lb_to_int(lb):
+    """ Returns the integer representation in VPP of a given load-balance strategy,
+        or -1 if 'lb' is not a valid string.
+
+        See src/vnet/bonding/bond.api and schema.yaml for valid pairs, although
+        bond.api defined more than we use in vppcfg. """
+
+    ret = { 'l2': 0, 'l34': 1, 'l23': 2, 'round-robin': 3, 'broadcast': 4, 'active-backup': 5 }
+    try:
+        return ret[lb]
+    except:
+        pass
+    return -1
+
+
 def validate_bondethernets(yaml):
     result = True
     msgs = []
@@ -74,6 +138,9 @@ def validate_bondethernets(yaml):
         if instance > 4294967294:
             msgs.append("bondethernet %s has instance %d which is too large" % (ifname, instance))
             result = False
+        if not get_mode(yaml, bond_ifname) in ['xor','lacp'] and 'load-balance' in iface:
+            msgs.append("bondethernet %s can only have load-balance if in mode XOR or LACP" % (ifname))
+            result = False
 
         for member in iface['interfaces']:
             if (None, None) == interface.get_by_name(yaml, member):

config/test_bondethernet.py

@@ -31,7 +31,34 @@ class TestBondEthernetMethods(unittest.TestCase):
 
     def test_enumerators(self):
         ifs = bondethernet.get_bondethernets(self.cfg)
-        self.assertEqual(len(ifs), 1)
+        self.assertEqual(len(ifs), 3)
         self.assertIn("BondEthernet0", ifs)
+        self.assertIn("BondEthernet1", ifs)
+        self.assertIn("BondEthernet2", ifs)
         self.assertNotIn("BondEthernet-noexist", ifs)
 
+    def test_get_mode(self):
+        self.assertEqual('lacp', bondethernet.get_mode(self.cfg, "BondEthernet0"))
+        self.assertEqual('xor', bondethernet.get_mode(self.cfg, "BondEthernet1"))
+
+    def test_mode_to_int(self):
+        self.assertEqual(1, bondethernet.mode_to_int("round-robin"))
+        self.assertEqual(2, bondethernet.mode_to_int("active-backup"))
+        self.assertEqual(3, bondethernet.mode_to_int("xor"))
+        self.assertEqual(4, bondethernet.mode_to_int("broadcast"))
+        self.assertEqual(5, bondethernet.mode_to_int("lacp"))
+        self.assertEqual(-1, bondethernet.mode_to_int("not-exist"))
+
+    def test_get_lb(self):
+        self.assertEqual('l34', bondethernet.get_lb(self.cfg, "BondEthernet0"))
+        self.assertEqual('l2', bondethernet.get_lb(self.cfg, "BondEthernet1"))
+        self.assertIsNone(bondethernet.get_lb(self.cfg, "BondEthernet2"))
+
+    def test_lb_to_int(self):
+        self.assertEqual(0, bondethernet.lb_to_int("l2"))
+        self.assertEqual(1, bondethernet.lb_to_int("l34"))
+        self.assertEqual(2, bondethernet.lb_to_int("l23"))
+        self.assertEqual(3, bondethernet.lb_to_int("round-robin"))
+        self.assertEqual(4, bondethernet.lb_to_int("broadcast"))
+        self.assertEqual(5, bondethernet.lb_to_int("active-backup"))
+        self.assertEqual(-1, bondethernet.lb_to_int("not-exist"))

docs/config-guide.md

@@ -156,20 +156,28 @@ BondEthernets are required to be named `BondEthernetN` (note the camelcase) wher
 *   ***interfaces***: A list of zero or more interfaces that are bond members. The interfaces
     must be PHYs, and in their `interface` configuration, members are allowed only to set the
     MTU.
+*   ***mode***: A mode to run the LAG in. Can be one of 'round-robin', 'active-backup', 'xor',
+    'broadcast' or 'lacp'. The default is LACP.
+*   ***load-balance***: A loadbalancing strategy to use, if the mode is either XOR or LACP.
+    Can be one of 'l2', 'l23', or 'l34'. The default is l34, which hashes on the source and
+    destination IPs and ports.
 
 Note that the configuration object here only specifies the link aggregation and its members.
 BondEthernets are expected to occur as well in the `interfaces` section, where their sub-interfaces
 and IP addresses and so on are specified.
 
-*Caveat*: Currently, BondEthernets are always created as `LACP` typed devices with a loadbalance
-strategy of `l34`. In a future release of `vppcfg`, the type and strategy will be configurable.
-
 Examples:
 ```
 bondethernets:
   BondEthernet0:
     description: "Core: LACP to fsw0.lab.ipng.ch"
+    interfaces: [ GigabitEthernet1/0/0, GigabitEthernet1/0/1 ]
+    mode: lacp
+    load-balance: l2
+  BondEthernet1:
+    description: "Core: RR LAG"
     interfaces: [ GigabitEthernet3/0/0, GigabitEthernet3/0/1 ]
+    mode: round-robin
 ```
 
 ### VXLAN Tunnels

example.yaml

@@ -1,6 +1,8 @@
 bondethernets:
   BondEthernet0:
     interfaces: [ GigabitEthernet3/0/0, GigabitEthernet3/0/1 ]
+    mode: xor
+    load-balance: l2
 
 interfaces:
   GigabitEthernet3/0/0:

intest/hippo10.yaml

@@ -1,6 +1,8 @@
 bondethernets:
   BondEthernet0:
     interfaces: [ GigabitEthernet3/0/0, GigabitEthernet3/0/1 ]
+    mode: lacp
+    load-balance: l2
 
 interfaces:
   GigabitEthernet3/0/0:

intest/hippo12.yaml

@@ -1,6 +1,8 @@
 bondethernets:
   BondEthernet0:
     interfaces: [ GigabitEthernet3/0/0, GigabitEthernet3/0/1 ]
+    mode: xor
+    load-balance: l2
 
 interfaces:
   GigabitEthernet3/0/0:

intest/hippo3.yaml

@@ -1,6 +1,7 @@
 bondethernets:
   BondEthernet1:
     interfaces: [ GigabitEthernet3/0/0, GigabitEthernet3/0/1 ]
+    mode: round-robin
   
 interfaces:
   GigabitEthernet3/0/0:

schema.yaml

@@ -35,6 +35,8 @@ loopback:
 bondethernet:
   description: str(exclude='\'"',len=64,required=False)
   interfaces: list(str(matches='.*GigabitEthernet[0-9]+/[0-9]+/[0-9]+'))
+  mode: enum('round-robin','active-backup','broadcast','lacp','xor',required=False)
+  load-balance: enum('l2','l23','l34',required=False)
 ---
 interface:
   description: str(exclude='\'"',len=64,required=False)

unittest/test_bondethernet.yaml

@@ -2,6 +2,15 @@ bondethernets:
   BondEthernet0:
     interfaces: [ GigabitEthernet1/0/0, GigabitEthernet1/0/1 ]
 
+  BondEthernet1:
+    interfaces: [ GigabitEthernet3/0/0, GigabitEthernet3/0/1 ]
+    mode: xor
+    load-balance: l2
+
+  BondEthernet2:
+    interfaces: [ GigabitEthernet4/0/0, GigabitEthernet4/0/1 ]
+    mode: round-robin
+
 interfaces:
   GigabitEthernet1/0/0:
     mtu: 3000
@@ -14,6 +23,16 @@ interfaces:
       100:
         mtu: 2000
 
+  GigabitEthernet3/0/0:
+    mtu: 3000
+  GigabitEthernet3/0/1:
+    mtu: 3000
+
+  GigabitEthernet4/0/0:
+    mtu: 3000
+  GigabitEthernet4/0/1:
+    mtu: 3000
+
   BondEthernet0:
     mtu: 3000
     lcp: "be012345678"
@@ -22,3 +41,9 @@ interfaces:
       100:
         mtu: 2000
         addresses: [ 192.0.2.9/29, 2001:db8:1::1/64 ]
+
+  BondEthernet1:
+    mtu: 3000
+
+  BondEthernet2:
+    mtu: 3000

unittest/yaml/error-bondethernet8.yaml

@@ -0,0 +1,44 @@
+test:
+  description: "BondEthernet can only have loadbalance if XOR or LACP"
+  errors:
+    expected: 
+     - "bondethernet BondEthernet2 can only have load-balance if in mode XOR or LACP"
+    count: 1
+---
+bondethernets:
+  BondEthernet0:
+    interfaces: [ GigabitEthernet1/0/0, GigabitEthernet1/0/1 ]
+    mode: xor
+    load-balance: l34
+
+  BondEthernet1:
+    interfaces: [ GigabitEthernet2/0/0, GigabitEthernet2/0/1 ]
+    mode: lacp
+    load-balance: l34
+
+  BondEthernet2:
+    interfaces: [ GigabitEthernet3/0/0, GigabitEthernet3/0/1 ]
+    mode: round-robin
+    load-balance: l34
+
+interfaces:
+  GigabitEthernet1/0/0:
+    mtu: 3000
+  GigabitEthernet1/0/1:
+    mtu: 3000
+  BondEthernet0:
+    mtu: 3000
+
+  GigabitEthernet2/0/0:
+    mtu: 3000
+  GigabitEthernet2/0/1:
+    mtu: 3000
+  BondEthernet1:
+    mtu: 3000
+
+  GigabitEthernet3/0/0:
+    mtu: 3000
+  GigabitEthernet3/0/1:
+    mtu: 3000
+  BondEthernet2:
+    mtu: 3000

vpp/reconciler.py

@@ -236,6 +236,33 @@ class Reconciler():
             self.vpp.cache_remove_l2xc(l2xc)
         return True
 
+    def __bond_has_diff(self, ifname):
+        """ Returns True if the given ifname (BondEthernet0) have different attributes,
+            or if either does not exist.
+
+            Returns False if they are identical.
+            """
+        if not ifname in self.vpp.cache['interface_names']:
+            return True
+
+        vpp_iface = self.vpp.cache['interface_names'][ifname]
+        if not vpp_iface.sw_if_index in self.vpp.cache['bondethernets']:
+            return True
+
+        config_ifname, config_iface = bondethernet.get_by_name(self.cfg, ifname)
+        if not config_iface:
+            return True
+
+        vpp_bond = self.vpp.cache['bondethernets'][vpp_iface.sw_if_index]
+        mode = bondethernet.mode_to_int(bondethernet.get_mode(self.cfg, config_ifname))
+        if mode != vpp_bond.mode:
+            return True
+        lb = bondethernet.lb_to_int(bondethernet.get_lb(self.cfg, config_ifname))
+        if lb != vpp_bond.lb:
+            return True
+
+        return False
+
     def prune_bondethernets(self):
         """ Remove all BondEthernets from VPP, if they are not in the config. If the bond has members,
             remove those from the bond before removing the bond. """
@@ -244,7 +271,8 @@ class Reconciler():
         for idx, bond in self.vpp.cache['bondethernets'].items():
             vpp_ifname = bond.interface_name
             config_ifname, config_iface = bondethernet.get_by_name(self.cfg, vpp_ifname)
-            if not config_iface:
+
+            if self.__bond_has_diff(vpp_ifname):
                 self.prune_addresses(vpp_ifname, [])
                 for member in self.vpp.cache['bondethernet_members'][idx]:
                     member_ifname = self.vpp.cache['interfaces'][member].interface_name
@@ -255,6 +283,7 @@ class Reconciler():
                 self.cli['prune'].append(cli);
                 removed_interfaces.append(vpp_ifname)
                 continue
+
             for member in self.vpp.cache['bondethernet_members'][idx]:
                 member_ifname = self.vpp.cache['interfaces'][member].interface_name
                 if 'interfaces' in config_iface and not member_ifname in config_iface['interfaces']:
@@ -326,7 +355,8 @@ class Reconciler():
         return match
 
     def prune_sub_interfaces(self):
-        """ Remove interfaces from VPP if they are not in the config, or if their encapsulation is different.
+        """ Remove interfaces from VPP if they are not in the config, if their encapsulation is different,
+            or if the BondEthernet they reside on is different.
             Start with inner-most (QinQ/QinAD), then Dot1Q/Dot1AD."""
         removed_interfaces=[]
         for numtags in [ 2, 1 ]:
@@ -338,21 +368,27 @@ class Reconciler():
                 if self.__tap_is_lcp(vpp_iface.sw_if_index):
                     continue
 
+                prune=False
                 config_ifname, config_iface = interface.get_by_name(self.cfg, vpp_ifname)
                 if not config_iface:
-                    self.prune_addresses(vpp_ifname, [])
+                    prune = True
-                    cli="delete sub %s" % (vpp_ifname)
+                elif vpp_iface.interface_dev_type=='bond' and vpp_iface.sub_number_of_tags > 0:
-                    self.cli['prune'].append(cli);
+                    config_parent_ifname, config_parent_iface = interface.get_parent_by_name(self.cfg, vpp_ifname)
-                    removed_interfaces.append(vpp_ifname)
+                    if self.__bond_has_diff(config_parent_ifname):
-                    continue
+                        prune = True
+
                 config_encap = interface.get_encapsulation(self.cfg, vpp_ifname)
                 vpp_encap = self.__get_encapsulation(vpp_iface)
                 if config_encap != vpp_encap:
+                    prune = True
+
+                if prune:
                     self.prune_addresses(vpp_ifname, [])
                     cli="delete sub %s" % (vpp_ifname)
                     self.cli['prune'].append(cli);
                     removed_interfaces.append(vpp_ifname)
                     continue
+
                 addresses = []
                 if 'addresses' in config_iface:
                     addresses = config_iface['addresses']
@@ -482,7 +518,7 @@ class Reconciler():
                         removed_lcps.append(lcp.host_if_name)
                         continue
 
-                if vpp_iface.sub_number_of_tags > 1:
+                if vpp_iface.sub_number_of_tags > 0:
                     config_encap = interface.get_encapsulation(self.cfg, config_ifname)
                     vpp_encap = self.__get_encapsulation(vpp_iface)
                     if config_encap != vpp_encap:
@@ -495,6 +531,14 @@ class Reconciler():
                 if vpp_iface.interface_dev_type=='Loopback':
                     ## Loopbacks will not have a PHY to check.
                     continue
+                if vpp_iface.interface_dev_type=='bond':
+                    bond_iface = self.vpp.cache['interfaces'][vpp_iface.sup_sw_if_index]
+                    if self.__bond_has_diff(bond_iface.interface_name):
+                        ## If BondEthernet changed, it has to be re-created, so all LCPs must be removed.
+                        cli="lcp delete %s" % (vpp_iface.interface_name)
+                        self.cli['prune'].append(cli);
+                        removed_lcps.append(lcp.host_if_name)
+                        continue
 
                 phy_lcp = lcps[vpp_iface.sup_sw_if_index]
                 config_phy_ifname, config_phy_iface = interface.get_by_lcp_name(self.cfg, phy_lcp.host_if_name)
@@ -578,7 +622,11 @@ class Reconciler():
                 continue
             ifname, iface = bondethernet.get_by_name(self.cfg, ifname)
             instance = int(ifname[12:])
-            cli="create bond mode lacp load-balance l34 id %d" % (instance)
+            mode = bondethernet.get_mode(self.cfg, ifname)
+            cli="create bond id %d mode %s" % (instance, mode)
+            lb = bondethernet.get_lb(self.cfg, ifname)
+            if lb:
+                cli += " load-balance %s" % lb
             self.cli['create'].append(cli);
         return True