small easy: support (0|1) or (off|on) attribute while keeping backward compatibility

Ticket: CM-8866
Reviewed By: Roopa
Testing Done:

added support for:
* bond-lacp-bypass-allow (0|1)
* bond-use-carrier (0|1)
* bridge-mcqifaddr (0|1)
* bridge-mcquerier (0|1)
* bridge-mcrouter (0|1)
* bridge-mcsnoop (0|1)
* bridge-portmcrouter (0|1)
* link-autoneg (off|on)
* vxlan-learning (off|on)

these 2 are not yet supported by ifupdown2:
* bond-lacp-bypass-all-active (0|1)
* bond-lacp-fallback-allow (0|1)

This one is left untouched. yes/no doesn't make sense for this attribute.
* bond-lacp-rate (0|1)

Signed-off-by: Julien Fortin <julien@cumulusnetworks.com>

addons/bond.py

@@ -13,6 +13,7 @@ from ifupdownaddons.iproute2 import iproute2
 from ifupdown.netlink import netlink
 import ifupdown.policymanager as policymanager
 import ifupdown.ifupdownflags as ifupdownflags
+from ifupdown.utils import utils
 
 class bond(moduleBase):
     """  ifupdown2 addon module to configure bond interfaces """
@@ -20,9 +21,9 @@ class bond(moduleBase):
                     'attrs' : {
                     'bond-use-carrier':
                          {'help' : 'bond use carrier',
-                          'validvals' : ['0', '1'],
+                          'validvals' : ['yes', 'no', '0', '1'],
-                          'default' : '1',
+                          'default' : 'yes',
-                          'example': ['bond-use-carrier 1']},
+                          'example': ['bond-use-carrier yes']},
                      'bond-num-grat-arp':
                          {'help' : 'bond use carrier',
                           'validrange' : ['0', '255'],
@@ -90,9 +91,9 @@ class bond(moduleBase):
                          'example' : ['bond-ad-actor-system 00:00:00:00:00:00'],},
                      'bond-lacp-bypass-allow':
                          {'help' : 'allow lacp bypass',
-                          'validvals' : ['0', '1'],
+                          'validvals' : ['yes', 'no', '0', '1'],
-                          'default' : '0',
+                          'default' : 'no',
-                          'example' : ['bond-lacp-bypass-allow 0']},
+                          'example' : ['bond-lacp-bypass-allow no']},
                      'bond-slaves' :
                         {'help' : 'bond slaves',
                          'required' : True,
@@ -245,6 +246,10 @@ class bond(moduleBase):
                     attrstoset[dstk] = v
             if not attrstoset:
                 return
+
+            # support yes/no attrs
+            utils.support_yesno_attrs(attrstoset, ['use_carrier', 'lacp_bypass'])
+
             have_attrs_to_set = 1
             self.bondcmd.set_attrs(ifaceobj.name, attrstoset,
                     self.ipcmd.link_down if linkup else None)
@@ -333,6 +338,11 @@ class bond(moduleBase):
         if not ifaceattrs: return
         runningattrs = self._query_running_attrs(ifaceobj.name)
 
+        # support yes/no attributes
+        utils.support_yesno_attrs(runningattrs, ['bond-use-carrier',
+                                                 'bond-lacp-bypass-allow'],
+                                  ifaceobj=ifaceobj)
+
         # support for numerical bond-mode
         mode = ifaceobj.get_attr_value_first('bond-mode')
         if mode in bond._bond_mode_num:

addons/bridge.py

@@ -7,6 +7,7 @@
 from sets import Set
 from ifupdown.iface import *
 import ifupdown.policymanager as policymanager
+from ifupdown.utils import utils
 from ifupdownaddons.modulebase import moduleBase
 from ifupdownaddons.bridgeutils import brctl
 from ifupdownaddons.iproute2 import iproute2
@@ -101,14 +102,14 @@ class bridge(moduleBase):
                           'default' : '2'},
                     'bridge-mcrouter' :
                         { 'help' : 'set multicast router',
-                          'validvals' : ['0', '1'],
+                          'validvals' : ['yes', 'no', '0', '1'],
-                          'default' : '1',
+                          'default' : 'yes',
-                          'example' : ['bridge-mcrouter 1']},
+                          'example' : ['bridge-mcrouter yes']},
                     'bridge-mcsnoop' :
                         { 'help' : 'set multicast snooping',
-                          'validvals' : ['0', '1'],
+                          'validvals' : ['yes', 'no', '0', '1'],
-                          'default' : '1',
+                          'default' : 'yes',
-                          'example' : ['bridge-mcsnoop 1']},
+                          'example' : ['bridge-mcsnoop yes']},
                     'bridge-mcsqc' :
                         { 'help' : 'set multicast startup query count',
                           'validrange' : ['0', '255'],
@@ -116,14 +117,14 @@ class bridge(moduleBase):
                           'example' : ['bridge-mcsqc 2']},
                     'bridge-mcqifaddr' :
                         { 'help' : 'set multicast query to use ifaddr',
-                          'validvals' : ['0', '1'],
+                          'validvals' : ['yes', 'no', '0', '1'],
-                          'default' : '0',
+                          'default' : 'no',
-                          'example' : ['bridge-mcqifaddr 0']},
+                          'example' : ['bridge-mcqifaddr no']},
                     'bridge-mcquerier' :
                         { 'help' : 'set multicast querier',
-                          'validvals' : ['0', '1'],
+                          'validvals' : ['yes', 'no', '0', '1'],
-                          'default' : '0',
+                          'default' : 'no',
-                          'example' : ['bridge-mcquerier 0']},
+                          'example' : ['bridge-mcquerier no']},
                     'bridge-hashel' :
                         { 'help' : 'set hash elasticity',
                           'validrange' : ['0', '4096'],
@@ -172,10 +173,10 @@ class bridge(moduleBase):
                           'example' : ['bridge-mcqv4src 100=172.16.100.1 101=172.16.101.1']},
                     'bridge-portmcrouter' :
                         { 'help' : 'set port multicast routers',
-                          'validvals' : ['0', '1'],
+                          'validvals' : ['yes', 'no', '0', '1'],
-                          'default' : '1',
+                          'default' : 'yes',
-                          'example' : ['under the bridge: bridge-portmcrouter swp1=1 swp2=1',
+                          'example' : ['under the bridge: bridge-portmcrouter swp1=yes swp2=yes',
-                                       'under the port (recommended): bridge-portmcrouter 1']},
+                                       'under the port (recommended): bridge-portmcrouter yes']},
                     'bridge-portmcfl' :
                         { 'help' : 'port multicast fast leave.',
                           'validrange' : ['0', '65535'],
@@ -721,6 +722,10 @@ class bridge(moduleBase):
                                }.items()
                             if v }
             if bridgeattrs:
+                utils.support_yesno_attrs(bridgeattrs, ['mcqifaddr',
+                                                        'mcquerier',
+                                                        'mcrouter',
+                                                        'mcsnoop'])
                 self.brctlcmd.set_bridge_attrs(ifaceobj.name, bridgeattrs)
             portattrs = {}
             for attrname, dstattrname in {'bridge-pathcosts' : 'pathcost',
@@ -741,7 +746,10 @@ class bridge(moduleBase):
                         (port, val) = p.split('=')
                         if not portattrs.get(port):
                             portattrs[port] = {}
-                        portattrs[port].update({dstattrname : val})
+                        if attrname == 'bridge-portmcrouter':
+                            portattrs[port].update({dstattrname: utils.boolean_support_binary(val)})
+                        else:
+                            portattrs[port].update({dstattrname : val})
                     except Exception, e:
                         self.log_error('%s: could not parse %s (%s)'
                                        %(ifaceobj.name, attrname, str(e)),
@@ -1454,6 +1462,9 @@ class bridge(moduleBase):
         except Exception, e:
             self.logger.warn(str(e))
             runningattrs = {}
+
+        self._query_check_support_yesno_attrs(runningattrs, ifaceobj)
+
         filterattrs = ['bridge-vids', 'bridge-port-vids',
                        'bridge-port-pvids']
         for k in Set(ifaceattrs).difference(filterattrs):
@@ -1662,6 +1673,12 @@ class bridge(moduleBase):
             try:
                 running_attrval = self.brctlcmd.get_bridgeport_attr(
                                        bridgename, ifaceobj.name, dstattr)
+
+                if dstattr == 'mcrouter':
+                    if not utils.is_binary_bool(attrval) and running_attrval:
+                        running_attrval = utils.get_yesno_boolean(
+                            utils.get_boolean_from_string(running_attrval))
+
                 if running_attrval != attrval:
                     ifaceobjcurr.update_config_with_status(attr,
                                             running_attrval, 1)
@@ -1755,6 +1772,29 @@ class bridge(moduleBase):
         if self.default_stp_on:
             ifaceobj.update_config('bridge-stp', 'yes')
 
+    def _query_check_support_yesno_attrs(self, runningattrs, ifaceobj):
+        for attrl in [['mcqifaddr', 'bridge-mcqifaddr'],
+                     ['mcquerier', 'bridge-mcquerier'],
+                     ['mcrouter', 'bridge-mcrouter'],
+                     ['mcsnoop', 'bridge-mcsnoop']]:
+            value = ifaceobj.get_attr_value_first(attrl[1])
+            if value and not utils.is_binary_bool(value):
+                if attrl[0] in runningattrs:
+                    bool = utils.get_boolean_from_string(runningattrs[attrl[0]])
+                    runningattrs[attrl[0]] = utils.get_yesno_boolean(bool)
+        attrval = ifaceobj.get_attr_value_first('bridge-portmcrouter')
+        if attrval:
+            portlist = self.parse_port_list(ifaceobj.name, attrval)
+            if portlist:
+                to_convert = []
+                for p in portlist:
+                    (port, val) = p.split('=')
+                    if not utils.is_binary_bool(val):
+                        to_convert.append(port)
+                for port in to_convert:
+                    runningattrs['ports'][port]['portmcrouter'] = utils.get_yesno_boolean(
+                        utils.get_boolean_from_string(runningattrs['ports'][port]['portmcrouter']))
+
     _run_ops = {'pre-up' : _up,
                'post-down' : _down,
                'query-checkcurr' : _query_check,

addons/ethtool.py

@@ -36,7 +36,7 @@ class ethtool(moduleBase,utilsBase):
                       'link-autoneg' :
                             {'help': 'set autonegotiation',
                              'example' : ['link-autoneg on'],
-                             'validvals' : ['on', 'off'],
+                             'validvals' : ['yes', 'no', 'on', 'off'],
                              'default' : 'varies by platform and port'}}}
 
     def __init__(self, *args, **kargs):
@@ -67,6 +67,10 @@ class ethtool(moduleBase,utilsBase):
                 continue
             # check running values
             running_val = self.get_running_attr(attr, ifaceobj)
+
+            if attr == 'autoneg':
+                config_val = utils.get_onoff_bool(config_val)
+
             # we need to track if an interface has a configured value
             # this will be used if there are duplicate iface stanza and
             # the configured interface will always take precedence.
@@ -154,6 +158,12 @@ class ethtool(moduleBase,utilsBase):
             if (not running_attr):
                 continue
 
+            if attr == 'autoneg':
+                if configured == 'yes' and running_attr == 'on':
+                    running_attr = 'yes'
+                elif configured == 'no' and running_attr == 'off':
+                    running_attr = 'no'
+
             # we make sure we can get a running value first
             if (running_attr and configured and running_attr == configured):
                 # PASS since running is what is configured 

addons/vxlan.py

@@ -1,6 +1,7 @@
 #!/usr/bin/python
 
 from ifupdown.iface import *
+from ifupdown.utils import utils
 from ifupdownaddons.modulebase import moduleBase
 from ifupdownaddons.iproute2 import iproute2
 from ifupdownaddons.systemutils import systemUtils
@@ -32,10 +33,10 @@ class vxlan(moduleBase):
                              'validvals' : [IPv4Address, ],
                              'example': ['vxlan-remoteip 172.16.22.127']},
                         'vxlan-learning' :
-                            {'help' : 'vxlan learning on/off',
+                            {'help' : 'vxlan learning yes/no',
-                             'validvals' : ['on', 'off'],
+                             'validvals' : ['yes', 'no', 'on', 'off'],
-                             'example': ['vxlan-learning off'],
+                             'example': ['vxlan-learning no'],
-                             'default': 'on'},
+                             'default': 'yes'},
                         'vxlan-ageing' :
                             {'help' : 'vxlan aging timer',
                              'validrange' : ['0', '4096'],
@@ -72,7 +73,7 @@ class vxlan(moduleBase):
             localtunnelip=ifaceobj.get_attr_value_first('vxlan-local-tunnelip'),
             svcnodeip=ifaceobj.get_attr_value_first('vxlan-svcnodeip'),
             remoteips=ifaceobj.get_attr_value('vxlan-remoteip'),
-            learning=ifaceobj.get_attr_value_first('vxlan-learning'),
+            learning=utils.get_onoff_bool(ifaceobj.get_attr_value_first('vxlan-learning')),
             ageing=ifaceobj.get_attr_value_first('vxlan-ageing'),
             anycastip=self._clagd_vxlan_anycast_ip)
             if ifaceobj.addr_method == 'manual':
@@ -140,7 +141,13 @@ class vxlan(moduleBase):
         learning = ifaceobj.get_attr_value_first('vxlan-learning')
         if not learning:
             learning = 'on'
+
         running_learning = vxlanattrs.get('learning')
+        if learning == 'yes' and running_learning == 'on':
+            running_learning = 'yes'
+        elif learning == 'no' and running_learning == 'off':
+            running_learning = 'no'
+
         if learning == running_learning:
            ifaceobjcurr.update_config_with_status('vxlan-learning',
                                                   running_learning, 0)

docs/examples/interfaces

@@ -21,7 +21,7 @@ iface swp30
     alias "test network"
     link-duplex full
     link-speed 1000
-    link-autoneg off
+    link-autoneg no
 
 # bond interface
 auto bond3
@@ -29,7 +29,7 @@ iface bond3 inet static
     bond-slaves swp1 swp2
     bond-mode 802.3ad
     bond-miimon 100
-    bond-use-carrier 1
+    bond-use-carrier yes
     bond-lacp-rate 1
     bond-min-links 1
     bond-xmit_hash_policy layer3+4
@@ -40,12 +40,12 @@ iface bond4 inet static
     bond-slaves swp3 swp4
     bond-mode 802.3ad
     bond-miimon 100
-    bond-use-carrier 1
+    bond-use-carrier yes
     bond-lacp-rate 1
     bond-min-links 1
     bond-xmit_hash_policy layer3+4
 
-# bond interface
+# bridge interface
 auto br0
 iface br0
     address 12.0.0.4/24

docs/examples/interfaces_bridge_igmp_mstp

@@ -31,11 +31,11 @@ iface br-300 inet static
            mstpctl-hello 2
            mstpctl-portnetwork swp13.300=no
            bridge-mclmc 3
-           bridge-mcrouter 0
+           bridge-mcrouter no
-           bridge-mcsnoop  1
+           bridge-mcsnoop  yes
            bridge-mcsqc    3
-           bridge-mcqifaddr 1
+           bridge-mcqifaddr yes
-           bridge-mcquerier 1
+           bridge-mcquerier yes
            bridge-hashel 3
            bridge-hashmax 4
            bridge-mclmi 3

docs/examples/vlan_aware_bridges/interfaces.with_bonds

@@ -23,7 +23,7 @@ iface uplink1
     bond-slaves swp32
     bond-mode 802.3ad
     bond-miimon 100
-    bond-use-carrier 1
+    bond-use-carrier yes
     bond-lacp-rate 1
     bond-min-links 1
     bond-xmit-hash-policy layer2
@@ -35,7 +35,7 @@ iface peerlink
     bond-slaves swp30 swp31
     bond-mode 802.3ad
     bond-miimon 100
-    bond-use-carrier 1
+    bond-use-carrier yes
     bond-lacp-rate 1
     bond-min-links 1
     bond-xmit-hash-policy layer3+4
@@ -47,7 +47,7 @@ iface downlink
     bond-slaves swp1
     bond-mode 802.3ad
     bond-miimon 100
-    bond-use-carrier 1
+    bond-use-carrier yes
     bond-lacp-rate 1
     bond-min-links 1
     bond-xmit-hash-policy layer3+4

docs/examples/vlan_aware_bridges/interfaces.with_clag

@@ -25,7 +25,7 @@ iface spine-bond
       bond-slaves glob swp19-22
       bond-mode 802.3ad
       bond-miimon 100
-      bond-use-carrier 1
+      bond-use-carrier yes
       bond-lacp-rate 1
       bond-min-links 1
       bond-xmit-hash-policy layer3+4
@@ -38,7 +38,7 @@ iface peer-bond
       bond-slaves glob swp23-24
       bond-mode 802.3ad
       bond-miimon 100
-      bond-use-carrier 1
+      bond-use-carrier yes
       bond-lacp-rate 1
       bond-min-links 1
       bond-xmit-hash-policy layer3+4
@@ -61,7 +61,7 @@ iface host-bond-01
       bond-slaves swp1
       bond-mode 802.3ad
       bond-miimon 100
-      bond-use-carrier 1
+      bond-use-carrier yes
       bond-lacp-rate 1
       bond-min-links 1
       bond-xmit-hash-policy layer3+4
@@ -72,7 +72,7 @@ iface host-bond-02
       bond-slaves swp2
       bond-mode 802.3ad
       bond-miimon 100
-      bond-use-carrier 1
+      bond-use-carrier yes
       bond-lacp-rate 1
       bond-min-links 1
       bond-xmit-hash-policy layer3+4

docs/source/userguide.rst

@@ -109,7 +109,7 @@ following example configuration::
         bond-slaves swp29 swp30
         bond-mode 802.3ad
         bond-miimon 100
-        bond-use-carrier 1
+        bond-use-carrier yes
         bond-lacp-rate 1
         bond-min-links 1
         bond-xmit-hash-policy layer3+4
@@ -120,7 +120,7 @@ following example configuration::
         bond-slaves swp31 swp32
         bond-mode 802.3ad
         bond-miimon 100
-        bond-use-carrier 1
+        bond-use-carrier yes
         bond-lacp-rate 1
         bond-min-links 1
         bond-xmit-hash-policy layer3+4
@@ -298,7 +298,7 @@ The contents of the sourced file used above are::
         bond-slaves swp25 swp26
         bond-mode 802.3ad
         bond-miimon 100
-        bond-use-carrier 1
+        bond-use-carrier yes
         bond-lacp-rate 1
         bond-min-links 1
         bond-xmit-hash-policy layer3+4
@@ -363,7 +363,7 @@ file, run::
         bond-slaves swp25 swp26
         bond-mode 802.3ad
         bond-miimon 100
-        bond-use-carrier 1
+        bond-use-carrier yes
         bond-lacp-rate 1
         bond-min-links 1
         bond-xmit-hash-policy layer3+4
@@ -379,7 +379,7 @@ does not match::
     iface bond0
             bond-mode 802.3ad  (✓)
             bond-miimon 100  (✓)
-            bond-use-carrier 1  (✓)
+            bond-use-carrier yes  (✓)
             bond-lacp-rate 1  (✓)
             bond-min-links 1  (✓)
             bond-xmit-hash-policy layer3+4  (✓)
@@ -417,10 +417,10 @@ the ``interfaces`` file. For complete syntax on the ``interfaces`` file, see
     {
         "auto": true, 
         "config": {
-            "bond-use-carrier": "1", 
+            "bond-use-carrier": "yes",
             "bond-xmit-hash-policy": "layer3+4", 
             "bond-miimon": "100", 
-            "bond-lacp-rate": "1", 
+            "bond-lacp-rate": "1",
             "bond-min-links": "1", 
             "bond-slaves": "swp25 swp26", 
             "bond-mode": "802.3ad", 

ifupdown/utils.py

@@ -28,6 +28,68 @@ class utils():
     logger = logging.getLogger('ifupdown')
     DEVNULL = open(os.devnull, 'w')
 
+    _string_values = {
+        "on": True,
+        "yes": True,
+        "1": True,
+        "off": False,
+        "no": False,
+        "0": False,
+    }
+
+    _binary_bool = {
+        True: "1",
+        False: "0",
+    }
+
+    _yesno_bool = {
+        True: 'yes',
+        False: 'no'
+    }
+
+    _onoff_bool = {
+        'yes': 'on',
+        'no': 'off'
+    }
+
+    @staticmethod
+    def get_onoff_bool(value):
+        if value in utils._onoff_bool:
+            return utils._onoff_bool[value]
+        return value
+
+    @staticmethod
+    def get_boolean_from_string(value):
+        if value in utils._string_values:
+            return utils._string_values[value]
+        return False
+
+    @staticmethod
+    def get_yesno_boolean(bool):
+        return utils._yesno_bool[bool]
+
+    @staticmethod
+    def boolean_support_binary(value):
+        return utils._binary_bool[utils.get_boolean_from_string(value)]
+
+    @staticmethod
+    def is_binary_bool(value):
+        return value == '0' or value == '1'
+
+    @staticmethod
+    def support_yesno_attrs(attrsdict, attrslist, ifaceobj=None):
+        if ifaceobj:
+            for attr in attrslist:
+                value = ifaceobj.get_attr_value_first(attr)
+                if value and not utils.is_binary_bool(value):
+                    if attr in attrsdict:
+                        bool = utils.get_boolean_from_string(attrsdict[attr])
+                        attrsdict[attr] = utils.get_yesno_boolean(bool)
+        else:
+            for attr in attrslist:
+                if attr in attrsdict:
+                    attrsdict[attr] = utils.boolean_support_binary(attrsdict[attr])
+
     @classmethod
     def importName(cls, modulename, name):
         """ Import a named object """

man.rst/ifupdown-addons-interfaces.5.rst

@@ -58,12 +58,12 @@ EXAMPLES
 
         **required**: False
 
-        **default**: off
+        **default**: no
 
-        **validvals**: on,off
+        **validvals**: yes,no
 
         **example**:
-            link-autoneg on
+            link-autoneg yes
 
 
       **link-speed**
@@ -182,10 +182,12 @@ EXAMPLES
 
         **required**: False
 
-        **default**: 0
+        **default**: no
+
+        **validvals**: yes,no
 
         **example**:
-            bridge-mcquerier 0
+            bridge-mcquerier no
 
 
       **bridge-mclmc**
@@ -221,10 +223,12 @@ EXAMPLES
 
         **required**: False
 
-        **default**: 1
+        **default**: yes
+
+        **validvals**: yes,no
 
         **example**:
-            bridge-mcrouter 1
+            bridge-mcrouter yes
 
 
       **bridge-stp**
@@ -333,10 +337,12 @@ EXAMPLES
 
         **required**: False
 
-        **default**: 0
+        **default**: no
+
+        **validvals**: yes,no
 
         **example**:
-            bridge-mcqifaddr 0
+            bridge-mcqifaddr no
 
 
       **bridge-waitport**
@@ -441,10 +447,12 @@ EXAMPLES
 
         **required**: False
 
-        **default**: 1
+        **default**: yes
+
+        **validvals**: yes,no
 
         **example**:
-            bridge-mcsnoop 1
+            bridge-mcsnoop yes
 
 
       **bridge-access**
@@ -978,12 +986,12 @@ EXAMPLES
 
         **required**: False
 
-        **default**: 1
+        **default**: yes
 
-        **validvals**: 0,1
+        **validvals**: yes,no
 
         **example**:
-            bond-use-carrier 1
+            bond-use-carrier yes
 
 
       **bond-lacp-bypass-period**
@@ -1077,27 +1085,12 @@ EXAMPLES
 
         **required**: False
 
-        **default**: 0
+        **default**: no
 
-        **validvals**: 0,1
+        **validvals**: yes,no
 
         **example**:
-            bond-lacp-bypass-allow 0
+            bond-lacp-bypass-allow no
-
-
-      **bond-lacp-bypass-allow-all-active**
-
-        **help**: allow all slaves to be active in lacp bypass irrespective of priority
-
-
-        **required**: False
-
-        **default**: 0
-
-        **validvals**: 0,1
-
-        **example**:
-            bond-lacp-bypass-all-active 1
 
 
       **bond-mode**
@@ -1323,15 +1316,17 @@ EXAMPLES
 
       **vxlan-learning**
 
-        **help**: vxlan learning on/off
+        **help**: vxlan learning yes/no
 
 
         **required**: False
 
-        **default**: on
+        **default**: yes
+
+        **validvals**: yes,no
 
         **example**:
-            vxlan-learning off
+            vxlan-learning no
 
 
       **vxlan-id**