From 384123406982efce4a209d9a51cc51fbc2ee0f4f Mon Sep 17 00:00:00 2001
From: Julien Fortin <julien@cumulusnetworks.com>
Date: Mon, 12 Dec 2016 07:34:43 +0100
Subject: [PATCH 1/8] bondutils: caching min_links value

Ticket: CM-13996
Reviewed By: Roopa, Nikhil G
Testing Done:

With the following configuration:
auto bond0
iface bond0
      bond-min-links	1
      bond-mode	802.3ad
      bond-slaves	eth0 eth1 eth2
      bond-xmit-hash-policy	layer3+4

auto vlan0
iface vlan0
      vlan-raw-device   bond0
      address 10.132.253.4/31
      address 2a03:2260:2342:fe09::1/126

On non cumulus distribution bond-min-links doesn't default to 1
For some reasons the min_links value wasn't cache with the other
bond values, if you issue an ifreload on a running/existing configuration
since the min_links value is not cache ifreload will down the bond, set
min_links to 1, then bond up. When taking the bond down the kernel will
also flush the ipv6 address but not the ipv4 address...

The issue was reported by an ifupdown2 contributor on github. He find out
that when running ifreload the ipv6 were flushed.

Signed-off-by: Julien Fortin <julien@cumulusnetworks.com>
---
 ifupdownaddons/bondutil.py | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/ifupdownaddons/bondutil.py b/ifupdownaddons/bondutil.py
index 366adfa..5cc64eb 100644
--- a/ifupdownaddons/bondutil.py
+++ b/ifupdownaddons/bondutil.py
@@ -31,36 +31,64 @@ class bondutil(utilsBase):
         except:
             linkCache.links[bondname] = {'linkinfo': {}}
 
+        try:
+            linkCache.set_attr([bondname, 'linkinfo', 'min_links'],
+                               self.read_file_oneline(
+                                   '/sys/class/net/%s/bonding/min_links'
+                                   % bondname))
+        except Exception as e:
+            self.logger.debug(str(e))
         try:
             linkCache.set_attr([bondname, 'linkinfo', 'slaves'],
                 self.read_file_oneline('/sys/class/net/%s/bonding/slaves'
                 %bondname).split())
+        except Exception as e:
+            self.logger.debug(str(e))
+        try:
             linkCache.set_attr([bondname, 'linkinfo', 'mode'],
                 self.read_file_oneline('/sys/class/net/%s/bonding/mode'
                 %bondname).split()[0])
+        except Exception as e:
+            self.logger.debug(str(e))
+        try:
             linkCache.set_attr([bondname, 'linkinfo', 'xmit_hash_policy'],
                 self.read_file_oneline(
                     '/sys/class/net/%s/bonding/xmit_hash_policy'
                     %bondname).split()[0])
+        except Exception as e:
+            self.logger.debug(str(e))
+        try:
             linkCache.set_attr([bondname, 'linkinfo', 'lacp_rate'],
                 self.read_file_oneline('/sys/class/net/%s/bonding/lacp_rate'
                                        %bondname).split()[1])
+        except Exception as e:
+            self.logger.debug(str(e))
+        try:
             linkCache.set_attr([bondname, 'linkinfo', 'ad_actor_sys_prio'],
                 self.read_file_oneline('/sys/class/net/%s/bonding/ad_actor_sys_prio'
                                        %bondname))
+        except Exception as e:
+            self.logger.debug(str(e))
+        try:
             linkCache.set_attr([bondname, 'linkinfo', 'ad_actor_system'],
                 self.read_file_oneline('/sys/class/net/%s/bonding/ad_actor_system'
                                        %bondname))
+        except Exception as e:
+            self.logger.debug(str(e))
+        try:
             linkCache.set_attr([bondname, 'linkinfo', 'lacp_bypass'],
                 self.read_file_oneline('/sys/class/net/%s/bonding/lacp_bypass'
                                        %bondname).split()[1])
+        except Exception as e:
+            self.logger.debug(str(e))
+        try:
             map(lambda x: linkCache.set_attr([bondname, 'linkinfo', x],
                    self.read_file_oneline('/sys/class/net/%s/bonding/%s'
                         %(bondname, x))),
                        ['use_carrier', 'miimon', 'min_links', 'num_unsol_na',
                         'num_grat_arp'])
-        except Exception, e:
-            pass
+        except Exception as e:
+            self.logger.debug(str(e))
 
     def _bond_linkinfo_fill_all(self):
         bondstr = self.read_file_oneline('/sys/class/net/bonding_masters')

From e3089aa89805a734162e2b01672c75f7cd8a2e3f Mon Sep 17 00:00:00 2001
From: Nikhil <nikhil@cumulusnetworks.com>
Date: Wed, 7 Dec 2016 09:53:33 -0800
Subject: [PATCH 2/8] addons: mstpctl: reset (default) bridge mstpctl options
 when users remove settings

Ticket: CM-8401
Reviewed By: Roopa, Julien
Testing Done: tested on all bridge mstpctl attributes.

This patch resets th following bridge attributes to defauls when
users remove settings from interface config file.

mstpctl-treeprio
mstpctl-ageing
mstpctl-fdelay
mstpctl-maxhops
mstpctl-maxage
mstpctl-txholdcount
mstpctl-forcevers
mstpctl-hello

Added an api in policy manager to get policy default value of any
module attribute.

Added a cache for bridge attributes to save some runtime

Signed-off-by: Nikhil <nikhil@cumulusnetworks.com>
---
 addons/mstpctl.py             | 33 ++++++++++---
 ifupdown/policymanager.py     |  1 +
 ifupdownaddons/mstpctlutil.py | 89 +++++++++++++++++++++++++----------
 3 files changed, 90 insertions(+), 33 deletions(-)

diff --git a/addons/mstpctl.py b/addons/mstpctl.py
index 54f659d..707103e 100644
--- a/addons/mstpctl.py
+++ b/addons/mstpctl.py
@@ -144,7 +144,7 @@ class mstpctl(moduleBase):
                           'validrange' : ['0', '255'],
                           'default' : '2',
                           'required' : False,
-                          'jsonAttr': 'portHelloTime',
+                          'jsonAttr': 'helloTime',
                           'example' : ['mstpctl-hello 2']},
                     'mstpctl-portnetwork' : 
                         { 'help' : 'enable/disable bridge assurance capability for a port',
@@ -329,16 +329,32 @@ class mstpctl(moduleBase):
         try:
             # set bridge attributes
             for attrname, dstattrname in self._attrs_map.items():
+                config_val = ifaceobj.get_attr_value_first(attrname)
+                default_val = policymanager.policymanager_api.get_iface_default(module_name=self.__class__.__name__, ifname=ifaceobj.name, attr=attrname)
+                if not default_val:
+                    default_val = self.get_mod_subattr(attrname,'default')
+                jsonAttr =  self.get_mod_subattr(attrname, 'jsonAttr')
+                try:
+                    running_val = self.mstpctlcmd.get_bridge_attr(
+                                    ifaceobj.name, jsonAttr)
+                except:
+                    self.logger.info('%s: could not get running %s value'
+                                     %(ifaceobj.name, attrname))
+                    running_val = None
+                if (not config_val and default_val and (running_val != default_val)):
+                    # this happens when users remove an attribute from a port
+                    # and expect the default to be restored with ifreload.
+                    config_val = default_val
+                elif not config_val:
+                    # there is nothing configured and no default to reset
+                    continue
                 try:
-                    v = ifaceobj.get_attr_value_first(attrname)
-                    if not v:
-                       continue
                     if attrname == 'mstpctl-treeprio':
                        self.mstpctlcmd.set_bridge_treeprio(ifaceobj.name,
-                                v, check)
+                                config_val, check)
                     else:
                        self.mstpctlcmd.set_bridge_attr(ifaceobj.name,
-                                dstattrname, v, check)
+                                dstattrname, config_val, check)
                 except Exception, e:
                     self.logger.warn('%s' %str(e))
                     pass
@@ -415,7 +431,10 @@ class mstpctl(moduleBase):
             else:
                 return 'yes'
         else:
-            return self.get_mod_subattr(attr,'default')
+            default_val = policymanager.policymanager_api.get_iface_default(module_name=self.__class__.__name__, ifname=ifaceobj.name, attr=attr)
+            if not default_val:
+                return self.get_mod_subattr(attr,'default')
+            return default_val
 
     def _apply_bridge_port_settings(self, ifaceobj, bridgename=None,
                                     bridgeifaceobj=None,
diff --git a/ifupdown/policymanager.py b/ifupdown/policymanager.py
index b5f3123..c624c18 100644
--- a/ifupdown/policymanager.py
+++ b/ifupdown/policymanager.py
@@ -210,4 +210,5 @@ class policymanager():
 
         return mod_array
 
+
 policymanager_api = policymanager()
diff --git a/ifupdownaddons/mstpctlutil.py b/ifupdownaddons/mstpctlutil.py
index 1b069b5..c9dcee9 100644
--- a/ifupdownaddons/mstpctlutil.py
+++ b/ifupdownaddons/mstpctlutil.py
@@ -28,6 +28,17 @@ class mstpctlutil(utilsBase):
                      'hello' : 'hello-time',
                      'forcevers' : 'force-protocol-version'}
 
+    _bridge_jsonAttr_map = {
+                            'treeprio': 'bridgeId',
+                            'maxage': 'maxAge',
+                            'fdelay': 'fwdDelay',
+                            'txholdcount': 'txHoldCounter',
+                            'maxhops': 'maxHops',
+                            'ageing': 'ageingTime',
+                            'hello': 'helloTime',
+                            'forcevers': 'forceProtocolVersion',
+                            }
+
     _bridgeportattrmap = {'portadminedge' : 'admin-edge-port',
                      'portp2p' : 'admin-point-to-point',
                      'portrestrrole' : 'restricted-role',
@@ -83,6 +94,33 @@ class mstpctlutil(utilsBase):
             self.logger.info('%s: cannot fetch mstpctl bridge port attributes: %s' % str(e))
         return mstpctl_bridgeport_attrs_dict
 
+    def _get_bridge_attrs_from_cache(self, bridgename):
+        attrs = MSTPAttrsCache.get(bridgename)
+        if attrs:
+            return attrs
+        mstpctl_bridge_attrs_dict = {}
+        try:
+            cmd = ['/sbin/mstpctl', 'showbridge', 'json', bridgename]
+            output = utils.exec_commandl(cmd)
+            if not output:
+                return mstpctl_bridge_attrs_dict
+        except Exception as e:
+            self.logger.info(str(e))
+            return mstpctl_bridge_attrs_dict
+        try:
+            mstpctl_bridge_cache = json.loads(output.strip('\n'))
+            for jsonAttr in mstpctl_bridge_cache[bridgename].keys():
+                mstpctl_bridge_attrs_dict[jsonAttr] = (
+                    str(mstpctl_bridge_cache[bridgename][jsonAttr]))
+            mstpctl_bridge_attrs_dict['treeprio'] = '%d' %(
+                                   int(mstpctl_bridge_attrs_dict.get('bridgeId',
+                                   '').split('.')[0], base=16) * 4096)
+            del mstpctl_bridge_attrs_dict['bridgeId']
+            MSTPAttrsCache.set(bridgename, mstpctl_bridge_attrs_dict)
+        except Exception as e:
+            self.logger.info('%s: cannot fetch mstpctl bridge attributes: %s' % str(e))
+        return mstpctl_bridge_attrs_dict
+
     def get_bridge_ports_attrs(self, bridgename):
         return self._get_bridge_port_attrs_from_cache(bridgename)
 
@@ -93,7 +131,7 @@ class mstpctlutil(utilsBase):
             return 'yes'
         return str(value)
 
-    def update_cache(self, bridgename, portname, attrname, value):
+    def update_bridge_port_cache(self, bridgename, portname, attrname, value):
         attrs = self.get_bridge_ports_attrs(bridgename)
         if not attrs:
             attrs = {}
@@ -102,6 +140,13 @@ class mstpctlutil(utilsBase):
         attrs[portname][attrname] = value
         MSTPAttrsCache.set(bridgename, attrs)
 
+    def update_bridge_cache(self, bridgename, attrname, value):
+        attrs = self.get_bridge_ports_attrs(bridgename)
+        if not attrs:
+            attrs = {}
+        attrs[attrname] = value
+        MSTPAttrsCache.set(bridgename, attrs)
+
     def set_bridge_port_attr(self, bridgename, portname, attrname, value, json_attr=None):
         cache_value = self.get_bridge_port_attr(bridgename, portname, json_attr)
         if cache_value and cache_value == value:
@@ -113,16 +158,13 @@ class mstpctlutil(utilsBase):
             utils.exec_commandl(['/sbin/mstpctl', 'set%s' % attrname,
                                  bridgename, portname, value])
         if json_attr:
-            self.update_cache(bridgename, portname, json_attr, value)
+            self.update_bridge_port_cache(bridgename, portname, json_attr, value)
 
     def get_bridge_attrs(self, bridgename):
         bridgeattrs = {}
         try:
-            bridgeattrs = dict((k, self.get_bridge_attr(bridgename, k))
-                                 for k in self._bridgeattrmap.keys())
-            bridgeattrs['treeprio'] = '%d' %(int(bridgeattrs.get('bridgeid',
-                                     '').split('.')[0], base=16) * 4096)
-            del bridgeattrs['bridgeid']
+            bridgeattrs = dict((k, self.get_bridge_attr(bridgename, v))
+                                 for k,v in self._bridge_jsonAttr_map.items())
         except Exception, e:
             self.logger.debug(bridgeattrs)
             self.logger.debug(str(e))
@@ -130,28 +172,32 @@ class mstpctlutil(utilsBase):
         return bridgeattrs
 
     def get_bridge_attr(self, bridgename, attrname):
-        try:
-            cmdl = ['/sbin/mstpctl', 'showbridge', bridgename,
-                    self._bridgeattrmap[attrname]]
-            return utils.exec_commandl(cmdl).strip('\n')
-        except Exception, e:
-            pass
-        return None
+        if attrname == 'bridgeId':
+            attrname = 'treeprio'
+        return self._get_bridge_attrs_from_cache(bridgename).get(attrname)
 
     def set_bridge_attr(self, bridgename, attrname, attrvalue, check=True):
 
         if check:
-            attrvalue_curr = self.get_bridge_attr(bridgename, attrname)
+            if attrname == 'treeprio':
+                attrvalue_curr = self.get_bridge_attr(bridgename, attrname)
+            else:
+                attrvalue_curr = self.get_bridge_attr(bridgename,
+                                        self._bridge_jsonAttr_map[attrname])
             if attrvalue_curr and attrvalue_curr == attrvalue:
                 return
         if attrname == 'treeprio':
             utils.exec_commandl(['/sbin/mstpctl', 'set%s' % attrname,
                                  '%s' % bridgename, '0', '%s' % attrvalue],
                                 stdout=False, stderr=None)
+            self.update_bridge_cache(bridgename, attrname, str(attrvalue))
         else:
             utils.exec_commandl(['/sbin/mstpctl', 'set%s' % attrname,
                                  '%s' % bridgename, '%s' % attrvalue],
                                 stdout=False, stderr=None)
+            self.update_bridge_cache(bridgename,
+                                     self._bridge_jsonAttr_map[attrname],
+                                     str(attrvalue))
 
     def set_bridge_attrs(self, bridgename, attrdict, check=True):
         for k, v in attrdict.iteritems():
@@ -163,17 +209,7 @@ class mstpctlutil(utilsBase):
                 self.logger.warn('%s: %s' %(bridgename, str(e)))
 
     def get_bridge_treeprio(self, bridgename):
-        try:
-            cmdl = ['/sbin/mstpctl',
-                    'showbridge',
-                    bridgename,
-                    self._bridgeattrmap['bridgeid']]
-
-            bridgeid = utils.exec_commandl(cmdl).strip('\n')
-            return '%d' %(int(bridgeid.split('.')[0], base=16) * 4096)
-        except:
-            pass
-        return None
+        return self.get_bridge_attr(bridgename, 'treeprio')
 
     def set_bridge_treeprio(self, bridgename, attrvalue, check=True):
         if check:
@@ -182,6 +218,7 @@ class mstpctlutil(utilsBase):
                 return
         utils.exec_commandl(['/sbin/mstpctl', 'settreeprio', bridgename, '0',
                              str(attrvalue)])
+        self.update_bridge_cache(bridgename, 'treeprio', str(attrvalue))
 
     def showbridge(self, bridgename=None):
         if bridgename:

From 3fc3c01ba614d6210e4cc7426aa278688c88e5d4 Mon Sep 17 00:00:00 2001
From: Julien Fortin <julien@cumulusnetworks.com>
Date: Tue, 27 Dec 2016 17:03:19 +0100
Subject: [PATCH 3/8] addons: address: purge ifalias when no alias keyword is
 provided

Ticket: CM-13044
Reviewed By: Roopa, Nikhil G, Daniel W,
Testing Done:

For some reason we can't simply write into a file when we want to purge the
ifalias, we have to exec a command. I wasn't able to make it work in any
other way.

add an alias to an interface, ifreload, ip link show interface
modify it, ifreload, ip link show interface
remove it, ifreload, ip link show interface

Signed-off-by: Julien Fortin <julien@cumulusnetworks.com>
---
 addons/address.py          | 11 ++++++++---
 ifupdownaddons/iproute2.py |  7 +++++--
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/addons/address.py b/addons/address.py
index aa86676..fc9fa89 100644
--- a/addons/address.py
+++ b/addons/address.py
@@ -446,6 +446,14 @@ class address(moduleBase):
     def _up(self, ifaceobj, ifaceobj_getfunc=None):
         if not self.ipcmd.link_exists(ifaceobj.name):
             return
+
+        alias = ifaceobj.get_attr_value_first('alias')
+        current_alias = self.ipcmd.link_get_alias(ifaceobj.name)
+        if alias and alias != current_alias:
+            self.ipcmd.link_set_alias(ifaceobj.name, alias)
+        elif not alias and current_alias:
+            self.ipcmd.link_set_alias(ifaceobj.name, '')
+
         addr_method = ifaceobj.addr_method
         force_reapply = False
         try:
@@ -472,9 +480,6 @@ class address(moduleBase):
                                       force_reapply)
         self._process_mtu_config(ifaceobj, ifaceobj_getfunc)
 
-        alias = ifaceobj.get_attr_value_first('alias')
-        if alias:
-           self.ipcmd.link_set_alias(ifaceobj.name, alias)
         try:
             self.ipcmd.batch_commit()
         except Exception as e:
diff --git a/ifupdownaddons/iproute2.py b/ifupdownaddons/iproute2.py
index a7aa8ac..72eda66 100644
--- a/ifupdownaddons/iproute2.py
+++ b/ifupdownaddons/iproute2.py
@@ -477,8 +477,11 @@ class iproute2(utilsBase):
         self._cache_update([ifacename, 'mtu'], mtu)
 
     def link_set_alias(self, ifacename, alias):
-        utils.exec_commandl(['ip', 'link', 'set', 'dev', ifacename,
-                             'alias', alias])
+        if not alias:
+            utils.exec_user_command('echo "" > /sys/class/net/%s/ifalias'
+                                    % ifacename)
+        else:
+            self.write_file('/sys/class/net/%s/ifalias' % ifacename, alias)
 
     def link_get_alias(self, ifacename):
         return self.read_file_oneline('/sys/class/net/%s/ifalias'

From bea8c8d105d12403c13c2bf5f17ec7143a2fe813 Mon Sep 17 00:00:00 2001
From: Julien Fortin <julien@cumulusnetworks.com>
Date: Tue, 17 Jan 2017 15:49:23 +0300
Subject: [PATCH 4/8] debian: ifupdown2.postinst: refine 'iface lo' regex

Ticket: CM-14362
Reviewed By: Roopa, Nikhil G
Testing Done:

Signed-off-by: Julien Fortin <julien@cumulusnetworks.com>
---
 debian/ifupdown2.postinst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/ifupdown2.postinst b/debian/ifupdown2.postinst
index fa40daf..19365db 100644
--- a/debian/ifupdown2.postinst
+++ b/debian/ifupdown2.postinst
@@ -56,7 +56,7 @@ case "$1" in
 
         # Generic stuff done on all configurations
         if [ -f /etc/network/interfaces ] ; then
-            if ! grep -q "^[[:space:]]*iface[[:space:]]\+lo0\?[[:space:]]\+inet[[:space:]]\+loopback\>" /etc/network/interfaces ; then
+            if ! grep -q -E "^[[:space:]]*iface[[:space:]]+l[o0]([[:space:]]+inet([[:space:]]+loopback)?)?[[:space:]]*$" /etc/network/interfaces ; then
                 report_warn "No 'iface lo' definition found in /etc/network/interfaces"
             fi
 

From 65beb82662576c047a281389bd663589dcba09db Mon Sep 17 00:00:00 2001
From: Julien Fortin <julien@cumulusnetworks.com>
Date: Thu, 19 Jan 2017 12:22:44 +0300
Subject: [PATCH 5/8] ifupdownaddons: bridgeutils: uncomment parsing code for
 brctl mc attributes

Ticket: None
Reviewed By: Roopa, Nikhil G
Testing Done: ssim ifupdown2-tests

runtests.sh -T ~/result_smoke/single/ifupdown2-tests.log -r ~/result_smoke/single/summary_ifupdown2tests.txt -d cel-redxp-06 -t nightly -l INFO -k $VM_BASE_KERNEL -i $VM_BASE_IMG $TESTS_HOME/tests/smoke/testOneConfigifupdown2.py:Testifupdown2OneConfig

Signed-off-by: Julien Fortin <julien@cumulusnetworks.com>
---
 ifupdownaddons/bridgeutils.py | 43 +++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/ifupdownaddons/bridgeutils.py b/ifupdownaddons/bridgeutils.py
index 159a87f..ed850fa 100644
--- a/ifupdownaddons/bridgeutils.py
+++ b/ifupdownaddons/bridgeutils.py
@@ -115,15 +115,40 @@ class brctl(utilsBase):
             except:
                 pass
 
-            # XXX: comment this out until mc attributes become available
-            # with brctl again
-            #battrs['hashel'] = broutlines[10].split('hash elasticity')[1].split()[0].strip()
-            #battrs['hashmax'] = broutlines[10].split('hash max')[1].strip()
-            #battrs['mclmc'] = broutlines[11].split('mc last member count')[1].split()[0].strip()
-            #battrs['mciqc'] = broutlines[11].split('mc init query count')[1].strip()
-            #battrs['mcrouter'] = broutlines[12].split('mc router')[1].split()[0].strip()
-            ##battrs['mcsnoop'] = broutlines[12].split('mc snooping')[1].strip()
-            #battrs['mclmt'] = broutlines[13].split('mc last member timer')[1].split()[0].strip()
+            try:
+                battrs['hashel'] = broutlines[10].split('hash elasticity')[1].split()[0].strip()
+            except:
+                pass
+
+            try:
+                battrs['hashmax'] = broutlines[10].split('hash max')[1].strip()
+            except:
+                pass
+
+            try:
+                battrs['mclmc'] = broutlines[11].split('mc last member count')[1].split()[0].strip()
+            except:
+                pass
+
+            try:
+                battrs['mciqc'] = broutlines[11].split('mc init query count')[1].strip()
+            except:
+                pass
+
+            try:
+                battrs['mcrouter'] = broutlines[12].split('mc router')[1].split()[0].strip()
+            except:
+                pass
+
+            try:
+                battrs['mcsnoop'] = broutlines[12].split('mc snooping')[1].strip()
+            except:
+                pass
+
+            try:
+                battrs['mclmt'] = broutlines[13].split('mc last member timer')[1].split()[0].strip().replace('.00', '')
+            except:
+                pass
         except Exception, e:
             self.logger.warn('%s: error while processing bridge attributes: %s' % (bridgename, str(e)))
             pass

From d03b0695e0fa6822f70e1671bc1292eed117c15a Mon Sep 17 00:00:00 2001
From: Nikhil <nikhil@cumulusnetworks.com>
Date: Thu, 5 Jan 2017 14:41:42 -0800
Subject: [PATCH 6/8] addons: mstpctl: ifquery -c --with-default, ignore
 mstpctl default attributes when stp is off

Ticket: CM-13779
Reviewed By: roopa, satish, julien
Testing Done: testing the config given in the CM

mstpctl showportdetail <bridge> command won't output anything when
bridge-stp is off, therefore ignore mstpctl default attributes during
ifquery -c --with-defaults

This patch also consistently updates bridge and bridge port cache
at the same time.
Earlier bridge and bridge port cache were not consistent
because of the early return condition
    attrs = MSTPAttrsCache.get(bridgename)
        if attrs:
            return attrs
If either of bridge port cache and bridge cache is updated, function used to
return inconsistent cache values

Signed-off-by: Nikhil Gajendrakumar <nikhil@cumulusnetworks.com>
---
 addons/mstpctl.py             | 20 ++++++++++++++++++++
 ifupdownaddons/mstpctlutil.py | 17 ++++++-----------
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/addons/mstpctl.py b/addons/mstpctl.py
index 707103e..0cfb70f 100644
--- a/addons/mstpctl.py
+++ b/addons/mstpctl.py
@@ -679,6 +679,18 @@ class mstpctl(moduleBase):
                                     if v})
         return bridgeattrdict
 
+    def _get_config_stp(self, ifaceobj):
+        stp = (ifaceobj.get_attr_value_first('mstpctl-stp') or
+               ifaceobj.get_attr_value_first('bridge-stp') or
+               policymanager.policymanager_api.get_iface_default(module_name=self.__class__.__name__, ifname=ifaceobj.name, attr='mstpctl-stp') or
+               # this is a temporary method to access policy default value of bridge-stp
+               policymanager.policymanager_api.get_iface_default(module_name='bridge', ifname=ifaceobj.name, attr='bridge-stp'))
+        return utils.get_boolean_from_string(stp)
+
+    def _get_running_stp(self, ifaceobj):
+        stp = self.brctlcmd.get_stp(ifaceobj.name)
+        return utils.get_boolean_from_string(stp)
+
     def _query_check_bridge(self, ifaceobj, ifaceobjcurr,
                             ifaceobj_getfunc=None):
         # list of attributes that are not supported currently
@@ -699,6 +711,8 @@ class mstpctl(moduleBase):
         #self.logger.info('B' + str(runningattrs))
         if not runningattrs:
             runningattrs = {}
+        config_stp = self._get_config_stp(ifaceobj)
+        running_stp = self._get_running_stp(ifaceobj)
         running_port_list = self.brctlcmd.get_bridge_ports(ifaceobj.name)
         for k in ifaceattrs:
             # for all mstpctl options
@@ -710,6 +724,8 @@ class mstpctl(moduleBase):
                 #unaware bridge
                 if not running_port_list:
                     continue
+                if (not config_stp or not running_stp):
+                    continue
                 v = ifaceobj.get_attr_value_first(k)
                 config_val = {}
                 running_val = {}
@@ -838,6 +854,10 @@ class mstpctl(moduleBase):
                 if (self._is_bridge(bifaceobj) and
                     self.default_vxlan_ports_set_bpduparams and
                     (bifaceobj.link_privflags & ifaceLinkPrivFlags.BRIDGE_VLAN_AWARE)):
+                        config_stp = self._get_config_stp(bifaceobj)
+                        running_stp = self._get_running_stp(bifaceobj)
+                        if (not config_stp or not running_stp):
+                            continue
                         for attr in ['mstpctl-portbpdufilter',
                                      'mstpctl-bpduguard']:
                             jsonAttr =  self.get_mod_subattr(attr, 'jsonAttr')
diff --git a/ifupdownaddons/mstpctlutil.py b/ifupdownaddons/mstpctlutil.py
index c9dcee9..1452bd4 100644
--- a/ifupdownaddons/mstpctlutil.py
+++ b/ifupdownaddons/mstpctlutil.py
@@ -67,7 +67,7 @@ class mstpctlutil(utilsBase):
         except:
             return mstpctlutil._DEFAULT_PORT_PRIO
 
-    def _get_bridge_port_attrs_from_cache(self, bridgename):
+    def _get_bridge_and_port_attrs_from_cache(self, bridgename):
         attrs = MSTPAttrsCache.get(bridgename)
         if attrs:
             return attrs
@@ -92,12 +92,7 @@ class mstpctlutil(utilsBase):
             MSTPAttrsCache.set(bridgename, mstpctl_bridgeport_attrs_dict)
         except Exception as e:
             self.logger.info('%s: cannot fetch mstpctl bridge port attributes: %s' % str(e))
-        return mstpctl_bridgeport_attrs_dict
 
-    def _get_bridge_attrs_from_cache(self, bridgename):
-        attrs = MSTPAttrsCache.get(bridgename)
-        if attrs:
-            return attrs
         mstpctl_bridge_attrs_dict = {}
         try:
             cmd = ['/sbin/mstpctl', 'showbridge', 'json', bridgename]
@@ -116,16 +111,16 @@ class mstpctlutil(utilsBase):
                                    int(mstpctl_bridge_attrs_dict.get('bridgeId',
                                    '').split('.')[0], base=16) * 4096)
             del mstpctl_bridge_attrs_dict['bridgeId']
-            MSTPAttrsCache.set(bridgename, mstpctl_bridge_attrs_dict)
+            MSTPAttrsCache.bridges[bridgename].update(mstpctl_bridge_attrs_dict)
         except Exception as e:
             self.logger.info('%s: cannot fetch mstpctl bridge attributes: %s' % str(e))
-        return mstpctl_bridge_attrs_dict
+        return MSTPAttrsCache.get(bridgename)
 
     def get_bridge_ports_attrs(self, bridgename):
-        return self._get_bridge_port_attrs_from_cache(bridgename)
+        return self._get_bridge_and_port_attrs_from_cache(bridgename)
 
     def get_bridge_port_attr(self, bridgename, portname, attrname):
-        attrs = self._get_bridge_port_attrs_from_cache(bridgename)
+        attrs = self._get_bridge_and_port_attrs_from_cache(bridgename)
         value = attrs.get(portname, {}).get(attrname, 'no')
         if value == 'True' or value == 'true':
             return 'yes'
@@ -174,7 +169,7 @@ class mstpctlutil(utilsBase):
     def get_bridge_attr(self, bridgename, attrname):
         if attrname == 'bridgeId':
             attrname = 'treeprio'
-        return self._get_bridge_attrs_from_cache(bridgename).get(attrname)
+        return self._get_bridge_and_port_attrs_from_cache(bridgename).get(attrname)
 
     def set_bridge_attr(self, bridgename, attrname, attrvalue, check=True):
 

From 82f84f82785eaac8ec0243fa835e45d7dfdb5c6f Mon Sep 17 00:00:00 2001
From: Julien Fortin <julien@cumulusnetworks.com>
Date: Fri, 27 Jan 2017 17:34:13 +0300
Subject: [PATCH 7/8] Revert "ifupdownaddons: bridgeutils: uncomment parsing
 code for brctl mc attributes"

This reverts commit 65beb82662576c047a281389bd663589dcba09db.

it's causing testifupdown2.py:TestMakoJson to fail... Basically this commit
uncomment codes which parse mc value from brctl output. So `ifquery -r` output
is different (this new attribute show up under the bridge). TestMakoJson at
some point does:
$ ifquery -a -r -t json > running_json
Then later
$ ifup -i running_json...
This ifup fails on:
...
warning: br0: unsupported attribute 'bridge-mclmt'
...
---
 ifupdownaddons/bridgeutils.py | 43 ++++++++---------------------------
 1 file changed, 9 insertions(+), 34 deletions(-)

diff --git a/ifupdownaddons/bridgeutils.py b/ifupdownaddons/bridgeutils.py
index ed850fa..159a87f 100644
--- a/ifupdownaddons/bridgeutils.py
+++ b/ifupdownaddons/bridgeutils.py
@@ -115,40 +115,15 @@ class brctl(utilsBase):
             except:
                 pass
 
-            try:
-                battrs['hashel'] = broutlines[10].split('hash elasticity')[1].split()[0].strip()
-            except:
-                pass
-
-            try:
-                battrs['hashmax'] = broutlines[10].split('hash max')[1].strip()
-            except:
-                pass
-
-            try:
-                battrs['mclmc'] = broutlines[11].split('mc last member count')[1].split()[0].strip()
-            except:
-                pass
-
-            try:
-                battrs['mciqc'] = broutlines[11].split('mc init query count')[1].strip()
-            except:
-                pass
-
-            try:
-                battrs['mcrouter'] = broutlines[12].split('mc router')[1].split()[0].strip()
-            except:
-                pass
-
-            try:
-                battrs['mcsnoop'] = broutlines[12].split('mc snooping')[1].strip()
-            except:
-                pass
-
-            try:
-                battrs['mclmt'] = broutlines[13].split('mc last member timer')[1].split()[0].strip().replace('.00', '')
-            except:
-                pass
+            # XXX: comment this out until mc attributes become available
+            # with brctl again
+            #battrs['hashel'] = broutlines[10].split('hash elasticity')[1].split()[0].strip()
+            #battrs['hashmax'] = broutlines[10].split('hash max')[1].strip()
+            #battrs['mclmc'] = broutlines[11].split('mc last member count')[1].split()[0].strip()
+            #battrs['mciqc'] = broutlines[11].split('mc init query count')[1].strip()
+            #battrs['mcrouter'] = broutlines[12].split('mc router')[1].split()[0].strip()
+            ##battrs['mcsnoop'] = broutlines[12].split('mc snooping')[1].strip()
+            #battrs['mclmt'] = broutlines[13].split('mc last member timer')[1].split()[0].strip()
         except Exception, e:
             self.logger.warn('%s: error while processing bridge attributes: %s' % (bridgename, str(e)))
             pass

From c3fc3b8f001705f4e34ec9755a2cc9f8a7fe024a Mon Sep 17 00:00:00 2001
From: Nikhil <nikhil@cumulusnetworks.com>
Date: Mon, 30 Jan 2017 14:09:58 -0800
Subject: [PATCH 8/8] addons: bond: adding attribute bond-(up|down)delay

Ticket: CM-8424
Reviewed By: Roopa, Julien
Testing Done: using the config mentioned in bug

updelay

    Specifies the time, in milliseconds, to wait before enabling a
    slave after a link recovery has been detected.  This option is
    only valid for the miimon link monitor.

downdelay

    Specifies the time, in milliseconds, to wait before disabling
    a slave after a link failure has been detected.  This option
    is only valid for the miimon link monitor.

Signed-off-by: Nikhil Gajendrakumar <nikhil@cumulusnetworks.com>

Conflicts:
	ifupdownaddons/bondutil.py
---
 addons/bond.py             | 21 ++++++++++++++++++---
 ifupdownaddons/bondutil.py | 18 ++++++++++++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/addons/bond.py b/addons/bond.py
index e6f60b0..9a13a6e 100644
--- a/addons/bond.py
+++ b/addons/bond.py
@@ -105,7 +105,16 @@ class bond(moduleBase):
                          'example' : ['bond-slaves swp1 swp2',
                                       'bond-slaves glob swp1-2',
                                       'bond-slaves regex (swp[1|2)'],
-                         'aliases': ['bond-ports']}}}
+                         'aliases': ['bond-ports']},
+                     'bond-updelay' :
+                        {'help' : 'bond updelay',
+                         'default' : '0',
+                         'example' : ['bond-updelay 100']},
+                     'bond-downdelay':
+                        {'help' : 'bond downdelay',
+                         'default' : '0',
+                         'example' : ['bond-downdelay 100']}
+                    }}
 
     _bond_mode_num = {'0': 'balance-rr',
                       '1': 'active-backup',
@@ -231,7 +240,9 @@ class bond(moduleBase):
                                  ('bond-ad-actor-system' , 'ad_actor_system'),
                                  ('bond-ad-sys-priority' , 'ad_actor_sys_prio'),
                                  ('bond-ad-actor-sys-prio' , 'ad_actor_sys_prio'),
-                                 ('bond-lacp-bypass-allow', 'lacp_bypass')])
+                                 ('bond-lacp-bypass-allow', 'lacp_bypass'),
+                                 ('bond-updelay', 'updelay'),
+                                 ('bond-downdelay', 'downdelay')])
         linkup = self.ipcmd.is_link_up(ifaceobj.name)
         try:
             # order of attributes set matters for bond, so
@@ -408,7 +419,11 @@ class bond(moduleBase):
                      'bond-num-unsol-na' :
                             self.bondcmd.get_num_unsol_na(bondname),
                      'bond-num-grat-arp' :
-                            self.bondcmd.get_num_grat_arp(bondname)}
+                            self.bondcmd.get_num_grat_arp(bondname),
+                     'bond-updelay' :
+                            self.bondcmd.get_updelay(bondname),
+                     'bond-downdelay' :
+                            self.bondcmd.get_downdelay(bondname)}
         slaves = self.bondcmd.get_slaves(bondname)
         if slaves:
             bondattrs['bond-slaves'] = slaves
diff --git a/ifupdownaddons/bondutil.py b/ifupdownaddons/bondutil.py
index 5cc64eb..03287e5 100644
--- a/ifupdownaddons/bondutil.py
+++ b/ifupdownaddons/bondutil.py
@@ -81,6 +81,18 @@ class bondutil(utilsBase):
                                        %bondname).split()[1])
         except Exception as e:
             self.logger.debug(str(e))
+        try:
+            linkCache.set_attr([bondname, 'linkinfo', 'updelay'],
+                self.read_file_oneline('/sys/class/net/%s/bonding/updelay'
+                                       %bondname))
+        except Exception as e:
+            self.logger.debug(str(e))
+        try:
+            linkCache.set_attr([bondname, 'linkinfo', 'downdelay'],
+                self.read_file_oneline('/sys/class/net/%s/bonding/downdelay'
+                                       %bondname))
+        except Exception as e:
+            self.logger.debug(str(e))
         try:
             map(lambda x: linkCache.set_attr([bondname, 'linkinfo', x],
                    self.read_file_oneline('/sys/class/net/%s/bonding/%s'
@@ -313,6 +325,12 @@ class bondutil(utilsBase):
     def get_num_grat_arp(self, bondname):
         return self._cache_get([bondname, 'linkinfo', 'num_grat_arp'])
 
+    def get_updelay(self, bondname):
+        return self._cache_get([bondname, 'linkinfo', 'updelay'])
+
+    def get_downdelay(self, bondname):
+        return self._cache_get([bondname, 'linkinfo', 'downdelay'])
+
     def enslave_slave(self, bondname, slave, prehook=None, posthook=None):
         slaves = self._cache_get([bondname, 'linkinfo', 'slaves'])
         if slaves and slave in slaves: return