diff --git a/src/sonic-bgpcfgd/bgpcfgd/main.py b/src/sonic-bgpcfgd/bgpcfgd/main.py index 48f46714d8c0..7b4291b4d4a3 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/main.py +++ b/src/sonic-bgpcfgd/bgpcfgd/main.py @@ -63,8 +63,7 @@ def do_work(): StaticRouteMgr(common_objs, "CONFIG_DB", "STATIC_ROUTE"), # Route Advertisement Managers AdvertiseRouteMgr(common_objs, "STATE_DB", swsscommon.STATE_ADVERTISE_NETWORK_TABLE_NAME), - # TBD, Change to schema name from swsscommon (swsscommon.APP_BGP_PROFILE_TABLE_NAME) when the submodule is advanced. - RouteMapMgr(common_objs, "APPL_DB", "BGP_PROFILE_TABLE"), + RouteMapMgr(common_objs, "APPL_DB", swsscommon.APP_BGP_PROFILE_TABLE_NAME), ] runner = Runner(common_objs['cfg_mgr']) for mgr in managers: diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_advertise_rt.py b/src/sonic-bgpcfgd/bgpcfgd/managers_advertise_rt.py index 891b9e786592..68c48b044f61 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_advertise_rt.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_advertise_rt.py @@ -3,11 +3,11 @@ from swsscommon import swsscommon from .managers_rm import ROUTE_MAPS import ipaddress -from .log import log_info, log_err +from .log import log_info, log_err, log_debug class AdvertiseRouteMgr(Manager): - """This class Advertises routes when ADVERTISE_NETWORK_TABLE in STATE_DB is updated""" + """ This class Advertises routes when ADVERTISE_NETWORK_TABLE in STATE_DB is updated """ def __init__(self, common_objs, db, table): """ @@ -23,20 +23,16 @@ def __init__(self, common_objs, db, table): table, ) - self.directory.subscribe( - [ - ("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"), - ], - self.on_bgp_asn_change, - ) + self.directory.subscribe([("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"),], self.on_bgp_asn_change) self.advertised_routes = dict() + OP_DELETE = "DELETE" OP_ADD = "ADD" def set_handler(self, key, data): - log_info("AdvertiseRouteMgr:: set handler") - if not self._set_handler_validate(key, data): + log_debug("AdvertiseRouteMgr:: set handler") + if not self.__set_handler_validate(key, data): return True vrf, ip_prefix = self.split_key(key) self.add_route_advertisement(vrf, ip_prefix, data) @@ -44,49 +40,21 @@ def set_handler(self, key, data): return True def del_handler(self, key): - log_info("AdvertiseRouteMgr:: del handler") - if not self._del_handler_validate(key): - return + log_debug("AdvertiseRouteMgr:: del handler") vrf, ip_prefix = self.split_key(key) self.remove_route_advertisement(vrf, ip_prefix) - def _ip_addr_validate(self, key): - if key: - _, ip_prefix = self.split_key(key) - ip_prefix = ip_prefix.split("/") - if len(ip_prefix) != 2: - log_err("BGPAdvertiseRouteMgr:: No valid ip prefix for advertised route %s" % key) - return False - try: - ip = ipaddress.ip_address(ip_prefix[0]) - if ip.version == 4 and int(ip_prefix[1]) not in range(0, 33): - log_err( - "BGPAdvertiseRouteMgr:: ipv4 prefix %s is illegal for advertised route %s" % (ip_prefix[1], key) - ) - return False - if ip.version == 6 and int(ip_prefix[1]) not in range(0, 129): - log_err( - "BGPAdvertiseRouteMgr:: ipv6 prefix %s is illegal for advertised route %s" % (ip_prefix[1], key) - ) - return False - except ValueError: - log_err("BGPAdvertiseRouteMgr:: No valid ip %s for advertised route %s" % (ip_prefix[0], key)) - return False - else: - return False - return True - - def _set_handler_validate(self, key, data): + def __set_handler_validate(self, key, data): if data: if ("profile" in data and data["profile"] in ROUTE_MAPS) or data == {"":""}: - return self._ip_addr_validate(key) + """ + APP which config the data should be responsible to pass a valid IP prefix + """ + return True log_err("BGPAdvertiseRouteMgr:: Invalid data %s for advertised route %s" % (data, key)) return False - def _del_handler_validate(self, key): - return self._ip_addr_validate(key) - def add_route_advertisement(self, vrf, ip_prefix, data): if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"): if not self.advertised_routes.get(vrf, dict()): @@ -110,9 +78,8 @@ def remove_route_advertisement(self, vrf, ip_prefix): def advertise_route_commands(self, ip_prefix, vrf, op, data=None): is_ipv6 = TemplateFabric.is_ipv6(ip_prefix) - bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"][ - "bgp_asn" - ] + bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"] + cmd_list = [] if vrf == "default": cmd_list.append("router bgp %s" % bgp_asn) @@ -123,24 +90,22 @@ def advertise_route_commands(self, ip_prefix, vrf, op, data=None): if data and "profile" in data: cmd_list.append(" network %s route-map %s" % (ip_prefix, "%s_RM" % data["profile"])) - log_info( + log_debug( "BGPAdvertiseRouteMgr:: Update bgp %s network %s with route-map %s" % (bgp_asn, vrf + "|" + ip_prefix, "%s_RM" % data["profile"]) ) else: cmd_list.append(" %snetwork %s" % ("no " if op == self.OP_DELETE else "", ip_prefix)) - log_info( + log_debug( "BGPAdvertiseRouteMgr:: %sbgp %s network %s" % ("Remove " if op == self.OP_DELETE else "Update ", bgp_asn, vrf + "|" + ip_prefix) ) self.cfg_mgr.push_list(cmd_list) - log_info("BGPAdvertiseRouteMgr::Done") + log_debug("BGPAdvertiseRouteMgr::Done") def bgp_network_import_check_commands(self, vrf, op): - bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"][ - "bgp_asn" - ] + bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"] cmd_list = [] if vrf == "default": cmd_list.append("router bgp %s" % bgp_asn) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_rm.py b/src/sonic-bgpcfgd/bgpcfgd/managers_rm.py index b8eb611b4aa2..08609c68f9a6 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_rm.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_rm.py @@ -1,5 +1,5 @@ from .manager import Manager -from .log import log_info, log_err +from .log import log_err, log_debug ROUTE_MAPS = ["FROM_SDN_SLB_ROUTES"] @@ -22,27 +22,27 @@ def __init__(self, common_objs, db, table): ) def set_handler(self, key, data): - log_info("BGPRouteMapMgr:: set handler") + log_debug("BGPRouteMapMgr:: set handler") """Only need a name as the key, and community id as the data""" - if not self._set_handler_validate(key, data): + if not self.__set_handler_validate(key, data): return True - self._update_rm(key, data) + self.__update_rm(key, data) return True def del_handler(self, key): - log_info("BGPRouteMapMgr:: del handler") - if not self._del_handler_validate(key): + log_debug("BGPRouteMapMgr:: del handler") + if not self.__del_handler_validate(key): return - self._remove_rm(key) + self.__remove_rm(key) - def _remove_rm(self, rm): + def __remove_rm(self, rm): cmds = ["no route-map %s permit 100" % ("%s_RM" % rm)] - log_info("BGPRouteMapMgr:: remove route-map %s" % ("%s_RM" % rm)) + log_debug("BGPRouteMapMgr:: remove route-map %s" % ("%s_RM" % rm)) self.cfg_mgr.push_list(cmds) - log_info("BGPRouteMapMgr::Done") + log_debug("BGPRouteMapMgr::Done") - def _set_handler_validate(self, key, data): + def __set_handler_validate(self, key, data): if key not in ROUTE_MAPS: log_err("BGPRouteMapMgr:: Invalid key for route-map %s" % key) return False @@ -65,14 +65,14 @@ def _set_handler_validate(self, key, data): return True - def _del_handler_validate(self, key): + def __del_handler_validate(self, key): if key not in ROUTE_MAPS: log_err("BGPRouteMapMgr:: Invalid key for route-map %s" % key) return False return True - def _update_rm(self, rm, data): + def __update_rm(self, rm, data): cmds = ["route-map %s permit 100" % ("%s_RM" % rm), " set community %s" % data["community_id"]] - log_info("BGPRouteMapMgr:: update route-map %s community %s" % ("%s_RM" % rm, data["community_id"])) + log_debug("BGPRouteMapMgr:: update route-map %s community %s" % ("%s_RM" % rm, data["community_id"])) self.cfg_mgr.push_list(cmds) - log_info("BGPRouteMapMgr::Done") + log_debug("BGPRouteMapMgr::Done")