diff --git a/dockers/docker-fpm-frr/base_image_files/prefix_list b/dockers/docker-fpm-frr/base_image_files/prefix_list index 3c8ff0e2d9..831d260f00 100755 --- a/dockers/docker-fpm-frr/base_image_files/prefix_list +++ b/dockers/docker-fpm-frr/base_image_files/prefix_list @@ -32,17 +32,37 @@ check_root_privileges() { fi } -# Function to check if the device is supported device with type spine routers and subtype UpstreamLC -check_spine_router() { - type=$(sonic-cfggen -d -v DEVICE_METADATA.localhost.type) - subtype=$(sonic-cfggen -d -v DEVICE_METADATA.localhost.subtype) - - # only supported on UpstreamLC or UpperSpineRouter - if [[ ("$type" == "SpineRouter" && "$subtype" == "UpstreamLC") || "$type" == "UpperSpineRouter" ]]; then - return +# Function to check if the device is a supported device type +DEVICE_TYPE="" +DEVICE_SUBTYPE="" + +read_device_metadata() { + if [[ -z "$DEVICE_TYPE" ]]; then + DEVICE_TYPE=$(sonic-cfggen -d -v DEVICE_METADATA.localhost.type) + DEVICE_SUBTYPE=$(sonic-cfggen -d -v DEVICE_METADATA.localhost.subtype) fi +} - echo "Operation is only supported on Upstream SpineRouter." >&2 +declare -A prefix_type_devices +prefix_type_devices=( + ["ANCHOR_PREFIX"]="SpineRouter:UpstreamLC UpperSpineRouter" + ["SUPPRESS_PREFIX"]="" +) + +validate_device_for_type() { + local prefix_type=$1 + local allowed="${prefix_type_devices[$prefix_type]}" + [[ -z "$allowed" ]] && return + read_device_metadata + local device_id="$DEVICE_TYPE:$DEVICE_SUBTYPE" + for entry in $allowed; do + if [[ "$entry" == *":"* ]]; then + [[ "$device_id" == "$entry" ]] && return + else + [[ "$DEVICE_TYPE" == "$entry" ]] && return + fi + done + echo "Prefix type '$prefix_type' is not supported on device type '$DEVICE_TYPE' (subtype: '$DEVICE_SUBTYPE')." >&2 exit 1 } @@ -147,7 +167,7 @@ handle_prefix_list_single() { } prefix_list_operations=("add" "remove" "status") -supported_prefix_types=("ANCHOR_PREFIX") +supported_prefix_types=("ANCHOR_PREFIX" "SUPPRESS_PREFIX") # Main script execution if [[ "$1" == "-h" || "$1" == "--help" ]]; then display_help @@ -156,10 +176,12 @@ fi if [ "$1" != "status" ]; then check_root_privileges fi -check_spine_router skip_chassis_supervisor validate_operation $1 $2 +if [[ "$1" == "add" || "$1" == "remove" ]]; then + validate_device_for_type "$2" +fi # Read SONiC immutable variables [ -f /etc/sonic/sonic-environment ] && . /etc/sonic/sonic-environment diff --git a/dockers/docker-fpm-frr/frr/bgpd/suppress_prefix/add_suppress_prefix.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/suppress_prefix/add_suppress_prefix.conf.j2 new file mode 100644 index 0000000000..501d9dd91a --- /dev/null +++ b/dockers/docker-fpm-frr/frr/bgpd/suppress_prefix/add_suppress_prefix.conf.j2 @@ -0,0 +1 @@ +{{ data.ipv }} prefix-list {{ data.prefix_list_name }} permit {{ data.prefix }} diff --git a/dockers/docker-fpm-frr/frr/bgpd/suppress_prefix/del_suppress_prefix.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/suppress_prefix/del_suppress_prefix.conf.j2 new file mode 100644 index 0000000000..efb817685d --- /dev/null +++ b/dockers/docker-fpm-frr/frr/bgpd/suppress_prefix/del_suppress_prefix.conf.j2 @@ -0,0 +1 @@ +no {{ data.ipv }} prefix-list {{ data.prefix_list_name }} permit {{ data.prefix }} diff --git a/files/image_config/constants/constants.yml b/files/image_config/constants/constants.yml index c3dfcc7b29..4ffbdedcb4 100644 --- a/files/image_config/constants/constants.yml +++ b/files/image_config/constants/constants.yml @@ -72,3 +72,7 @@ constants: enabled: true db_table: "BGP_SENTINELS" template_dir: "sentinels" + prefix_list: + SUPPRESS_PREFIX: + ipv4_name: "SUPPRESS_IPV4_PREFIX" + ipv6_name: "SUPPRESS_IPV6_PREFIX" diff --git a/src/sonic-bgpcfgd/bgpcfgd/main.py b/src/sonic-bgpcfgd/bgpcfgd/main.py index ef61624315..836beec83f 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/main.py +++ b/src/sonic-bgpcfgd/bgpcfgd/main.py @@ -120,16 +120,16 @@ def do_work(): managers.append(BfdMgr(common_objs, "STATE_DB", swsscommon.STATE_BFD_SOFTWARE_SESSION_TABLE_NAME)) device_metadata = config_db.get_table("DEVICE_METADATA") - # Enable Prefix List Manager and AsPath Manager for UpperSpineRouter/UpstreamLC + # Enable AsPath Manager for UpperSpineRouter/UpstreamLC is_upstream_lc = ("localhost" in device_metadata and "type" in device_metadata["localhost"] and "subtype" in device_metadata["localhost"] and device_metadata["localhost"]["type"] == "SpineRouter" and device_metadata["localhost"]["subtype"] == "UpstreamLC") is_upper_spine_router = ("localhost" in device_metadata and "type" in device_metadata["localhost"] and device_metadata["localhost"]["type"] == "UpperSpineRouter") if is_upstream_lc or is_upper_spine_router: - # Prefix List Manager - managers.append(PrefixListMgr(common_objs, "CONFIG_DB", "PREFIX_LIST")) managers.append(AsPathMgr(common_objs, "CONFIG_DB", "DEVICE_METADATA")) - log_notice("Prefix List Manager and AsPath Manager are enabled for UpperSpineRouter/UpstreamLC") + log_notice("AsPath Manager is enabled for %s" % device_metadata["localhost"]["type"]) + + managers.append(PrefixListMgr(common_objs, "CONFIG_DB", "PREFIX_LIST")) runner = Runner(common_objs['cfg_mgr']) for mgr in managers: diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_prefix_list.py b/src/sonic-bgpcfgd/bgpcfgd/managers_prefix_list.py index 1686cfc416..57817d892c 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_prefix_list.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_prefix_list.py @@ -3,84 +3,115 @@ from swsscommon import swsscommon import netaddr +PREFIX_TYPE_CONFIG = { + "ANCHOR_PREFIX": { + "add_template": "bgpd/radian/add_radian", + "del_template": "bgpd/radian/del_radian", + "allowed_devices": [ + ("SpineRouter", "UpstreamLC"), + ("UpperSpineRouter", None), + ], + "prefix_list_name": lambda ipv: "ANCHOR_CONTRIBUTING_ROUTES", + "log_label": "Anchor prefix", + "log_label_target": "radian", + }, + "SUPPRESS_PREFIX": { + "add_template": "bgpd/suppress_prefix/add_suppress_prefix", + "del_template": "bgpd/suppress_prefix/del_suppress_prefix", + "allowed_devices": None, + "prefix_list_name": lambda ipv: "SUPPRESS_IPV4_PREFIX" if ipv == "ip" else "SUPPRESS_IPV6_PREFIX", + "log_label": "Suppress prefix", + "log_label_target": "suppress_prefix", + }, +} + class PrefixListMgr(Manager): """This class responds to changes in the PREFIX_LIST table""" 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 - """ self.directory = common_objs['directory'] self.cfg_mgr = common_objs['cfg_mgr'] self.constants = common_objs['constants'] - self.templates = { - "add_radian": common_objs['tf'].from_file("bgpd/radian/add_radian.conf.j2"), - "del_radian": common_objs['tf'].from_file("bgpd/radian/del_radian.conf.j2"), - } + self.templates = {} + for cfg in PREFIX_TYPE_CONFIG.values(): + self.templates[cfg["add_template"]] = common_objs['tf'].from_file(cfg["add_template"] + ".conf.j2") + self.templates[cfg["del_template"]] = common_objs['tf'].from_file(cfg["del_template"] + ".conf.j2") super(PrefixListMgr, self).__init__( common_objs, - [], + [ + ("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/type"), + ("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"), + ], db, table, ) - def generate_prefix_list_config(self, data, add): - """ - Generate the prefix list configuration from the template - :param data: data from the PREFIX_LIST table - :return: rendered configuration - """ - cmd = "\n" - metadata = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"] + def _is_device_allowed(self, device_type, device_subtype, allowed_list): + if allowed_list is None: + return True + for allowed_type, allowed_subtype in allowed_list: + if device_type == allowed_type: + if allowed_subtype is None or device_subtype == allowed_subtype: + return True + return False + + def generate_prefix_list_config(self, prefix_type, data, add): + type_cfg = PREFIX_TYPE_CONFIG.get(prefix_type) + if type_cfg is None: + log_warn("PrefixListMgr:: Prefix type '%s' is not supported" % prefix_type) + return False + + if not self.directory.path_exist("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost"): + log_info("PrefixListMgr:: Device metadata is not ready yet") + return False + metadata = self.directory.get_path("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost") + # bgp_asn is required for all prefix types: ANCHOR_PREFIX templates use it + # directly (router bgp ), and while SUPPRESS_PREFIX templates don't + # reference it today, prefix-list operations are inherently BGP features — + # any device managing prefix lists will be running BGP with an ASN configured. + # Requiring bgp_asn upfront keeps templates free to use it when expanded. try: + device_type = metadata["type"] + device_subtype = metadata.get("subtype", "") bgp_asn = metadata["bgp_asn"] - localhost_type = metadata["type"] - subtype = metadata["subtype"] except KeyError as e: - log_warn(f"PrefixListMgr:: Missing metadata key: {e}") + log_warn("PrefixListMgr:: Missing metadata key: %s" % e) return False - if data["prefix_list_name"] != "ANCHOR_PREFIX": - log_warn("PrefixListMgr:: Prefix list %s is not supported" % data["prefix_list_name"]) - return False - if localhost_type not in ["UpperSpineRouter", "SpineRouter"] or (localhost_type == "SpineRouter" and subtype != "UpstreamLC"): - log_warn("PrefixListMgr:: Prefix list %s is only supported on Upstream SpineRouter" % data["prefix_list_name"]) + if not self._is_device_allowed(device_type, device_subtype, type_cfg["allowed_devices"]): + device_desc = "%s/%s" % (device_type, device_subtype) if device_subtype else device_type + log_warn("PrefixListMgr:: Device type %s not supported for %s" % (device_desc, prefix_type)) return False - # Add the anchor prefix to the radian configuration data["bgp_asn"] = bgp_asn - if add: - # add some way of getting this asn list from the database in the future - cmd += self.templates["add_radian"].render(data=data) - log_debug("PrefixListMgr:: Anchor prefix %s added to radian configuration" % data["prefix"]) - else: - cmd += self.templates["del_radian"].render(data=data) - log_debug("PrefixListMgr:: Anchor prefix %s removed from radian configuration" % data["prefix"]) + data["device_type"] = device_type + data["device_subtype"] = device_subtype + pl_overrides = self.constants.get("bgp", {}).get("prefix_list", {}).get(prefix_type, {}) + name_key = "ipv4_name" if data["ipv"] == "ip" else "ipv6_name" + data["prefix_list_name"] = pl_overrides.get(name_key, type_cfg["prefix_list_name"](data["ipv"])) + + template_key = type_cfg["add_template"] if add else type_cfg["del_template"] + cmd = "\n" + self.templates[template_key].render(data=data) self.cfg_mgr.push(cmd) + + action = "added to" if add else "removed from" + log_debug("PrefixListMgr:: %s %s %s %s configuration" % (type_cfg["log_label"], data["prefix"], action, type_cfg["log_label_target"])) return True - - def set_handler(self, key, data): log_debug("PrefixListMgr:: set handler") if '|' in key: - prefix_list_name, prefix_str = key.split('|', 1) + prefix_type, prefix_str = key.split('|', 1) try: prefix = netaddr.IPNetwork(str(prefix_str)) except (netaddr.NotRegisteredError, netaddr.AddrFormatError, netaddr.AddrConversionError): - log_warn("PrefixListMgr:: Prefix '%s' format is wrong for prefix list '%s'" % (prefix_str, prefix_list_name)) + log_warn("PrefixListMgr:: Prefix '%s' format is wrong for prefix list '%s'" % (prefix_str, prefix_type)) return True - data["prefix_list_name"] = prefix_list_name data["prefix"] = str(prefix.cidr) data["prefixlen"] = prefix.prefixlen data["ipv"] = self.get_ip_type(prefix) - # Generate the prefix list configuration - if self.generate_prefix_list_config(data, add=True): - log_info("PrefixListMgr:: %s %s configuration generated" % (prefix_list_name, data["prefix"])) - + if self.generate_prefix_list_config(prefix_type, data, add=True): + log_info("PrefixListMgr:: %s %s configuration generated" % (prefix_type, data["prefix"])) self.directory.put(self.db_name, self.table_name, key, data) log_info("PrefixListMgr:: set %s" % key) return True @@ -88,31 +119,23 @@ def set_handler(self, key, data): def del_handler(self, key): log_debug("PrefixListMgr:: del handler") if '|' in key: - prefix_list_name, prefix_str = key.split('|', 1) + prefix_type, prefix_str = key.split('|', 1) try: prefix = netaddr.IPNetwork(str(prefix_str)) except (netaddr.NotRegisteredError, netaddr.AddrFormatError, netaddr.AddrConversionError): - log_warn("PrefixListMgr:: Prefix '%s' format is wrong for prefix list '%s'" % (prefix_str, prefix_list_name)) + log_warn("PrefixListMgr:: Prefix '%s' format is wrong for prefix list '%s'" % (prefix_str, prefix_type)) return True data = {} - data["prefix_list_name"] = prefix_list_name data["prefix"] = str(prefix.cidr) data["prefixlen"] = prefix.prefixlen data["ipv"] = self.get_ip_type(prefix) - # remove the prefix list configuration - if self.generate_prefix_list_config(data, add=False): - log_info("PrefixListMgr:: %s %s configuration deleted" % (prefix_list_name, data["prefix"])) + if self.generate_prefix_list_config(prefix_type, data, add=False): + log_info("PrefixListMgr:: %s %s configuration deleted" % (prefix_type, data["prefix"])) self.directory.remove(self.db_name, self.table_name, key) log_info("PrefixListMgr:: deleted %s" % key) - # Implement deletion logic if necessary return True def get_ip_type(self, prefix: netaddr.IPNetwork): - """ - Determine the IP type (IPv4 or IPv6) of a prefix. - :param prefix: The prefix to check (e.g., "192.168.1.0/24" or "2001:db8::/32") - :return: "ip" if the prefix is an IPv4 address, "ipv6" if it is an IPv6 address, None if invalid - """ if prefix.version == 4: return "ip" elif prefix.version == 6: diff --git a/src/sonic-bgpcfgd/tests/test_prefix_list.py b/src/sonic-bgpcfgd/tests/test_prefix_list.py index 9f1ef6a48c..58e4682c0f 100644 --- a/src/sonic-bgpcfgd/tests/test_prefix_list.py +++ b/src/sonic-bgpcfgd/tests/test_prefix_list.py @@ -4,6 +4,10 @@ from bgpcfgd.directory import Directory from bgpcfgd.template import TemplateFabric from . import swsscommon_test + +import sys +sys.modules["swsscommon"] = swsscommon_test + from swsscommon import swsscommon from bgpcfgd.managers_prefix_list import PrefixListMgr @@ -67,4 +71,90 @@ def test_del_handler_ipv6(mocked_log_debug): m = constructor() set_handler_test(m, "ANCHOR_PREFIX|fc02:100::/64", {}) del_handler_test(m, "ANCHOR_PREFIX|fc02:100::/64") - mocked_log_debug.assert_called_with("PrefixListMgr:: Anchor prefix fc02:100::/64 removed from radian configuration") \ No newline at end of file + mocked_log_debug.assert_called_with("PrefixListMgr:: Anchor prefix fc02:100::/64 removed from radian configuration") + +def constructor_with_constants(constants): + cfg_mgr = MagicMock() + common_objs = { + 'directory': Directory(), + 'cfg_mgr': cfg_mgr, + 'tf': TemplateFabric(TEMPLATE_PATH), + 'constants': constants, + } + m = PrefixListMgr(common_objs, "CONFIG_DB", "PREFIX_LIST") + m.directory.put("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost", + {"bgp_asn": "65100", "type": "ToRRouter", "subtype": ""}) + return m + +@patch('bgpcfgd.managers_prefix_list.log_warn') +def test_unsupported_prefix_type(mocked_log_warn): + m = constructor_with_constants({}) + set_handler_test(m, "UNKNOWN_TYPE|10.0.0.0/24", {}) + mocked_log_warn.assert_called_with("PrefixListMgr:: Prefix type 'UNKNOWN_TYPE' is not supported") + +@patch('bgpcfgd.managers_prefix_list.log_warn') +def test_anchor_prefix_wrong_device(mocked_log_warn): + m = constructor_with_constants({}) + set_handler_test(m, "ANCHOR_PREFIX|192.168.0.0/24", {}) + mocked_log_warn.assert_called_with("PrefixListMgr:: Device type ToRRouter not supported for ANCHOR_PREFIX") + +@patch('bgpcfgd.managers_prefix_list.log_debug') +def test_suppress_prefix_ipv4(mocked_log_debug): + m = constructor_with_constants({}) + set_handler_test(m, "SUPPRESS_PREFIX|10.0.0.0/24", {}) + mocked_log_debug.assert_called_with("PrefixListMgr:: Suppress prefix 10.0.0.0/24 added to suppress_prefix configuration") + +@patch('bgpcfgd.managers_prefix_list.log_debug') +def test_suppress_prefix_ipv6(mocked_log_debug): + m = constructor_with_constants({}) + set_handler_test(m, "SUPPRESS_PREFIX|fc00::/64", {}) + mocked_log_debug.assert_called_with("PrefixListMgr:: Suppress prefix fc00::/64 added to suppress_prefix configuration") + +@patch('bgpcfgd.managers_prefix_list.log_debug') +def test_suppress_prefix_del_ipv4(mocked_log_debug): + m = constructor_with_constants({}) + set_handler_test(m, "SUPPRESS_PREFIX|10.0.0.0/24", {}) + del_handler_test(m, "SUPPRESS_PREFIX|10.0.0.0/24") + mocked_log_debug.assert_called_with("PrefixListMgr:: Suppress prefix 10.0.0.0/24 removed from suppress_prefix configuration") + +@patch('bgpcfgd.managers_prefix_list.log_debug') +def test_suppress_prefix_del_ipv6(mocked_log_debug): + m = constructor_with_constants({}) + set_handler_test(m, "SUPPRESS_PREFIX|fc00::/64", {}) + del_handler_test(m, "SUPPRESS_PREFIX|fc00::/64") + mocked_log_debug.assert_called_with("PrefixListMgr:: Suppress prefix fc00::/64 removed from suppress_prefix configuration") + +@patch('bgpcfgd.managers_prefix_list.log_debug') +def test_suppress_prefix_any_device(mocked_log_debug): + m = constructor() + set_handler_test(m, "SUPPRESS_PREFIX|10.0.0.0/24", {}) + mocked_log_debug.assert_called_with("PrefixListMgr:: Suppress prefix 10.0.0.0/24 added to suppress_prefix configuration") + +@patch('bgpcfgd.managers_prefix_list.log_debug') +def test_suppress_prefix_constants_override(mocked_log_debug): + constants = {"bgp": {"prefix_list": {"SUPPRESS_PREFIX": { + "ipv4_name": "CUSTOM_IPV4_PREFIX", + "ipv6_name": "CUSTOM_IPV6_PREFIX"}}}} + m = constructor_with_constants(constants) + set_handler_test(m, "SUPPRESS_PREFIX|10.0.0.0/24", {}) + push_call = m.cfg_mgr.push.call_args[0][0] + assert "CUSTOM_IPV4_PREFIX" in push_call + assert "SUPPRESS_IPV4_PREFIX" not in push_call + +@patch('bgpcfgd.managers_prefix_list.log_debug') +def test_suppress_prefix_constants_override_ipv6(mocked_log_debug): + constants = {"bgp": {"prefix_list": {"SUPPRESS_PREFIX": { + "ipv4_name": "CUSTOM_IPV4_PREFIX", + "ipv6_name": "CUSTOM_IPV6_PREFIX"}}}} + m = constructor_with_constants(constants) + set_handler_test(m, "SUPPRESS_PREFIX|fc00::/64", {}) + push_call = m.cfg_mgr.push.call_args[0][0] + assert "CUSTOM_IPV6_PREFIX" in push_call + assert "SUPPRESS_IPV6_PREFIX" not in push_call + +@patch('bgpcfgd.managers_prefix_list.log_debug') +def test_suppress_prefix_no_constants_fallback(mocked_log_debug): + m = constructor_with_constants({}) + set_handler_test(m, "SUPPRESS_PREFIX|10.0.0.0/24", {}) + push_call = m.cfg_mgr.push.call_args[0][0] + assert "SUPPRESS_IPV4_PREFIX" in push_call