ifupdownaddons: mstpctl: treeportprio value is cached. It prevents ifupdown2 from...

Ticket: CM-11773
Reviewed By: Roopa, Nikhil G
Testing Done: smoke + hand testings :

$ cat /etc/network/interfaces
auto br2
iface br2
      bridge-vlan-aware yes
      bridge-vids 100
      bridge-pvid  1
      bridge-ports swp1 swp4 swp5
      bridge-stp on
$ ifreload -a -v
[...]
info: netlink: set link swp1 up
info: netlink: set link swp4 up
info: netlink: set link swp5 up
info: br2: applying mstp configuration specific to ports
info: br2: processing mstp config for port swp1
info: executing /sbin/mstpctl showportdetail br2 json
info: executing /sbin/mstpctl settreeportprio br2 swp1 0 128
info: br2: processing mstp config for port swp4
info: executing /sbin/mstpctl settreeportprio br2 swp4 0 128
info: br2: processing mstp config for port swp5
info: executing /sbin/mstpctl settreeportprio br2 swp5 0 128
info: netlink: set link br2 up
[...]
$

/sbin/mstpctl settreeportprio br2 swpX 0 128 is executed for every port.
ifupdown2 tries to set this attr to 128 (the default value). This is a
problem at scale. We shouldn't try to set this value if the running value
is already 128.

With the attached patch:

$ ifreload -a
[...]
info: netlink: set link swp1 up
info: netlink: set link swp4 up
info: netlink: set link swp5 up
info: executing /sbin/mstpctl showportdetail br2 json
info: br2: applying mstp configuration specific to ports
info: br2: processing mstp config for port swp1
info: br2: processing mstp config for port swp4
info: br2: processing mstp config for port swp5
info: netlink: set link br2 up
[...]
$

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

diff --git a/ifupdownaddons/mstpctlutil.py b/ifupdownaddons/mstpctlutil.py
index 115626d890fda481ca8f01a014a3d6a7d012e915..468244feef5ff83ec37286bb5f7cea3ebb29c3b0 100644 (file)
--- a/ifupdownaddons/mstpctlutil.py
+++ b/ifupdownaddons/mstpctlutil.py
@@ -16,6 +16,8 @@ class mstpctlutil(utilsBase):
     """ This class contains helper methods to interact with mstpd using
     mstputils commands """
 
+    _DEFAULT_PORT_PRIO = '128'
+
     _cache_fill_done = False
 
     _bridgeattrmap = {'bridgeid' : 'bridge-id',
@@ -70,6 +72,13 @@ class mstpctlutil(utilsBase):
             pass
         return bridgeattrs
 
+    def _extract_bridge_port_prio(self, portid):
+        try:
+            return str(int(portid[0], 16) * 16)
+        except:
+            pass
+        return mstpctlutil._DEFAULT_PORT_PRIO
+
     def _get_mstpctl_bridgeport_attr_from_cache(self, bridgename):
         attrs = MSTPAttrsCache.get(bridgename)
         if not attrs:
@@ -89,6 +98,7 @@ class mstpctlutil(utilsBase):
                     # by bridgename, portname, and json attribute
                     for portid in mstpctl_bridge_cache[portname].keys():
                         mstpctl_bridgeport_attrs_dict[portname] = {}
+                        mstpctl_bridgeport_attrs_dict[portname]['treeportprio'] = self._extract_bridge_port_prio(portid)
                         for jsonAttr in mstpctl_bridge_cache[portname][portid].keys():
                             jsonVal = mstpctl_bridge_cache[portname][portid][jsonAttr]
                             mstpctl_bridgeport_attrs_dict[portname][jsonAttr] = str(jsonVal)
@@ -119,11 +129,26 @@ class mstpctlutil(utilsBase):
             except Exception, e:
                 self.logger.warn(str(e))
 
+    def _get_bridge_port_attr_with_prio(self,
+                                        bridgename,
+                                        bridgeportname,
+                                        attrname):
+        attrvalue_curr = self.get_bridgeport_attr(bridgename,
+                                                  bridgeportname, attrname)
+        if attrname == 'treeportprio':
+            try:
+                attrs = self._get_mstpctl_bridgeport_attr_from_cache(bridgename)
+                attrvalue_curr = attrs[bridgeportname]['treeportprio']
+            except:
+                pass
+        return attrvalue_curr
+
     def set_bridgeport_attr(self, bridgename, bridgeportname, attrname,
                             attrvalue, check=True):
         if check:
-            attrvalue_curr = self.get_bridgeport_attr(bridgename,
-                                    bridgeportname, attrname)
+            attrvalue_curr = self._get_bridge_port_attr_with_prio(bridgename,
+                                                                  bridgeportname,
+                                                                  attrname)
             if attrvalue_curr and attrvalue_curr == attrvalue:
                 return
         if attrname == 'treeportcost' or attrname == 'treeportprio':