[bgp-cfgd] BGP allow list enhancement (sonic-net#11586)

Why I did it
2 things are missing in current allow-prefix list implementation.

In some usecase, need to tell the BGP neighbor and have different allow-prefix list for different neighbors, which is not supported.
for the prefix list, can't support flexible le and ge.
How I did it
To enhance the bgp allow-prefix list feature to have:

To include the neighbor type info for the allow-prefix list.
To support flexible le and ge length for allow-prefix list.
How to verify it
4 new unit test cases are added in this PR to cover changes.

Author: StormLiangMS

src/sonic-bgpcfgd/bgpcfgd/managers_allow_list.py

@@ -14,8 +14,11 @@ class BGPAllowListMgr(Manager):
     ALLOW_ADDRESS_PL_NAME_TMPL = "ALLOW_ADDRESS_%d_%s"  # template for a name for the ALLOW_ADDRESS prefix-list ???
     EMPTY_COMMUNITY = "empty"
     PL_NAME_TMPL = "PL_ALLOW_LIST_DEPLOYMENT_ID_%d_COMMUNITY_%s_V%s"
+    PL_NAME_TMPL_WITH_NEIGH = "PL_ALLOW_LIST_DEPLOYMENT_ID_%d_NEIGHBOR_%s_COMMUNITY_%s_V%s"
     COMMUNITY_NAME_TMPL = "COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_%d_COMMUNITY_%s"
+    COMMUNITY_NAME_TMPL_WITH_NEIGH = "COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_%d_NEIGHBOR_%s_COMMUNITY_%s"
     RM_NAME_TMPL = "ALLOW_LIST_DEPLOYMENT_ID_%d_V%s"
+    RM_NAME_TMPL_WITH_NEIGH = "ALLOW_LIST_DEPLOYMENT_ID_%d_NEIGHBOR_%s_V%s"
     ROUTE_MAP_ENTRY_WITH_COMMUNITY_START = 10
     ROUTE_MAP_ENTRY_WITH_COMMUNITY_END = 29990
     ROUTE_MAP_ENTRY_WITHOUT_COMMUNITY_START = 30000
@@ -38,7 +41,7 @@ def __init__(self, common_objs, db, table):
             db,
             table,
         )
-        self.key_re = re.compile(r"^DEPLOYMENT_ID\|\d+\|\S+$|^DEPLOYMENT_ID\|\d+$")
+        self.key_re = re.compile(r"^DEPLOYMENT_ID\|\d+\|\S+$|^DEPLOYMENT_ID\|\d+$|^DEPLOYMENT_ID\|\d+\|\S+\|NEIGHBOR_TYPE\|\S+$|^DEPLOYMENT_ID\|\d+\|NEIGHBOR_TYPE\|\S+")
         self.enabled = self.__get_enabled()
         self.prefix_match_tag = self.__get_routemap_tag()
         self.__load_constant_lists()
@@ -55,8 +58,14 @@ def set_handler(self, key, data):
             return True
         if not self.__set_handler_validate(key, data):
             return True
-        key = key.replace("DEPLOYMENT_ID|", "")
-        deployment_id, community_value = key.split('|', 1) if '|' in key else (key, BGPAllowListMgr.EMPTY_COMMUNITY)
+        if 'NEIGHBOR_TYPE' in key:
+            keys = key.split('|NEIGHBOR_TYPE|', 1)
+            deployment_id = keys[0].replace("DEPLOYMENT_ID|", "")
+            neighbor_type, community_value = keys[1].split('|', 1) if '|' in keys[1] else (keys[1], BGPAllowListMgr.EMPTY_COMMUNITY)
+        else:
+            key = key.replace("DEPLOYMENT_ID|", "")
+            deployment_id, community_value = key.split('|', 1) if '|' in key else (key, BGPAllowListMgr.EMPTY_COMMUNITY)
+            neighbor_type = ''
         deployment_id = int(deployment_id)
         prefixes_v4 = []
         prefixes_v6 = []
@@ -65,7 +74,7 @@ def set_handler(self, key, data):
         if "prefixes_v6" in data:
             prefixes_v6 = str(data['prefixes_v6']).split(",")
         default_action_community = self.__get_default_action_community(data)
-        self.__update_policy(deployment_id, community_value, prefixes_v4, prefixes_v6, default_action_community)
+        self.__update_policy(deployment_id, community_value, prefixes_v4, prefixes_v6, default_action_community, neighbor_type)
         return True
 
     def __set_handler_validate(self, key, data):
@@ -85,13 +94,13 @@ def __set_handler_validate(self, key, data):
         prefixes_v6 = []
         if "prefixes_v4" in data:
             prefixes_v4 = str(data["prefixes_v4"]).split(",")
-            if not all(TemplateFabric.is_ipv4(prefix) for prefix in prefixes_v4):
+            if not all(TemplateFabric.is_ipv4(re.split('ge|le', prefix)[0]) for prefix in prefixes_v4):
                 arguments = "prefixes_v4", str(data["prefixes_v4"])
                 log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message with invalid input[%s]:'%s'" % arguments)
                 return False
         if "prefixes_v6" in data:
             prefixes_v6 = str(data["prefixes_v6"]).split(",")
-            if not all(TemplateFabric.is_ipv6(prefix) for prefix in prefixes_v6):
+            if not all(TemplateFabric.is_ipv6(re.split('ge|le', prefix)[0]) for prefix in prefixes_v6):
                 arguments = "prefixes_v6", str(data["prefixes_v6"])
                 log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message with invalid input[%s]:'%s'" % arguments)
                 return False
@@ -113,10 +122,18 @@ def del_handler(self, key):
             return
         if not self.__del_handler_validate(key):
             return
-        key = key.replace('DEPLOYMENT_ID|', '')
-        deployment_id, community = key.split('|', 1) if '|' in key else (key, BGPAllowListMgr.EMPTY_COMMUNITY)
+
+        if 'NEIGHBOR_TYPE' in key:
+            keys = key.split('|NEIGHBOR_TYPE|', 1)
+            deployment_id = keys[0].replace("DEPLOYMENT_ID|", "")
+            neighbor_type, community_value = keys[1].split('|', 1) if '|' in keys[1] else (keys[1], BGPAllowListMgr.EMPTY_COMMUNITY)
+        else:
+            key = key.replace("DEPLOYMENT_ID|", "")
+            deployment_id, community_value = key.split('|', 1) if '|' in key else (key, BGPAllowListMgr.EMPTY_COMMUNITY)
+            neighbor_type = ''
+
         deployment_id = int(deployment_id)
-        self.__remove_policy(deployment_id, community)
+        self.__remove_policy(deployment_id, community_value, neighbor_type)
 
     def __del_handler_validate(self, key):
         """
@@ -129,7 +146,7 @@ def __del_handler_validate(self, key):
             return False
         return True
 
-    def __update_policy(self, deployment_id, community_value, prefixes_v4, prefixes_v6, default_action):
+    def __update_policy(self, deployment_id, community_value, prefixes_v4, prefixes_v6, default_action, neighbor_type):
         """
         Update "allow list" policy with parameters
         :param deployment_id: deployment id which policy will be changed
@@ -139,12 +156,13 @@ def __update_policy(self, deployment_id, community_value, prefixes_v4, prefixes_
         :param default_action: the default action for the policy. should be either 'permit' or 'deny'
         """
         # update all related entries with the information
-        info = deployment_id, community_value, str(prefixes_v4), str(prefixes_v6)
+        info = deployment_id, community_value, str(prefixes_v4), str(prefixes_v6), neighbor_type
         msg = "BGPAllowListMgr::Updating 'Allow list' policy."
         msg += " deployment_id '%s'. community: '%s'"
         msg += " prefix_v4 '%s'. prefix_v6: '%s'"
+        msg += " neighbor_type %s"
         log_info(msg % info)
-        names = self.__generate_names(deployment_id, community_value)
+        names = self.__generate_names(deployment_id, community_value, neighbor_type)
         self.cfg_mgr.update()
         cmds = []
         cmds += self.__update_prefix_list(self.V4, names['pl_v4'], prefixes_v4)
@@ -156,14 +174,14 @@ def __update_policy(self, deployment_id, community_value, prefixes_v4, prefixes_
         cmds += self.__update_default_route_map_entry(names['rm_v6'], default_action)
         if cmds:
             self.cfg_mgr.push_list(cmds)
-            peer_groups = self.__find_peer_group_by_deployment_id(deployment_id)
+            peer_groups = self.__find_peer_group(deployment_id, neighbor_type)
             self.cfg_mgr.restart_peer_groups(peer_groups)
             log_debug("BGPAllowListMgr::__update_policy. The peers configuration scheduled for updates")
         else:
             log_debug("BGPAllowListMgr::__update_policy. Nothing to update")
         log_info("BGPAllowListMgr::Done")
 
-    def __remove_policy(self, deployment_id, community_value):
+    def __remove_policy(self, deployment_id, community_value, neighbor_type):
         """
         Remove "allow list" policy for given deployment_id and community_value
         :param deployment_id: deployment id which policy will be removed
@@ -177,7 +195,7 @@ def __remove_policy(self, deployment_id, community_value):
         log_info(msg % info)
 
         default_action = self.__get_default_action_community()
-        names = self.__generate_names(deployment_id, community_value)
+        names = self.__generate_names(deployment_id, community_value, neighbor_type)
         self.cfg_mgr.update()
         cmds = []
         cmds += self.__remove_allow_route_map_entry(self.V4, names['pl_v4'], names['community'], names['rm_v4'])
@@ -189,34 +207,50 @@ def __remove_policy(self, deployment_id, community_value):
         cmds += self.__update_default_route_map_entry(names['rm_v6'], default_action)
         if cmds:
             self.cfg_mgr.push_list(cmds)
-            peer_groups = self.__find_peer_group_by_deployment_id(deployment_id)
+            peer_groups = self.__find_peer_group(deployment_id, neighbor_type)
             self.cfg_mgr.restart_peer_groups(peer_groups)
             log_debug("BGPAllowListMgr::__remove_policy. 'Allow list' policy was scheduled for removal")
         else:
             log_debug("BGPAllowListMgr::__remove_policy. Nothing to remove")
         log_info('BGPAllowListMgr::Done')
 
     @staticmethod
-    def __generate_names(deployment_id, community_value):
+    def __generate_names(deployment_id, community_value, neighbor_type):
         """
         Generate prefix-list names for a given peer_ip and community value
         :param deployment_id: deployment_id for which we're going to filter prefixes
         :param community_value: community, which we want to use to filter prefixes
         :return: a dictionary with names
         """
-        if community_value == BGPAllowListMgr.EMPTY_COMMUNITY:
-            community_name = BGPAllowListMgr.EMPTY_COMMUNITY
+        if not neighbor_type:
+            if community_value == BGPAllowListMgr.EMPTY_COMMUNITY:
+                community_name = BGPAllowListMgr.EMPTY_COMMUNITY
+            else:
+                community_name = BGPAllowListMgr.COMMUNITY_NAME_TMPL % (deployment_id, community_value)
+            names = {
+                "pl_v4": BGPAllowListMgr.PL_NAME_TMPL % (deployment_id, community_value, '4'),
+                "pl_v6": BGPAllowListMgr.PL_NAME_TMPL % (deployment_id, community_value, '6'),
+                "rm_v4": BGPAllowListMgr.RM_NAME_TMPL % (deployment_id, '4'),
+                "rm_v6": BGPAllowListMgr.RM_NAME_TMPL % (deployment_id, '6'),
+                "community": community_name,
+                'neigh_type': neighbor_type,
+            }
+            arguments = deployment_id, community_value, str(names)
+            log_debug("BGPAllowListMgr::__generate_names. deployment_id: %d, community: %s. names: %s" % arguments)
         else:
-            community_name = BGPAllowListMgr.COMMUNITY_NAME_TMPL % (deployment_id, community_value)
-        names = {
-            "pl_v4": BGPAllowListMgr.PL_NAME_TMPL % (deployment_id, community_value, '4'),
-            "pl_v6": BGPAllowListMgr.PL_NAME_TMPL % (deployment_id, community_value, '6'),
-            "rm_v4": BGPAllowListMgr.RM_NAME_TMPL % (deployment_id, '4'),
-            "rm_v6": BGPAllowListMgr.RM_NAME_TMPL % (deployment_id, '6'),
-            "community": community_name,
-        }
-        arguments = deployment_id, community_value, str(names)
-        log_debug("BGPAllowListMgr::__generate_names. deployment_id: %d, community: %s. names: %s" % arguments)
+            if community_value == BGPAllowListMgr.EMPTY_COMMUNITY:
+                community_name = BGPAllowListMgr.EMPTY_COMMUNITY
+            else:
+                community_name = BGPAllowListMgr.COMMUNITY_NAME_TMPL_WITH_NEIGH % (deployment_id, neighbor_type, community_value)
+            names = {
+                "pl_v4": BGPAllowListMgr.PL_NAME_TMPL_WITH_NEIGH % (deployment_id, neighbor_type, community_value, '4'),
+                "pl_v6": BGPAllowListMgr.PL_NAME_TMPL_WITH_NEIGH % (deployment_id, neighbor_type, community_value, '6'),
+                "rm_v4": BGPAllowListMgr.RM_NAME_TMPL_WITH_NEIGH % (deployment_id, neighbor_type, '4'),
+                "rm_v6": BGPAllowListMgr.RM_NAME_TMPL_WITH_NEIGH % (deployment_id, neighbor_type, '6'),
+                "community": community_name,
+            }
+            arguments = deployment_id, neighbor_type, community_value, str(names)
+            log_debug("BGPAllowListMgr::__generate_names. deployment_id: %d, neighbor_type: %s, community: %s. names: %s" % arguments)
         return names
 
     def __update_prefix_list(self, af, pl_name, allow_list):
@@ -630,7 +664,7 @@ def __get_routemap_tag(self):
         return prefix_match_tag
 
     @staticmethod
-    def __get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call):
+    def __get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call, neighbor_type):
         """
         Get peer_groups which are assigned to deployment_id
         :deployment_id: deployment_id number
@@ -639,14 +673,17 @@ def __get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call):
         :rm_2_call: a dictionary: key - name of a route-map, value - name of a route-map call defined for the route-map
         """
         ret = set()
-        target_allow_list_prefix = 'ALLOW_LIST_DEPLOYMENT_ID_%d_V' % deployment_id
+        if not neighbor_type:
+            target_allow_list_prefix = 'ALLOW_LIST_DEPLOYMENT_ID_%d_V' % deployment_id
+        else:
+            target_allow_list_prefix = 'ALLOW_LIST_DEPLOYMENT_ID_%d_NEIGHBOR_%s_V' % (deployment_id, neighbor_type)
         for peer_group, route_map in pg_2_rm.items():
             if route_map in rm_2_call:
                 if rm_2_call[route_map].startswith(target_allow_list_prefix):
                     ret.add(peer_group)
         return list(ret)
 
-    def __find_peer_group_by_deployment_id(self, deployment_id):
+    def __find_peer_group(self, deployment_id, neighbor_type):
         """
         Deduce peer-group names which are connected to devices with requested deployment_id
         :param deployment_id: deployment_id number
@@ -656,7 +693,7 @@ def __find_peer_group_by_deployment_id(self, deployment_id):
         peer_groups = self.__extract_peer_group_names()
         pg_2_rm = self.__get_peer_group_to_route_map(peer_groups)
         rm_2_call = self.__get_route_map_calls(set(pg_2_rm.values()))
-        ret = self.__get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call)
+        ret = self.__get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call, neighbor_type)
         return list(ret)
 
     def __get_enabled(self):
@@ -706,11 +743,14 @@ def __to_prefix_list(self, af, allow_list):
         res = []
         prefix_mask_default = 32 if af == self.V4 else 128
         for prefix in allow_list:
-            prefix_mask = int(prefix.split("/")[1])
-            if prefix_mask == prefix_mask_default:
+            if 'le' in prefix or 'ge' in prefix:
                 res.append("permit %s" % prefix)
             else:
-                res.append("permit %s le %d" % (prefix, prefix_mask_default))
+                prefix_mask = int(prefix.split("/")[1])
+                if prefix_mask == prefix_mask_default:
+                    res.append("permit %s" % prefix)
+                else:
+                    res.append("permit %s le %d" % (prefix, prefix_mask_default))
         return res
 
     def __af_to_family(self, af):