[route_check]: Ignore standalone tunnel routes (sonic-net#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 <lawlee@microsoft.com>
EdenGri · Oct 12, 2022 · d9d3ee4 · d9d3ee4
1 parent c8d1c88
commit d9d3ee4
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 15 deletions.
diff --git a/scripts/route_check.py b/scripts/route_check.py
@@ -450,6 +450,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.
@@ -486,6 +525,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
@@ -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
@@ -222,7 +222,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)
@@ -484,7 +484,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)
@@ -501,7 +501,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)
@@ -526,7 +526,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)
@@ -545,7 +545,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
@@ -828,7 +828,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
@@ -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'
@@ -295,6 +296,22 @@
         }
     },
     "7": {
+        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: {},
+                }
+            }
+        }
+    },
+    "8": {
         DESCR: "Good one with VRF routes",
         ARGS: "route_check",
         PRE: {
@@ -403,6 +420,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]