Skip to content

Commit 8f966ee

Browse files
dgsudharsandgsudharsan
authored
[FRR]bgpd: Fix JSON wrapper brace consistency in neighbor commands (#24115)
Manual cherry-pick of #24111 Why I did it Ported fix FRRouting/frr#19552 from FRR community. Without this fix, this command may fail in some scenarios. Work item tracking Microsoft ADO (number only): How I did it Added patch. How to verify it Run basic sanity tests signed-off-by: [email protected]
1 parent bfbd8a3 commit 8f966ee

File tree

2 files changed

+172
-0
lines changed

2 files changed

+172
-0
lines changed

‎src/sonic-frr/patch/0061-bgpd-Fix-JSON-wrapper-brace-consistency-in-neighbor.patch‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
From 63e37a76cb4f1282b0d8ee4dd655221bec01a1de Mon Sep 17 00:00:00 2001
2+
From: Vaideesh Ravi Shankar <[email protected]>
3+
Date: Mon, 15 Sep 2025 14:42:00 -0700
4+
Subject: [PATCH] bgpd: Fix JSON wrapper brace consistency in neighbor commands
5+
6+
The BGP neighbor adj-route commands (advertised-routes, received-routes,
7+
filtered-routes) had inconsistent JSON wrapper brace handling. Only
8+
advertised-routes and received-routes got wrapper braces from the command
9+
handler, while filtered-routes used complete JSON objects.
10+
11+
This caused malformed JSON output like:
12+
{ { warning: message } }
13+
14+
Fix by conditionally handling JSON output based on route type:
15+
- advertised/received routes: output raw JSON fragments for CLI brace wrapping
16+
- filtered routes: output complete JSON objects with vty_json
17+
18+
This ensures proper JSON formatting for all route types while maintaining
19+
backward compatibility.
20+
21+
Signed-off-by: Vaideesh Ravi Shankar <[email protected]>
22+
23+
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
24+
index 4742ccabc6..4ba2c68c0c 100644
25+
--- a/bgpd/bgp_route.c
26+
+++ b/bgpd/bgp_route.c
27+
@@ -6730,7 +6730,7 @@ static int clear_batch_rib_helper(struct bgp_clearing_info *cinfo)
28+
/* This will resume the "inner" walk if necessary */
29+
ret = walk_batch_table_helper(cinfo, table, true /*inner*/);
30+
if (ret != 0) {
31+
- /* The "inner" resume info will be set;
32+
+ /* The "inner" resume info will be set;
33+
* capture the resume info we need
34+
* from the outer afi/safi and dest
35+
*/
36+
@@ -15534,11 +15534,18 @@ static int peer_adj_routes(struct vty *vty, struct peer *peer, afi_t afi,
37+
38+
if (!peer || !peer->afc[afi][safi]) {
39+
if (use_json) {
40+
- json_object_string_add(
41+
- json, "warning",
42+
- "No such neighbor or address family");
43+
- vty_out(vty, "%s\n", json_object_to_json_string(json));
44+
- json_object_free(json);
45+
+ if (type == bgp_show_adj_route_advertised ||
46+
+ type == bgp_show_adj_route_received) {
47+
+ /* Raw fragment for CLI brace wrapping */
48+
+ vty_out(vty,
49+
+ "\"warning\": \"No such neighbor or address family\"\n");
50+
+ json_object_free(json);
51+
+ } else {
52+
+ /* Complete object for filtered/bestpath */
53+
+ json_object_string_add(json, "warning",
54+
+ "No such neighbor or address family");
55+
+ vty_json(vty, json);
56+
+ }
57+
json_object_free(json_ar);
58+
} else
59+
vty_out(vty, "%% No such neighbor or address family\n");
60+
@@ -15551,11 +15558,17 @@ static int peer_adj_routes(struct vty *vty, struct peer *peer, afi_t afi,
61+
&& !CHECK_FLAG(peer->af_flags[afi][safi],
62+
PEER_FLAG_SOFT_RECONFIG)) {
63+
if (use_json) {
64+
- json_object_string_add(
65+
- json, "warning",
66+
- "Inbound soft reconfiguration not enabled");
67+
- vty_out(vty, "%s\n", json_object_to_json_string(json));
68+
- json_object_free(json);
69+
+ if (type == bgp_show_adj_route_received) {
70+
+ /* Raw fragment for CLI brace wrapping */
71+
+ vty_out(vty,
72+
+ "\"warning\": \"Inbound soft reconfiguration not enabled\"\n");
73+
+ json_object_free(json);
74+
+ } else {
75+
+ /* Complete object for filtered routes */
76+
+ json_object_string_add(json, "warning",
77+
+ "Inbound soft reconfiguration not enabled");
78+
+ vty_json(vty, json);
79+
+ }
80+
json_object_free(json_ar);
81+
} else
82+
vty_out(vty,
83+
diff --git a/tests/topotests/bgp_received_routes_with_soft_inbound/test_bgp_received_routes_with_soft_inbound.py b/tests/topotests/bgp_received_routes_with_soft_inbound/test_bgp_received_routes_with_soft_inbound.py
84+
index 0b933add2f..07a14b2767 100644
85+
--- a/tests/topotests/bgp_received_routes_with_soft_inbound/test_bgp_received_routes_with_soft_inbound.py
86+
+++ b/tests/topotests/bgp_received_routes_with_soft_inbound/test_bgp_received_routes_with_soft_inbound.py
87+
@@ -98,6 +98,81 @@ def test_bgp_received_routes_with_soft_inbound():
88+
assert result is None, "Can't converge"
89+
90+
91+
+def test_bgp_adj_routes_json_error_paths():
92+
+ """Test JSON formatting consistency for BGP adj-route error paths"""
93+
+ tgen = get_topogen()
94+
+
95+
+ if tgen.routers_have_failure():
96+
+ pytest.skip(tgen.errors)
97+
+
98+
+ r1 = tgen.gears["r1"]
99+
+
100+
+ # Disable soft-reconfiguration to trigger warning paths
101+
+ r1.vtysh_cmd(
102+
+ """
103+
+ configure terminal
104+
+ router bgp 65001
105+
+ address-family ipv4 unicast
106+
+ no neighbor 192.168.1.2 soft-reconfiguration inbound
107+
+ end
108+
+ """
109+
+ )
110+
+
111+
+ def _check_adj_route_json_consistency():
112+
+ """Check that all adj-route JSON outputs are well-formed"""
113+
+ test_commands = [
114+
+ "show bgp ipv4 unicast neighbors 192.168.1.2 received-routes json",
115+
+ "show bgp ipv4 unicast neighbors 192.168.1.2 advertised-routes json",
116+
+ "show bgp ipv4 unicast neighbors 192.168.1.2 filtered-routes json",
117+
+ ]
118+
+
119+
+ for cmd in test_commands:
120+
+ output = r1.vtysh_cmd(cmd)
121+
+
122+
+ # Critical: JSON should ALWAYS be valid after our fix
123+
+ try:
124+
+ parsed = json.loads(output)
125+
+ except json.JSONDecodeError as e:
126+
+ pytest.fail(f"Malformed JSON in command '{cmd}': {e}\nOutput: {output}")
127+
+
128+
+ # Test CONTENT for expected error messages
129+
+ if "received-routes" in cmd or "filtered-routes" in cmd:
130+
+ warning_msg = parsed.get("warning", "")
131+
+ expected = "Inbound soft reconfiguration not enabled"
132+
+ if warning_msg != expected:
133+
+ return (
134+
+ f"Expected soft reconfig warning in {cmd}, "
135+
+ f"got: '{warning_msg}'"
136+
+ )
137+
+
138+
+ # Verify JSON structure is a single object, not double-nested
139+
+ if not isinstance(parsed, dict):
140+
+ return f"Expected dict object for {cmd}"
141+
+
142+
+ # Critical: Should not have nested structure { { "warning": ... } }
143+
+ warning_value = parsed.get("warning", "")
144+
+ if isinstance(warning_value, dict):
145+
+ # This indicates our fix failed - fail test immediately
146+
+ pytest.fail(f"Warning should be string, not nested object in {cmd}")
147+
+
148+
+ return None
149+
+
150+
+ test_func = functools.partial(_check_adj_route_json_consistency)
151+
+ _, result = topotest.run_and_expect(test_func, None, count=5, wait=3)
152+
+ assert result is None, f"JSON consistency check failed: {result}"
153+
+
154+
+ # Restore original configuration
155+
+ r1.vtysh_cmd(
156+
+ """
157+
+ configure terminal
158+
+ router bgp 65001
159+
+ address-family ipv4 unicast
160+
+ neighbor 192.168.1.2 soft-reconfiguration inbound
161+
+ end
162+
+ """
163+
+ )
164+
+
165+
+
166+
if __name__ == "__main__":
167+
args = ["-s"] + sys.argv[1:]
168+
sys.exit(pytest.main(args))
169+
--
170+
2.50.1
171+

‎src/sonic-frr/patch/series‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,4 @@
5858
0058-mgmtd-normalize-argument-order-to-copy-dst-src.patch
5959
0059-zebra-Ensure-that-the-dplane-can-send-the-full-packe.patch
6060
0060-zebra-if-speed-change-check-fix.patch
61+
0061-bgpd-Fix-JSON-wrapper-brace-consistency-in-neighbor.patch

0 commit comments

Comments
 (0)