From 3ffcab71f1ae2829139b45fbf78c0c8735d3dd73 Mon Sep 17 00:00:00 2001 From: qiwang4 Date: Mon, 13 Feb 2023 21:41:34 +0800 Subject: [PATCH 01/16] staticroutebfd implementation --- .../frr/supervisord/critical_processes.j2 | 1 + .../frr/supervisord/supervisord.conf.j2 | 14 + src/sonic-bgpcfgd/bgpcfgd/log.py | 2 +- .../bgpcfgd/managers_static_rt.py | 5 + src/sonic-bgpcfgd/setup.py | 1 + src/sonic-bgpcfgd/staticroutebfd/__init__.py | 0 src/sonic-bgpcfgd/staticroutebfd/main.py | 664 ++++++++++++++++++ 7 files changed, 686 insertions(+), 1 deletion(-) create mode 100644 src/sonic-bgpcfgd/staticroutebfd/__init__.py create mode 100644 src/sonic-bgpcfgd/staticroutebfd/main.py diff --git a/dockers/docker-fpm-frr/frr/supervisord/critical_processes.j2 b/dockers/docker-fpm-frr/frr/supervisord/critical_processes.j2 index 69f4e8e6931e..dad38888e883 100644 --- a/dockers/docker-fpm-frr/frr/supervisord/critical_processes.j2 +++ b/dockers/docker-fpm-frr/frr/supervisord/critical_processes.j2 @@ -9,4 +9,5 @@ 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 e7312466051b..fd891960c8d1 100644 --- a/dockers/docker-fpm-frr/frr/supervisord/supervisord.conf.j2 +++ b/dockers/docker-fpm-frr/frr/supervisord/supervisord.conf.j2 @@ -141,6 +141,20 @@ stderr_logfile=syslog dependent_startup=true dependent_startup_wait_for=bgpd:running +{% if DEVICE_METADATA.localhost.frr_mgmt_framework_config is defined and DEVICE_METADATA.localhost.frr_mgmt_framework_config == "true" %} +{% else %} +[program:staticroutebfd] +command=/usr/local/bin/staticroutebfd +priority=6 +autostart=false +autorestart=false +startsecs=0 +stdout_logfile=syslog +stderr_logfile=syslog +dependent_startup=true +dependent_startup_wait_for=bgpd:running +{% endif %} + [program:bgpmon] command=/usr/local/bin/bgpmon priority=6 diff --git a/src/sonic-bgpcfgd/bgpcfgd/log.py b/src/sonic-bgpcfgd/bgpcfgd/log.py index 4083b13aa6ad..28c3cc39bcfd 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/log.py +++ b/src/sonic-bgpcfgd/bgpcfgd/log.py @@ -30,4 +30,4 @@ def log_err(msg): def log_crit(msg): """ Send a message msg to the syslog as CRIT """ - syslog.syslog(syslog.LOG_CRIT, msg) \ No newline at end of file + syslog.syslog(syslog.LOG_CRIT, msg) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py b/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py index fae7a5365aba..59a9854fb311 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py @@ -41,8 +41,13 @@ def set_handler(self, key, data): 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 + bfd_enable = arg_list(data['bfd']) if 'bfd' in data else None route_tag = self.ROUTE_ADVERTISE_DISABLE_TAG if 'advertise' in data and data['advertise'] == "false" else self.ROUTE_ADVERTISE_ENABLE_TAG + # bfd enabled route would be handled in staticroutebfd, skip here + if bfd_enable and bfd_enable[0].lower() == "true": + return True + try: ip_nh_set = IpNextHopSet(is_ipv6, bkh_list, nh_list, intf_list, dist_list, nh_vrf_list) cur_nh_set, cur_route_tag = self.static_routes.get(vrf, {}).get(ip_prefix, (IpNextHopSet(is_ipv6), route_tag)) diff --git a/src/sonic-bgpcfgd/setup.py b/src/sonic-bgpcfgd/setup.py index b451accb9792..30298fea989a 100755 --- a/src/sonic-bgpcfgd/setup.py +++ b/src/sonic-bgpcfgd/setup.py @@ -11,6 +11,7 @@ entry_points = { 'console_scripts': [ 'bgpcfgd = bgpcfgd.main:main', + 'staticroutebfd = staticroutebfd.main:main', 'bgpmon = bgpmon.bgpmon:main', ] }, diff --git a/src/sonic-bgpcfgd/staticroutebfd/__init__.py b/src/sonic-bgpcfgd/staticroutebfd/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py new file mode 100644 index 000000000000..fb8fed943d10 --- /dev/null +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -0,0 +1,664 @@ +import os +import signal +import sys +import syslog +import threading +import traceback +from collections import defaultdict +from ipaddress import IPv4Address, IPv6Address + +from swsscommon import swsscommon + +g_debug = True +g_run = True + +ROUTE_ADVERTISE_ENABLE_TAG = '1' +ROUTE_ADVERTISE_DISABLE_TAG = '2' + +CONFIG_DB_NAME = "CONFIG_DB" +APPL_DB_NAME = "APPL_DB" +STATE_DB_NAME = "STATE_DB" + +LOOPBACK_TABLE_NAME = "LOOPBACK_INTERFACE" +INTERFACE_TABLE_NAME = "INTERFACE" +PORTCHANNEL_INTERFACE_TABLE_NAME = "PORTCHANNEL_INTERFACE" +STATIC_ROUTE_TABLE_NAME = "STATIC_ROUTE" +BFD_SESSION_TABLE_NAME = "BFD_SESSION_TABLE" + +LOCAL_CONFIG_TABLE = "config" +LOCAL_NEXTHOP_TABLE = "nexthop" +LOCAL_SR_TABLE = "sr" +LOCAL_BFD_TABLE = "bfd" +LOCAL_INTERFACE_TABLE = "interface" +LOCAL_LOOPBACK_TABLE = "loopback" + +LOOPBACK0_NAME = "Loopback0" +LOOPBACK4096_NAME = "Loopback4096" + +def log_debug(msg): + """ Send a message msg to the syslog as DEBUG """ + if g_debug: + syslog.syslog(syslog.LOG_DEBUG, msg) + +def log_notice(msg): + """ Send a message msg to the syslog as NOTICE """ + syslog.syslog(syslog.LOG_NOTICE, msg) + +def log_info(msg): + """ Send a message msg to the syslog as INFO """ + syslog.syslog(syslog.LOG_INFO, msg) + +def log_warn(msg): + """ Send a message msg to the syslog as WARNING """ + syslog.syslog(syslog.LOG_WARNING, msg) + +def log_err(msg): + """ Send a message msg to the syslog as ERR """ + syslog.syslog(syslog.LOG_ERR, msg) + +def log_crit(msg): + """ Send a message msg to the syslog as CRIT """ + syslog.syslog(syslog.LOG_CRIT, msg) + +def signal_handler(_, __): # signal_handler(signum, frame) + """ signal handler """ + global g_run + g_run = False + +def static_route_split_key(key): + """ + Split key into vrf name and prefix. + :param key: key to split + :return: valid, vrf name extracted from the key, ip prefix extracted from the key + """ + l = tuple(key.split('|')) + + if len(l) == 1: + return True, 'default', l[0] + + return True, l[0], l[1] + +def check_ip(ip): + if len(ip) == 0: + return False, False, "" + + value = ip.split('/',1) + v = value[0] + try: + IPv4Address(v) + valid = True + is_ipv4 = True + except: + is_ipv4 = False + try: + IPv6Address(v) + valid = True + except: + valid = False + return valid, is_ipv4, v + +class StaticRouteBfd(object): + + SELECT_TIMEOUT = 1000 + BFD_DEFAULT_CFG = {"multihop": "true", "rx_interval": "600", "tx_interval": "600"} + + def __init__(self): + self.local_db = defaultdict(dict) + self.local_db[LOCAL_CONFIG_TABLE] = defaultdict(dict) + self.local_db[LOCAL_NEXTHOP_TABLE] = defaultdict(set) + self.local_db[LOCAL_SR_TABLE] = defaultdict(set) + self.local_db[LOCAL_BFD_TABLE] = defaultdict(dict) + #interface, portchannel_interface and loopback_interface share same table, assume name is unique + #assume only one ipv4 and/or one ipv6 for each interface + self.local_db[LOCAL_INTERFACE_TABLE] = defaultdict(dict) + + self.config_db = swsscommon.DBConnector(CONFIG_DB_NAME, 0, True) + self.appl_db = swsscommon.DBConnector(APPL_DB_NAME, 0, True) + self.state_db = swsscommon.DBConnector(STATE_DB_NAME, 0, True) + + self.bfd_appl_tbl = swsscommon.ProducerStateTable(self.appl_db, BFD_SESSION_TABLE_NAME) + + self.static_route_appl_tbl = swsscommon.Table(self.appl_db, STATIC_ROUTE_TABLE_NAME) + + #to use SonicV2Connector get_all method, DBConnector doesn't have get_all + self.db = swsscommon.SonicV2Connector() + self.db.connect(self.db.CONFIG_DB) + self.db.connect(self.db.APPL_DB) + self.db.connect(self.db.STATE_DB) + + self.selector = swsscommon.Select() + self.callbacks = defaultdict(lambda: defaultdict(list)) # db -> table -> handlers[] + self.subscribers = set() + self.first_time = True + + def get_ip_from_key(self, key): + """ + Get ip address from key for LOOPBACK/INTERFACE/PORTCHANNEL_INTERFACE table + :param key: key in the tables + :return: valid, is_ipv4, ip address + """ + if '|' not in key: + return False, False, "", "" + else: + if_ip = key.split('|') + if len(if_ip) < 2: + return False, False, "", "" + if_name = if_ip[0] + value = if_ip[1] + valid, is_ipv4, ip = check_ip(value) + log_debug("get ip from intf key: valid %s is_ipv4 %s, if_name %s ip %s"%(str(valid), str(is_ipv4), if_name, ip)) + return valid, is_ipv4, if_name, ip + + def set_local_db(self, table, key, data): + try: + self.local_db[table][key] = data + return + except: + log_err("set_local_db error, table %s key %s"%(table, key)) + pass + + def get_local_db(self, table, key): + #FIXME how to handle key does not exist case, to add item + try: + v = self.local_db[table][key] + return v + except: + return {} + + def remove_from_local_db(self, table, key): + if table in self.local_db: + if key in self.local_db[table]: + del self.local_db[table][key] + + def append_to_nh_table_entry(self, nh_key, ip_prefix): + #FIXME consider first time adding nh_key + entry = self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key) + entry.add(ip_prefix) + + def remove_from_nh_table_entry(self, nh_key, ip_prefix): + entry = self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key) + if ip_prefix in entry: + entry.remove(ip_prefix) + if len(entry) == 0: + self.remove_from_local_db(LOCAL_NEXTHOP_TABLE, nh_key) + + def set_bfd_session_into_appl_db(self, key, data): + fvs = swsscommon.FieldValuePairs(list(data.items())) + self.bfd_appl_tbl.set(key, fvs) + log_debug("set bfd session to appl_db, key %s, data %s"%(key, str(data))) + + def del_bfd_session_from_appl_db(self, key): + self.appl_db.delete("BFD_SESSION_TABLE:{}".format(key)) + + def interface_set_handler(self, key, data): + valid, is_ipv4, if_name, ip = self.get_ip_from_key(key) + if not valid: + return True + value = self.get_local_db(LOCAL_INTERFACE_TABLE, if_name) + if len(value) == 0: + value = {is_ipv4: ip} + else: + value[is_ipv4] = ip + self.set_local_db(LOCAL_INTERFACE_TABLE, if_name, value) + return True + + def interface_del_handler(self, key, data): + valid, is_ipv4, if_name, ip = self.get_ip_from_key(key) + if not valid: + return True + value = self.get_local_db(LOCAL_INTERFACE_TABLE, if_name) + if len(value) == 0: + return + else: + value[is_ipv4] = "" #remove the IP address for the interface + self.set_local_db(LOCAL_INTERFACE_TABLE, if_name, value) + + def find_interface_ip(self, if_name, ip_example): + valid, is_ipv4, nh = check_ip(ip_example) + if not valid: + return False, "" + + value = self.get_local_db(LOCAL_INTERFACE_TABLE, if_name) + ip = value.get(is_ipv4, "") + if len(ip)>0: #ip should be verified when add to local_db + return True, ip + + #if there is no ip for the interface, try loopback ip address + value = self.get_local_db(LOCAL_INTERFACE_TABLE, LOOPBACK4096_NAME) + + lp4096_ip = value.get(is_ipv4, "") + if len(lp4096_ip)>0: + return True, lp4096_ip + value = self.get_local_db(LOCAL_INTERFACE_TABLE, LOOPBACK0_NAME) + + lp0_ip = value.get(is_ipv4, "") + if len(lp0_ip)>0: + return True, lp0_ip + + return False, "" + + def strip_table_name(self, key, splitter): + return key.split(splitter, 1)[1] + + def reconciliation(self): + #MUST keep the restore sequene + #restore interface(loopback/interface/portchannel_interface) tables + + #restore interface tables + log_info("restore interface table -->") + keys = self.db.keys(self.db.CONFIG_DB, "LOOPBACK_INTERFACE|*") + for key in keys: + key_new = self.strip_table_name(key, "|") + self.interface_set_handler(key_new, "") + keys = self.db.keys(self.db.CONFIG_DB, "INTERFACE|*") + for key in keys: + key_new = self.strip_table_name(key, "|") + self.interface_set_handler(key_new, "") + keys = self.db.keys(self.db.CONFIG_DB, "PORTCHANNEL_INTERFACE|*") + for key in keys: + key_new = self.strip_table_name(key, "|") + self.interface_set_handler(key_new, "") + + #restore bfd session table, static route won't create bfd session if it is already in appl_db + log_info("restore bfd session table -->") + keys = self.db.keys(self.db.APPL_DB, "BFD_SESSION_TABLE:*") + for key in keys: + data = self.db.get_all(self.db.APPL_DB, key) + key_new = self.strip_table_name(key, ":") + self.set_local_db(LOCAL_BFD_TABLE, key_new, data) + + #restore static route table + log_info("restore static route table -->") + keys = self.db.keys(self.db.CONFIG_DB, "STATIC_ROUTE|*") + for key in keys: + data = self.db.get_all(self.db.CONFIG_DB, key) + key_new = self.strip_table_name(key, "|") + self.static_route_set_handler(key_new, data) + + #clean up local bfd table, remove non static route bfd session + log_info("cleanup bfd session table -->") + self.cleanup_local_bfd_table() + + #restore bfd state table + log_info("restore bfd state table -->") + keys = self.db.keys(self.db.STATE_DB, "BFD_SESSION_TABLE|*") + for key in keys: + data = self.db.get_all(self.db.STATE_DB, key) + key_new = self.strip_table_name(key, "|") + self.bfd_state_set_handler(key_new, data) + + def cleanup_local_bfd_table(self): + kl=[] + for key in self.local_db[LOCAL_BFD_TABLE]: + kl.append(key) + for key in kl: + bfd_session = self.local_db[LOCAL_BFD_TABLE][key] + if "static_route" not in bfd_session or bfd_session["static_route"] != "true": + self.local_db[LOCAL_BFD_TABLE].pop(key) + + def check_skip(self, bfd_enable): + if isinstance(bfd_enable, list): + if len(bfd_enable) == 1: + if isinstance(bfd_enable[0], str): + if bfd_enable[0].lower() == "true": + return False + return True + + def static_route_set_handler(self, key, data): + global ROUTE_ADVERTISE_ENABLE_TAG + global ROUTE_ADVERTISE_DISABLE_TAG + + #sanity checking + if len(data) == 0: + return True + + valid, vrf, ip_prefix = static_route_split_key(key) + local_db_key = vrf + "|" + ip_prefix + if not valid: + return True + + valid, is_ipv4, ip = check_ip(ip_prefix) + if not valid: + log_err("invalid ip prefix for static route: ", key) + return True + + arg_list = lambda v: v.split(',') if len(v.strip()) != 0 else None + bfd_enable = arg_list(data['bfd']) if 'bfd' in data else False + # this process, staticroutebfd, only handle the bfd enabled case, other cases would be handled in bgpcfgd/StaticRouteMgr + if self.check_skip(bfd_enable): + 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 + route_tag = ROUTE_ADVERTISE_DISABLE_TAG if 'advertise' in data and data['advertise'] == "false" else ROUTE_ADVERTISE_ENABLE_TAG + 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.") + return True + + + data_exist = self.get_local_db(LOCAL_CONFIG_TABLE, local_db_key) + if data_exist: + # route with the prefix already exist, remove the deleted nexthops + nh_list_exist = arg_list(data_exist['nexthop']) if 'nexthop' in data else None + nh_vrf_list_exist = arg_list(data_exist['nexthop-vrf']) if 'nexthop-vrf' in data else None + if nh_vrf_list_exist is None: + nh_vrf_list_exist = [] + for nh in nh_list: + nh_vrf_list_exist.append(vrf) + + intf_list_exist = arg_list(data_exist['ifname']) if 'ifname' in data else None + nh_key_list_exist = list(zip(nh_vrf_list_exist, intf_list_exist, nh_list_exist)) + nh_key_list_new = list(zip(nh_vrf_list, intf_list, nh_list)) + for nh in nh_key_list_exist: + if nh not in nh_key_list_new: + nh_vrf = nh[0] + nh_ip = nh[2] + nh_key = nh_vrf + "|" + nh_ip + self.remove_from_nh_table_entry(nh_key, vrf + "|" + ip_prefix) + 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) + + self.set_local_db(LOCAL_CONFIG_TABLE, local_db_key, data) + for index in range(len(nh_list)): + nh_ip = nh_list[index] + intf = intf_list[index] + nh_vrf = nh_vrf_list[index] + nh_key = nh_vrf + "|" + nh_ip + + #check if the bfd session is already created + bfd_key = nh_vrf + ":default:" + nh_ip + bfd_session = self.get_local_db(LOCAL_BFD_TABLE, bfd_key) + if len(bfd_session)>0: + self.local_db[LOCAL_BFD_TABLE][bfd_key]["static_route"] = "true" + + if len(self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key)) == 0 and len(bfd_session) == 0: + valid, local_addr = self.find_interface_ip(intf, nh_ip) + if not valid: + log_warn("cannot find ip for interface: ", intf) + continue + bfd_entry_cfg = self.BFD_DEFAULT_CFG.copy() + bfd_entry_cfg["local_addr"] = local_addr + self.set_bfd_session_into_appl_db(bfd_key, bfd_entry_cfg) + bfd_entry_cfg["static_route"] = "true" + self.set_local_db(LOCAL_BFD_TABLE, bfd_key, bfd_entry_cfg) + + self.append_to_nh_table_entry(nh_key, vrf + "|" + ip_prefix) + + return True + + def static_route_del_handler(self, key): + valid, vrf, ip_prefix = self.static_route_split_key(key) + if not valid: + return True + local_db_key = vrf + "|" + ip_prefix + + valid, is_ipv4, ip = check_ip(ip_prefix) + if not valid: + return True + + data = self.get_local_db(LOCAL_CONFIG_TABLE, local_db_key) + if len(data) == 0: + # this route is not handled by StaticRouteBfd, skip + return True + + arg_list = lambda v: 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, vrf + "|" + ip_prefix) + + 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) + + return True + + def interface_callback(self, key, op, data): + if op == swsscommon.SET_COMMAND: + self.interface_set_handler(key, data) + elif op == swsscommon.DEL_COMMAND: + self.interface_del_handler(key, data) + else: + log_err("Invalid operation '%s' for key '%s'" % (op, key)) + + def static_route_callback(self, key, op, data): + global ROUTE_ADVERTISE_ENABLE_TAG + global ROUTE_ADVERTISE_DISABLE_TAG + ROUTE_ADVERTISE_ENABLE_TAG = '1' + ROUTE_ADVERTISE_DISABLE_TAG = '2' + + if op == swsscommon.SET_COMMAND: + self.static_route_set_handler(key, data) + elif op == swsscommon.DEL_COMMAND: + self.static_route_del_handler(key) + else: + log_err("Invalid operation '%s' for key '%s'" % (op, key)) + + def bfd_state_split_key(self, key): + """ + Split key into table name, vrf name, interface name and peer ip. + :param key: key to split + :return: table name, vrf name, interface name and peer ip extracted from the key + """ + if key.count("|") < 2: + return 'default', 'default', key + else: + return tuple(key.split('|')) + + def append_to_sr_table_entry(self, sr_key, nh_info): + entry = self.get_local_db(LOCAL_SR_TABLE, sr_key) + entry.add(nh_info) + + def remove_from_sr_table_entry(self, sr_key, nh_info): + entry = self.get_local_db(LOCAL_SR_TABLE, sr_key) + if nh_info in entry: + entry.remove(nh_info) + if len(entry) == 0: + self.remove_from_local_db(LOCAL_SR_TABLE, sr_key) + + def set_static_route_into_appl_db(self, key, data): + fvs = swsscommon.FieldValuePairs(list(data.items())) + self.static_route_appl_tbl.set(key, fvs) + log_debug("set static route to appl_db, key %s, data %s"%(key, str(data))) + + def del_static_route_from_appl_db(self, key): + self.static_route_appl_tbl.delete(key) + + def reconstruct_static_route_config(self, original_config, reachable_nexthops): + arg_list = lambda v: v.split(',') if len(v.strip()) != 0 else None + bkh_list = arg_list(original_config['blackhole']) if 'blackhole' in original_config else None + nh_list = arg_list(original_config['nexthop']) if 'nexthop' in original_config else None + intf_list = arg_list(original_config['ifname']) if 'ifname' in original_config else None + dist_list = arg_list(original_config['distance']) if 'distance' in original_config else None + nh_vrf_list = arg_list(original_config['nexthop-vrf']) if 'nexthop-vrf' in original_config else None + + bkh_candidate = "" + nh_candidate = "" + intf_candidate = "" + dist_candidate = "" + nh_vrf_candidate = "" + + + for i in range(len(nh_list)): + if (nh_vrf_list[i], nh_list[i]) in reachable_nexthops: + bkh_candidate += "," + (bkh_list[i] if bkh_list else "") + nh_candidate += "," + (nh_list[i] if nh_list else "") + intf_candidate += "," + (intf_list[i] if intf_list else "") + dist_candidate += "," + (dist_list[i] if dist_list else "") + nh_vrf_candidate += "," + (nh_vrf_list[i] if nh_vrf_list else "") + + new_config = dict() + for key in original_config: + if key == "bfd": + continue + if key == "blackhole": + new_config[key] = bkh_candidate[1:] + elif key == "nexthop": + new_config[key] = nh_candidate[1:] + elif key == "ifname": + new_config[key] = intf_candidate[1:] + elif key == "distance": + new_config[key] = dist_candidate[1:] + elif key == "nexthop-vrf": + new_config[key] = nh_vrf_candidate[1:] + else: + new_config[key] = original_config[key] + new_config["expiry"] = "false" + + return new_config + + def bfd_state_set_handler(self, key, data): + #key are diff in state db and appl_db + vrf, intf, peer_ip = self.bfd_state_split_key(key) + bfd_key = vrf + ":" + intf + ":" + peer_ip + + #check if the BFD session is in local table + bfd_session = self.get_local_db(LOCAL_BFD_TABLE, bfd_key) + if len(bfd_session) == 0: + return True + + nh_key = vrf + "|" + peer_ip + state = data['state'] if 'state' in data else "DOWN" + log_info("bfd seesion %s state %s" %(bfd_key, state)) + + if state.upper() == "UP": + for prefix in self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key): + sr_key = prefix + config_key = prefix + self.append_to_sr_table_entry(sr_key, (vrf, peer_ip)) + config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) + new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SR_TABLE, sr_key)) + self.set_static_route_into_appl_db(sr_key.replace("|", ":"), new_config) + + elif state.upper() == "DOWN": + for prefix in self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key): + sr_key = prefix + config_key = prefix + self.remove_from_sr_table_entry(sr_key, (vrf, peer_ip)) + if len(self.get_local_db(LOCAL_SR_TABLE, sr_key)) == 0: + self.del_static_route_from_appl_db(sr_key.replace("|", ":")) + else: + config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) + new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SR_TABLE, sr_key)) + self.set_static_route_into_appl_db(sr_key.replace("|", ":"), new_config) + + + def bfd_state_del_handler(self, key, data): + vrf, intf, peer_ip = self.bfd_state_split_key(key) + bfd_key = vrf + ":" + intf + ":" + peer_ip + + #check if the BFD session is in local table + bfd_session = self.get_local_db(LOCAL_BFD_TABLE, bfd_key) + if len(bfd_session) == 0: + return True + + nh_key = vrf + "|" + peer_ip + + for prefix in self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key): + sr_key = prefix + config_key = prefix + self.remove_from_sr_table_entry(sr_key, (vrf, peer_ip)) + if len(self.get_local_db(LOCAL_SR_TABLE, bfd_key)) == 0: + self.del_static_route_from_appl_db(sr_key.replace("|", ":")) + else: + config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) + new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SR_TABLE, sr_key)) + self.set_static_route_into_appl_db(sr_key.replace("|", ":"), new_config) + + def bfd_state_callback(self, key, op, data): + if op == swsscommon.SET_COMMAND: + self.bfd_state_set_handler(key, data) + elif op == swsscommon.DEL_COMMAND: + self.bfd_state_route_del_handler(key) + else: + log_err("Invalid operation '%s' for key '%s'" % (op, key)) + + + def prepare_selector(self): + interface_subscriber = swsscommon.SubscriberStateTable(self.config_db, INTERFACE_TABLE_NAME) + portchannel_interface_subscriber = swsscommon.SubscriberStateTable(self.config_db, PORTCHANNEL_INTERFACE_TABLE_NAME) + static_route_subscriber = swsscommon.SubscriberStateTable(self.config_db, STATIC_ROUTE_TABLE_NAME) + bfd_state_subscriber = swsscommon.SubscriberStateTable(self.state_db, swsscommon.STATE_BFD_SESSION_TABLE_NAME) + + self.selector.addSelectable(interface_subscriber) + self.selector.addSelectable(portchannel_interface_subscriber) + self.selector.addSelectable(static_route_subscriber) + self.selector.addSelectable(bfd_state_subscriber) + + self.subscribers.add(interface_subscriber) + self.subscribers.add(portchannel_interface_subscriber) + self.subscribers.add(static_route_subscriber) + self.subscribers.add(bfd_state_subscriber) + + self.callbacks[self.config_db.getDbId()][LOOPBACK_TABLE_NAME].append(self.interface_callback) + self.callbacks[self.config_db.getDbId()][INTERFACE_TABLE_NAME].append(self.interface_callback) + self.callbacks[self.config_db.getDbId()][PORTCHANNEL_INTERFACE_TABLE_NAME].append(self.interface_callback) + self.callbacks[self.config_db.getDbId()][STATIC_ROUTE_TABLE_NAME].append(self.static_route_callback) + self.callbacks[self.state_db.getDbId()][swsscommon.STATE_BFD_SESSION_TABLE_NAME].append(self.bfd_state_callback) + + def run(self): + self.prepare_selector() + while g_run: + state, _ = self.selector.select(self.SELECT_TIMEOUT) + if state == self.selector.TIMEOUT: + continue + elif state == self.selector.ERROR: + raise Exception("Received error from select") + + if self.first_time: + self.first_time = False + self.reconciliation() + + for sub in self.subscribers: + while True: + key, op, fvs = sub.pop() + if len(key) == 0: + break + log_debug("Received message : '%s'" % str((key, op, fvs))) + for callback in self.callbacks[sub.getDbConnector().getDbId()][sub.getTableName()]: + callback(key, op, dict(fvs)) + + +def do_work(): + sr_bfd = StaticRouteBfd() + sr_bfd.run() + +def main(): + rc = 0 + try: + syslog.openlog('staticroutebfd') + signal.signal(signal.SIGTERM, signal_handler) + signal.signal(signal.SIGINT, signal_handler) + do_work() + except KeyboardInterrupt: + syslog.syslog(syslog.LOG_NOTICE, "Keyboard interrupt") + pass + except RuntimeError as exc: + syslog.syslog(syslog.LOG_CRIT, str(exc)) + rc = -2 + if g_debug: + raise + except Exception as exc: + syslog.syslog(syslog.LOG_CRIT, "Got an exception %s: Traceback: %s" % (str(exc), traceback.format_exc())) + rc = -1 + if g_debug: + raise + finally: + syslog.closelog() + try: + sys.exit(rc) + except SystemExit: + os._exit(rc) From 5e96ff3d65c67b6515e537b273430ee64f74fa39 Mon Sep 17 00:00:00 2001 From: qiwang4 Date: Fri, 3 Mar 2023 15:16:01 +0800 Subject: [PATCH 02/16] fix some known issues --- src/sonic-bgpcfgd/staticroutebfd/main.py | 71 ++++++++++++++++-------- 1 file changed, 49 insertions(+), 22 deletions(-) diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py index fb8fed943d10..9c4aa2524ae3 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/main.py +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -188,7 +188,7 @@ def set_bfd_session_into_appl_db(self, key, data): log_debug("set bfd session to appl_db, key %s, data %s"%(key, str(data))) def del_bfd_session_from_appl_db(self, key): - self.appl_db.delete("BFD_SESSION_TABLE:{}".format(key)) + self.bfd_appl_tbl.delete(key) def interface_set_handler(self, key, data): valid, is_ipv4, if_name, ip = self.get_ip_from_key(key) @@ -202,7 +202,7 @@ def interface_set_handler(self, key, data): self.set_local_db(LOCAL_INTERFACE_TABLE, if_name, value) return True - def interface_del_handler(self, key, data): + def interface_del_handler(self, key): valid, is_ipv4, if_name, ip = self.get_ip_from_key(key) if not valid: return True @@ -304,6 +304,28 @@ def check_skip(self, bfd_enable): return False return True + def refresh_active_nh(self, route_cfg_key): + data = self.get_local_db(LOCAL_CONFIG_TABLE, route_cfg_key) + + arg_list = lambda v: 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 + bfd_key = nh_vrf + ":default:" + nh_ip + + bfd_session = self.get_local_db(LOCAL_BFD_TABLE, bfd_key) + if len(bfd_session) == 0: + continue + if "state" in bfd_session and bfd_session["state"].upper() == "UP": + self.append_to_sr_table_entry(route_cfg_key, (nh_vrf, nh_ip)) + + new_config = self.reconstruct_static_route_config(data, self.get_local_db(LOCAL_SR_TABLE, route_cfg_key)) + self.set_static_route_into_appl_db(route_cfg_key.replace("|", ":"), new_config) + def static_route_set_handler(self, key, data): global ROUTE_ADVERTISE_ENABLE_TAG global ROUTE_ADVERTISE_DISABLE_TAG @@ -313,7 +335,7 @@ def static_route_set_handler(self, key, data): return True valid, vrf, ip_prefix = static_route_split_key(key) - local_db_key = vrf + "|" + ip_prefix + route_cfg_key = vrf + "|" + ip_prefix if not valid: return True @@ -343,7 +365,7 @@ def static_route_set_handler(self, key, data): return True - data_exist = self.get_local_db(LOCAL_CONFIG_TABLE, local_db_key) + data_exist = self.get_local_db(LOCAL_CONFIG_TABLE, route_cfg_key) if data_exist: # route with the prefix already exist, remove the deleted nexthops nh_list_exist = arg_list(data_exist['nexthop']) if 'nexthop' in data else None @@ -361,13 +383,14 @@ def static_route_set_handler(self, key, data): nh_vrf = nh[0] nh_ip = nh[2] nh_key = nh_vrf + "|" + nh_ip - self.remove_from_nh_table_entry(nh_key, vrf + "|" + ip_prefix) + self.remove_from_sr_table_entry(route_cfg_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) - self.set_local_db(LOCAL_CONFIG_TABLE, local_db_key, data) + self.set_local_db(LOCAL_CONFIG_TABLE, route_cfg_key, data) for index in range(len(nh_list)): nh_ip = nh_list[index] intf = intf_list[index] @@ -383,7 +406,7 @@ def static_route_set_handler(self, key, data): if len(self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key)) == 0 and len(bfd_session) == 0: valid, local_addr = self.find_interface_ip(intf, nh_ip) if not valid: - log_warn("cannot find ip for interface: ", intf) + log_warn("cannot find ip for interface: %s" %intf) continue bfd_entry_cfg = self.BFD_DEFAULT_CFG.copy() bfd_entry_cfg["local_addr"] = local_addr @@ -393,21 +416,23 @@ def static_route_set_handler(self, key, data): self.append_to_nh_table_entry(nh_key, vrf + "|" + ip_prefix) + self.refresh_active_nh(route_cfg_key) + return True def static_route_del_handler(self, key): - valid, vrf, ip_prefix = self.static_route_split_key(key) + valid, vrf, ip_prefix = static_route_split_key(key) if not valid: return True - local_db_key = vrf + "|" + ip_prefix + route_cfg_key = vrf + "|" + ip_prefix valid, is_ipv4, ip = check_ip(ip_prefix) if not valid: return True - data = self.get_local_db(LOCAL_CONFIG_TABLE, local_db_key) + data = self.get_local_db(LOCAL_CONFIG_TABLE, route_cfg_key) if len(data) == 0: - # this route is not handled by StaticRouteBfd, skip + # this route is not handled by StaticRouteBfd, skip return True arg_list = lambda v: v.split(',') if len(v.strip()) != 0 else None @@ -417,20 +442,23 @@ def static_route_del_handler(self, key): 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, vrf + "|" + ip_prefix) + 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) + self.del_static_route_from_appl_db(route_cfg_key.replace("|", ":")) + self.remove_from_local_db(LOCAL_INTERFACE_TABLE, route_cfg_key) + return True def interface_callback(self, key, op, data): if op == swsscommon.SET_COMMAND: self.interface_set_handler(key, data) elif op == swsscommon.DEL_COMMAND: - self.interface_del_handler(key, data) + self.interface_del_handler(key) else: log_err("Invalid operation '%s' for key '%s'" % (op, key)) @@ -520,8 +548,10 @@ def reconstruct_static_route_config(self, original_config, reachable_nexthops): return new_config + def bfd_state_set_handler(self, key, data): - #key are diff in state db and appl_db + #key are diff in state db and appl_db, + #intf is always default for multihop bfd vrf, intf, peer_ip = self.bfd_state_split_key(key) bfd_key = vrf + ":" + intf + ":" + peer_ip @@ -534,6 +564,8 @@ def bfd_state_set_handler(self, key, data): state = data['state'] if 'state' in data else "DOWN" log_info("bfd seesion %s state %s" %(bfd_key, state)) + self.local_db[LOCAL_BFD_TABLE][bfd_key]["state"] = state + if state.upper() == "UP": for prefix in self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key): sr_key = prefix @@ -556,22 +588,17 @@ def bfd_state_set_handler(self, key, data): self.set_static_route_into_appl_db(sr_key.replace("|", ":"), new_config) - def bfd_state_del_handler(self, key, data): + def bfd_state_del_handler(self, key): vrf, intf, peer_ip = self.bfd_state_split_key(key) bfd_key = vrf + ":" + intf + ":" + peer_ip - #check if the BFD session is in local table - bfd_session = self.get_local_db(LOCAL_BFD_TABLE, bfd_key) - if len(bfd_session) == 0: - return True - nh_key = vrf + "|" + peer_ip for prefix in self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key): sr_key = prefix config_key = prefix self.remove_from_sr_table_entry(sr_key, (vrf, peer_ip)) - if len(self.get_local_db(LOCAL_SR_TABLE, bfd_key)) == 0: + if len(self.get_local_db(LOCAL_SR_TABLE, sr_key)) == 0: self.del_static_route_from_appl_db(sr_key.replace("|", ":")) else: config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) @@ -582,7 +609,7 @@ def bfd_state_callback(self, key, op, data): if op == swsscommon.SET_COMMAND: self.bfd_state_set_handler(key, data) elif op == swsscommon.DEL_COMMAND: - self.bfd_state_route_del_handler(key) + self.bfd_state_del_handler(key) else: log_err("Invalid operation '%s' for key '%s'" % (op, key)) From fa52a10ce735ef41d8bb3590f6ddbbe5325b34b3 Mon Sep 17 00:00:00 2001 From: qiwang4 Date: Tue, 4 Apr 2023 12:03:14 +0800 Subject: [PATCH 03/16] address Baorong and Venkatesan's comments --- src/sonic-bgpcfgd/bgpcfgd/log.py | 2 +- src/sonic-bgpcfgd/staticroutebfd/main.py | 77 +++++++++++------------- src/sonic-bgpcfgd/staticroutebfd/vars.py | 5 ++ 3 files changed, 42 insertions(+), 42 deletions(-) create mode 100644 src/sonic-bgpcfgd/staticroutebfd/vars.py diff --git a/src/sonic-bgpcfgd/bgpcfgd/log.py b/src/sonic-bgpcfgd/bgpcfgd/log.py index 28c3cc39bcfd..4083b13aa6ad 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/log.py +++ b/src/sonic-bgpcfgd/bgpcfgd/log.py @@ -30,4 +30,4 @@ def log_err(msg): def log_crit(msg): """ Send a message msg to the syslog as CRIT """ - syslog.syslog(syslog.LOG_CRIT, msg) + syslog.syslog(syslog.LOG_CRIT, msg) \ No newline at end of file diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py index 9c4aa2524ae3..431355b54724 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/main.py +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -9,11 +9,9 @@ from swsscommon import swsscommon -g_debug = True -g_run = True +from .vars import g_debug, bfd_multihop, bfd_rx_interval, bfd_tx_interval, bfd_multiplier -ROUTE_ADVERTISE_ENABLE_TAG = '1' -ROUTE_ADVERTISE_DISABLE_TAG = '2' +g_run = True CONFIG_DB_NAME = "CONFIG_DB" APPL_DB_NAME = "APPL_DB" @@ -27,10 +25,9 @@ LOCAL_CONFIG_TABLE = "config" LOCAL_NEXTHOP_TABLE = "nexthop" -LOCAL_SR_TABLE = "sr" +LOCAL_SRT_TABLE = "srt" LOCAL_BFD_TABLE = "bfd" LOCAL_INTERFACE_TABLE = "interface" -LOCAL_LOOPBACK_TABLE = "loopback" LOOPBACK0_NAME = "Loopback0" LOOPBACK4096_NAME = "Loopback4096" @@ -100,13 +97,13 @@ def check_ip(ip): class StaticRouteBfd(object): SELECT_TIMEOUT = 1000 - BFD_DEFAULT_CFG = {"multihop": "true", "rx_interval": "600", "tx_interval": "600"} + BFD_DEFAULT_CFG = {"multihop": "false", "rx_interval": "50", "tx_interval": "50"} def __init__(self): self.local_db = defaultdict(dict) self.local_db[LOCAL_CONFIG_TABLE] = defaultdict(dict) self.local_db[LOCAL_NEXTHOP_TABLE] = defaultdict(set) - self.local_db[LOCAL_SR_TABLE] = defaultdict(set) + self.local_db[LOCAL_SRT_TABLE] = defaultdict(set) self.local_db[LOCAL_BFD_TABLE] = defaultdict(dict) #interface, portchannel_interface and loopback_interface share same table, assume name is unique #assume only one ipv4 and/or one ipv6 for each interface @@ -321,14 +318,12 @@ def refresh_active_nh(self, route_cfg_key): if len(bfd_session) == 0: continue if "state" in bfd_session and bfd_session["state"].upper() == "UP": - self.append_to_sr_table_entry(route_cfg_key, (nh_vrf, nh_ip)) + self.append_to_srt_table_entry(route_cfg_key, (nh_vrf, nh_ip)) - new_config = self.reconstruct_static_route_config(data, self.get_local_db(LOCAL_SR_TABLE, route_cfg_key)) + new_config = self.reconstruct_static_route_config(data, self.get_local_db(LOCAL_SRT_TABLE, route_cfg_key)) self.set_static_route_into_appl_db(route_cfg_key.replace("|", ":"), new_config) def static_route_set_handler(self, key, data): - global ROUTE_ADVERTISE_ENABLE_TAG - global ROUTE_ADVERTISE_DISABLE_TAG #sanity checking if len(data) == 0: @@ -355,7 +350,6 @@ def static_route_set_handler(self, key, data): 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 - route_tag = ROUTE_ADVERTISE_DISABLE_TAG if 'advertise' in data and data['advertise'] == "false" else ROUTE_ADVERTISE_ENABLE_TAG 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 '' @@ -383,7 +377,7 @@ def static_route_set_handler(self, key, data): nh_vrf = nh[0] nh_ip = nh[2] nh_key = nh_vrf + "|" + nh_ip - self.remove_from_sr_table_entry(route_cfg_key, (nh_vrf, nh_ip)) + self.remove_from_srt_table_entry(route_cfg_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 @@ -408,7 +402,13 @@ def static_route_set_handler(self, key, data): if not valid: log_warn("cannot find ip for interface: %s" %intf) continue - bfd_entry_cfg = self.BFD_DEFAULT_CFG.copy() + if all([bfd_rx_interval, bfd_tx_interval, bfd_multiplier, bfd_multihop]): + bfd_entry_cfg["multihop"] = bfd_multihop + bfd_entry_cfg["rx_interval"] = bfd_rx_interval + bfd_entry_cfg["tx_interval"] = bfd_tx_interval + bfd_entry_cfg["multiplier"] = bfd_multiplier + else: + bfd_entry_cfg = self.BFD_DEFAULT_CFG.copy() bfd_entry_cfg["local_addr"] = local_addr self.set_bfd_session_into_appl_db(bfd_key, bfd_entry_cfg) bfd_entry_cfg["static_route"] = "true" @@ -463,11 +463,6 @@ def interface_callback(self, key, op, data): log_err("Invalid operation '%s' for key '%s'" % (op, key)) def static_route_callback(self, key, op, data): - global ROUTE_ADVERTISE_ENABLE_TAG - global ROUTE_ADVERTISE_DISABLE_TAG - ROUTE_ADVERTISE_ENABLE_TAG = '1' - ROUTE_ADVERTISE_DISABLE_TAG = '2' - if op == swsscommon.SET_COMMAND: self.static_route_set_handler(key, data) elif op == swsscommon.DEL_COMMAND: @@ -486,16 +481,16 @@ def bfd_state_split_key(self, key): else: return tuple(key.split('|')) - def append_to_sr_table_entry(self, sr_key, nh_info): - entry = self.get_local_db(LOCAL_SR_TABLE, sr_key) + def append_to_srt_table_entry(self, srt_key, nh_info): + entry = self.get_local_db(LOCAL_SRT_TABLE, srt_key) entry.add(nh_info) - def remove_from_sr_table_entry(self, sr_key, nh_info): - entry = self.get_local_db(LOCAL_SR_TABLE, sr_key) + def remove_from_srt_table_entry(self, srt_key, nh_info): + entry = self.get_local_db(LOCAL_SRT_TABLE, srt_key) if nh_info in entry: entry.remove(nh_info) if len(entry) == 0: - self.remove_from_local_db(LOCAL_SR_TABLE, sr_key) + self.remove_from_local_db(LOCAL_SRT_TABLE, srt_key) def set_static_route_into_appl_db(self, key, data): fvs = swsscommon.FieldValuePairs(list(data.items())) @@ -568,24 +563,24 @@ def bfd_state_set_handler(self, key, data): if state.upper() == "UP": for prefix in self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key): - sr_key = prefix + srt_key = prefix config_key = prefix - self.append_to_sr_table_entry(sr_key, (vrf, peer_ip)) + self.append_to_srt_table_entry(srt_key, (vrf, peer_ip)) config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) - new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SR_TABLE, sr_key)) - self.set_static_route_into_appl_db(sr_key.replace("|", ":"), new_config) + new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SRT_TABLE, srt_key)) + self.set_static_route_into_appl_db(srt_key.replace("|", ":"), new_config) elif state.upper() == "DOWN": for prefix in self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key): - sr_key = prefix + srt_key = prefix config_key = prefix - self.remove_from_sr_table_entry(sr_key, (vrf, peer_ip)) - if len(self.get_local_db(LOCAL_SR_TABLE, sr_key)) == 0: - self.del_static_route_from_appl_db(sr_key.replace("|", ":")) + self.remove_from_srt_table_entry(srt_key, (vrf, peer_ip)) + if len(self.get_local_db(LOCAL_SRT_TABLE, srt_key)) == 0: + self.del_static_route_from_appl_db(srt_key.replace("|", ":")) else: config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) - new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SR_TABLE, sr_key)) - self.set_static_route_into_appl_db(sr_key.replace("|", ":"), new_config) + new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SRT_TABLE, srt_key)) + self.set_static_route_into_appl_db(srt_key.replace("|", ":"), new_config) def bfd_state_del_handler(self, key): @@ -595,15 +590,15 @@ def bfd_state_del_handler(self, key): nh_key = vrf + "|" + peer_ip for prefix in self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key): - sr_key = prefix + srt_key = prefix config_key = prefix - self.remove_from_sr_table_entry(sr_key, (vrf, peer_ip)) - if len(self.get_local_db(LOCAL_SR_TABLE, sr_key)) == 0: - self.del_static_route_from_appl_db(sr_key.replace("|", ":")) + self.remove_from_srt_table_entry(srt_key, (vrf, peer_ip)) + if len(self.get_local_db(LOCAL_SRT_TABLE, srt_key)) == 0: + self.del_static_route_from_appl_db(srt_key.replace("|", ":")) else: config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) - new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SR_TABLE, sr_key)) - self.set_static_route_into_appl_db(sr_key.replace("|", ":"), new_config) + new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SRT_TABLE, srt_key)) + self.set_static_route_into_appl_db(srt_key.replace("|", ":"), new_config) def bfd_state_callback(self, key, op, data): if op == swsscommon.SET_COMMAND: diff --git a/src/sonic-bgpcfgd/staticroutebfd/vars.py b/src/sonic-bgpcfgd/staticroutebfd/vars.py new file mode 100644 index 000000000000..71f451a6f4f1 --- /dev/null +++ b/src/sonic-bgpcfgd/staticroutebfd/vars.py @@ -0,0 +1,5 @@ +g_debug = True # FIXME: read from env variable, or from constants +bfd_multihop = "false" +bfd_rx_interval = "40" +bfd_tx_interval = "40" +bfd_multiplier = "3" From 8fe89b5b928d926ad7448060ea082d7b8cd91fee Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Fri, 14 Apr 2023 11:21:15 -0700 Subject: [PATCH 04/16] change default value --- src/sonic-bgpcfgd/staticroutebfd/vars.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sonic-bgpcfgd/staticroutebfd/vars.py b/src/sonic-bgpcfgd/staticroutebfd/vars.py index 71f451a6f4f1..527986827129 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/vars.py +++ b/src/sonic-bgpcfgd/staticroutebfd/vars.py @@ -1,5 +1,5 @@ -g_debug = True # FIXME: read from env variable, or from constants +g_debug = True bfd_multihop = "false" -bfd_rx_interval = "40" -bfd_tx_interval = "40" +bfd_rx_interval = "50" +bfd_tx_interval = "50" bfd_multiplier = "3" From 054f104c513678813d36f7ad5c294be3007975ff Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Fri, 14 Apr 2023 11:52:38 -0700 Subject: [PATCH 05/16] change default value --- src/sonic-bgpcfgd/staticroutebfd/vars.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sonic-bgpcfgd/staticroutebfd/vars.py b/src/sonic-bgpcfgd/staticroutebfd/vars.py index 71f451a6f4f1..527986827129 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/vars.py +++ b/src/sonic-bgpcfgd/staticroutebfd/vars.py @@ -1,5 +1,5 @@ -g_debug = True # FIXME: read from env variable, or from constants +g_debug = True bfd_multihop = "false" -bfd_rx_interval = "40" -bfd_tx_interval = "40" +bfd_rx_interval = "50" +bfd_tx_interval = "50" bfd_multiplier = "3" From 3b03463c0dff43bc2cabadb749ad673144572655 Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Sun, 16 Apr 2023 22:21:26 -0700 Subject: [PATCH 06/16] to support bfd flag dynamic change --- .../bgpcfgd/managers_static_rt.py | 13 +- src/sonic-bgpcfgd/staticroutebfd/main.py | 134 +++++++++++++----- 2 files changed, 104 insertions(+), 43 deletions(-) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py b/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py index 59a9854fb311..aaec3d1038df 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py @@ -46,6 +46,11 @@ def set_handler(self, key, data): # bfd enabled route would be handled in staticroutebfd, skip here if bfd_enable and bfd_enable[0].lower() == "true": + log_debug("{} static route {} bfd flag is true".format(self.db_name, key)) + tmp_nh_set, tmp_route_tag = self.static_routes.get(vrf, {}).get(ip_prefix, (IpNextHopSet(is_ipv6), route_tag)) + if tmp_nh_set: #clear nexthop set if it is not empty + log_debug("{} static route {} bfd flag is true, cur_nh is not empty, clear it".format(self.db_name, key)) + self.static_routes.setdefault(vrf, {}).pop(ip_prefix, None) return True try: @@ -65,9 +70,9 @@ def set_handler(self, key, data): if cmd_list: self.cfg_mgr.push_list(cmd_list) - log_debug("Static route {} is scheduled for updates".format(key)) + log_debug("{} Static route {} is scheduled for updates. {}".format(self.db_name, key, str(cmd_list))) else: - log_debug("Nothing to update for static route {}".format(key)) + log_debug("{} Nothing to update for static route {}".format(self.db_name, key)) self.static_routes.setdefault(vrf, {})[ip_prefix] = (ip_nh_set, route_tag) @@ -90,9 +95,9 @@ def del_handler(self, key): if cmd_list: self.cfg_mgr.push_list(cmd_list) - log_debug("Static route {} is scheduled for updates".format(key)) + log_debug("{} Static route {} is scheduled for updates. {}".format(self.db_name, key, str(cmd_list))) else: - log_debug("Nothing to update for static route {}".format(key)) + log_debug("{} Nothing to update for static route {}".format(self.db_name, key)) self.static_routes.setdefault(vrf, {}).pop(ip_prefix, None) diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py index 431355b54724..460d32963f11 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/main.py +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -4,8 +4,10 @@ import syslog import threading import traceback +import time from collections import defaultdict from ipaddress import IPv4Address, IPv6Address +from copy import deepcopy from swsscommon import swsscommon @@ -117,12 +119,6 @@ def __init__(self): self.static_route_appl_tbl = swsscommon.Table(self.appl_db, STATIC_ROUTE_TABLE_NAME) - #to use SonicV2Connector get_all method, DBConnector doesn't have get_all - self.db = swsscommon.SonicV2Connector() - self.db.connect(self.db.CONFIG_DB) - self.db.connect(self.db.APPL_DB) - self.db.connect(self.db.STATE_DB) - self.selector = swsscommon.Select() self.callbacks = defaultdict(lambda: defaultdict(list)) # db -> table -> handlers[] self.subscribers = set() @@ -155,7 +151,6 @@ def set_local_db(self, table, key, data): pass def get_local_db(self, table, key): - #FIXME how to handle key does not exist case, to add item try: v = self.local_db[table][key] return v @@ -168,7 +163,6 @@ def remove_from_local_db(self, table, key): del self.local_db[table][key] def append_to_nh_table_entry(self, nh_key, ip_prefix): - #FIXME consider first time adding nh_key entry = self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key) entry.add(ip_prefix) @@ -238,38 +232,45 @@ def strip_table_name(self, key, splitter): return key.split(splitter, 1)[1] def reconciliation(self): + #to use SonicV2Connector get_all method, DBConnector doesn't have get_all + db = swsscommon.SonicV2Connector() + db.connect(db.CONFIG_DB) + db.connect(db.APPL_DB) + db.connect(db.STATE_DB) + #MUST keep the restore sequene #restore interface(loopback/interface/portchannel_interface) tables #restore interface tables log_info("restore interface table -->") - keys = self.db.keys(self.db.CONFIG_DB, "LOOPBACK_INTERFACE|*") + keys = db.keys(db.CONFIG_DB, "LOOPBACK_INTERFACE|*") for key in keys: key_new = self.strip_table_name(key, "|") self.interface_set_handler(key_new, "") - keys = self.db.keys(self.db.CONFIG_DB, "INTERFACE|*") + keys = db.keys(db.CONFIG_DB, "INTERFACE|*") for key in keys: key_new = self.strip_table_name(key, "|") self.interface_set_handler(key_new, "") - keys = self.db.keys(self.db.CONFIG_DB, "PORTCHANNEL_INTERFACE|*") + keys = db.keys(db.CONFIG_DB, "PORTCHANNEL_INTERFACE|*") for key in keys: key_new = self.strip_table_name(key, "|") self.interface_set_handler(key_new, "") #restore bfd session table, static route won't create bfd session if it is already in appl_db log_info("restore bfd session table -->") - keys = self.db.keys(self.db.APPL_DB, "BFD_SESSION_TABLE:*") + keys = db.keys(db.APPL_DB, "BFD_SESSION_TABLE:*") for key in keys: - data = self.db.get_all(self.db.APPL_DB, key) + data = db.get_all(db.APPL_DB, key) key_new = self.strip_table_name(key, ":") self.set_local_db(LOCAL_BFD_TABLE, key_new, data) #restore static route table log_info("restore static route table -->") - keys = self.db.keys(self.db.CONFIG_DB, "STATIC_ROUTE|*") + keys = db.keys(db.CONFIG_DB, "STATIC_ROUTE|*") for key in keys: - data = self.db.get_all(self.db.CONFIG_DB, key) + data = db.get_all(db.CONFIG_DB, key) key_new = self.strip_table_name(key, "|") + log_debug("SRT_BFD: restore static route from config_db, key %s, data %s"%(key, str(data))) self.static_route_set_handler(key_new, data) #clean up local bfd table, remove non static route bfd session @@ -278,9 +279,9 @@ def reconciliation(self): #restore bfd state table log_info("restore bfd state table -->") - keys = self.db.keys(self.db.STATE_DB, "BFD_SESSION_TABLE|*") + keys = db.keys(db.STATE_DB, "BFD_SESSION_TABLE|*") for key in keys: - data = self.db.get_all(self.db.STATE_DB, key) + data = db.get_all(db.STATE_DB, key) key_new = self.strip_table_name(key, "|") self.bfd_state_set_handler(key_new, data) @@ -293,13 +294,13 @@ def cleanup_local_bfd_table(self): if "static_route" not in bfd_session or bfd_session["static_route"] != "true": self.local_db[LOCAL_BFD_TABLE].pop(key) - def check_skip(self, bfd_enable): - if isinstance(bfd_enable, list): - if len(bfd_enable) == 1: - if isinstance(bfd_enable[0], str): - if bfd_enable[0].lower() == "true": - return False - return True + def isFieldTrue(self, bfd_field): + if isinstance(bfd_field, list): + if len(bfd_field) == 1: + if isinstance(bfd_field[0], str): + if bfd_field[0].lower() == "true": + return True + return False def refresh_active_nh(self, route_cfg_key): data = self.get_local_db(LOCAL_CONFIG_TABLE, route_cfg_key) @@ -307,6 +308,7 @@ def refresh_active_nh(self, route_cfg_key): arg_list = lambda v: 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 + nh_cnt = 0 for index in range(len(nh_list)): nh_ip = nh_list[index] @@ -319,10 +321,42 @@ def refresh_active_nh(self, route_cfg_key): continue if "state" in bfd_session and bfd_session["state"].upper() == "UP": self.append_to_srt_table_entry(route_cfg_key, (nh_vrf, nh_ip)) + nh_cnt += 1 + + if nh_cnt == 0: + return new_config = self.reconstruct_static_route_config(data, self.get_local_db(LOCAL_SRT_TABLE, route_cfg_key)) self.set_static_route_into_appl_db(route_cfg_key.replace("|", ":"), new_config) + def handle_bfd_change(self, cfg_key, data, to_bfd_enable): + valid, vrf, ip_prefix = static_route_split_key(cfg_key) + key = vrf + ":" + ip_prefix + log_debug("SRT_BFD: handle_bfd_change. key %s, data %s, to_bfd_enable %s"%(key, str(data), str(to_bfd_enable))) + if to_bfd_enable: + #uninstall existing route first, and then start bfd processing + #1, write route with full_nh_list to appl_db, let StaticRouteMgr(appl_db) install this route + #2, delete the above route to uninstall the the route, and then start normal processing + data['bfd'] = "false" + self.set_static_route_into_appl_db(key, data) + time.sleep(0.1) + self.del_static_route_from_appl_db(key) + data['bfd'] = "false" + log_debug("SRT_BFD: bfd toggle to true. uninstall the route first, delete static route from appl_db, key %s"%(key)) + else: + #safely delete the static_route entry created by StaticRouteBfd + #1, write a route with bfd=true to appl_db to clean up cur_nh in StaticRouteMgr + #2, delete the entry from appl_db STATIC_ROUTE_TABLE, StaticRouteMgr do nothing because of empty cur_nh + data['bfd'] = "true" + self.set_static_route_into_appl_db(key, data) + time.sleep(0.1) + self.del_static_route_from_appl_db(key) + log_debug("SRT_BFD: bfd toggle to false. delete static route from appl_db, key %s"%(key)) + + data['bfd'] = "false" + #treat it as static route deletion, but do not delete it from LOCAL_CONFIG_TABLE + self.static_route_del_handler(cfg_key, False) + def static_route_set_handler(self, key, data): #sanity checking @@ -339,10 +373,28 @@ def static_route_set_handler(self, key, data): log_err("invalid ip prefix for static route: ", key) return True - arg_list = lambda v: v.split(',') if len(v.strip()) != 0 else None - bfd_enable = arg_list(data['bfd']) if 'bfd' in data else False + arg_list = lambda v: v.split(',') if len(v.strip()) != 0 else None + bfd_field = arg_list(data['bfd']) if 'bfd' in data else ["false"] + + cur_data = self.get_local_db(LOCAL_CONFIG_TABLE, route_cfg_key) + cur_bfd_enabled = False + if cur_data: + cur_bfd_field = arg_list(cur_data['bfd']) if 'bfd' in cur_data else ["false"] + cur_bfd_enabled = self.isFieldTrue(cur_bfd_field) + # this process, staticroutebfd, only handle the bfd enabled case, other cases would be handled in bgpcfgd/StaticRouteMgr - if self.check_skip(bfd_enable): + bfd_enabled = self.isFieldTrue(bfd_field) + + if cur_data: + if cur_bfd_enabled and not bfd_enabled: # dynamic bfd flag change from TRUE to FALSE + self.handle_bfd_change(key, data, False) + if not cur_bfd_enabled and bfd_enabled: # dynamic bfd flag change from FALSE to TRUE + self.handle_bfd_change(key, data, True) + + 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" + self.set_local_db(LOCAL_CONFIG_TABLE, route_cfg_key, data) return True bkh_list = arg_list(data['blackhole']) if 'blackhole' in data else None @@ -359,17 +411,16 @@ def static_route_set_handler(self, key, data): return True - data_exist = self.get_local_db(LOCAL_CONFIG_TABLE, route_cfg_key) - if data_exist: + if cur_data: # route with the prefix already exist, remove the deleted nexthops - nh_list_exist = arg_list(data_exist['nexthop']) if 'nexthop' in data else None - nh_vrf_list_exist = arg_list(data_exist['nexthop-vrf']) if 'nexthop-vrf' in data else None + nh_list_exist = arg_list(cur_data['nexthop']) if 'nexthop' in cur_data else None + nh_vrf_list_exist = arg_list(cur_data['nexthop-vrf']) if 'nexthop-vrf' in cur_data else None if nh_vrf_list_exist is None: nh_vrf_list_exist = [] for nh in nh_list: nh_vrf_list_exist.append(vrf) - intf_list_exist = arg_list(data_exist['ifname']) if 'ifname' in data else None + intf_list_exist = arg_list(cur_data['ifname']) if 'ifname' in cur_data else None nh_key_list_exist = list(zip(nh_vrf_list_exist, intf_list_exist, nh_list_exist)) nh_key_list_new = list(zip(nh_vrf_list, intf_list, nh_list)) for nh in nh_key_list_exist: @@ -402,13 +453,14 @@ def static_route_set_handler(self, key, data): if not valid: log_warn("cannot find ip for interface: %s" %intf) continue + + bfd_entry_cfg = self.BFD_DEFAULT_CFG.copy() if all([bfd_rx_interval, bfd_tx_interval, bfd_multiplier, bfd_multihop]): bfd_entry_cfg["multihop"] = bfd_multihop bfd_entry_cfg["rx_interval"] = bfd_rx_interval bfd_entry_cfg["tx_interval"] = bfd_tx_interval bfd_entry_cfg["multiplier"] = bfd_multiplier - else: - bfd_entry_cfg = self.BFD_DEFAULT_CFG.copy() + bfd_entry_cfg["local_addr"] = local_addr self.set_bfd_session_into_appl_db(bfd_key, bfd_entry_cfg) bfd_entry_cfg["static_route"] = "true" @@ -420,7 +472,7 @@ def static_route_set_handler(self, key, data): return True - def static_route_del_handler(self, key): + def static_route_del_handler(self, key, redis_del): valid, vrf, ip_prefix = static_route_split_key(key) if not valid: return True @@ -450,7 +502,9 @@ def static_route_del_handler(self, key): self.del_bfd_session_from_appl_db(bfd_key) self.del_static_route_from_appl_db(route_cfg_key.replace("|", ":")) - self.remove_from_local_db(LOCAL_INTERFACE_TABLE, route_cfg_key) + + if redis_del: + self.remove_from_local_db(LOCAL_CONFIG_TABLE, route_cfg_key) return True @@ -466,7 +520,7 @@ def static_route_callback(self, key, op, data): if op == swsscommon.SET_COMMAND: self.static_route_set_handler(key, data) elif op == swsscommon.DEL_COMMAND: - self.static_route_del_handler(key) + self.static_route_del_handler(key, True) else: log_err("Invalid operation '%s' for key '%s'" % (op, key)) @@ -495,7 +549,7 @@ def remove_from_srt_table_entry(self, srt_key, nh_info): def set_static_route_into_appl_db(self, key, data): fvs = swsscommon.FieldValuePairs(list(data.items())) self.static_route_appl_tbl.set(key, fvs) - log_debug("set static route to appl_db, key %s, data %s"%(key, str(data))) + log_debug("SRT_BFD: set static route to appl_db, key %s, data %s"%(key, str(data))) def del_static_route_from_appl_db(self, key): self.static_route_appl_tbl.delete(key) @@ -574,8 +628,10 @@ def bfd_state_set_handler(self, key, data): for prefix in self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key): srt_key = prefix config_key = prefix + config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) self.remove_from_srt_table_entry(srt_key, (vrf, peer_ip)) if len(self.get_local_db(LOCAL_SRT_TABLE, srt_key)) == 0: + log_debug("SRT_BFD: bfd_state DOWN. nh_list is empty, delete static route from appl_db, key %s"%(srt_key.replace("|", ":"))) self.del_static_route_from_appl_db(srt_key.replace("|", ":")) else: config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) @@ -594,6 +650,7 @@ def bfd_state_del_handler(self, key): config_key = prefix self.remove_from_srt_table_entry(srt_key, (vrf, peer_ip)) if len(self.get_local_db(LOCAL_SRT_TABLE, srt_key)) == 0: + log_debug("SRT_BFD: bfd_state deletion. nh_list is empty, delete static route from appl_db, key %s"%(srt_key.replace("|", ":"))) self.del_static_route_from_appl_db(srt_key.replace("|", ":")) else: config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) @@ -653,7 +710,6 @@ def run(self): for callback in self.callbacks[sub.getDbConnector().getDbId()][sub.getTableName()]: callback(key, op, dict(fvs)) - def do_work(): sr_bfd = StaticRouteBfd() sr_bfd.run() From 7170bdbd620666d8fe794cc7f28d883256311e04 Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Fri, 21 Apr 2023 10:58:44 -0700 Subject: [PATCH 07/16] change logic for bfd false to true --- src/sonic-bgpcfgd/staticroutebfd/main.py | 32 ++++++++++++++++-------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py index 460d32963f11..6fcd3dc1cf59 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/main.py +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -305,7 +305,7 @@ def isFieldTrue(self, bfd_field): def refresh_active_nh(self, route_cfg_key): data = self.get_local_db(LOCAL_CONFIG_TABLE, route_cfg_key) - arg_list = lambda v: v.split(',') if len(v.strip()) != 0 else None + 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 nh_cnt = 0 @@ -323,9 +323,12 @@ def refresh_active_nh(self, route_cfg_key): self.append_to_srt_table_entry(route_cfg_key, (nh_vrf, nh_ip)) nh_cnt += 1 + #do not write to appl_db is no nexthop reachable if nh_cnt == 0: return + #if there is any bfd session state UP, we don't need to hold the static route update. + data['bfd_nh_hold'] = "false" new_config = self.reconstruct_static_route_config(data, self.get_local_db(LOCAL_SRT_TABLE, route_cfg_key)) self.set_static_route_into_appl_db(route_cfg_key.replace("|", ":"), new_config) @@ -334,15 +337,12 @@ def handle_bfd_change(self, cfg_key, data, to_bfd_enable): key = vrf + ":" + ip_prefix log_debug("SRT_BFD: handle_bfd_change. key %s, data %s, to_bfd_enable %s"%(key, str(data), str(to_bfd_enable))) if to_bfd_enable: - #uninstall existing route first, and then start bfd processing - #1, write route with full_nh_list to appl_db, let StaticRouteMgr(appl_db) install this route - #2, delete the above route to uninstall the the route, and then start normal processing + #write route with full_nh_list to appl_db, let StaticRouteMgr(appl_db) install this route to update its cache data['bfd'] = "false" self.set_static_route_into_appl_db(key, data) time.sleep(0.1) - self.del_static_route_from_appl_db(key) - data['bfd'] = "false" - log_debug("SRT_BFD: bfd toggle to true. uninstall the route first, delete static route from appl_db, key %s"%(key)) + data['bfd'] = "true" + log_debug("SRT_BFD: bfd toggle to true. write the route to appl_db, update StaticRouteMgr(appl_db), key %s"%(key)) else: #safely delete the static_route entry created by StaticRouteBfd #1, write a route with bfd=true to appl_db to clean up cur_nh in StaticRouteMgr @@ -373,7 +373,7 @@ def static_route_set_handler(self, key, data): log_err("invalid ip prefix for static route: ", key) return True - arg_list = lambda v: v.split(',') if len(v.strip()) != 0 else None + arg_list = lambda v: [x.strip() for x in v.split(',')] if len(v.strip()) != 0 else None bfd_field = arg_list(data['bfd']) if 'bfd' in data else ["false"] cur_data = self.get_local_db(LOCAL_CONFIG_TABLE, route_cfg_key) @@ -385,11 +385,16 @@ def static_route_set_handler(self, key, data): # this process, staticroutebfd, only handle the bfd enabled case, other cases would be handled in bgpcfgd/StaticRouteMgr bfd_enabled = self.isFieldTrue(bfd_field) + #when bfd changed from "false" to "true", before bfd session created and state becomes up, + #the installed static route need to be kept in the system system, so put this route in "hold" state until at least one + #bfd session becomes UP. + data['bfd_nh_hold'] = "false" if cur_data: if cur_bfd_enabled and not bfd_enabled: # dynamic bfd flag change from TRUE to FALSE self.handle_bfd_change(key, data, False) if not cur_bfd_enabled and bfd_enabled: # dynamic bfd flag change from FALSE to TRUE self.handle_bfd_change(key, data, True) + data['bfd_nh_hold'] = "true" if not bfd_enabled: #skip if bfd is not enabled, but store it to local_db to detect bfd field dynamic change @@ -411,7 +416,7 @@ def static_route_set_handler(self, key, data): return True - if cur_data: + if cur_data and cur_bfd_enabled: # route with the prefix already exist, remove the deleted nexthops nh_list_exist = arg_list(cur_data['nexthop']) if 'nexthop' in cur_data else None nh_vrf_list_exist = arg_list(cur_data['nexthop-vrf']) if 'nexthop-vrf' in cur_data else None @@ -487,7 +492,7 @@ def static_route_del_handler(self, key, redis_del): # this route is not handled by StaticRouteBfd, skip return True - arg_list = lambda v: v.split(',') if len(v.strip()) != 0 else None + 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)): @@ -555,7 +560,7 @@ def del_static_route_from_appl_db(self, key): self.static_route_appl_tbl.delete(key) def reconstruct_static_route_config(self, original_config, reachable_nexthops): - arg_list = lambda v: v.split(',') if len(v.strip()) != 0 else None + arg_list = lambda v: [x.strip() for x in v.split(',')] if len(v.strip()) != 0 else None bkh_list = arg_list(original_config['blackhole']) if 'blackhole' in original_config else None nh_list = arg_list(original_config['nexthop']) if 'nexthop' in original_config else None intf_list = arg_list(original_config['ifname']) if 'ifname' in original_config else None @@ -621,6 +626,8 @@ def bfd_state_set_handler(self, key, data): config_key = prefix self.append_to_srt_table_entry(srt_key, (vrf, peer_ip)) config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) + #exit "hold" state when any BFD session becomes UP + data['bfd_nh_hold'] = "false" new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SRT_TABLE, srt_key)) self.set_static_route_into_appl_db(srt_key.replace("|", ":"), new_config) @@ -629,6 +636,9 @@ def bfd_state_set_handler(self, key, data): srt_key = prefix config_key = prefix config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) + #skip if the static route is in "hold" state + if data['bfd_nh_hold'] == "true": + continue self.remove_from_srt_table_entry(srt_key, (vrf, peer_ip)) if len(self.get_local_db(LOCAL_SRT_TABLE, srt_key)) == 0: log_debug("SRT_BFD: bfd_state DOWN. nh_list is empty, delete static route from appl_db, key %s"%(srt_key.replace("|", ":"))) From 9245cc098e75380a8d65ea900341df17fc4c9af0 Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Sat, 22 Apr 2023 08:23:42 -0700 Subject: [PATCH 08/16] skil flag field bfd_nh_hold when write to appl_db --- src/sonic-bgpcfgd/staticroutebfd/main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py index 6fcd3dc1cf59..965d97da90eb 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/main.py +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -586,6 +586,8 @@ def reconstruct_static_route_config(self, original_config, reachable_nexthops): for key in original_config: if key == "bfd": continue + if key == "bfd_nh_hold": + continue if key == "blackhole": new_config[key] = bkh_candidate[1:] elif key == "nexthop": From 72b6e5929155d56c4e4f6415803a58cf963a9272 Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Sat, 22 Apr 2023 16:24:25 -0700 Subject: [PATCH 09/16] [static_route]add UT for staticroutebfd and design fixes --- src/sonic-bgpcfgd/staticroutebfd/main.py | 13 +- src/sonic-bgpcfgd/tests/test_static_rt.py | 106 ++++++ src/sonic-bgpcfgd/tests/test_static_rt_bfd.py | 352 ++++++++++++++++++ 3 files changed, 465 insertions(+), 6 deletions(-) create mode 100644 src/sonic-bgpcfgd/tests/test_static_rt_bfd.py diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py index 965d97da90eb..12e736d2d525 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/main.py +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -339,9 +339,9 @@ def handle_bfd_change(self, cfg_key, data, to_bfd_enable): if to_bfd_enable: #write route with full_nh_list to appl_db, let StaticRouteMgr(appl_db) install this route to update its cache data['bfd'] = "false" + data['expiry'] = "false" self.set_static_route_into_appl_db(key, data) time.sleep(0.1) - data['bfd'] = "true" log_debug("SRT_BFD: bfd toggle to true. write the route to appl_db, update StaticRouteMgr(appl_db), key %s"%(key)) else: #safely delete the static_route entry created by StaticRouteBfd @@ -353,7 +353,6 @@ def handle_bfd_change(self, cfg_key, data, to_bfd_enable): self.del_static_route_from_appl_db(key) log_debug("SRT_BFD: bfd toggle to false. delete static route from appl_db, key %s"%(key)) - data['bfd'] = "false" #treat it as static route deletion, but do not delete it from LOCAL_CONFIG_TABLE self.static_route_del_handler(cfg_key, False) @@ -388,12 +387,13 @@ def static_route_set_handler(self, key, data): #when bfd changed from "false" to "true", before bfd session created and state becomes up, #the installed static route need to be kept in the system system, so put this route in "hold" state until at least one #bfd session becomes UP. + data_copy = data.copy() data['bfd_nh_hold'] = "false" if cur_data: if cur_bfd_enabled and not bfd_enabled: # dynamic bfd flag change from TRUE to FALSE - self.handle_bfd_change(key, data, False) + self.handle_bfd_change(key, data_copy, False) if not cur_bfd_enabled and bfd_enabled: # dynamic bfd flag change from FALSE to TRUE - self.handle_bfd_change(key, data, True) + self.handle_bfd_change(key, data_copy, True) data['bfd_nh_hold'] = "true" if not bfd_enabled: @@ -507,6 +507,7 @@ def static_route_del_handler(self, key, redis_del): self.del_bfd_session_from_appl_db(bfd_key) 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: self.remove_from_local_db(LOCAL_CONFIG_TABLE, route_cfg_key) @@ -629,7 +630,7 @@ def bfd_state_set_handler(self, key, data): self.append_to_srt_table_entry(srt_key, (vrf, peer_ip)) config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) #exit "hold" state when any BFD session becomes UP - data['bfd_nh_hold'] = "false" + config_data['bfd_nh_hold'] = "false" new_config = self.reconstruct_static_route_config(config_data, self.get_local_db(LOCAL_SRT_TABLE, srt_key)) self.set_static_route_into_appl_db(srt_key.replace("|", ":"), new_config) @@ -639,7 +640,7 @@ def bfd_state_set_handler(self, key, data): config_key = prefix config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) #skip if the static route is in "hold" state - if data['bfd_nh_hold'] == "true": + if config_data['bfd_nh_hold'] == "true": continue self.remove_from_srt_table_entry(srt_key, (vrf, peer_ip)) if len(self.get_local_db(LOCAL_SRT_TABLE, srt_key)) == 0: diff --git a/src/sonic-bgpcfgd/tests/test_static_rt.py b/src/sonic-bgpcfgd/tests/test_static_rt.py index 881bba1563b7..ba12f17f2128 100644 --- a/src/sonic-bgpcfgd/tests/test_static_rt.py +++ b/src/sonic-bgpcfgd/tests/test_static_rt.py @@ -821,3 +821,109 @@ def test_set_tag_change(): "ip route 10.1.0.0/24 10.0.0.57 tag 2", ] ) + +def test_set_bfd_false(): + mgr = constructor() + set_del_test( + mgr, + "SET", + ("10.1.0.0/24", { + "bfd": "false", + "nexthop": "PortChannel0001", + }), + True, + [ + "ip route 10.1.0.0/24 PortChannel0001 tag 1", + "route-map STATIC_ROUTE_FILTER permit 10", + " match tag 1", + "router bgp 65100", + " address-family ipv4", + " redistribute static route-map STATIC_ROUTE_FILTER", + " address-family ipv6", + " redistribute static route-map STATIC_ROUTE_FILTER" + ] + ) + + set_del_test( + mgr, + "DEL", + ("10.1.0.0/24",), + True, + [ + "no ip route 10.1.0.0/24 PortChannel0001 tag 1", + "router bgp 65100", + " address-family ipv4", + " no redistribute static route-map STATIC_ROUTE_FILTER", + " address-family ipv6", + " no redistribute static route-map STATIC_ROUTE_FILTER", + "no route-map STATIC_ROUTE_FILTER" + ] + ) + +def test_set_bfd_true(): + mgr = constructor() + set_del_test( + mgr, + "SET", + ("10.1.0.0/24", { + "bfd": "false", + "nexthop": "PortChannel0001", + }), + True, + [ + "ip route 10.1.0.0/24 PortChannel0001 tag 1", + "route-map STATIC_ROUTE_FILTER permit 10", + " match tag 1", + "router bgp 65100", + " address-family ipv4", + " redistribute static route-map STATIC_ROUTE_FILTER", + " address-family ipv6", + " redistribute static route-map STATIC_ROUTE_FILTER" + ] + ) + #do nothing for adding smae route second time + set_del_test( + mgr, + "SET", + ("10.1.0.0/24", { + "bfd": "false", + "nexthop": "PortChannel0001", + }), + True, + [ + ] + ) + #clear internal cache if bfd flag is true + set_del_test( + mgr, + "SET", + ("10.1.0.0/24", { + "bfd": "true", + "nexthop": "PortChannel0001", + }), + True, + [ + ] + ) + + #install the route becasue that cache was cleared above + set_del_test( + mgr, + "SET", + ("10.1.0.0/24", { + "bfd": "false", + "nexthop": "PortChannel0001", + }), + True, + [ + "ip route 10.1.0.0/24 PortChannel0001 tag 1", + "route-map STATIC_ROUTE_FILTER permit 10", + " match tag 1", + "router bgp 65100", + " address-family ipv4", + " redistribute static route-map STATIC_ROUTE_FILTER", + " address-family ipv6", + " redistribute static route-map STATIC_ROUTE_FILTER" + ] + ) + diff --git a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py new file mode 100644 index 000000000000..1255b968af9f --- /dev/null +++ b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py @@ -0,0 +1,352 @@ +from unittest.mock import patch +#from unittest.mock import MagicMock, patch + +from staticroutebfd.main import * +from swsscommon import swsscommon + +@patch('swsscommon.swsscommon.DBConnector.__init__') +@patch('swsscommon.swsscommon.ProducerStateTable.__init__') +@patch('swsscommon.swsscommon.Table.__init__') +def constructor(mock_db, mock_producer, mock_tbl): + mock_db.return_value = None + mock_producer.return_value = None + mock_tbl.return_value = None + + srt_bfd = StaticRouteBfd() + return srt_bfd + +def set_del_test(dut, hdlr, op, args, e_bfd_dict, e_srt_dict): + set_del_test.bfd_dict = {} + set_del_test.srt_dict = {} + + def bfd_app_set(key, data): + set_del_test.bfd_dict[key] = data.copy() + def bfd_app_del(key): + set_del_test.bfd_dict[key] = {} + def srt_app_set(key, data): + set_del_test.srt_dict[key] = data.copy() + def srt_app_del(key): + set_del_test.srt_dict[key] = {} + + def compare_dict(r, e): + if len(r) == 0 and len(e) == 0: + return True + if len(r) != len(e): + return False + for k in e: + if k not in r: + return False + if type(e[k]) is str: + r_sort = "".join(sorted([x.strip() for x in r[k].split(',')])) + e_sort = "".join(sorted([x.strip() for x in e[k].split(',')])) + if r_sort != e_sort: + return False + if type(e[k]) is dict: + ret = compare_dict(r[k], e[k]) + if not ret: + return False + return True + + dut.set_bfd_session_into_appl_db = bfd_app_set + dut.del_bfd_session_from_appl_db = bfd_app_del + dut.set_static_route_into_appl_db = srt_app_set + dut.del_static_route_from_appl_db = srt_app_del + + if op == "SET": + if hdlr == "bfd": + dut.bfd_state_set_handler(*args) + if hdlr == "srt": + dut.static_route_set_handler(*args) + if hdlr == "intf": + dut.interface_set_handler(*args) + elif op == "DEL": + if hdlr == "bfd": + dut.bfd_state_del_handler(*args) + if hdlr == "srt": + dut.static_route_del_handler(*args) + if hdlr == "intf": + dut.interface_del_handler(*args) + else: + assert False, "Wrong operation" + + assert compare_dict(set_del_test.bfd_dict, e_bfd_dict) + assert compare_dict(set_del_test.srt_dict, e_srt_dict) + +def intf_setup(dut): + set_del_test(dut, "intf", + "SET", + ("if1|192.168.1.1/24", {} + ), + {}, + {} + ) + set_del_test(dut, "intf", + "SET", + ("if2|192.168.2.1/24", {} + ), + {}, + {} + ) + set_del_test(dut, "intf", + "SET", + ("if3|192.168.3.1/24", {} + ), + {}, + {} + ) + +def test_set(): + dut = constructor() + intf_setup(dut) + + #test #1 + set_del_test(dut, "srt", + "SET", + ("2.2.2.0/24", { + "bfd": "true", + "nexthop": "192.168.1.2 , 192.168.2.2, 192.168.3.2", + "ifname": "if1, if2, if3", + }), + { + "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + }, + {} + ) + + set_del_test(dut, "bfd", + "SET", + ("192.168.1.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.2.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.3.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + ) + + #test #2 + set_del_test(dut, "srt", + "SET", + ("2.2.2.0/24", { + "bfd": "true", + "nexthop": "192.168.1.2 , 192.168.2.2", + "ifname": "if1, if2", + }), + { + "default:default:192.168.3.2" : {} + }, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + ) + + #test #3 + set_del_test(dut, "srt", + "DEL", + ("2.2.2.0/24", { + "bfd": "true", + "nexthop": "192.168.1.2 , 192.168.2.2", + "ifname": "if1, if2", + }), + { + "default:default:192.168.1.2" : {}, + "default:default:192.168.2.2" : {} + }, + {'default:2.2.2.0/24': {}} + ) + +def test_set_2routes(): + dut = constructor() + intf_setup(dut) + + #test #4 + set_del_test(dut, "srt", + "SET", + ("2.2.2.0/24", { + "bfd": "true", + "nexthop": "192.168.1.2 , 192.168.2.2, 192.168.3.2", + "ifname": "if1, if2, if3", + }), + { + "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + }, + {} + ) + + set_del_test(dut, "bfd", + "SET", + ("192.168.1.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.2.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.3.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + ) + + set_del_test(dut, "srt", + "SET", + ("3.3.3.0/24", { + "bfd": "true", + "nexthop": "192.168.2.2", + "ifname": "if2", + }), + {}, + {'default:3.3.3.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} + ) + + #test #5 + set_del_test(dut, "bfd", + "SET", + ("192.168.2.2", { + "state": "Down" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.3.2,192.168.1.2 ', 'ifname': 'if3,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}, 'default:3.3.3.0/24': {}} + ) + + #test #6 + set_del_test(dut, "bfd", + "SET", + ("192.168.2.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}, + 'default:3.3.3.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} + ) + +def test_set_bfd_flag_change(): + dut = constructor() + intf_setup(dut) + + #test #9 bfd: true -> false + set_del_test(dut, "srt", + "SET", + ("2.2.2.0/24", { + "bfd": "true", + "nexthop": "192.168.1.2 , 192.168.2.2, 192.168.3.2", + "ifname": "if1, if2, if3", + }), + { + "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + }, + {} + ) + + set_del_test(dut, "bfd", + "SET", + ("192.168.1.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.2.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.3.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + ) + + set_del_test(dut, "srt", + "SET", + ("2.2.2.0/24", { + "bfd": "false", + "nexthop": "192.168.1.2 , 192.168.2.2, 192.168.3.2", + "ifname": "if1, if2, if3", + }), + { + "default:default:192.168.1.2" : {}, + "default:default:192.168.2.2" : {}, + "default:default:192.168.3.2" : {} + }, + {'default:2.2.2.0/24': {'bfd':'true', 'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}, + 'default:2.2.2.0/24': {} + } + ) + + #test #10 'bfd': false --> true, write original rout first + set_del_test(dut, "srt", + "SET", + ("2.2.2.0/24", { + "bfd": "true", + "nexthop": "192.168.1.2 , 192.168.2.2, 192.168.3.2", + "ifname": "if1, if2, if3", + }), + { + "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + }, + {'default:2.2.2.0/24': {'bfd':'false', 'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'expiry': 'false'}} + ) + + set_del_test(dut, "bfd", + "SET", + ("192.168.1.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.2.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.3.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + ) + + From 59d14cb0367020c67b013fb605e921d194b3b2d1 Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Sat, 22 Apr 2023 17:31:12 -0700 Subject: [PATCH 10/16] [static_route]add UT cases for staticroutebfd --- src/sonic-bgpcfgd/staticroutebfd/main.py | 3 + src/sonic-bgpcfgd/tests/test_static_rt_bfd.py | 113 +++++++++++++++++- 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py index 12e736d2d525..dc334c928f3f 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/main.py +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -627,6 +627,9 @@ def bfd_state_set_handler(self, key, data): for prefix in self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key): srt_key = prefix config_key = prefix + #skip if the (vrf, peer_ip) is already in the nexthop list + if (vrf, peer_ip) in self.get_local_db(LOCAL_SRT_TABLE, srt_key): + continue self.append_to_srt_table_entry(srt_key, (vrf, peer_ip)) config_data = self.get_local_db(LOCAL_CONFIG_TABLE, config_key) #exit "hold" state when any BFD session becomes UP diff --git a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py index 1255b968af9f..1ead8a639841 100644 --- a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py +++ b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py @@ -246,7 +246,7 @@ def test_set_2routes(): 'default:3.3.3.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) -def test_set_bfd_flag_change(): +def test_set_bfd_change_hold(): dut = constructor() intf_setup(dut) @@ -350,3 +350,114 @@ def test_set_bfd_flag_change(): ) +def test_set_bfd_change_no_hold(): + dut = constructor() + intf_setup(dut) + + #setup runtime "bfd"="false" condition`` + set_del_test(dut, "srt", + "SET", + ("2.2.2.0/24", { + "bfd": "true", + "nexthop": "192.168.1.2 , 192.168.2.2, 192.168.3.2", + "ifname": "if1, if2, if3", + }), + { + "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + }, + {} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.1.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.2.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.3.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + ) + set_del_test(dut, "srt", + "SET", + ("3.3.3.0/24", { + "bfd": "true", + "nexthop": "192.168.2.2", + "ifname": "if2", + }), + {}, + {'default:3.3.3.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} + ) + + set_del_test(dut, "srt", + "SET", + ("2.2.2.0/24", { + "bfd": "false", + "nexthop": "192.168.1.2 , 192.168.2.2, 192.168.3.2", + "ifname": "if1, if2, if3", + }), + { + "default:default:192.168.1.2" : {}, + "default:default:192.168.3.2" : {} + }, + {'default:2.2.2.0/24': {'bfd':'true', 'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}, + 'default:2.2.2.0/24': {} + } + ) + + #test #10 change 'bfd': false to true, because the bfd session "default:default:192.168.2.2" is up, so add that nexthop right after "bfd" change to "true" + set_del_test(dut, "srt", + "SET", + ("2.2.2.0/24", { + "bfd": "true", + "nexthop": "192.168.1.2 , 192.168.2.2, 192.168.3.2", + "ifname": "if1, if2, if3", + }), + { + "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + }, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} + ) + + set_del_test(dut, "bfd", + "SET", + ("192.168.1.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.2.2", { + "state": "Up" + }), + {}, + {} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.3.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + ) + + From 7273f8ac3c77cca61b857c73e3ae947ca418cf47 Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Mon, 24 Apr 2023 12:00:21 -0700 Subject: [PATCH 11/16] [static_toute]add testcase --- src/sonic-bgpcfgd/tests/test_static_rt_bfd.py | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py index 1ead8a639841..9f883f44ada7 100644 --- a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py +++ b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py @@ -95,7 +95,7 @@ def intf_setup(dut): {} ) -def test_set(): +def test_set_del(): dut = constructor() intf_setup(dut) @@ -169,6 +169,58 @@ def test_set(): {'default:2.2.2.0/24': {}} ) +def test_bfd_del(): + dut = constructor() + intf_setup(dut) + + set_del_test(dut, "srt", + "SET", + ("2.2.2.0/24", { + "bfd": "true", + "nexthop": "192.168.1.2 , 192.168.2.2, 192.168.3.2", + "ifname": "if1, if2, if3", + }), + { + "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + }, + {} + ) + + set_del_test(dut, "bfd", + "SET", + ("192.168.1.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.2.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + ) + set_del_test(dut, "bfd", + "SET", + ("192.168.3.2", { + "state": "Up" + }), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + ) + + #test bfd state del + set_del_test(dut, "bfd", + "DEL", + ({"192.168.2.2"}), + {}, + {'default:2.2.2.0/24': {'nexthop': '192.168.1.2,192.168.3.2 ', 'ifname': 'if1,if3', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + ) + def test_set_2routes(): dut = constructor() intf_setup(dut) From f47387204600918d260b2d80e15c782000bee830 Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Wed, 26 Apr 2023 10:17:25 -0700 Subject: [PATCH 12/16] [static_route] fix UT -- log checking --- src/sonic-bgpcfgd/tests/test_static_rt.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sonic-bgpcfgd/tests/test_static_rt.py b/src/sonic-bgpcfgd/tests/test_static_rt.py index ba12f17f2128..3d947a47ac73 100644 --- a/src/sonic-bgpcfgd/tests/test_static_rt.py +++ b/src/sonic-bgpcfgd/tests/test_static_rt.py @@ -604,7 +604,7 @@ def test_set_no_action(mocked_log_debug): True, [] ) - mocked_log_debug.assert_called_with("Nothing to update for static route default|10.1.1.0/24") + mocked_log_debug.assert_called_with("CONFIG_DB Nothing to update for static route default|10.1.1.0/24") @patch('bgpcfgd.managers_static_rt.log_debug') def test_del_no_action(mocked_log_debug): @@ -616,7 +616,7 @@ def test_del_no_action(mocked_log_debug): True, [] ) - mocked_log_debug.assert_called_with("Nothing to update for static route default|10.1.1.0/24") + mocked_log_debug.assert_called_with("CONFIG_DB Nothing to update for static route default|10.1.1.0/24") def test_set_invalid_arg(): mgr = constructor() From 25e47540ea7b66e7e2a6fc27b1d2ff00c74d01da Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Fri, 5 May 2023 10:57:29 -0700 Subject: [PATCH 13/16] [static_route]update bfd config dynamic design and UT --- .../bgpcfgd/managers_static_rt.py | 29 ++++ src/sonic-bgpcfgd/staticroutebfd/main.py | 8 -- src/sonic-bgpcfgd/staticroutebfd/vars.py | 2 +- src/sonic-bgpcfgd/tests/test_static_rt_bfd.py | 133 +++++++++--------- 4 files changed, 97 insertions(+), 75 deletions(-) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py b/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py index aaec3d1038df..7e26c1d0a456 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py @@ -25,6 +25,7 @@ def __init__(self, common_objs, db, table): self.directory.subscribe([("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"),], self.on_bgp_asn_change) self.static_routes = {} self.vrf_pending_redistribution = set() + self.config_db = None OP_DELETE = 'DELETE' OP_ADD = 'ADD' @@ -79,10 +80,38 @@ def set_handler(self, key, data): return True + def skip_appl_del(self, vrf, ip_prefix): + if self.db_name == "CONFIG_DB": + return False + + if self.config_db is None: + self.config_db = swsscommon.SonicV2Connector() + self.config_db.connect(self.config_db.CONFIG_DB) + + #just pop local cache if the route exist in config_db + cfg_key = "STATIC_ROUTE|" + vrf + "|" + ip_prefix + nexthop = self.config_db.get(self.config_db.CONFIG_DB, cfg_key, "nexthop") + if nexthop and len(nexthop)>0: + self.static_routes.setdefault(vrf, {}).pop(ip_prefix, None) + return True + + if vrf == "default": + cfg_key = "STATIC_ROUTE|" + ip_prefix + nexthop = self.config_db.get(self.config_db.CONFIG_DB, cfg_key, "nexthop") + if nexthop and len(nexthop)>0: + self.static_routes.setdefault(vrf, {}).pop(ip_prefix, None) + return True + + return False + def del_handler(self, key): vrf, ip_prefix = self.split_key(key) is_ipv6 = TemplateFabric.is_ipv6(ip_prefix) + if self.skip_appl_del(vrf, ip_prefix): + log_debug("{} ignore appl_db static route deletion because of key {} exist in config_db".format(self.db_name, key)) + return + ip_nh_set = IpNextHopSet(is_ipv6) cur_nh_set, route_tag = self.static_routes.get(vrf, {}).get(ip_prefix, (IpNextHopSet(is_ipv6), self.ROUTE_ADVERTISE_DISABLE_TAG)) cmd_list = self.static_route_commands(ip_nh_set, cur_nh_set, ip_prefix, vrf, route_tag, route_tag) diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py index dc334c928f3f..4198d0ad79b7 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/main.py +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -4,7 +4,6 @@ import syslog import threading import traceback -import time from collections import defaultdict from ipaddress import IPv4Address, IPv6Address from copy import deepcopy @@ -341,15 +340,8 @@ def handle_bfd_change(self, cfg_key, data, to_bfd_enable): data['bfd'] = "false" data['expiry'] = "false" self.set_static_route_into_appl_db(key, data) - time.sleep(0.1) log_debug("SRT_BFD: bfd toggle to true. write the route to appl_db, update StaticRouteMgr(appl_db), key %s"%(key)) else: - #safely delete the static_route entry created by StaticRouteBfd - #1, write a route with bfd=true to appl_db to clean up cur_nh in StaticRouteMgr - #2, delete the entry from appl_db STATIC_ROUTE_TABLE, StaticRouteMgr do nothing because of empty cur_nh - data['bfd'] = "true" - self.set_static_route_into_appl_db(key, data) - time.sleep(0.1) self.del_static_route_from_appl_db(key) log_debug("SRT_BFD: bfd toggle to false. delete static route from appl_db, key %s"%(key)) diff --git a/src/sonic-bgpcfgd/staticroutebfd/vars.py b/src/sonic-bgpcfgd/staticroutebfd/vars.py index 527986827129..e880844687f6 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/vars.py +++ b/src/sonic-bgpcfgd/staticroutebfd/vars.py @@ -1,5 +1,5 @@ g_debug = True -bfd_multihop = "false" +bfd_multihop = "true" bfd_rx_interval = "50" bfd_tx_interval = "50" bfd_multiplier = "3" diff --git a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py index 9f883f44ada7..aa908a52bc86 100644 --- a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py +++ b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py @@ -20,13 +20,13 @@ def set_del_test(dut, hdlr, op, args, e_bfd_dict, e_srt_dict): set_del_test.srt_dict = {} def bfd_app_set(key, data): - set_del_test.bfd_dict[key] = data.copy() + set_del_test.bfd_dict["set_"+key] = data.copy() def bfd_app_del(key): - set_del_test.bfd_dict[key] = {} + set_del_test.bfd_dict["del_"+key] = {} def srt_app_set(key, data): - set_del_test.srt_dict[key] = data.copy() + set_del_test.srt_dict["set_"+key] = data.copy() def srt_app_del(key): - set_del_test.srt_dict[key] = {} + set_del_test.srt_dict["del_"+key] = {} def compare_dict(r, e): if len(r) == 0 and len(e) == 0: @@ -108,9 +108,9 @@ def test_set_del(): "ifname": "if1, if2, if3", }), { - "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} }, {} ) @@ -121,7 +121,7 @@ def test_set_del(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -129,7 +129,7 @@ def test_set_del(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -137,7 +137,7 @@ def test_set_del(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} ) #test #2 @@ -149,9 +149,9 @@ def test_set_del(): "ifname": "if1, if2", }), { - "default:default:192.168.3.2" : {} + "del_default:default:192.168.3.2" : {} }, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} ) #test #3 @@ -163,10 +163,10 @@ def test_set_del(): "ifname": "if1, if2", }), { - "default:default:192.168.1.2" : {}, - "default:default:192.168.2.2" : {} + "del_default:default:192.168.1.2" : {}, + "del_default:default:192.168.2.2" : {} }, - {'default:2.2.2.0/24': {}} + {'del_default:2.2.2.0/24': {}} ) def test_bfd_del(): @@ -181,9 +181,9 @@ def test_bfd_del(): "ifname": "if1, if2, if3", }), { - "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} }, {} ) @@ -194,7 +194,7 @@ def test_bfd_del(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -202,7 +202,7 @@ def test_bfd_del(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -210,7 +210,7 @@ def test_bfd_del(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} ) #test bfd state del @@ -218,7 +218,7 @@ def test_bfd_del(): "DEL", ({"192.168.2.2"}), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.1.2,192.168.3.2 ', 'ifname': 'if1,if3', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.1.2,192.168.3.2 ', 'ifname': 'if1,if3', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} ) def test_set_2routes(): @@ -234,9 +234,9 @@ def test_set_2routes(): "ifname": "if1, if2, if3", }), { - "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} }, {} ) @@ -247,7 +247,7 @@ def test_set_2routes(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -255,7 +255,7 @@ def test_set_2routes(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -263,7 +263,7 @@ def test_set_2routes(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} ) set_del_test(dut, "srt", @@ -274,7 +274,7 @@ def test_set_2routes(): "ifname": "if2", }), {}, - {'default:3.3.3.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} + {'set_default:3.3.3.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) #test #5 @@ -284,7 +284,7 @@ def test_set_2routes(): "state": "Down" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.3.2,192.168.1.2 ', 'ifname': 'if3,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}, 'default:3.3.3.0/24': {}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.3.2,192.168.1.2 ', 'ifname': 'if3,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}, 'del_default:3.3.3.0/24': {}} ) #test #6 @@ -294,8 +294,8 @@ def test_set_2routes(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}, - 'default:3.3.3.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}, + 'set_default:3.3.3.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) def test_set_bfd_change_hold(): @@ -311,9 +311,9 @@ def test_set_bfd_change_hold(): "ifname": "if1, if2, if3", }), { - "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} }, {} ) @@ -324,7 +324,7 @@ def test_set_bfd_change_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -332,7 +332,7 @@ def test_set_bfd_change_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -340,7 +340,7 @@ def test_set_bfd_change_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} ) set_del_test(dut, "srt", @@ -351,14 +351,15 @@ def test_set_bfd_change_hold(): "ifname": "if1, if2, if3", }), { - "default:default:192.168.1.2" : {}, - "default:default:192.168.2.2" : {}, - "default:default:192.168.3.2" : {} + "del_default:default:192.168.1.2" : {}, + "del_default:default:192.168.2.2" : {}, + "del_default:default:192.168.3.2" : {} }, - {'default:2.2.2.0/24': {'bfd':'true', 'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}, - 'default:2.2.2.0/24': {} + { + 'del_default:2.2.2.0/24': {} } ) + return #test #10 'bfd': false --> true, write original rout first set_del_test(dut, "srt", @@ -369,11 +370,11 @@ def test_set_bfd_change_hold(): "ifname": "if1, if2, if3", }), { - "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} }, - {'default:2.2.2.0/24': {'bfd':'false', 'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'bfd':'false', 'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'expiry': 'false'}} ) set_del_test(dut, "bfd", @@ -382,7 +383,7 @@ def test_set_bfd_change_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -390,7 +391,7 @@ def test_set_bfd_change_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -398,7 +399,7 @@ def test_set_bfd_change_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} ) @@ -415,9 +416,9 @@ def test_set_bfd_change_no_hold(): "ifname": "if1, if2, if3", }), { - "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, + "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} }, {} ) @@ -427,7 +428,7 @@ def test_set_bfd_change_no_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.1.2', 'ifname': 'if1', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -435,7 +436,7 @@ def test_set_bfd_change_no_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -443,7 +444,7 @@ def test_set_bfd_change_no_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} ) set_del_test(dut, "srt", "SET", @@ -453,7 +454,7 @@ def test_set_bfd_change_no_hold(): "ifname": "if2", }), {}, - {'default:3.3.3.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} + {'set_default:3.3.3.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) set_del_test(dut, "srt", @@ -464,11 +465,11 @@ def test_set_bfd_change_no_hold(): "ifname": "if1, if2, if3", }), { - "default:default:192.168.1.2" : {}, - "default:default:192.168.3.2" : {} + "del_default:default:192.168.1.2" : {}, + "del_default:default:192.168.3.2" : {} }, - {'default:2.2.2.0/24': {'bfd':'true', 'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}, - 'default:2.2.2.0/24': {} + { + 'del_default:2.2.2.0/24': {} } ) @@ -481,10 +482,10 @@ def test_set_bfd_change_no_hold(): "ifname": "if1, if2, if3", }), { - "default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} }, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", @@ -493,7 +494,7 @@ def test_set_bfd_change_no_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2 ', 'ifname': 'if2,if1', 'nexthop-vrf': 'default,default', 'expiry': 'false'}} ) set_del_test(dut, "bfd", "SET", @@ -509,7 +510,7 @@ def test_set_bfd_change_no_hold(): "state": "Up" }), {}, - {'default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} + {'set_default:2.2.2.0/24': {'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'nexthop-vrf': 'default,default,default', 'expiry': 'false'}} ) From 765aebcc11e0cb0b1c0195224cd58653ae916d3b Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Fri, 5 May 2023 12:21:59 -0700 Subject: [PATCH 14/16] [static_rotue] use multihop for T2 before singlehop is ready --- src/sonic-bgpcfgd/tests/test_static_rt_bfd.py | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py index aa908a52bc86..e2eb32cb85d5 100644 --- a/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py +++ b/src/sonic-bgpcfgd/tests/test_static_rt_bfd.py @@ -108,9 +108,9 @@ def test_set_del(): "ifname": "if1, if2, if3", }), { - "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'true', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default: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'} }, {} ) @@ -181,9 +181,9 @@ def test_bfd_del(): "ifname": "if1, if2, if3", }), { - "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'true', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default: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'} }, {} ) @@ -234,9 +234,9 @@ def test_set_2routes(): "ifname": "if1, if2, if3", }), { - "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'true', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default: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'} }, {} ) @@ -311,9 +311,9 @@ def test_set_bfd_change_hold(): "ifname": "if1, if2, if3", }), { - "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'true', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default: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'} }, {} ) @@ -370,9 +370,9 @@ def test_set_bfd_change_hold(): "ifname": "if1, if2, if3", }), { - "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'true', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default: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_default:2.2.2.0/24': {'bfd':'false', 'nexthop': '192.168.2.2,192.168.1.2,192.168.3.2 ', 'ifname': 'if2,if1,if3', 'expiry': 'false'}} ) @@ -416,9 +416,9 @@ def test_set_bfd_change_no_hold(): "ifname": "if1, if2, if3", }), { - "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "set_default:default:192.168.2.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.2.1'}, - "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'true', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, + "set_default: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'} }, {} ) @@ -482,8 +482,8 @@ def test_set_bfd_change_no_hold(): "ifname": "if1, if2, if3", }), { - "set_default:default:192.168.1.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.1'}, - "set_default:default:192.168.3.2" : {'multihop': 'false', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.3.1'} + "set_default:default:192.168.1.2" : {'multihop': 'true', 'rx_interval': '50', 'tx_interval': '50', 'multiplier': '3', 'local_addr': '192.168.1.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_default:2.2.2.0/24': {'nexthop': '192.168.2.2', 'ifname': 'if2', 'nexthop-vrf': 'default', 'expiry': 'false'}} ) From 6e68a89607e7a401823d51bdf2a7f6c54255d14f Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Thu, 11 May 2023 21:29:42 -0700 Subject: [PATCH 15/16] postpone bfd session creation if no ip for the interface --- src/sonic-bgpcfgd/staticroutebfd/main.py | 46 +++++++++++++++--------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/sonic-bgpcfgd/staticroutebfd/main.py b/src/sonic-bgpcfgd/staticroutebfd/main.py index 4198d0ad79b7..4483d0589cca 100644 --- a/src/sonic-bgpcfgd/staticroutebfd/main.py +++ b/src/sonic-bgpcfgd/staticroutebfd/main.py @@ -18,7 +18,6 @@ APPL_DB_NAME = "APPL_DB" STATE_DB_NAME = "STATE_DB" -LOOPBACK_TABLE_NAME = "LOOPBACK_INTERFACE" INTERFACE_TABLE_NAME = "INTERFACE" PORTCHANNEL_INTERFACE_TABLE_NAME = "PORTCHANNEL_INTERFACE" STATIC_ROUTE_TABLE_NAME = "STATIC_ROUTE" @@ -28,11 +27,9 @@ LOCAL_NEXTHOP_TABLE = "nexthop" LOCAL_SRT_TABLE = "srt" LOCAL_BFD_TABLE = "bfd" +LOCAL_BFD_PENDING_TABLE = "bfd_pending" LOCAL_INTERFACE_TABLE = "interface" -LOOPBACK0_NAME = "Loopback0" -LOOPBACK4096_NAME = "Loopback4096" - def log_debug(msg): """ Send a message msg to the syslog as DEBUG """ if g_debug: @@ -106,6 +103,7 @@ def __init__(self): self.local_db[LOCAL_NEXTHOP_TABLE] = defaultdict(set) self.local_db[LOCAL_SRT_TABLE] = defaultdict(set) self.local_db[LOCAL_BFD_TABLE] = defaultdict(dict) + self.local_db[LOCAL_BFD_PENDING_TABLE] = defaultdict(dict) #interface, portchannel_interface and loopback_interface share same table, assume name is unique #assume only one ipv4 and/or one ipv6 for each interface self.local_db[LOCAL_INTERFACE_TABLE] = defaultdict(dict) @@ -190,6 +188,7 @@ def interface_set_handler(self, key, data): else: value[is_ipv4] = ip self.set_local_db(LOCAL_INTERFACE_TABLE, if_name, value) + self.update_bfd_pending(if_name) return True def interface_del_handler(self, key): @@ -213,19 +212,32 @@ def find_interface_ip(self, if_name, ip_example): if len(ip)>0: #ip should be verified when add to local_db return True, ip - #if there is no ip for the interface, try loopback ip address - value = self.get_local_db(LOCAL_INTERFACE_TABLE, LOOPBACK4096_NAME) + return False, "" - lp4096_ip = value.get(is_ipv4, "") - if len(lp4096_ip)>0: - return True, lp4096_ip - value = self.get_local_db(LOCAL_INTERFACE_TABLE, LOOPBACK0_NAME) + def update_bfd_pending(self, if_name): + del_list=[] + for k, v in self.local_db[LOCAL_BFD_PENDING_TABLE].items(): + if len(v) == 3 and v[0] == if_name: + intf, nh_ip, bfd_key = v[0], v[1], v[2] + valid, local_addr = self.find_interface_ip(intf, nh_ip) + if not valid: #IP address might not be available for this type of nh_ip (IPv4 or IPv6) yet + continue + log_notice("bfd_pending: get ip for interface: %s, create bfd session for %s"%(intf, bfd_key)) + bfd_entry_cfg = self.BFD_DEFAULT_CFG.copy() + if all([bfd_rx_interval, bfd_tx_interval, bfd_multiplier, bfd_multihop]): + bfd_entry_cfg["multihop"] = bfd_multihop + bfd_entry_cfg["rx_interval"] = bfd_rx_interval + bfd_entry_cfg["tx_interval"] = bfd_tx_interval + bfd_entry_cfg["multiplier"] = bfd_multiplier - lp0_ip = value.get(is_ipv4, "") - if len(lp0_ip)>0: - return True, lp0_ip + bfd_entry_cfg["local_addr"] = local_addr + self.set_bfd_session_into_appl_db(bfd_key, bfd_entry_cfg) + bfd_entry_cfg["static_route"] = "true" + self.set_local_db(LOCAL_BFD_TABLE, bfd_key, bfd_entry_cfg) + del_list.append(k) - return False, "" + for k in del_list: + self.local_db[LOCAL_BFD_PENDING_TABLE].pop(k) def strip_table_name(self, key, splitter): return key.split(splitter, 1)[1] @@ -448,7 +460,10 @@ def static_route_set_handler(self, key, data): if len(self.get_local_db(LOCAL_NEXTHOP_TABLE, nh_key)) == 0 and len(bfd_session) == 0: valid, local_addr = self.find_interface_ip(intf, nh_ip) if not valid: - log_warn("cannot find ip for interface: %s" %intf) + #interface IP is not available yet, put this request to cache + self.set_local_db(LOCAL_BFD_PENDING_TABLE, intf+"_"+bfd_key, [intf, nh_ip, bfd_key]) + self.append_to_nh_table_entry(nh_key, vrf + "|" + ip_prefix) + log_warn("bfd_pending: cannot find ip for interface: %s, postpone bfd session creation" %intf) continue bfd_entry_cfg = self.BFD_DEFAULT_CFG.copy() @@ -690,7 +705,6 @@ def prepare_selector(self): self.subscribers.add(static_route_subscriber) self.subscribers.add(bfd_state_subscriber) - self.callbacks[self.config_db.getDbId()][LOOPBACK_TABLE_NAME].append(self.interface_callback) self.callbacks[self.config_db.getDbId()][INTERFACE_TABLE_NAME].append(self.interface_callback) self.callbacks[self.config_db.getDbId()][PORTCHANNEL_INTERFACE_TABLE_NAME].append(self.interface_callback) self.callbacks[self.config_db.getDbId()][STATIC_ROUTE_TABLE_NAME].append(self.static_route_callback) From 862ec93cd1e1eeb25a5a4fd0f31935b10048deb0 Mon Sep 17 00:00:00 2001 From: Baorong Liu Date: Mon, 22 May 2023 12:00:34 -0700 Subject: [PATCH 16/16] add comment for skip_appl_del --- src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py b/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py index 7e26c1d0a456..951ee4a43863 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_static_rt.py @@ -81,6 +81,18 @@ def set_handler(self, key, data): def skip_appl_del(self, vrf, ip_prefix): + """ + If a static route is bfd enabled, the processed static route is written into application DB by staticroutebfd. + When we disable bfd for that route at runtime, that static route entry will be removed from APPL_DB STATIC_ROUTE_TABLE. + In the case, the StaticRouteMgr(appl_db) cannot uninstall the static route from FRR if the static route is still in CONFIG_DB, + so need this checking (skip appl_db deletion) to avoid race condition between StaticRouteMgr(appl_db) and StaticRouteMgr(config_db) + For more detailed information: + https://github.com/sonic-net/SONiC/blob/master/doc/static-route/SONiC_static_route_bfd_hld.md#bfd-field-changes-from-true-to-false + + :param vrf: vrf from the split_key(key) return + :param ip_prefix: ip_prefix from the split_key(key) return + :return: True if the deletion comes from APPL_DB and the vrf|ip_prefix exists in CONFIG_DB, otherwise return False + """ if self.db_name == "CONFIG_DB": return False