diff --git a/dockers/docker-fpm-frr/frr/supervisord/critical_processes.j2 b/dockers/docker-fpm-frr/frr/supervisord/critical_processes.j2 index dad38888e8..69f4e8e693 100644 --- a/dockers/docker-fpm-frr/frr/supervisord/critical_processes.j2 +++ b/dockers/docker-fpm-frr/frr/supervisord/critical_processes.j2 @@ -9,5 +9,4 @@ program:pimd program:frrcfgd {%- else %} program:bgpcfgd -program:staticroutebfd {%- endif %} diff --git a/dockers/docker-fpm-frr/frr/supervisord/supervisord.conf.j2 b/dockers/docker-fpm-frr/frr/supervisord/supervisord.conf.j2 index 584e7fdc2e..f6edc17a55 100644 --- a/dockers/docker-fpm-frr/frr/supervisord/supervisord.conf.j2 +++ b/dockers/docker-fpm-frr/frr/supervisord/supervisord.conf.j2 @@ -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 diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py index 4483d0589c..e3b2ed10be 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/main.py +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -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: diff --git a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py index e2eb32cb85..0e4d624759 100644 --- a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py +++ b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py @@ -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(): ) +