[filter-fdb] Check VLAN Presence When Filter FDB (#957) (#975)

* [filter-fdb] Check VLAN Presence When Filter FDB

FTOS fast conversion script generates bogus vlan that does not exist.
This PR uses config_db in order to verify that provided vlans exist
in the switch configuration.

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>

* review comments
making lgtm happy
Added two more test cases

* Update existing test case and adding new one

* adding support for filter ou based on vlan ip network

scripts/fast-reboot

@@ -414,6 +414,7 @@ if [[ "$REBOOT_TYPE" = "fast-reboot" ]]; then
     # Dump the ARP and FDB tables to files also as default routes for both IPv4 and IPv6
     # into /host/fast-reboot
     DUMP_DIR=/host/fast-reboot
+    CONFIG_DB_FILE=/etc/sonic/config_db.json
     mkdir -p $DUMP_DIR
     FAST_REBOOT_DUMP_RC=0
     /usr/bin/fast-reboot-dump.py -t $DUMP_DIR || FAST_REBOOT_DUMP_RC=$?
@@ -425,7 +426,7 @@ if [[ "$REBOOT_TYPE" = "fast-reboot" ]]; then
 
     FILTER_FDB_ENTRIES_RC=0
     # Filter FDB entries using MAC addresses from ARP table
-    /usr/bin/filter_fdb_entries.py -f $DUMP_DIR/fdb.json -a $DUMP_DIR/arp.json || FILTER_FDB_ENTRIES_RC=$?
+    /usr/bin/filter_fdb_entries.py -f $DUMP_DIR/fdb.json -a $DUMP_DIR/arp.json -c $CONFIG_DB_FILE || FILTER_FDB_ENTRIES_RC=$?
     if [[ FILTER_FDB_ENTRIES_RC -ne 0 ]]; then
         error "Failed to filter FDb entries. Exit code: $FILTER_FDB_ENTRIES_RC"
         unload_kernel

scripts/filter_fdb_entries.py

@@ -8,33 +8,67 @@
 import traceback
 import time
 
+from ipaddress import ip_address, ip_network, ip_interface
 from collections import defaultdict
 
-def get_arp_entries_map(filename):
+def get_vlan_cidr_map(filename):
+    """
+        Generate Vlan CIDR information from Config DB file
+
+        fdb entries could be contaminated with foreigh Vlan entries as seen in the case of 
+        FTOS fast conversion. SONiC Vlan CIDR configuration will be used to filter out
+        those invalid Vlan entries out.
+
+        Args:
+            filename(str): Config DB data file
+
+        Returns:
+            vlan_cidr(dict) map of Vlan CIDR configuration for SONiC device
+    """
+    with open(filename, 'r') as fp:
+        config_db_entries = json.load(fp)
+
+    vlan_cidr = defaultdict()
+    if "VLAN_INTERFACE" in config_db_entries.keys() and "VLAN" in config_db_entries.keys():
+        for vlan_key in config_db_entries["VLAN_INTERFACE"].keys():
+            vlan, cidr = tuple(vlan_key.split('|'))
+            if vlan in config_db_entries["VLAN"]:
+                vlan_cidr[vlan] = ip_interface(cidr).network
+
+    return vlan_cidr
+
+def get_arp_entries_map(arp_filename, config_db_filename):
     """
         Generate map for ARP entries
 
         ARP entry map is using the MAC as a key for the arp entry. The map key is reformated in order
         to match FDB table formatting
 
         Args:
-            filename(str): ARP entry file name
+            arp_filename(str): ARP entry file name
+            config_db_filename(str): Config DB file name
 
         Returns:
             arp_map(dict) map of ARP entries using MAC as key.
     """
-    with open(filename, 'r') as fp:
+    vlan_cidr = get_vlan_cidr_map(config_db_filename)
+
+    with open(arp_filename, 'r') as fp:
         arp_entries = json.load(fp)
 
     arp_map = defaultdict()
     for arp in arp_entries:
         for key, config in arp.items():
-            if 'NEIGH_TABLE' in key:
+            if "NEIGH_TABLE" not in key:
+                continue
+            table, vlan, ip = tuple(key.split(':'))
+            if "NEIGH_TABLE" in table and vlan in vlan_cidr.keys() \
+                and ip_address(ip) in ip_network(vlan_cidr[vlan]) and "neigh" in config.keys():
                 arp_map[config["neigh"].replace(':', '-')] = ""
 
     return arp_map
 
-def filter_fdb_entries(fdb_filename, arp_filename, backup_file):
+def filter_fdb_entries(fdb_filename, arp_filename, config_db_filename, backup_file):
     """
         Filter FDB entries based on MAC presence into ARP entries
 
@@ -44,12 +78,13 @@ def filter_fdb_entries(fdb_filename, arp_filename, backup_file):
         Args:
             fdb_filename(str): FDB entries file name
             arp_filename(str): ARP entry file name
+            config_db_filename(str): Config DB file name
             backup_file(bool): Create backup copy of FDB file before creating new one
 
         Returns:
             None
     """
-    arp_map = get_arp_entries_map(arp_filename)
+    arp_map = get_arp_entries_map(arp_filename, config_db_filename)
 
     with open(fdb_filename, 'r') as fp:
         fdb_entries = json.load(fp)
@@ -91,20 +126,23 @@ def main():
     parser = argparse.ArgumentParser()
     parser.add_argument('-f', '--fdb', type=str, default='/tmp/fdb.json', help='fdb file name')
     parser.add_argument('-a', '--arp', type=str, default='/tmp/arp.json', help='arp file name')
+    parser.add_argument('-c', '--config_db', type=str, default='/tmp/config_db.json', help='config db file name')
     parser.add_argument('-b', '--backup_file', type=bool, default=True, help='Back up old fdb entries file')
     args = parser.parse_args()
 
     fdb_filename = args.fdb
     arp_filename = args.arp
+    config_db_filename = args.config_db
     backup_file = args.backup_file
 
     try:
         file_exists_or_raise(fdb_filename)
         file_exists_or_raise(arp_filename)
+        file_exists_or_raise(config_db_filename)
     except Exception as e:
         syslog.syslog(syslog.LOG_ERR, "Got an exception %s: Traceback: %s" % (str(e), traceback.format_exc()))
     else:
-        filter_fdb_entries(fdb_filename, arp_filename, backup_file)
+        filter_fdb_entries(fdb_filename, arp_filename, config_db_filename, backup_file)
 
     return 0
 

sonic-utilities-tests/filter_fdb_entries_test.py

@@ -14,6 +14,7 @@ class TestFilterFdbEntries(object):
     """
     ARP_FILENAME = "/tmp/arp.json"
     FDB_FILENAME = "/tmp/fdb.json"
+    CONFIG_DB_FILENAME = "/tmp/config_db.json"
     EXPECTED_FDB_FILENAME = "/tmp/expected_fdb.json"
 
     def __setUp(self, testData):
@@ -45,16 +46,17 @@ def create_file_or_raise(data, filename):
                 Raises:
                     Exception if data type is not supported
             """
-            if isinstance(data, list):
+            if isinstance(data, list) or isinstance(data, dict):
                 with open(filename, 'w') as fp:
                     json.dump(data, fp, indent=2, separators=(',', ': '))
             elif isinstance(data, str):
                 shutil.copyfile(data, filename)
             else:
-                raise Exception("Unknown test data type: {0}".format(type(test_data)))
+                raise Exception("Unknown test data type: {0}".format(type(data)))
 
         create_file_or_raise(testData["arp"], self.ARP_FILENAME)
         create_file_or_raise(testData["fdb"], self.FDB_FILENAME)
+        create_file_or_raise(testData["config_db"], self.CONFIG_DB_FILENAME)
         create_file_or_raise(testData["expected_fdb"], self.EXPECTED_FDB_FILENAME)
 
     def __tearDown(self):
@@ -72,6 +74,7 @@ def __tearDown(self):
         fdbFiles = glob.glob(self.FDB_FILENAME + '*')
         for file in fdbFiles:
             os.remove(file)
+        os.remove(self.CONFIG_DB_FILENAME)
 
     def __runCommand(self, cmds):
         """
@@ -166,8 +169,10 @@ def testFilterFdbEntries(self, testData):
                 self.ARP_FILENAME,
                 "-f",
                 self.FDB_FILENAME,
+                "-c",
+                self.CONFIG_DB_FILENAME,
             ])
-            assert rc == 0, "CFilter_fbd_entries.py failed with '{0}'".format(stderr)
+            assert rc == 0, "Filter_fdb_entries.py failed with '{0}'".format(stderr)
             assert self.__verifyOutput(), "Test failed for test data: {0}".format(testData)
         finally:
             self.__tearDown()