From c620272fd04482ec1f722208393fc31fcba9700b Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 6 Jan 2025 16:07:17 +0100 Subject: [PATCH 1/2] bgpd: fix display the attr from adj-rib-out with detail used The 'show bgp neighbors advertised-routes detail' command displays all the bgp paths of the loc-rib. The expectation is that it displays only the bgp path for the peer this path is sent to. Fix this by replacing the extra attr attribute with the bgp_adj_out pointer of the path to display. Filter out the appropriate bgp path and display the adj->attr. Fixes: e960b4ca0662 ("bgpd: add "detail" for advertised/received-routes") Signed-off-by: Philippe Guibert --- bgpd/bgp_route.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 0f899d961744..1abf0b455cfd 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -12730,17 +12730,23 @@ void route_vty_out_detail_header(struct vty *vty, struct bgp *bgp, static void bgp_show_path_info(const struct prefix_rd *pfx_rd, struct bgp_dest *bgp_node, struct vty *vty, struct bgp *bgp, afi_t afi, safi_t safi, json_object *json, enum bgp_path_type pathtype, int *display, - enum rpki_states rpki_target_state, struct attr *attr) + enum rpki_states rpki_target_state, struct bgp_adj_out *adj_out) { - struct bgp_path_info *pi; + struct bgp_path_info *pi, *pi_adj_out = NULL; int header = 1; json_object *json_header = NULL; json_object *json_paths = NULL; const struct prefix *p = bgp_dest_get_prefix(bgp_node); + if (adj_out && adj_out->adv) + pi_adj_out = adj_out->adv->pathi; + for (pi = bgp_dest_get_bgp_path_info(bgp_node); pi; pi = pi->next) { enum rpki_states rpki_curr_state = RPKI_NOT_BEING_USED; + if (pi_adj_out && pi != pi_adj_out) + continue; + if (p->family == AF_INET || p->family == AF_INET6) rpki_curr_state = hook_call(bgp_rpki_prefix_status, pi->peer, pi->attr, p); @@ -12774,7 +12780,8 @@ static void bgp_show_path_info(const struct prefix_rd *pfx_rd, struct bgp_dest * && (CHECK_FLAG(pi->flags, BGP_PATH_MULTIPATH) || CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)))) route_vty_out_detail(vty, bgp, bgp_node, bgp_dest_get_prefix(bgp_node), pi, - afi, safi, rpki_curr_state, json_paths, attr); + afi, safi, rpki_curr_state, json_paths, + adj_out ? adj_out->attr : NULL); } if (json && json_paths) { @@ -15032,7 +15039,7 @@ show_adj_route(struct vty *vty, struct peer *peer, struct bgp_table *table, bgp_show_path_info(NULL, dest, vty, bgp, afi, safi, json_net, BGP_PATH_SHOW_ALL, &display, RPKI_NOT_BEING_USED, - adj->attr); + adj); if (use_json) json_object_object_addf(json_ar, json_net, "%pFX", rn_p); From b7fd45c5ca2e2e7b61a7ffd9b35c64e194cad267 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 6 Jan 2025 17:03:26 +0100 Subject: [PATCH 2/2] topotests: bgp_vpnv4_noretain, check presence of locpref in adj-rib-out Add a test that check that the detailed command of show bgp advertised neighbors 10.125.0.2 displays the locpref value. Signed-off-by: Philippe Guibert --- ...ipv4_vpn_routes_advertised_10_125_0_2.json | 105 ++++++++++++++++++ .../test_bgp_vpnv4_noretain.py | 38 +++++++ 2 files changed, 143 insertions(+) create mode 100644 tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_advertised_10_125_0_2.json diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_advertised_10_125_0_2.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_advertised_10_125_0_2.json new file mode 100644 index 000000000000..789198265341 --- /dev/null +++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_advertised_10_125_0_2.json @@ -0,0 +1,105 @@ +{ + "bgpLocalRouterId":"192.0.2.1", + "defaultLocPrf":100, + "localAS":65500, + "advertisedRoutes":{ + "192.0.2.1:1":{ + "rd":"192.0.2.1:1", + "10.101.0.0/24":{ + "prefix":"10.101.0.0/24", + "advertisedTo":{ + "10.125.0.2":{ + "hostname":"r2" + } + }, + "paths":[{ + "aspath":{ + "string":"Local", + "segments":[], + "length":0 + }, + "nhVrfName":"vrf1", + "announceNexthopSelf":true, + "origin":"incomplete", + "metric":0, + "locPrf":100, + "weight":32768, + "valid":true, + "sourced":true, + "local":true, + "bestpath":{ + "overall":true, + "selectionReason":"First path received" + }, + "extendedCommunity":{ + "string":"RT:192.0.2.1:100" + }, + "originatorId":"192.0.2.1", + "remoteLabel":101, + "nexthops":[{ + "ip":"0.0.0.0", + "hostname":"r1", + "afi":"ipv4", + "metric":0, + "accessible":true, + "used":true + }], + "peer":{ + "peerId":"0.0.0.0", + "routerId":"192.0.2.1" + } + }] + } + }, + "192.0.2.1:3":{ + "rd":"192.0.2.1:3", + "10.103.0.0/24":{ + "prefix":"10.103.0.0/24", + "advertisedTo":{ + "10.125.0.2":{ + "hostname":"r2" + } + }, + "paths":[{ + "aspath":{ + "string":"Local", + "segments":[], + "length":0 + }, + "nhVrfName":"vrf3", + "announceNexthopSelf":true, + "origin":"incomplete", + "metric":0, + "locPrf":100, + "weight":32768, + "valid":true, + "sourced":true, + "local":true, + "bestpath":{ + "overall":true, + "selectionReason":"First path received" + }, + "extendedCommunity":{ + "string":"RT:192.0.2.1:300" + }, + "originatorId":"192.0.2.1", + "remoteLabel":103, + "nexthops":[{ + "ip":"0.0.0.0", + "hostname":"r1", + "afi":"ipv4", + "metric":0, + "accessible":true, + "used":true + }], + "peer":{ + "peerId":"0.0.0.0", + "routerId":"192.0.2.1" + } + }] + } + } + }, + "totalPrefixCounter":2, + "filteredPrefixCounter":0 +} diff --git a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py index ee84e375fb52..3099f4b40146 100644 --- a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py +++ b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py @@ -218,6 +218,29 @@ def check_show_bgp_ipv4_vpn(rname, json_file): assert result is None, assertmsg +def check_show_bgp_ipv4_vpn_peer_advertised_routes(rname, peer, json_file): + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + router = tgen.gears[rname] + + logger.info( + "Checking VPNv4 advertised routes for on {} for peer {}".format(rname, peer) + ) + + json_file = "{}/{}/{}".format(CWD, router.name, json_file) + expected = json.loads(open(json_file).read()) + test_func = partial( + topotest.router_json_cmp, + router, + "show bgp ipv4 vpn neighbors {} advertised-routes detail json".format(peer), + expected, + ) + _, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5) + assertmsg = '"{}" JSON output mismatches'.format(router.name) + assert result is None, assertmsg + + def check_show_bgp_vrf_ipv4(rname, json_file): tgen = get_topogen() if tgen.routers_have_failure(): @@ -563,6 +586,21 @@ def test_bgp_retain_step12(): check_show_bgp_vrf_ipv4(rname, "ipv4_vrf_all_routes_init.json") +def test_bgp_advertised_routes_step13(): + """ + Dump advertised routes from r1 to r2 + Check that the localpref attribute is set on the show command + """ + + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + check_show_bgp_ipv4_vpn_peer_advertised_routes( + "r1", "10.125.0.2", "ipv4_vpn_routes_advertised_10_125_0_2.json" + ) + + def test_memory_leak(): "Run the memory leak test and report results." tgen = get_topogen()