Skip to content

Commit 669c5f6

Browse files
BYGX-wcrUbuntu
BYGX-wcr
authored and
Ubuntu
committed
[Bgpcfgd]Add correct terminating commands at the end of BGP config command groups (sonic-net#21889)
Fix sonic-net#21829 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.
1 parent 50bf976 commit 669c5f6

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)