[filter-fdb] Call Filter FDB Main From Within Test Code #1051 (#1086)

* [filter-fdb] Call Filter FDB Main From Within Test Code (#1051) Code coverage requires that python code be run with the same process. Current test code was invoking filter fdb via shell which launches new process and so coverage is not available. This PR calls the main method from within test code. signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com> * [filter-fdb] Fix Filter FDB With IPv6 Present in Config DB (#1059) Filter fdb was wiping out IPv4 entries when both IPv4 and IPv6 are associated with VLan interface. The reason is IPv6 network was overwriting IPv4 network. This pr add support to filter both IPv4 and IPv6 addresses signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
sonic-net · Sep 3, 2020 · 072ae4a · 072ae4a
1 parent 8e30ac1
commit 072ae4a
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 41 deletions.
diff --git a/fdbutil/__init__.py b/fdbutil/__init__.py
diff --git a/scripts/filter_fdb_entries.py → fdbutil/filter_fdb_entries.py b/scripts/filter_fdb_entries.py → fdbutil/filter_fdb_entries.py
@@ -1,15 +1,13 @@
-#!/usr/bin/env python
-
+import argparse
 import json
-import sys
 import os
-import argparse
+import sys
 import syslog
-import traceback
 import time
+import traceback
 
-from ipaddress import ip_address, ip_network, ip_interface
 from collections import defaultdict
+from ipaddress import ip_address, ip_network, ip_interface
 
 def get_vlan_cidr_map(filename):
     """
@@ -35,7 +33,9 @@ def get_vlan_cidr_map(filename):
                 continue
             vlan, cidr = tuple(vlan_key.split('|'))
             if vlan in config_db_entries["VLAN"]:
-                vlan_cidr[vlan] = ip_interface(cidr).network
+                if vlan not in vlan_cidr:
+                    vlan_cidr[vlan] = {4: ip_address("0.0.0.0".decode()), 6: ip_address("::".decode())}
+                vlan_cidr[vlan][ip_interface(cidr).version] = ip_interface(cidr).network
 
     return vlan_cidr
 
@@ -65,8 +65,9 @@ def get_arp_entries_map(arp_filename, config_db_filename):
                 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(':', '-')] = ""
+                and ip_address(ip) in ip_network(vlan_cidr[vlan][ip_interface(ip).version]) \
+                and "neigh" in config.keys():
+                arp_map[config["neigh"].replace(':', '-').upper()] = ""
 
     return arp_map
 
@@ -94,7 +95,7 @@ def filter_fdb_entries(fdb_filename, arp_filename, config_db_filename, backup_fi
     def filter_fdb_entry(fdb_entry):
         for key, _ in fdb_entry.items():
             if 'FDB_TABLE' in key:
-                return key.split(':')[-1] in arp_map
+                return key.split(':')[-1].upper() in arp_map
 
         # malformed entry, default to False so it will be deleted
         return False
@@ -124,44 +125,33 @@ def file_exists_or_raise(filename):
     if not os.path.exists(filename):
         raise Exception("file '{0}' does not exist".format(filename))
 
-def main():
+def main(argv=sys.argv):
     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()
+    args = parser.parse_args(argv[1:])
 
     fdb_filename = args.fdb
     arp_filename = args.arp
     config_db_filename = args.config_db
     backup_file = args.backup_file
 
+    res = 0
     try:
+        syslog.openlog('filter_fdb_entries')
         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, config_db_filename, backup_file)
-
-    return 0
-
-if __name__ == '__main__':
-    res = 0
-    try:
-        syslog.openlog('filter_fdb_entries')
-        res = main()
     except KeyboardInterrupt:
         syslog.syslog(syslog.LOG_NOTICE, "SIGINT received. Quitting")
         res = 1
-    except Exception as e:
-        syslog.syslog(syslog.LOG_ERR, "Got an exception %s: Traceback: %s" % (str(e), traceback.format_exc()))
-        res = 2
+    else:
+        filter_fdb_entries(fdb_filename, arp_filename, config_db_filename, backup_file)
     finally:
         syslog.closelog()
-    try:
-        sys.exit(res)
-    except SystemExit:
-        os._exit(res)
+
+    return res
diff --git a/scripts/fast-reboot b/scripts/fast-reboot
@@ -378,7 +378,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 -c $CONFIG_DB_FILE || FILTER_FDB_ENTRIES_RC=$?
+    /usr/bin/filter_fdb_entries -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

diff --git a/setup.py b/setup.py
@@ -46,6 +46,7 @@ def get_test_suite():
         'sfputil',
         'pfc',
         'psuutil',
+        'fdbutil',
         'show',
         'sonic_installer',
         'sonic-utilities-tests',
@@ -70,7 +71,6 @@ def get_test_suite():
         'scripts/fast-reboot-dump.py',
         'scripts/fdbclear',
         'scripts/fdbshow',
-        'scripts/filter_fdb_entries.py',
         'scripts/generate_dump',
         'scripts/intfutil',
         'scripts/lldpshow',
@@ -102,6 +102,7 @@ def get_test_suite():
             'counterpoll = counterpoll.main:cli',
             'crm = crm.main:cli',
             'debug = debug.main:cli',
+            'filter_fdb_entries = fdbutil.filter_fdb_entries:main',
             'pfcwd = pfcwd.main:cli',
             'sfputil = sfputil.main:cli',
             'pfc = pfc.main:cli',

diff --git a/sonic-utilities-tests/filter_fdb_entries_test.py b/sonic-utilities-tests/filter_fdb_entries_test.py
@@ -7,6 +7,7 @@
 
 from collections import defaultdict
 from filter_fdb_input.test_vectors import filterFdbEntriesTestVector
+from fdbutil.filter_fdb_entries import main as filterFdbMain
 
 class TestFilterFdbEntries(object):
     """
@@ -162,16 +163,16 @@ def testFilterFdbEntries(self, testData):
         """
         try:
             self.__setUp(testData)
-
-            stdout, stderr, rc = self.__runCommand([
-                "scripts/filter_fdb_entries.py",
+            argv = [
+                "filter_fdb_entries",
                 "-a",
                 self.ARP_FILENAME,
                 "-f",
                 self.FDB_FILENAME,
                 "-c",
                 self.CONFIG_DB_FILENAME,
-            ])
+            ]
+            rc = filterFdbMain(argv)
             assert rc == 0, "Filter_fdb_entries.py failed with '{0}'".format(stderr)
             assert self.__verifyOutput(), "Test failed for test data: {0}".format(testData)
         finally:

diff --git a/sonic-utilities-tests/filter_fdb_input/config_db.json b/sonic-utilities-tests/filter_fdb_input/config_db.json
@@ -1125,7 +1125,9 @@
         }
     }, 
     "VLAN_INTERFACE": {
-        "Vlan1000|192.168.0.1/21": {}
+        "Vlan1000": {}, 
+        "Vlan1000|192.168.0.1/21": {}, 
+        "Vlan1000|fc02:1000::1/64": {}
     }, 
     "BUFFER_PG": {
         "Ethernet4|0": {

diff --git a/sonic-utilities-tests/filter_fdb_input/expected_fdb.json b/sonic-utilities-tests/filter_fdb_input/expected_fdb.json
@@ -181,6 +181,13 @@
     },
     "OP": "SET"
   },
+  {
+    "FDB_TABLE:Vlan1000:24-8A-07-4C-F5-18": {
+      "type": "dynamic",
+      "port": "Ethernet24"
+    },
+    "OP": "SET"
+  },
   {
     "FDB_TABLE:Vlan1000:72-06-00-01-02-72": {
       "type": "dynamic",
@@ -398,4 +405,4 @@
     },
     "OP": "SET"
   }
-]
+]
diff --git a/sonic-utilities-tests/filter_fdb_input/test_vectors.py b/sonic-utilities-tests/filter_fdb_input/test_vectors.py
@@ -43,7 +43,9 @@
                 "Vlan1000": {}
             },
             "VLAN_INTERFACE": {
-                "Vlan1000|192.168.0.1/21": {}
+                "Vlan1000": {}, 
+                "Vlan1000|192.168.0.1/21": {},
+                "Vlan1000|fc02:1000::1/64": {}
             }, 
         },
         "expected_fdb": [
@@ -80,7 +82,8 @@
                 "Vlan1000": {}
             },
             "VLAN_INTERFACE": {
-                "Vlan1000|192.168.0.1/21": {}
+                "Vlan1000|192.168.0.1/21": {},
+                "Vlan1000|fc02:1000::1/64": {}
             }, 
         },
         "expected_fdb": [
@@ -154,8 +157,10 @@
                 "Vlan1": {}
             },
             "VLAN_INTERFACE": {
-                "Vlan1|25.103.178.1/21": {}
-            }, 
+                "Vlan1|25.103.178.1/21": {},
+                "Vlan1": {}, 
+                "Vlan1|fc02:1000::1/64": {}
+            },
         },
         "expected_fdb": [
             {