ifupdown2 patch to properly remove address-virtual mac addresses

Ticket: CM-6702
Reviewed By: roopa
Testing Done: unit and smoke tested with ifupdown2 suite

When address-virtual mac adddress is modified, removed from an SVI, or the SVI is removed,
the permanent mac address is not removed.

This patch addresses all three cases but creating a global statemanager instance
and removing address-virtual FDB entries that were previously configured.

addons/addressvirtual.py

@@ -7,6 +7,7 @@
 from ifupdown.iface import *
 from ifupdownaddons.modulebase import moduleBase
 from ifupdownaddons.iproute2 import iproute2
+import ifupdown.statemanager as statemanager
 import ifupdown.rtnetlink_api as rtnetlink_api
 from ipaddr import IPNetwork
 import logging
@@ -52,21 +53,22 @@ class addressvirtual(moduleBase):
 
     def _remove_addresses_from_bridge(self, ifaceobj, hwaddress):
         # XXX: batch the addresses
-        bridgename = None
         if '.' in ifaceobj.name:
+            (bridgename, vlan) = ifaceobj.name.split('.')
             if self.ipcmd.bridge_is_vlan_aware(bridgename):
-                (bridgename, vlan) = ifaceobj.name.split('.')
+                for addr in hwaddress:
+                    try:
+                        self.ipcmd.bridge_fdb_del(bridgename, addr, vlan)
+                    except Exception, e:
+                        self.logger.debug("%s: %s" %(ifaceobj.name, str(e)))
+                        pass
         elif self.ipcmd.is_bridge(ifaceobj.name):
-            vlan = None
+            for addr in hwaddress:
-            bridgename = ifaceobj.name
+                try:
-        if not bridgename:
+                    self.ipcmd.bridge_fdb_del(ifaceobj.name, addr)
-            return
+                except Exception, e:
-        for addr in hwaddress:
+                    self.logger.debug("%s: %s" %(ifaceobj.name, str(e)))
-            try:
+                    pass
-                self.ipcmd.bridge_fdb_del(bridgename, addr, vlan)
-            except Exception, e:
-                self.logger.debug("%s: %s" %(ifaceobj.name, str(e)))
-                pass
 
     def _get_bridge_fdbs(self, bridgename, vlan):
         fdbs = self._bridge_fdb_query_cache.get(bridgename)
@@ -119,6 +121,26 @@ class addressvirtual(moduleBase):
                               %str(e))
             pass
 
+    def _get_macs_from_old_config(self, ifaceobj=None):
+        """ This method returns a list of the mac addresses
+        in the address-virtual attribute for the bridge. """
+        maclist = []
+        saved_ifaceobjs = statemanager.statemanager_api.get_ifaceobjs(ifaceobj.name)
+        if not saved_ifaceobjs:
+            return maclist
+        # we need the old saved configs from the statemanager
+        for oldifaceobj in saved_ifaceobjs:
+            if not oldifaceobj.get_attr_value('address-virtual'):
+                continue
+            for av in oldifaceobj.get_attr_value('address-virtual'):
+                macip = av.split()
+                if len(macip) < 2:
+                    self.logger.debug("%s: incorrect old address-virtual attrs '%s'"
+                                      %(oldifaceobj.name,  av))
+                    continue
+                maclist.append(macip[0])
+        return maclist
+
     def _apply_address_config(self, ifaceobj, address_virtual_list):
         purge_existing = False if self.PERFMODE else True
 
@@ -154,6 +176,13 @@ class addressvirtual(moduleBase):
             av_idx += 1
         self.ipcmd.batch_commit()
 
+        # check the statemanager for old configs.
+        # We need to remove only the previously configured FDB entries
+        oldmacs = self._get_macs_from_old_config(ifaceobj)
+        # get a list of fdbs in old that are not in new config meaning they should
+        # be removed since they are gone from the config
+        removed_macs = [mac for mac in oldmacs if mac not in hwaddress]
+        self._remove_addresses_from_bridge(ifaceobj, removed_macs)
         # if ifaceobj is a bridge and bridge is a vlan aware bridge
         # add the vid to the bridge
         self._add_addresses_to_bridge(ifaceobj, hwaddress)
@@ -164,7 +193,7 @@ class addressvirtual(moduleBase):
         hwaddress = []
         self.ipcmd.batch_start()
         macvlan_prefix = self._get_macvlan_prefix(ifaceobj)
-        for macvlan_ifacename in glob.glob("/sys/class/net/%s-*" %macvlan_prefix):
+        for macvlan_ifacename in glob.glob("/sys/class/net/%s*" %macvlan_prefix):
             macvlan_ifacename = os.path.basename(macvlan_ifacename)
             if not self.ipcmd.link_exists(macvlan_ifacename):
                 continue

ifupdown/ifupdownmain.py

@@ -15,7 +15,7 @@ import logging
 import sys, traceback
 import copy
 import json
-from statemanager import *
+import ifupdown.statemanager as statemanager
 from networkinterfaces import *
 from iface import *
 from scheduler import *
@@ -227,7 +227,7 @@ class ifupdownMain(ifupdownBase):
 
         if self.STATEMANAGER_ENABLE:
             try:
-                self.statemanager = stateManager()
+                self.statemanager = statemanager.statemanager_api
                 self.statemanager.read_saved_state()
             except Exception, e:
                 # XXX Maybe we should continue by ignoring old state

ifupdown/statemanager.py

@@ -174,3 +174,5 @@ class stateManager():
         else:
             for ifacename, ifaceobjs in self.ifaceobjdict.items():
                 [i.dump(self.logger) for i in ifaceobjs]
+
+statemanager_api = stateManager()