Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bgp-cfgd] BGP allow list enhancement #11586

Merged
merged 2 commits into from
Aug 11, 2022
Merged

[bgp-cfgd] BGP allow list enhancement #11586

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7446098
BGP allow list enhancement
StormLiangMS Jul 30, 2022
8993397
fix comments
StormLiangMS Aug 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 76 additions & 36 deletions src/sonic-bgpcfgd/bgpcfgd/managers_allow_list.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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 = []
Expand All @@ -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):
Expand All @@ -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
Expand All @@ -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):
"""
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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'])
Expand All @@ -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 neighbor_type == '':
StormLiangMS marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just thinking, why not ignore neighbor_type completely and use the existing flow? Will there be any conflicts?

For e.g:

ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 10 deny 0.0.0.0/0 le 17
instead of
ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_NEIGHBOR_OpticalLonghaulTerminal_COMMUNITY_empty_V4 seq 10 deny 0.0.0.0/0 le 17

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prsunny The neighbor of Mgmt ToR could be T1 or OLT, to add neighbor type info to tell the difference. Since it is possible to add different allow prefix list towards T1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

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):
Expand Down Expand Up @@ -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
Expand All @@ -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 neighbor_type == '':
StormLiangMS marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
Loading