From 072ae4ae9be49db6d1e3742fbb8a33cccc4dadc7 Mon Sep 17 00:00:00 2001 From: Tamer Ahmed Date: Thu, 3 Sep 2020 12:32:52 -0700 Subject: [PATCH] [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 * [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 --- fdbutil/__init__.py | 0 {scripts => fdbutil}/filter_fdb_entries.py | 48 ++++++++----------- scripts/fast-reboot | 2 +- setup.py | 3 +- .../filter_fdb_entries_test.py | 9 ++-- .../filter_fdb_input/config_db.json | 4 +- .../filter_fdb_input/expected_fdb.json | 9 +++- .../filter_fdb_input/test_vectors.py | 13 +++-- 8 files changed, 47 insertions(+), 41 deletions(-) create mode 100644 fdbutil/__init__.py rename {scripts => fdbutil}/filter_fdb_entries.py (89%) diff --git a/fdbutil/__init__.py b/fdbutil/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/scripts/filter_fdb_entries.py b/fdbutil/filter_fdb_entries.py similarity index 89% rename from scripts/filter_fdb_entries.py rename to fdbutil/filter_fdb_entries.py index 31d4204ec9..736827d34e 100755 --- a/scripts/filter_fdb_entries.py +++ b/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 index 3c315cf466..640cf3c1c0 100755 --- 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 index af49a99cde..77a48049ab 100644 --- 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 index af1f7712c3..fbbff67ea9 100644 --- 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 index 8c34fcc5b6..0f4d548c3c 100644 --- 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 index 1993cf24a3..2dc3cd0e18 100644 --- 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" } -] \ No newline at end of file +] diff --git a/sonic-utilities-tests/filter_fdb_input/test_vectors.py b/sonic-utilities-tests/filter_fdb_input/test_vectors.py index 2321da47af..daa6863a3b 100644 --- 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": [ {