From a3533a904bf2137cf48aba4ad953956f00e0d82d Mon Sep 17 00:00:00 2001 From: Lawrence Lee Date: Tue, 30 Aug 2022 14:45:58 -0700 Subject: [PATCH] [route_check]: Ignore standalone tunnel routes (#2325) When checking for route entry mismatches, ignore mismatched routes where an APPL_DB neighbor entry with a zero MAC is present. These routes are introduced by this change in SWSS and are expected: sonic-net/sonic-swss#2137 Signed-off-by: Lawrence Lee --- scripts/route_check.py | 40 ++++++++++++++++++++++++++++++++ scripts/route_check_test.sh | 32 ++++++++++++++++++------- tests/config_test.py | 10 ++++---- tests/mock_tables/config_db.json | 3 ++- tests/route_check_test.py | 22 ++++++++++++++++++ 5 files changed, 92 insertions(+), 15 deletions(-) diff --git a/scripts/route_check.py b/scripts/route_check.py index fdbdd0a5f8..eab3615c46 100755 --- a/scripts/route_check.py +++ b/scripts/route_check.py @@ -447,6 +447,45 @@ def filter_out_vnet_routes(routes): return updated_routes +def filter_out_standalone_tunnel_routes(routes): + config_db = swsscommon.ConfigDBConnector() + config_db.connect() + device_metadata = config_db.get_table('DEVICE_METADATA') + subtype = device_metadata['localhost'].get('subtype', '') + + if subtype.lower() != 'dualtor': + return routes + + app_db = swsscommon.DBConnector('APPL_DB', 0) + neigh_table = swsscommon.Table(app_db, 'NEIGH_TABLE') + neigh_keys = neigh_table.getKeys() + standalone_tunnel_route_ips = [] + updated_routes = [] + + for neigh in neigh_keys: + _, mac = neigh_table.hget(neigh, 'neigh') + if mac == '00:00:00:00:00:00': + # remove preceding 'VlanXXXX' to get just the neighbor IP + neigh_ip = ':'.join(neigh.split(':')[1:]) + standalone_tunnel_route_ips.append(neigh_ip) + + if not standalone_tunnel_route_ips: + return routes + + for route in routes: + ip, subnet = route.split('/') + ip_version = ipaddress.ip_address(ip).version + + # we want to keep the route if it is not a standalone tunnel route. + # if the route subnet contains more than one address, it is not a + # standalone tunnel route + if (ip not in standalone_tunnel_route_ips) or \ + ((ip_version == 6 and subnet != '128') or (ip_version == 4 and subnet != '32')): + updated_routes.append(route) + + return updated_routes + + def check_routes(): """ The heart of this script which runs the checks. @@ -483,6 +522,7 @@ def check_routes(): _, rt_asic_miss = diff_sorted_lists(intf_appl, rt_asic_miss) rt_asic_miss = filter_out_default_routes(rt_asic_miss) rt_asic_miss = filter_out_vnet_routes(rt_asic_miss) + rt_asic_miss = filter_out_standalone_tunnel_routes(rt_asic_miss) # Check APPL-DB INTF_TABLE with ASIC table route entries intf_appl_miss, _ = diff_sorted_lists(intf_appl, rt_asic) diff --git a/scripts/route_check_test.sh b/scripts/route_check_test.sh index 505253863e..989cbfae0b 100755 --- a/scripts/route_check_test.sh +++ b/scripts/route_check_test.sh @@ -2,22 +2,36 @@ # add a route, interface & route-entry to simulate error # -sonic-db-cli APPL_DB hmset "ROUTE_TABLE:20c0:d9b8:99:80::/64" "nexthop" "fc00::72,fc00::76,fc00::7a,fc00::7e" "ifname" "PortChannel01,PortChannel02,PortChannel03,PortChannel04" +sonic-db-cli APPL_DB hmset "ROUTE_TABLE:20c0:d9b8:99:80::/64" "nexthop" "fc00::72,fc00::76,fc00::7a,fc00::7e" "ifname" "PortChannel01,PortChannel02,PortChannel03,PortChannel04" > /dev/null +sonic-db-cli ASIC_DB hmset "ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY:{\"dest\":\"192.193.120.255/25\",\"switch_id\":\"oid:0x21000000000000\",\"vr\":\"oid:0x3000000000022\"}" "SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID" "oid:0x5000000000614" > /dev/null +sonic-db-cli APPL_DB hmset "INTF_TABLE:PortChannel01:10.0.0.99/31" "scope" "global" "family" "IPv4" > /dev/null +echo "------" +echo "expect errors!" +echo "Running Route Check..." +./route_check.py +echo "return value: $?" -sonic-db-cli ASIC_DB hmset "ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY:{\"dest\":\"192.193.120.255/25\",\"switch_id\":\"oid:0x21000000000000\",\"vr\":\"oid:0x3000000000022\"}" "SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID" "oid:0x5000000000614" +sonic-db-cli APPL_DB del "ROUTE_TABLE:20c0:d9b8:99:80::/64" > /dev/null +sonic-db-cli ASIC_DB del "ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY:{\"dest\":\"192.193.120.255/25\",\"switch_id\":\"oid:0x21000000000000\",\"vr\":\"oid:0x3000000000022\"}" > /dev/null +sonic-db-cli APPL_DB del "INTF_TABLE:PortChannel01:10.0.0.99/31" > /dev/null -sonic-db-cli APPL_DB hmset "INTF_TABLE:PortChannel01:10.0.0.99/31" "scope" "global" "family" "IPv4" +# add standalone tunnel route to simulate unreachable neighbor scenario on dual ToR +# in this scenario, we expect the route mismatch to be ignored +sonic-db-cli APPL_DB hmset "NEIGH_TABLE:Vlan1000:fc02:1000::99" "neigh" "00:00:00:00:00:00" "family" "IPv6" > /dev/null +sonic-db-cli ASIC_DB hmset 'ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"fc02:1000::99/128","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}' "SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID" "oid:0x400000000167d" "SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION" "SAI_PACKET_ACTION_FORWARD" > /dev/null -echo "expect errors!\n------\nRunning Route Check...\n" +echo "------" +echo "expect success on dualtor, expect error on all other devices!" +echo "Running Route Check..." ./route_check.py echo "return value: $?" -sonic-db-cli APPL_DB del "ROUTE_TABLE:20c0:d9b8:99:80::/64" -sonic-db-cli ASIC_DB del "ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY:{\"dest\":\"192.193.120.255/25\",\"switch_id\":\"oid:0x21000000000000\",\"vr\":\"oid:0x3000000000022\"}" -sonic-db-cli APPL_DB del "INTF_TABLE:PortChannel01:10.0.0.99/31" - +sonic-db-cli APPL_DB del "NEIGH_TABLE:Vlan1000:fc02:1000::99" > /dev/null +sonic-db-cli ASIC_DB del 'ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"fc02:1000::99/128","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}' > /dev/null -echo "expect success!\n------\nRunning Route Check...\n" +echo "------" +echo "expect success!" +echo "Running Route Check..." ./route_check.py echo "return value: $?" diff --git a/tests/config_test.py b/tests/config_test.py index d480fd356d..e184698ec0 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -205,7 +205,7 @@ def test_config_reload(self, get_cmd_module, setup_single_broadcom_asic): obj = {'config_db': db.cfgdb} # simulate 'config reload' to provoke load_sys_info option - result = runner.invoke(config.config.commands["reload"], ["-l", "-n", "-y"], obj=obj) + result = runner.invoke(config.config.commands["reload"], ["-l", "-n", "-y", "--disable_arp_cache"], obj=obj) print(result.exit_code) print(result.output) @@ -451,7 +451,7 @@ def test_reload_config(self, get_cmd_module, setup_single_broadcom_asic): result = runner.invoke( config.config.commands["reload"], - [self.dummy_cfg_file, '-y', '-f']) + [self.dummy_cfg_file, '-y', '-f', "--disable_arp_cache"]) print(result.exit_code) print(result.output) @@ -468,7 +468,7 @@ def test_config_reload_disabled_service(self, get_cmd_module, setup_single_broad (config, show) = get_cmd_module runner = CliRunner() - result = runner.invoke(config.config.commands["reload"], [self.dummy_cfg_file, "-y"]) + result = runner.invoke(config.config.commands["reload"], [self.dummy_cfg_file, "-y", "--disable_arp_cache"]) print(result.exit_code) print(result.output) @@ -493,7 +493,7 @@ def test_reload_config_masic(self, get_cmd_module, setup_multi_broadcom_masic): self.dummy_cfg_file) result = runner.invoke( config.config.commands["reload"], - [cfg_files, '-y', '-f']) + [cfg_files, '-y', '-f', "--disable_arp_cache"]) print(result.exit_code) print(result.output) @@ -512,7 +512,7 @@ def test_reload_yang_config(self, get_cmd_module, runner = CliRunner() result = runner.invoke(config.config.commands["reload"], - [self.dummy_cfg_file, '-y','-f' ,'-t', 'config_yang']) + [self.dummy_cfg_file, "--disable_arp_cache", '-y', '-f', '-t', 'config_yang']) print(result.exit_code) print(result.output) diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index e3c038d29f..148e3600e1 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -824,7 +824,8 @@ "mac": "1d:34:db:16:a6:00", "platform": "x86_64-mlnx_msn3800-r0", "peer_switch": "sonic-switch", - "type": "ToRRouter" + "type": "ToRRouter", + "subtype": "DualToR" }, "SNMP_COMMUNITY|msft": { "TYPE": "RO" diff --git a/tests/route_check_test.py b/tests/route_check_test.py index b4fd3ce17d..4cbbfa87b8 100644 --- a/tests/route_check_test.py +++ b/tests/route_check_test.py @@ -24,6 +24,7 @@ OP_SET = "SET" OP_DEL = "DEL" +NEIGH_TABLE = 'NEIGH_TABLE' ROUTE_TABLE = 'ROUTE_TABLE' VNET_ROUTE_TABLE = 'VNET_ROUTE_TABLE' INTF_TABLE = 'INTF_TABLE' @@ -256,6 +257,22 @@ } }, "6": { + DESCR: "dualtor standalone tunnel route case", + ARGS: "route_check", + PRE: { + APPL_DB: { + NEIGH_TABLE: { + "Vlan1000:fc02:1000::99": { "neigh": "00:00:00:00:00:00", "family": "IPv6"} + } + }, + ASIC_DB: { + RT_ENTRY_TABLE: { + RT_ENTRY_KEY_PREFIX + "fc02:1000::99/128" + RT_ENTRY_KEY_SUFFIX: {}, + } + } + } + }, + "7": { DESCR: "Good one with VNET routes", ARGS: "route_check", PRE: { @@ -376,6 +393,11 @@ def get(self, key): return (True, ret) + def hget(self, key, field): + ret = copy.deepcopy(self.data.get(key, {}).get(field, {})) + return True, ret + + db_conns = {"APPL_DB": APPL_DB, "ASIC_DB": ASIC_DB} def conn_side_effect(arg, _): return db_conns[arg]