diff --git a/src/sonic-bgpcfgd/bgpcfgd/main.py b/src/sonic-bgpcfgd/bgpcfgd/main.py index 52d591d1fc..698e1756e6 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/main.py +++ b/src/sonic-bgpcfgd/bgpcfgd/main.py @@ -17,6 +17,7 @@ from .managers_db import BGPDataBaseMgr from .managers_intf import InterfaceMgr from .managers_setsrc import ZebraSetSrc from .managers_static_rt import StaticRouteMgr +from .managers_rm import RouteMapMgr from .runner import Runner, signal_handler from .template import TemplateFabric from .utils import read_constants @@ -60,6 +61,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), + 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 352b89f728..68c48b044f 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_advertise_rt.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_advertise_rt.py @@ -1,10 +1,14 @@ from .manager import Manager from .template import TemplateFabric from swsscommon import swsscommon +from .managers_rm import ROUTE_MAPS +import ipaddress +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 """ + def __init__(self, common_objs, db, table): """ Initialize the object @@ -18,82 +22,105 @@ class AdvertiseRouteMgr(Manager): db, table, ) - + 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' - + OP_DELETE = "DELETE" + OP_ADD = "ADD" def set_handler(self, 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) + self.add_route_advertisement(vrf, ip_prefix, data) return True - def del_handler(self, key): + log_debug("AdvertiseRouteMgr:: del handler") vrf, ip_prefix = self.split_key(key) self.remove_route_advertisement(vrf, ip_prefix) + def __set_handler_validate(self, key, data): + if data: + if ("profile" in data and data["profile"] in ROUTE_MAPS) or data == {"":""}: + """ + 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 add_route_advertisement(self, vrf, ip_prefix): + 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, set()): + if not self.advertised_routes.get(vrf, dict()): self.bgp_network_import_check_commands(vrf, self.OP_ADD) - self.advertise_route_commands(ip_prefix, vrf, self.OP_ADD) - - self.advertised_routes.setdefault(vrf, set()).add(ip_prefix) + self.advertise_route_commands(ip_prefix, vrf, self.OP_ADD, data) + self.advertised_routes.setdefault(vrf, dict()).update({ip_prefix: data}) def remove_route_advertisement(self, vrf, ip_prefix): - self.advertised_routes.setdefault(vrf, set()).discard(ip_prefix) - if not self.advertised_routes.get(vrf, set()): + if ip_prefix not in self.advertised_routes.get(vrf, dict()): + log_info("BGPAdvertiseRouteMgr:: %s|%s does not exist" % (vrf, ip_prefix)) + return + self.advertised_routes.get(vrf, dict()).pop(ip_prefix) + if not self.advertised_routes.get(vrf, dict()): self.advertised_routes.pop(vrf, None) if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"): - if not self.advertised_routes.get(vrf, set()): + if not self.advertised_routes.get(vrf, dict()): self.bgp_network_import_check_commands(vrf, self.OP_DELETE) self.advertise_route_commands(ip_prefix, vrf, self.OP_DELETE) - - def advertise_route_commands(self, ip_prefix, vrf, op): + 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"] cmd_list = [] - if vrf == 'default': + if vrf == "default": cmd_list.append("router bgp %s" % bgp_asn) else: cmd_list.append("router bgp %s vrf %s" % (bgp_asn, vrf)) cmd_list.append(" address-family %s unicast" % ("ipv6" if is_ipv6 else "ipv4")) - cmd_list.append(" %snetwork %s" % ('no ' if op == self.OP_DELETE else '', ip_prefix)) + + if data and "profile" in data: + cmd_list.append(" network %s route-map %s" % (ip_prefix, "%s_RM" % data["profile"])) + 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_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_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"] cmd_list = [] - if vrf == 'default': + if vrf == "default": cmd_list.append("router bgp %s" % bgp_asn) else: cmd_list.append("router bgp %s vrf %s" % (bgp_asn, vrf)) - cmd_list.append(" %sbgp network import-check" % ('' if op == self.OP_DELETE else 'no ')) + cmd_list.append(" %sbgp network import-check" % ("" if op == self.OP_DELETE else "no ")) self.cfg_mgr.push_list(cmd_list) - def on_bgp_asn_change(self): if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"): for vrf, ip_prefixes in self.advertised_routes.items(): self.bgp_network_import_check_commands(vrf, self.OP_ADD) for ip_prefix in ip_prefixes: - self.add_route_advertisement(vrf, ip_prefix) - + self.add_route_advertisement(vrf, ip_prefix, ip_prefixes[ip_prefix]) @staticmethod def split_key(key): @@ -102,7 +129,7 @@ class AdvertiseRouteMgr(Manager): :param key: key to split :return: vrf name extracted from the key, ip prefix extracted from the key """ - if '|' not in key: - return 'default', key + if "|" not in key: + return "default", key else: - return tuple(key.split('|', 1)) + return tuple(key.split("|", 1)) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_rm.py b/src/sonic-bgpcfgd/bgpcfgd/managers_rm.py new file mode 100644 index 0000000000..08609c68f9 --- /dev/null +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_rm.py @@ -0,0 +1,78 @@ +from .manager import Manager +from .log import log_err, log_debug + +ROUTE_MAPS = ["FROM_SDN_SLB_ROUTES"] + + +class RouteMapMgr(Manager): + """This class add route-map when BGP_PROFILE_TABLE in APPL_DB is updated""" + + def __init__(self, common_objs, db, table): + """ + Initialize the object + :param common_objs: common object dictionary + :param db: name of the db + :param table: name of the table in the db + """ + super(RouteMapMgr, self).__init__( + common_objs, + [], + db, + table, + ) + + def set_handler(self, key, data): + 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): + return True + + self.__update_rm(key, data) + return True + + def del_handler(self, key): + log_debug("BGPRouteMapMgr:: del handler") + if not self.__del_handler_validate(key): + return + self.__remove_rm(key) + + def __remove_rm(self, rm): + cmds = ["no route-map %s permit 100" % ("%s_RM" % rm)] + log_debug("BGPRouteMapMgr:: remove route-map %s" % ("%s_RM" % rm)) + self.cfg_mgr.push_list(cmds) + log_debug("BGPRouteMapMgr::Done") + + 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 + + if not data: + log_err("BGPRouteMapMgr:: data is None") + return False + community_ids = data["community_id"].split(":") + try: + if ( + len(community_ids) != 2 + or int(community_ids[0]) not in range(0, 65536) + or int(community_ids[1]) not in range(0, 65536) + ): + log_err("BGPRouteMapMgr:: data %s doesn't include valid community id %s" % (data, community_ids)) + return False + except ValueError: + log_err("BGPRouteMapMgr:: data %s includes illegal input" % (data)) + return False + + return True + + 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): + cmds = ["route-map %s permit 100" % ("%s_RM" % rm), " set community %s" % 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_debug("BGPRouteMapMgr::Done") diff --git a/src/sonic-bgpcfgd/tests/test_advertise_rt.py b/src/sonic-bgpcfgd/tests/test_advertise_rt.py index 26f7b66176..7515406000 100644 --- a/src/sonic-bgpcfgd/tests/test_advertise_rt.py +++ b/src/sonic-bgpcfgd/tests/test_advertise_rt.py @@ -48,7 +48,7 @@ def test_set_del(): set_del_test( mgr, "SET", - ("10.1.0.0/24", {}), + ("10.1.0.0/24", {"":""}), True, [ ["router bgp 65100", @@ -62,7 +62,7 @@ def test_set_del(): set_del_test( mgr, "SET", - ("fc00:10::/64", {}), + ("fc00:10::/64", {"":""}), True, [ ["router bgp 65100", @@ -103,7 +103,7 @@ def test_set_del_vrf(): set_del_test( mgr, "SET", - ("vrfRED|10.2.0.0/24", {}), + ("vrfRED|10.2.0.0/24", {"":""}), True, [ ["router bgp 65100 vrf vrfRED", @@ -117,7 +117,7 @@ def test_set_del_vrf(): set_del_test( mgr, "SET", - ("vrfRED|fc00:20::/64", {}), + ("vrfRED|fc00:20::/64", {"":""}), True, [ ["router bgp 65100 vrf vrfRED", @@ -158,7 +158,9 @@ def test_set_del_bgp_asn_change(): set_del_test( mgr, "SET", - ("vrfRED|10.3.0.0/24", {}), + ("vrfRED|10.3.0.0/24", { + "profile": "FROM_SDN_SLB_ROUTES" + }), True, [] ) @@ -170,7 +172,7 @@ def test_set_del_bgp_asn_change(): " no bgp network import-check"], ["router bgp 65100 vrf vrfRED", " address-family ipv4 unicast", - " network 10.3.0.0/24"] + " network 10.3.0.0/24 route-map FROM_SDN_SLB_ROUTES_RM"] ] def push_list(cmds): test_set_del_bgp_asn_change.push_list_called = True @@ -183,3 +185,61 @@ def test_set_del_bgp_asn_change(): mgr.directory.put("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost", {"bgp_asn": "65100"}) assert test_set_del_bgp_asn_change.push_list_called + +def test_set_del_with_community(): + mgr = constructor() + set_del_test( + mgr, + "SET", + ("10.1.0.0/24", { + "profile": "FROM_SDN_SLB_ROUTES" + }), + True, + [ + ["router bgp 65100", + " no bgp network import-check"], + ["router bgp 65100", + " address-family ipv4 unicast", + " network 10.1.0.0/24 route-map FROM_SDN_SLB_ROUTES_RM"] + ] + ) + + set_del_test( + mgr, + "SET", + ("fc00:10::/64", { + "profile": "FROM_SDN_SLB_ROUTES" + }), + True, + [ + ["router bgp 65100", + " address-family ipv6 unicast", + " network fc00:10::/64 route-map FROM_SDN_SLB_ROUTES_RM"] + ] + ) + + set_del_test( + mgr, + "DEL", + ("10.1.0.0/24",), + True, + [ + ["router bgp 65100", + " address-family ipv4 unicast", + " no network 10.1.0.0/24"] + ] + ) + + set_del_test( + mgr, + "DEL", + ("fc00:10::/64",), + True, + [ + ["router bgp 65100", + " bgp network import-check"], + ["router bgp 65100", + " address-family ipv6 unicast", + " no network fc00:10::/64"] + ] + ) \ No newline at end of file diff --git a/src/sonic-bgpcfgd/tests/test_rm.py b/src/sonic-bgpcfgd/tests/test_rm.py new file mode 100644 index 0000000000..fe89055d27 --- /dev/null +++ b/src/sonic-bgpcfgd/tests/test_rm.py @@ -0,0 +1,62 @@ +from unittest.mock import MagicMock +from bgpcfgd.directory import Directory +from bgpcfgd.managers_rm import RouteMapMgr +from swsscommon import swsscommon + +def constructor(): + cfg_mgr = MagicMock() + + common_objs = { + 'directory': Directory(), + 'cfg_mgr': cfg_mgr, + 'constants': {}, + } + + mgr = RouteMapMgr(common_objs, "APPL_DB", "BGP_PROFILE_TABLE") + return mgr + +def set_del_test(mgr, op, args, expected_ret, expected_cmds): + set_del_test.push_list_called = False + def push_list(cmds): + set_del_test.push_list_called = True + assert cmds in expected_cmds + return True + mgr.cfg_mgr.push_list = push_list + + if op == "SET": + ret = mgr.set_handler(*args) + assert ret == expected_ret + elif op == "DEL": + mgr.del_handler(*args) + else: + assert False, "Wrong operation" + + if expected_cmds: + assert set_del_test.push_list_called, "cfg_mgr.push_list wasn't called" + else: + assert not set_del_test.push_list_called, "cfg_mgr.push_list was called" + +def test_set_del(): + mgr = constructor() + set_del_test( + mgr, + "SET", + ("FROM_SDN_SLB_ROUTES", { + "community_id": "1234:1234" + }), + True, + [ + ["route-map FROM_SDN_SLB_ROUTES_RM permit 100", + " set community 1234:1234"] + ] + ) + + set_del_test( + mgr, + "DEL", + ("FROM_SDN_SLB_ROUTES",), + True, + [ + ["no route-map FROM_SDN_SLB_ROUTES_RM permit 100"] + ] + )