Skip to content

Commit 147f736

Browse files
StormLiangMSStormLiangMS
authored
[bgp-cfgd] BGP allow list enhancement (#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.
1 parent 2d42993 commit 147f736

File tree

2 files changed

+203
-38
lines changed

2 files changed

+203
-38
lines changed

‎src/sonic-bgpcfgd/bgpcfgd/managers_allow_list.py‎

Lines changed: 76 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@ class BGPAllowListMgr(Manager):
1414
ALLOW_ADDRESS_PL_NAME_TMPL = "ALLOW_ADDRESS_%d_%s" # template for a name for the ALLOW_ADDRESS prefix-list ???
1515
EMPTY_COMMUNITY = "empty"
1616
PL_NAME_TMPL = "PL_ALLOW_LIST_DEPLOYMENT_ID_%d_COMMUNITY_%s_V%s"
17+
PL_NAME_TMPL_WITH_NEIGH = "PL_ALLOW_LIST_DEPLOYMENT_ID_%d_NEIGHBOR_%s_COMMUNITY_%s_V%s"
1718
COMMUNITY_NAME_TMPL = "COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_%d_COMMUNITY_%s"
19+
COMMUNITY_NAME_TMPL_WITH_NEIGH = "COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_%d_NEIGHBOR_%s_COMMUNITY_%s"
1820
RM_NAME_TMPL = "ALLOW_LIST_DEPLOYMENT_ID_%d_V%s"
21+
RM_NAME_TMPL_WITH_NEIGH = "ALLOW_LIST_DEPLOYMENT_ID_%d_NEIGHBOR_%s_V%s"
1922
ROUTE_MAP_ENTRY_WITH_COMMUNITY_START = 10
2023
ROUTE_MAP_ENTRY_WITH_COMMUNITY_END = 29990
2124
ROUTE_MAP_ENTRY_WITHOUT_COMMUNITY_START = 30000
@@ -38,7 +41,7 @@ def __init__(self, common_objs, db, table):
3841
db,
3942
table,
4043
)
41-
self.key_re = re.compile(r"^DEPLOYMENT_ID\|\d+\|\S+$|^DEPLOYMENT_ID\|\d+$")
44+
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+")
4245
self.enabled = self.__get_enabled()
4346
self.prefix_match_tag = self.__get_routemap_tag()
4447
self.__load_constant_lists()
@@ -55,8 +58,14 @@ def set_handler(self, key, data):
5558
return True
5659
if not self.__set_handler_validate(key, data):
5760
return True
58-
key = key.replace("DEPLOYMENT_ID|", "")
59-
deployment_id, community_value = key.split('|', 1) if '|' in key else (key, BGPAllowListMgr.EMPTY_COMMUNITY)
61+
if 'NEIGHBOR_TYPE' in key:
62+
keys = key.split('|NEIGHBOR_TYPE|', 1)
63+
deployment_id = keys[0].replace("DEPLOYMENT_ID|", "")
64+
neighbor_type, community_value = keys[1].split('|', 1) if '|' in keys[1] else (keys[1], BGPAllowListMgr.EMPTY_COMMUNITY)
65+
else:
66+
key = key.replace("DEPLOYMENT_ID|", "")
67+
deployment_id, community_value = key.split('|', 1) if '|' in key else (key, BGPAllowListMgr.EMPTY_COMMUNITY)
68+
neighbor_type = ''
6069
deployment_id = int(deployment_id)
6170
prefixes_v4 = []
6271
prefixes_v6 = []
@@ -65,7 +74,7 @@ def set_handler(self, key, data):
6574
if "prefixes_v6" in data:
6675
prefixes_v6 = str(data['prefixes_v6']).split(",")
6776
default_action_community = self.__get_default_action_community(data)
68-
self.__update_policy(deployment_id, community_value, prefixes_v4, prefixes_v6, default_action_community)
77+
self.__update_policy(deployment_id, community_value, prefixes_v4, prefixes_v6, default_action_community, neighbor_type)
6978
return True
7079

7180
def __set_handler_validate(self, key, data):
@@ -85,13 +94,13 @@ def __set_handler_validate(self, key, data):
8594
prefixes_v6 = []
8695
if "prefixes_v4" in data:
8796
prefixes_v4 = str(data["prefixes_v4"]).split(",")
88-
if not all(TemplateFabric.is_ipv4(prefix) for prefix in prefixes_v4):
97+
if not all(TemplateFabric.is_ipv4(re.split('ge|le', prefix)[0]) for prefix in prefixes_v4):
8998
arguments = "prefixes_v4", str(data["prefixes_v4"])
9099
log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message with invalid input[%s]:'%s'" % arguments)
91100
return False
92101
if "prefixes_v6" in data:
93102
prefixes_v6 = str(data["prefixes_v6"]).split(",")
94-
if not all(TemplateFabric.is_ipv6(prefix) for prefix in prefixes_v6):
103+
if not all(TemplateFabric.is_ipv6(re.split('ge|le', prefix)[0]) for prefix in prefixes_v6):
95104
arguments = "prefixes_v6", str(data["prefixes_v6"])
96105
log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message with invalid input[%s]:'%s'" % arguments)
97106
return False
@@ -113,10 +122,18 @@ def del_handler(self, key):
113122
return
114123
if not self.__del_handler_validate(key):
115124
return
116-
key = key.replace('DEPLOYMENT_ID|', '')
117-
deployment_id, community = key.split('|', 1) if '|' in key else (key, BGPAllowListMgr.EMPTY_COMMUNITY)
125+
126+
if 'NEIGHBOR_TYPE' in key:
127+
keys = key.split('|NEIGHBOR_TYPE|', 1)
128+
deployment_id = keys[0].replace("DEPLOYMENT_ID|", "")
129+
neighbor_type, community_value = keys[1].split('|', 1) if '|' in keys[1] else (keys[1], BGPAllowListMgr.EMPTY_COMMUNITY)
130+
else:
131+
key = key.replace("DEPLOYMENT_ID|", "")
132+
deployment_id, community_value = key.split('|', 1) if '|' in key else (key, BGPAllowListMgr.EMPTY_COMMUNITY)
133+
neighbor_type = ''
134+
118135
deployment_id = int(deployment_id)
119-
self.__remove_policy(deployment_id, community)
136+
self.__remove_policy(deployment_id, community_value, neighbor_type)
120137

121138
def __del_handler_validate(self, key):
122139
"""
@@ -129,7 +146,7 @@ def __del_handler_validate(self, key):
129146
return False
130147
return True
131148

132-
def __update_policy(self, deployment_id, community_value, prefixes_v4, prefixes_v6, default_action):
149+
def __update_policy(self, deployment_id, community_value, prefixes_v4, prefixes_v6, default_action, neighbor_type):
133150
"""
134151
Update "allow list" policy with parameters
135152
: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_
139156
:param default_action: the default action for the policy. should be either 'permit' or 'deny'
140157
"""
141158
# update all related entries with the information
142-
info = deployment_id, community_value, str(prefixes_v4), str(prefixes_v6)
159+
info = deployment_id, community_value, str(prefixes_v4), str(prefixes_v6), neighbor_type
143160
msg = "BGPAllowListMgr::Updating 'Allow list' policy."
144161
msg += " deployment_id '%s'. community: '%s'"
145162
msg += " prefix_v4 '%s'. prefix_v6: '%s'"
163+
msg += " neighbor_type %s"
146164
log_info(msg % info)
147-
names = self.__generate_names(deployment_id, community_value)
165+
names = self.__generate_names(deployment_id, community_value, neighbor_type)
148166
self.cfg_mgr.update()
149167
cmds = []
150168
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_
156174
cmds += self.__update_default_route_map_entry(names['rm_v6'], default_action)
157175
if cmds:
158176
self.cfg_mgr.push_list(cmds)
159-
peer_groups = self.__find_peer_group_by_deployment_id(deployment_id)
177+
peer_groups = self.__find_peer_group(deployment_id, neighbor_type)
160178
self.cfg_mgr.restart_peer_groups(peer_groups)
161179
log_debug("BGPAllowListMgr::__update_policy. The peers configuration scheduled for updates")
162180
else:
163181
log_debug("BGPAllowListMgr::__update_policy. Nothing to update")
164182
log_info("BGPAllowListMgr::Done")
165183

166-
def __remove_policy(self, deployment_id, community_value):
184+
def __remove_policy(self, deployment_id, community_value, neighbor_type):
167185
"""
168186
Remove "allow list" policy for given deployment_id and community_value
169187
:param deployment_id: deployment id which policy will be removed
@@ -177,7 +195,7 @@ def __remove_policy(self, deployment_id, community_value):
177195
log_info(msg % info)
178196

179197
default_action = self.__get_default_action_community()
180-
names = self.__generate_names(deployment_id, community_value)
198+
names = self.__generate_names(deployment_id, community_value, neighbor_type)
181199
self.cfg_mgr.update()
182200
cmds = []
183201
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):
189207
cmds += self.__update_default_route_map_entry(names['rm_v6'], default_action)
190208
if cmds:
191209
self.cfg_mgr.push_list(cmds)
192-
peer_groups = self.__find_peer_group_by_deployment_id(deployment_id)
210+
peer_groups = self.__find_peer_group(deployment_id, neighbor_type)
193211
self.cfg_mgr.restart_peer_groups(peer_groups)
194212
log_debug("BGPAllowListMgr::__remove_policy. 'Allow list' policy was scheduled for removal")
195213
else:
196214
log_debug("BGPAllowListMgr::__remove_policy. Nothing to remove")
197215
log_info('BGPAllowListMgr::Done')
198216

199217
@staticmethod
200-
def __generate_names(deployment_id, community_value):
218+
def __generate_names(deployment_id, community_value, neighbor_type):
201219
"""
202220
Generate prefix-list names for a given peer_ip and community value
203221
:param deployment_id: deployment_id for which we're going to filter prefixes
204222
:param community_value: community, which we want to use to filter prefixes
205223
:return: a dictionary with names
206224
"""
207-
if community_value == BGPAllowListMgr.EMPTY_COMMUNITY:
208-
community_name = BGPAllowListMgr.EMPTY_COMMUNITY
225+
if not neighbor_type:
226+
if community_value == BGPAllowListMgr.EMPTY_COMMUNITY:
227+
community_name = BGPAllowListMgr.EMPTY_COMMUNITY
228+
else:
229+
community_name = BGPAllowListMgr.COMMUNITY_NAME_TMPL % (deployment_id, community_value)
230+
names = {
231+
"pl_v4": BGPAllowListMgr.PL_NAME_TMPL % (deployment_id, community_value, '4'),
232+
"pl_v6": BGPAllowListMgr.PL_NAME_TMPL % (deployment_id, community_value, '6'),
233+
"rm_v4": BGPAllowListMgr.RM_NAME_TMPL % (deployment_id, '4'),
234+
"rm_v6": BGPAllowListMgr.RM_NAME_TMPL % (deployment_id, '6'),
235+
"community": community_name,
236+
'neigh_type': neighbor_type,
237+
}
238+
arguments = deployment_id, community_value, str(names)
239+
log_debug("BGPAllowListMgr::__generate_names. deployment_id: %d, community: %s. names: %s" % arguments)
209240
else:
210-
community_name = BGPAllowListMgr.COMMUNITY_NAME_TMPL % (deployment_id, community_value)
211-
names = {
212-
"pl_v4": BGPAllowListMgr.PL_NAME_TMPL % (deployment_id, community_value, '4'),
213-
"pl_v6": BGPAllowListMgr.PL_NAME_TMPL % (deployment_id, community_value, '6'),
214-
"rm_v4": BGPAllowListMgr.RM_NAME_TMPL % (deployment_id, '4'),
215-
"rm_v6": BGPAllowListMgr.RM_NAME_TMPL % (deployment_id, '6'),
216-
"community": community_name,
217-
}
218-
arguments = deployment_id, community_value, str(names)
219-
log_debug("BGPAllowListMgr::__generate_names. deployment_id: %d, community: %s. names: %s" % arguments)
241+
if community_value == BGPAllowListMgr.EMPTY_COMMUNITY:
242+
community_name = BGPAllowListMgr.EMPTY_COMMUNITY
243+
else:
244+
community_name = BGPAllowListMgr.COMMUNITY_NAME_TMPL_WITH_NEIGH % (deployment_id, neighbor_type, community_value)
245+
names = {
246+
"pl_v4": BGPAllowListMgr.PL_NAME_TMPL_WITH_NEIGH % (deployment_id, neighbor_type, community_value, '4'),
247+
"pl_v6": BGPAllowListMgr.PL_NAME_TMPL_WITH_NEIGH % (deployment_id, neighbor_type, community_value, '6'),
248+
"rm_v4": BGPAllowListMgr.RM_NAME_TMPL_WITH_NEIGH % (deployment_id, neighbor_type, '4'),
249+
"rm_v6": BGPAllowListMgr.RM_NAME_TMPL_WITH_NEIGH % (deployment_id, neighbor_type, '6'),
250+
"community": community_name,
251+
}
252+
arguments = deployment_id, neighbor_type, community_value, str(names)
253+
log_debug("BGPAllowListMgr::__generate_names. deployment_id: %d, neighbor_type: %s, community: %s. names: %s" % arguments)
220254
return names
221255

222256
def __update_prefix_list(self, af, pl_name, allow_list):
@@ -630,7 +664,7 @@ def __get_routemap_tag(self):
630664
return prefix_match_tag
631665

632666
@staticmethod
633-
def __get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call):
667+
def __get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call, neighbor_type):
634668
"""
635669
Get peer_groups which are assigned to deployment_id
636670
:deployment_id: deployment_id number
@@ -639,14 +673,17 @@ def __get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call):
639673
:rm_2_call: a dictionary: key - name of a route-map, value - name of a route-map call defined for the route-map
640674
"""
641675
ret = set()
642-
target_allow_list_prefix = 'ALLOW_LIST_DEPLOYMENT_ID_%d_V' % deployment_id
676+
if not neighbor_type:
677+
target_allow_list_prefix = 'ALLOW_LIST_DEPLOYMENT_ID_%d_V' % deployment_id
678+
else:
679+
target_allow_list_prefix = 'ALLOW_LIST_DEPLOYMENT_ID_%d_NEIGHBOR_%s_V' % (deployment_id, neighbor_type)
643680
for peer_group, route_map in pg_2_rm.items():
644681
if route_map in rm_2_call:
645682
if rm_2_call[route_map].startswith(target_allow_list_prefix):
646683
ret.add(peer_group)
647684
return list(ret)
648685

649-
def __find_peer_group_by_deployment_id(self, deployment_id):
686+
def __find_peer_group(self, deployment_id, neighbor_type):
650687
"""
651688
Deduce peer-group names which are connected to devices with requested deployment_id
652689
:param deployment_id: deployment_id number
@@ -656,7 +693,7 @@ def __find_peer_group_by_deployment_id(self, deployment_id):
656693
peer_groups = self.__extract_peer_group_names()
657694
pg_2_rm = self.__get_peer_group_to_route_map(peer_groups)
658695
rm_2_call = self.__get_route_map_calls(set(pg_2_rm.values()))
659-
ret = self.__get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call)
696+
ret = self.__get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call, neighbor_type)
660697
return list(ret)
661698

662699
def __get_enabled(self):
@@ -706,11 +743,14 @@ def __to_prefix_list(self, af, allow_list):
706743
res = []
707744
prefix_mask_default = 32 if af == self.V4 else 128
708745
for prefix in allow_list:
709-
prefix_mask = int(prefix.split("/")[1])
710-
if prefix_mask == prefix_mask_default:
746+
if 'le' in prefix or 'ge' in prefix:
711747
res.append("permit %s" % prefix)
712748
else:
713-
res.append("permit %s le %d" % (prefix, prefix_mask_default))
749+
prefix_mask = int(prefix.split("/")[1])
750+
if prefix_mask == prefix_mask_default:
751+
res.append("permit %s" % prefix)
752+
else:
753+
res.append("permit %s le %d" % (prefix, prefix_mask_default))
714754
return res
715755

716756
def __af_to_family(self, af):

0 commit comments

Comments
 (0)