Skip to content

Commit df21f2f

Browse files
mssonicbldmssonicbld
authored
[Bgpcfgd]Add correct terminating commands at the end of BGP config command groups (sonic-net#774)
<!-- Please make sure you've read and understood our contributing guidelines: https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md failure_prs.log skip_prs.log Make sure all your commits include a signature generated with `git commit -s` ** If this is a bug fix, make sure your description includes "fixes #xxxx", or "closes #xxxx" or "resolves #xxxx" Please provide the following information: --> #### Why I did it Previously, Bgpcfgd generated groups of BGP configuration commands that did not have proper terminating commands. As a consequence, non-BGP configuration commands following the BGP configuration command may be rejected by the FRR because of FRR's context was not properly switched. You check the details in Issue sonic-net#21829. Fix sonic-net#21829 ##### Work item tracking - Microsoft ADO **(number only)**: #### How I did it Add exit and exit-address-family command at the end of BGP configuration command groups. #### How to verify it <!-- If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012. --> #### Which release branch to backport (provide reason below if selected) <!-- - Note we only backport fixes to a release branch, *not* features! - Please also provide a reason for the backporting below. - e.g. - [x] 202006 --> - [ ] 201811 - [ ] 201911 - [ ] 202006 - [ ] 202012 - [ ] 202106 - [ ] 202111 - [ ] 202205 - [ ] 202211 - [ ] 202305 #### Tested branch (Please provide the tested image version) <!-- - Please provide tested image version - e.g. - [x] 20201231.100 --> - [ ] <!-- image version 1 --> - [ ] <!-- image version 2 --> #### Description for the changelog <!-- Write a short (one line) summary that describes the changes in this pull request for inclusion in the changelog: --> <!-- Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU. --> #### Link to config_db schema for YANG module changes <!-- Provide a link to config_db schema for the table for which YANG model is defined Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md --> #### A picture of a cute animal (not mandatory but encouraged)
1 parent a54ac27 commit df21f2f

File tree

7 files changed

+214
-55
lines changed

7 files changed

+214
-55
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ def advertise_route_commands(self, ip_prefix, vrf, op, data=None):
100100
"BGPAdvertiseRouteMgr:: %sbgp %s network %s"
101101
% ("Remove " if op == self.OP_DELETE else "Update ", bgp_asn, vrf + "|" + ip_prefix)
102102
)
103+
cmd_list.append(" exit-address-family")
104+
cmd_list.append("exit")
103105

104106
self.cfg_mgr.push_list(cmd_list)
105107
log_debug("BGPAdvertiseRouteMgr::Done")
@@ -112,6 +114,7 @@ def bgp_network_import_check_commands(self, vrf, op):
112114
else:
113115
cmd_list.append("router bgp %s vrf %s" % (bgp_asn, vrf))
114116
cmd_list.append(" %sbgp network import-check" % ("" if op == self.OP_DELETE else "no "))
117+
cmd_list.append("exit")
115118

116119
self.cfg_mgr.push_list(cmd_list)
117120

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ def __set_prepare_config(self, status):
158158
if peer_group_name.startswith(pg_name) and af in self.bbr_enabled_pgs[pg_name]:
159159
cmds.append(" %sneighbor %s allowas-in 1" % (prefix_of_commands, peer_group_name))
160160
peer_groups_to_restart.add(peer_group_name)
161+
cmds.append(" exit-address-family")
162+
cmds.append("exit")
161163
return cmds, list(peer_groups_to_restart)
162164

163165
def __get_available_peer_groups(self):

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ def update_pg(self, name, **kwargs):
6565
return False
6666

6767
if kwargs['vrf'] == 'default':
68-
cmd = ('router bgp %s\n' % kwargs['bgp_asn']) + pg + tsa_rm + idf_isolation_rm
68+
cmd = ('router bgp %s\n' % kwargs['bgp_asn']) + pg + tsa_rm + idf_isolation_rm + "\nexit"
6969
else:
70-
cmd = ('router bgp %s vrf %s\n' % (kwargs['bgp_asn'], kwargs['vrf'])) + pg + tsa_rm + idf_isolation_rm
70+
cmd = ('router bgp %s vrf %s\n' % (kwargs['bgp_asn'], kwargs['vrf'])) + pg + tsa_rm + idf_isolation_rm + "\nexit"
7171
self.update_entity(cmd, "Peer-group for peer '%s'" % name)
7272
return True
7373

@@ -314,9 +314,9 @@ def apply_op(self, cmd, vrf):
314314
bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"]
315315
enable_bgp_suppress_fib_pending_cmd = 'bgp suppress-fib-pending'
316316
if vrf == 'default':
317-
cmd = ('router bgp %s\n %s\n' % (bgp_asn, enable_bgp_suppress_fib_pending_cmd)) + cmd
317+
cmd = ('router bgp %s\n %s\n' % (bgp_asn, enable_bgp_suppress_fib_pending_cmd)) + cmd + "\nexit"
318318
else:
319-
cmd = ('router bgp %s vrf %s\n %s\n' % (bgp_asn, vrf, enable_bgp_suppress_fib_pending_cmd)) + cmd
319+
cmd = ('router bgp %s vrf %s\n %s\n' % (bgp_asn, vrf, enable_bgp_suppress_fib_pending_cmd)) + cmd + "\nexit"
320320
self.cfg_mgr.push(cmd)
321321
return True
322322

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def set_handler(self, key, data):
4343
dist_list = arg_list(data['distance']) if 'distance' in data else None
4444
nh_vrf_list = arg_list(data['nexthop-vrf']) if 'nexthop-vrf' in data else None
4545
bfd_enable = arg_list(data['bfd']) if 'bfd' in data else None
46-
route_tag = self.ROUTE_ADVERTISE_DISABLE_TAG if 'advertise' in data and data['advertise'] == "false" else self.ROUTE_ADVERTISE_ENABLE_TAG
46+
route_tag = self.ROUTE_ADVERTISE_DISABLE_TAG if 'advertise' in data and data['advertise'] == "false" else self.ROUTE_ADVERTISE_ENABLE_TAG
4747

4848
# bfd enabled route would be handled in staticroutebfd, skip here
4949
if bfd_enable and bfd_enable[0].lower() == "true":
@@ -231,6 +231,8 @@ def enable_redistribution_command(self, vrf):
231231
for af in ["ipv4", "ipv6"]:
232232
cmd_list.append(" address-family %s" % af)
233233
cmd_list.append(" redistribute static route-map STATIC_ROUTE_FILTER")
234+
cmd_list.append(" exit-address-family")
235+
cmd_list.append("exit")
234236
return cmd_list
235237

236238
def disable_redistribution_command(self, vrf):
@@ -244,6 +246,8 @@ def disable_redistribution_command(self, vrf):
244246
for af in ["ipv4", "ipv6"]:
245247
cmd_list.append(" address-family %s" % af)
246248
cmd_list.append(" no redistribute static route-map STATIC_ROUTE_FILTER")
249+
cmd_list.append(" exit-address-family")
250+
cmd_list.append("exit")
247251
cmd_list.append("no route-map STATIC_ROUTE_FILTER")
248252
return cmd_list
249253

‎src/sonic-bgpcfgd/tests/test_advertise_rt.py

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,13 @@ def test_set_del():
5252
True,
5353
[
5454
["router bgp 65100",
55-
" no bgp network import-check"],
55+
" no bgp network import-check",
56+
"exit"],
5657
["router bgp 65100",
5758
" address-family ipv4 unicast",
58-
" network 10.1.0.0/24"]
59+
" network 10.1.0.0/24",
60+
" exit-address-family",
61+
"exit"]
5962
]
6063
)
6164

@@ -67,7 +70,9 @@ def test_set_del():
6770
[
6871
["router bgp 65100",
6972
" address-family ipv6 unicast",
70-
" network fc00:10::/64"]
73+
" network fc00:10::/64",
74+
" exit-address-family",
75+
"exit"]
7176
]
7277
)
7378

@@ -79,7 +84,9 @@ def test_set_del():
7984
[
8085
["router bgp 65100",
8186
" address-family ipv4 unicast",
82-
" no network 10.1.0.0/24"]
87+
" no network 10.1.0.0/24",
88+
" exit-address-family",
89+
"exit"]
8390
]
8491
)
8592

@@ -90,10 +97,13 @@ def test_set_del():
9097
True,
9198
[
9299
["router bgp 65100",
93-
" bgp network import-check"],
100+
" bgp network import-check",
101+
"exit"],
94102
["router bgp 65100",
95103
" address-family ipv6 unicast",
96-
" no network fc00:10::/64"]
104+
" no network fc00:10::/64",
105+
" exit-address-family",
106+
"exit"]
97107
]
98108
)
99109

@@ -107,10 +117,13 @@ def test_set_del_vrf():
107117
True,
108118
[
109119
["router bgp 65100 vrf vrfRED",
110-
" no bgp network import-check"],
120+
" no bgp network import-check",
121+
"exit"],
111122
["router bgp 65100 vrf vrfRED",
112123
" address-family ipv4 unicast",
113-
" network 10.2.0.0/24"]
124+
" network 10.2.0.0/24",
125+
" exit-address-family",
126+
"exit"]
114127
]
115128
)
116129

@@ -122,7 +135,9 @@ def test_set_del_vrf():
122135
[
123136
["router bgp 65100 vrf vrfRED",
124137
" address-family ipv6 unicast",
125-
" network fc00:20::/64"]
138+
" network fc00:20::/64",
139+
" exit-address-family",
140+
"exit"]
126141
]
127142
)
128143

@@ -134,7 +149,9 @@ def test_set_del_vrf():
134149
[
135150
["router bgp 65100 vrf vrfRED",
136151
" address-family ipv4 unicast",
137-
" no network 10.2.0.0/24"]
152+
" no network 10.2.0.0/24",
153+
" exit-address-family",
154+
"exit"]
138155
]
139156
)
140157

@@ -145,10 +162,13 @@ def test_set_del_vrf():
145162
True,
146163
[
147164
["router bgp 65100 vrf vrfRED",
148-
" bgp network import-check"],
165+
" bgp network import-check",
166+
"exit"],
149167
["router bgp 65100 vrf vrfRED",
150168
" address-family ipv6 unicast",
151-
" no network fc00:20::/64"]
169+
" no network fc00:20::/64",
170+
" exit-address-family",
171+
"exit"]
152172
]
153173
)
154174

@@ -169,10 +189,13 @@ def test_set_del_bgp_asn_change():
169189
test_set_del_bgp_asn_change.push_list_called = False
170190
expected_cmds = [
171191
["router bgp 65100 vrf vrfRED",
172-
" no bgp network import-check"],
192+
" no bgp network import-check",
193+
"exit"],
173194
["router bgp 65100 vrf vrfRED",
174195
" address-family ipv4 unicast",
175-
" network 10.3.0.0/24 route-map FROM_SDN_SLB_ROUTES_RM"]
196+
" network 10.3.0.0/24 route-map FROM_SDN_SLB_ROUTES_RM",
197+
" exit-address-family",
198+
"exit"]
176199
]
177200
def push_list(cmds):
178201
test_set_del_bgp_asn_change.push_list_called = True
@@ -197,10 +220,13 @@ def test_set_del_with_community():
197220
True,
198221
[
199222
["router bgp 65100",
200-
" no bgp network import-check"],
223+
" no bgp network import-check",
224+
"exit"],
201225
["router bgp 65100",
202226
" address-family ipv4 unicast",
203-
" network 10.1.0.0/24 route-map FROM_SDN_SLB_ROUTES_RM"]
227+
" network 10.1.0.0/24 route-map FROM_SDN_SLB_ROUTES_RM",
228+
" exit-address-family",
229+
"exit"]
204230
]
205231
)
206232

@@ -214,7 +240,9 @@ def test_set_del_with_community():
214240
[
215241
["router bgp 65100",
216242
" address-family ipv6 unicast",
217-
" network fc00:10::/64 route-map FROM_SDN_SLB_ROUTES_RM"]
243+
" network fc00:10::/64 route-map FROM_SDN_SLB_ROUTES_RM",
244+
" exit-address-family",
245+
"exit"]
218246
]
219247
)
220248

@@ -226,7 +254,9 @@ def test_set_del_with_community():
226254
[
227255
["router bgp 65100",
228256
" address-family ipv4 unicast",
229-
" no network 10.1.0.0/24"]
257+
" no network 10.1.0.0/24",
258+
" exit-address-family",
259+
"exit"]
230260
]
231261
)
232262

@@ -237,9 +267,12 @@ def test_set_del_with_community():
237267
True,
238268
[
239269
["router bgp 65100",
240-
" bgp network import-check"],
270+
" bgp network import-check",
271+
"exit"],
241272
["router bgp 65100",
242273
" address-family ipv6 unicast",
243-
" no network fc00:10::/64"]
274+
" no network fc00:10::/64",
275+
" exit-address-family",
276+
"exit"]
244277
]
245278
)

‎src/sonic-bgpcfgd/tests/test_bbr.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,12 @@ def test___set_prepare_config_enabled():
309309
'router bgp 65500',
310310
' address-family ipv4',
311311
' neighbor PEER_V4 allowas-in 1',
312+
" exit-address-family",
312313
' address-family ipv6',
313314
' neighbor PEER_V4 allowas-in 1',
314315
' neighbor PEER_V6 allowas-in 1',
316+
" exit-address-family",
317+
"exit"
315318
])
316319

317320
def test___set_prepare_config_disabled():
@@ -322,9 +325,12 @@ def test___set_prepare_config_disabled():
322325
'router bgp 65500',
323326
' address-family ipv4',
324327
' no neighbor PEER_V4 allowas-in 1',
328+
" exit-address-family",
325329
' address-family ipv6',
326330
' no neighbor PEER_V4 allowas-in 1',
327331
' no neighbor PEER_V6 allowas-in 1',
332+
" exit-address-family",
333+
"exit"
328334
])
329335

330336
def test___set_prepare_config_enabled_part():
@@ -336,9 +342,12 @@ def test___set_prepare_config_enabled_part():
336342
'router bgp 65500',
337343
' address-family ipv4',
338344
' neighbor PEER_V4 allowas-in 1',
345+
" exit-address-family",
339346
' address-family ipv6',
340347
' neighbor PEER_V4 allowas-in 1',
341348
' neighbor PEER_V6 allowas-in 1',
349+
" exit-address-family",
350+
"exit"
342351
])
343352

344353
def test___set_prepare_config_disabled_part():
@@ -350,9 +359,12 @@ def test___set_prepare_config_disabled_part():
350359
'router bgp 65500',
351360
' address-family ipv4',
352361
' no neighbor PEER_V4 allowas-in 1',
362+
" exit-address-family",
353363
' address-family ipv6',
354364
' no neighbor PEER_V4 allowas-in 1',
355365
' no neighbor PEER_V6 allowas-in 1',
366+
" exit-address-family",
367+
"exit"
356368
])
357369
def test___set_prepare_config_enabled_multiple_peers():
358370
__set_prepare_config_common("enabled", {
@@ -365,10 +377,13 @@ def test___set_prepare_config_enabled_multiple_peers():
365377
' neighbor PEER_V4 allowas-in 1',
366378
' neighbor PEER_V4_DEPLOYMENT_ID_0 allowas-in 1',
367379
' neighbor PEER_V4_DEPLOYMENT_ID_1 allowas-in 1',
380+
" exit-address-family",
368381
' address-family ipv6',
369382
' neighbor PEER_V6 allowas-in 1',
370383
' neighbor PEER_V6_DEPLOYMENT_ID_0 allowas-in 1',
371384
' neighbor PEER_V6_DEPLOYMENT_ID_1 allowas-in 1',
385+
" exit-address-family",
386+
"exit"
372387
],
373388
{"PEER_V4", "PEER_V4_DEPLOYMENT_ID_0", "PEER_V4_DEPLOYMENT_ID_1", "PEER_V6", "PEER_V6_DEPLOYMENT_ID_0", "PEER_V6_DEPLOYMENT_ID_1"})
374389

@@ -383,10 +398,13 @@ def test___set_prepare_config_disabled_multiple_peers():
383398
' no neighbor PEER_V4 allowas-in 1',
384399
' no neighbor PEER_V4_DEPLOYMENT_ID_0 allowas-in 1',
385400
' no neighbor PEER_V4_DEPLOYMENT_ID_1 allowas-in 1',
401+
" exit-address-family",
386402
' address-family ipv6',
387403
' no neighbor PEER_V6 allowas-in 1',
388404
' no neighbor PEER_V6_DEPLOYMENT_ID_0 allowas-in 1',
389405
' no neighbor PEER_V6_DEPLOYMENT_ID_1 allowas-in 1',
406+
" exit-address-family",
407+
"exit"
390408
],
391409
{"PEER_V4", "PEER_V4_DEPLOYMENT_ID_0", "PEER_V4_DEPLOYMENT_ID_1", "PEER_V6", "PEER_V6_DEPLOYMENT_ID_0", "PEER_V6_DEPLOYMENT_ID_1"})
392410

0 commit comments

Comments
 (0)