Skip to content

Commit

Permalink
[route_check]: Ignore standalone tunnel routes (sonic-net#2325)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
theasianpianist committed Sep 1, 2022
1 parent 5892f6d commit a3533a9
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 15 deletions.
40 changes: 40 additions & 0 deletions scripts/route_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
32 changes: 23 additions & 9 deletions scripts/route_check_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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: $?"
10 changes: 5 additions & 5 deletions tests/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion tests/mock_tables/config_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
22 changes: 22 additions & 0 deletions tests/route_check_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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]
Expand Down

0 comments on commit a3533a9

Please sign in to comment.