matthew/sonic-buildimage
Archived
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[staticroutebfd]fix an issue on deleting a non-bfd static route (#15269)

Browse Source 
* [static_route][staticroutebfd]fix an issue on deleting a non-bfd static route

Fix an issue for deleting a non-bfd static route also remove the staticroutebfd from critical_processes list and make it auto restart in the case of crash.
This commit is contained in:
Baorong Liu 2023-06-02 11:46:56 -07:00 committed by GitHub
parent c044e6e34e
commit acb423b255
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 120 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
dockers/docker-fpm-frr/frr/supervisord/critical_processes.j2
View File

@ -9,5 +9,4 @@ program:pimd
program:frrcfgd
{%- else %}
program:bgpcfgd
program:staticroutebfd
{%- endif %}

2
dockers/docker-fpm-frr/frr/supervisord/supervisord.conf.j2
View File

@ -147,7 +147,7 @@ dependent_startup_wait_for=bgpd:running
command=/usr/local/bin/staticroutebfd
priority=6
autostart=false
autorestart=false
autorestart=true
startsecs=0
stdout_logfile=syslog
stderr_logfile=syslog

45
src/sonic-bgpcfgd/staticroutebfd/main.py
View File

@ -400,6 +400,18 @@ class StaticRouteBfd(object):
self.handle_bfd_change(key, data_copy, True)
data['bfd_nh_hold'] = "true"
# preprocess empty nexthop-vrf list before save to LOCAL_CONFIG_TABLE, bfd session need this information
nh_list = arg_list(data['nexthop']) if 'nexthop' in data else None
nh_vrf_list = arg_list(data['nexthop-vrf']) if 'nexthop-vrf' in data else None
if nh_vrf_list is None:
nh_vrf_list = [vrf] * len(nh_list) if len(nh_list) > 0 else None
data['nexthop-vrf'] = ','.join(nh_vrf_list) if nh_vrf_list else ''
else: # preprocess empty nexthop-vrf member
for index in range(len(nh_vrf_list)):
if len(nh_vrf_list[index]) == 0:
nh_vrf_list[index] = vrf
data['nexthop-vrf'] = ','.join(nh_vrf_list)
if not bfd_enabled:
#skip if bfd is not enabled, but store it to local_db to detect bfd field dynamic change
data['bfd'] = "false"
@ -407,13 +419,8 @@ class StaticRouteBfd(object):
return True
bkh_list = arg_list(data['blackhole']) if 'blackhole' in data else None
nh_list = arg_list(data['nexthop']) if 'nexthop' in data else None
intf_list = arg_list(data['ifname']) if 'ifname' in data else None
dist_list = arg_list(data['distance']) if 'distance' in data else None
nh_vrf_list = arg_list(data['nexthop-vrf']) if 'nexthop-vrf' in data else None
if nh_vrf_list is None:
nh_vrf_list = [vrf] * len(nh_list) if len(nh_list) > 0 else None
data['nexthop-vrf'] = ','.join(nh_vrf_list) if nh_vrf_list else ''
if intf_list is None or nh_list is None or nh_vrf_list is None or \
len(intf_list) != len(nh_list) or len(intf_list) != len(nh_vrf_list):
log_err("Static route bfd set Failed, nexthop, interface and vrf lists do not match.")
@ -502,18 +509,26 @@ class StaticRouteBfd(object):
arg_list = lambda v: [x.strip() for x in v.split(',')] if len(v.strip()) != 0 else None
nh_list = arg_list(data['nexthop']) if 'nexthop' in data else None
nh_vrf_list = arg_list(data['nexthop-vrf']) if 'nexthop-vrf' in data else None
for index in range(len(nh_list)):
nh_ip = nh_list[index]
nh_vrf = nh_vrf_list[index]
nh_key = nh_vrf + "|" + nh_ip
self.remove_from_nh_table_entry(nh_key, route_cfg_key)
bfd_field = arg_list(data['bfd']) if 'bfd' in data else ["false"]
bfd_enabled = self.isFieldTrue(bfd_field)
if len(self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key)) == 0:
bfd_key = nh_vrf + ":default:" + nh_ip
self.remove_from_local_db(LOCAL_BFD_TABLE, bfd_key)
self.del_bfd_session_from_appl_db(bfd_key)
# for a bfd_enabled static route, the nh_vrf_list was processed, has same length with nh_list
if bfd_enabled and nh_list and nh_vrf_list and len(nh_list) == len(nh_vrf_list):
for index in range(len(nh_list)):
nh_ip = nh_list[index]
nh_vrf = nh_vrf_list[index]
nh_key = nh_vrf + "|" + nh_ip
self.remove_from_nh_table_entry(nh_key, route_cfg_key)
if len(self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key)) == 0:
bfd_key = nh_vrf + ":default:" + nh_ip
self.remove_from_local_db(LOCAL_BFD_TABLE, bfd_key)
self.del_bfd_session_from_appl_db(bfd_key)
# do not delete it from appl_db if the route is not bfd enabled
if bfd_enabled:
self.del_static_route_from_appl_db(route_cfg_key.replace("|", ":"))
self.del_static_route_from_appl_db(route_cfg_key.replace("|", ":"))
self.remove_from_local_db(LOCAL_SRT_TABLE, route_cfg_key)
if redis_del:

89
src/sonic-bgpcfgd/tests/test_static_rt_bfd.py
View File

@ -169,6 +169,94 @@ def test_set_del():
{'del_default:2.2.2.0/24': {}}
)
# test add a non-bfd static route
set_del_test(dut, "srt",
"SET",
("3.3.3.0/24", {
"nexthop": "192.168.1.2 , 192.168.2.2",
"ifname": "if1, if2",
}),
{},
{}
)
# test delete a non-bfd static route
set_del_test(dut, "srt",
"DEL",
("3.3.3.0/24", {}),
{},
{}
)
def test_set_del_vrf():
dut = constructor()
intf_setup(dut)
set_del_test(dut, "srt",
"SET",
("vrfred|2.2.2.0/24", {
"bfd": "true",
"nexthop": "192.168.1.2 , 192.168.2.2, 192.168.3.2",
"ifname": "if1, if2, if3",
"nexthop-vrf": "testvrf1, , default",
}),
{
"set_testvrf1:default:192.168.1.2" : {'multihop': 'true', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'},
"set_vrfred:default:192.168.2.2" : {'multihop': 'true', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'},
"set_default:default:192.168.3.2" : {'multihop': 'true', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'}
},
{}
)
set_del_test(dut, "bfd",
"SET",
("testvrf1|default|192.168.1.2", {
"state": "Up"
}),
{},
{'set_vrfred:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'testvrf1', 'expiry': 'false'}}
)
set_del_test(dut, "bfd",
"SET",
("vrfred|default|192.168.2.2", {
"state": "Up"
}),
{},
{'set_vrfred:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'testvrf1,vrfred', 'expiry': 'false'}}
)
set_del_test(dut, "bfd",
"SET",
("default|default|192.168.3.2", {
"state": "Up"
}),
{},
{'set_vrfred:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'testvrf1,vrfred,default', 'expiry': 'false'}}
)
set_del_test(dut, "srt",
"SET",
("vrfred|2.2.2.0/24", {
"bfd": "true",
"nexthop": "192.168.1.2 , 192.168.2.2",
"ifname": "if1, if2",
"nexthop-vrf": "testvrf1,",
}),
{
"del_default:default:192.168.3.2" : {}
},
{'set_vrfred:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'vrfred, testvrf1', 'expiry': 'false'}}
)
set_del_test(dut, "srt",
"DEL",
("vrfred|2.2.2.0/24", { }),
{
"del_testvrf1:default:192.168.1.2" : {},
"del_vrfred:default:192.168.2.2" : {}
},
{'del_vrfred:2.2.2.0/24': {}}
)
def test_bfd_del():
dut = constructor()
intf_setup(dut)
@ -514,3 +602,4 @@ def test_set_bfd_change_no_hold():
)