From 1cbbc94e5528981c981453a5d5f5d2143c142831 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 3 Mar 2025 14:40:42 +0100 Subject: [PATCH 1/6] bgpd: fix export, and selects l3vpn aggregated prefix On a L3VPN setup, an aggretated prefix can not be exported and selected. The below example illustrates the 172.31.0.0/24 aggregated prefix, which is valid as a VRF prefix, but invalid as a VPN prefix: > r1# show bgp ipv4 vpn 172.31.0.0/24 > BGP routing table entry for 444:1:172.31.0.0/24, version 0 > not allocated > Paths: (1 available, no best path) > Not advertised to any peer > Local, (aggregated by 65500 192.0.2.1) > 0.0.0.0 from 0.0.0.0 (192.0.2.1) vrf vrf1(4) announce-nh-self > Origin incomplete, metric 0, weight 32768, invalid, sourced, local, atomic-aggregate > Extended Community: RT:52:100 > Originator: 192.0.2.1 > Remote label: 101 > Last update: Mon Mar 3 14:35:04 2025 > r1# show bgp vrf vrf1 ipv4 172.31.0.0/24 > BGP routing table entry for 172.31.0.0/24, version 1 > Paths: (1 available, best #1, vrf vrf1) > Not advertised to any peer > Local, (aggregated by 65500 192.0.2.1) > 0.0.0.0 from 0.0.0.0 (192.0.2.1) > Origin incomplete, metric 0, weight 32768, valid, aggregated, local, atomic-aggregate, best (First path received) > Last update: Mon Mar 3 14:35:03 2025 > r1# Actually, the aggregated prefix nexthop is considered, and 0.0.0.0 is an invalid nexthop. > r1# show bgp vrf vrf1 nexthop > Current BGP nexthop cache: > 0.0.0.0 invalid, #paths 1 > Is not Registered > Last update: Thu Feb 13 18:33:43 2025 Fix this by considering the L3VPN prefix selected, if the VRF prefix is selected too. Signed-off-by: Philippe Guibert --- bgpd/bgp_mplsvpn.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index ac2f90e867..8057b3d56a 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1097,6 +1097,9 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn, * then mark the nexthop as valid. */ nh_valid = true; + } else if (bpi_ultimate->type == ZEBRA_ROUTE_BGP && + bpi_ultimate->sub_type == BGP_ROUTE_AGGREGATE) { + nh_valid = true; } else /* * TBD do we need to do anything about the From bccf1e5447994ca349de8ba944aba267b397380f Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 3 Mar 2025 15:31:09 +0100 Subject: [PATCH 2/6] topotests: add vpn test to control aggregated prefix is exported Add a test in bgp_vpnv4_ebgp test to control that the aggregated prefix is exported and selected as a VPN prefix. Signed-off-by: Philippe Guibert --- tests/topotests/bgp_vpnv4_ebgp/r1/bgpd.conf | 2 + .../r1/show_bgp_ipv4_172_31_1_0.json | 52 +++++++ tests/topotests/bgp_vpnv4_ebgp/r1/zebra.conf | 4 + tests/topotests/bgp_vpnv4_ebgp/r100/bgpd.conf | 6 + .../bgp_vpnv4_ebgp/r100/show_bgp_ipv4.json | 147 ++++++++++++++++++ .../topotests/bgp_vpnv4_ebgp/r100/zebra.conf | 4 + .../bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py | 50 ++++++ 7 files changed, 265 insertions(+) create mode 100644 tests/topotests/bgp_vpnv4_ebgp/r1/show_bgp_ipv4_172_31_1_0.json create mode 100644 tests/topotests/bgp_vpnv4_ebgp/r100/bgpd.conf create mode 100644 tests/topotests/bgp_vpnv4_ebgp/r100/show_bgp_ipv4.json create mode 100644 tests/topotests/bgp_vpnv4_ebgp/r100/zebra.conf diff --git a/tests/topotests/bgp_vpnv4_ebgp/r1/bgpd.conf b/tests/topotests/bgp_vpnv4_ebgp/r1/bgpd.conf index d8a45ce274..65ae4b5c3d 100644 --- a/tests/topotests/bgp_vpnv4_ebgp/r1/bgpd.conf +++ b/tests/topotests/bgp_vpnv4_ebgp/r1/bgpd.conf @@ -15,8 +15,10 @@ router bgp 65500 ! router bgp 65500 vrf vrf1 bgp router-id 192.0.2.1 + neighbor 172.31.2.100 remote-as 65500 address-family ipv4 unicast redistribute connected + aggregate-address 172.31.1.0/24 label vpn export 101 rd vpn export 444:1 rt vpn both 52:100 diff --git a/tests/topotests/bgp_vpnv4_ebgp/r1/show_bgp_ipv4_172_31_1_0.json b/tests/topotests/bgp_vpnv4_ebgp/r1/show_bgp_ipv4_172_31_1_0.json new file mode 100644 index 0000000000..5947893a82 --- /dev/null +++ b/tests/topotests/bgp_vpnv4_ebgp/r1/show_bgp_ipv4_172_31_1_0.json @@ -0,0 +1,52 @@ +{ + "444:1":{ + "prefix":"172.31.1.0/24", + "advertisedTo":{ + "192.168.0.2":{ + "hostname":"r2" + }, + "192.168.0.3":{ + "hostname":"r3" + } + }, + "paths":[ + { + "aspath":{ + "string":"Local", + "segments":[ + ], + "length":0 + }, + "aggregatorAs":65500, + "aggregatorId":"192.0.2.1", + "nhVrfName":"vrf1", + "announceNexthopSelf":true, + "origin":"incomplete", + "metric":0, + "weight":32768, + "valid":true, + "sourced":true, + "local":true, + "atomicAggregate":true, + "bestpath":{ + "overall":true, + "selectionReason":"First path received" + }, + "originatorId":"192.0.2.1", + "nexthops":[ + { + "ip":"0.0.0.0", + "afi":"ipv4", + "metric":0, + "accessible":true, + "used":true + } + ], + "peer":{ + "peerId":"0.0.0.0", + "routerId":"192.0.2.1" + } + } + ] + } +} diff --git a/tests/topotests/bgp_vpnv4_ebgp/r1/zebra.conf b/tests/topotests/bgp_vpnv4_ebgp/r1/zebra.conf index f626e448a7..17766d1840 100644 --- a/tests/topotests/bgp_vpnv4_ebgp/r1/zebra.conf +++ b/tests/topotests/bgp_vpnv4_ebgp/r1/zebra.conf @@ -1,6 +1,10 @@ log stdout interface r1-eth1 vrf vrf1 ip address 172.31.0.1/32 + ip address 172.31.1.1/32 + ip address 172.31.1.2/32 + ip address 172.31.1.3/32 + ip address 172.31.2.1/24 ! interface r1-eth0 ip address 192.168.0.1/24 diff --git a/tests/topotests/bgp_vpnv4_ebgp/r100/bgpd.conf b/tests/topotests/bgp_vpnv4_ebgp/r100/bgpd.conf new file mode 100644 index 0000000000..9f6e0d2376 --- /dev/null +++ b/tests/topotests/bgp_vpnv4_ebgp/r100/bgpd.conf @@ -0,0 +1,6 @@ +router bgp 65500 + bgp router-id 192.0.2.100 + neighbor 172.31.2.1 remote-as 65500 + address-family ipv4 unicast + exit-address-family +exit diff --git a/tests/topotests/bgp_vpnv4_ebgp/r100/show_bgp_ipv4.json b/tests/topotests/bgp_vpnv4_ebgp/r100/show_bgp_ipv4.json new file mode 100644 index 0000000000..5bf2627243 --- /dev/null +++ b/tests/topotests/bgp_vpnv4_ebgp/r100/show_bgp_ipv4.json @@ -0,0 +1,147 @@ +{ + "vrfId": 0, + "vrfName": "default", + "routerId": "192.0.2.100", + "defaultLocPrf": 100, + "localAS": 65500, + "routes": { + "172.31.0.1/32": [{ + "valid":true, + "bestpath":true, + "selectionReason":"First path received", + "pathFrom":"internal", + "prefix":"172.31.0.1", + "prefixLen":32, + "network":"172.31.0.1/32", + "metric":0, + "locPrf":100, + "weight":0, + "peerId":"172.31.2.1", + "origin":"incomplete", + "nexthops":[{ + "ip":"172.31.2.1", + "afi":"ipv4", + "used":true + }] + }], + "172.31.0.10/32": [{ + "valid":true, + "bestpath":true, + "selectionReason":"First path received", + "pathFrom":"internal", + "prefix":"172.31.0.10", + "prefixLen":32, + "network":"172.31.0.10/32", + "metric":0, + "locPrf":100, + "weight":0, + "peerId":"172.31.2.1", + "path":"65501", + "origin":"incomplete", + "nexthops":[{ + "ip":"172.31.2.1", + "afi":"ipv4", + "used":true + }] + }], + "172.31.1.0/24": [{ + "valid":true, + "bestpath":true, + "selectionReason":"First path received", + "pathFrom":"internal", + "prefix":"172.31.1.0", + "prefixLen":24, + "network":"172.31.1.0/24", + "metric":0, + "locPrf":100, + "weight":0, + "peerId":"172.31.2.1", + "path":"", + "origin":"incomplete", + "nexthops":[{ + "ip":"172.31.2.1", + "afi":"ipv4", + "used":true + }] + }], + "172.31.1.1/32": [{ + "valid":true, + "bestpath":true, + "selectionReason":"First path received", + "pathFrom":"internal", + "prefix":"172.31.1.1", + "prefixLen":32, + "network":"172.31.1.1/32", + "metric":0, + "locPrf":100, + "weight":0, + "peerId":"172.31.2.1", + "path":"", + "origin":"incomplete", + "nexthops":[{ + "ip":"172.31.2.1", + "afi":"ipv4", + "used":true}] + }], + "172.31.1.2/32": [{ + "valid":true, + "bestpath":true, + "selectionReason":"First path received", + "pathFrom":"internal", + "prefix":"172.31.1.2", + "prefixLen":32, + "network":"172.31.1.2/32", + "metric":0, + "locPrf":100, + "weight":0, + "peerId":"172.31.2.1", + "path":"", + "origin":"incomplete", + "nexthops":[{ + "ip":"172.31.2.1", + "afi":"ipv4", + "used":true}] + }], + "172.31.1.3/32": [{ + "valid":true, + "bestpath":true, + "selectionReason":"First path received", + "pathFrom":"internal", + "prefix":"172.31.1.3", + "prefixLen":32, + "network":"172.31.1.3/32", + "metric":0, + "locPrf":100, + "weight":0, + "peerId":"172.31.2.1", + "path":"", + "origin":"incomplete", + "nexthops":[{ + "ip":"172.31.2.1", + "afi":"ipv4", + "used":true + }] + }], + "172.31.2.0/24": [{ + "valid":true, + "bestpath":true, + "selectionReason":"First path received", + "pathFrom":"internal", + "prefix":"172.31.2.0", + "prefixLen":24, + "network":"172.31.2.0/24", + "metric":0, + "locPrf":100, + "weight":0, + "peerId":"172.31.2.1", + "path":"", + "origin":"incomplete", + "nexthops":[{ + "ip":"172.31.2.1", + "afi":"ipv4", + "used":true}] + }] + } , + "totalRoutes": 7, + "totalPaths": 7 +} diff --git a/tests/topotests/bgp_vpnv4_ebgp/r100/zebra.conf b/tests/topotests/bgp_vpnv4_ebgp/r100/zebra.conf new file mode 100644 index 0000000000..f4154b644b --- /dev/null +++ b/tests/topotests/bgp_vpnv4_ebgp/r100/zebra.conf @@ -0,0 +1,4 @@ +log stdout +interface r100-eth0 + ip address 172.31.2.100/24 +! diff --git a/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py b/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py index dd9d54742b..e2727a3ed7 100644 --- a/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py +++ b/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py @@ -45,6 +45,7 @@ def build_topo(tgen): tgen.add_router("r1") tgen.add_router("r2") tgen.add_router("r3") + tgen.add_router("r100") switch = tgen.add_switch("s1") switch.add_link(tgen.gears["r1"]) @@ -53,6 +54,7 @@ def build_topo(tgen): switch = tgen.add_switch("s2") switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r100"]) switch = tgen.add_switch("s3") switch.add_link(tgen.gears["r2"]) @@ -524,6 +526,54 @@ router bgp 65501 assert result is None, assertmsg +def test_aggregated_route_on_r100(): + """ + Check that only aggregated route on r100 is received + """ + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r100 = tgen.gears["r100"] + logger.info("Checking prefixes list on R100") + json_file = "{}/{}/show_bgp_ipv4.json".format(CWD, r100.name) + + expected = json.loads(open(json_file).read()) + test_func = partial( + topotest.router_json_cmp, + r100, + "show bgp ipv4 json", + expected, + ) + _, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5) + assertmsg = '"{}" JSON output mismatches'.format(r100.name) + assert result is None, assertmsg + + +def test_aggregated_exported_route_on_r1(): + """ + Check that the aggregated route 172.31.1.0/24 is exported + """ + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + logger.info("Checking 172.31.1.0/24 VPN prefix on R1") + json_file = "{}/{}/show_bgp_ipv4_172_31_1_0.json".format(CWD, r1.name) + + expected = json.loads(open(json_file).read()) + test_func = partial( + topotest.router_json_cmp, + r1, + "show bgp ipv4 vpn 172.31.1.0/24 json", + expected, + ) + _, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5) + assertmsg = '"{}" JSON output mismatches'.format(r1.name) + assert result is None, assertmsg + + def test_memory_leak(): "Run the memory leak test and report results." tgen = get_topogen() From 32088c43a81d7e5d0ef8d21c3da15676d006a0d9 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 3 Mar 2025 17:21:19 +0100 Subject: [PATCH 3/6] bgpd: fix remove vpn aggregated prefix upon unconfiguration When unconfiguring an aggregated prefix, the VPN prefix is not removed. Fix this by refreshing the VPN leak when the aggregated route is or is not available. Signed-off-by: Philippe Guibert --- bgpd/bgp_route.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 3bbbdee161..910fa97493 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -8015,6 +8015,9 @@ static void bgp_aggregate_install( bgp_process(bgp, dest, new, afi, safi); if (debug) zlog_debug(" aggregate %pFX: installed", p); + if (SAFI_UNICAST == safi && (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) + vpn_leak_from_vrf_update(bgp_get_default(), bgp, new); } else { uninstall_aggregate_route: /* Withdraw the aggregate route from routing table. */ @@ -8023,6 +8026,11 @@ static void bgp_aggregate_install( bgp_process(bgp, dest, pi, afi, safi); if (debug) zlog_debug(" aggregate %pFX: uninstall", p); + if (SAFI_UNICAST == safi && + (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) { + vpn_leak_from_vrf_withdraw(bgp_get_default(), bgp, pi); + } } } From e0f585fcab3e5ab6c1bc6e2174262d4b18872068 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 3 Mar 2025 17:22:59 +0100 Subject: [PATCH 4/6] topotests: add a test to unconfigure aggregated prefix on VPN That test will ensure the VPN prefix associated is removed. Signed-off-by: Philippe Guibert --- .../bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py b/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py index e2727a3ed7..f6bfed5bb0 100644 --- a/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py +++ b/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py @@ -574,6 +574,40 @@ def test_aggregated_exported_route_on_r1(): assert result is None, assertmsg +def test_aggregated_suppress_aggregate_r1(): + """ + Check that only the suppressed networks are exported + """ + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r1.vtysh_cmd( + """ + configure terminal + router bgp 65500 vrf vrf1 + address-family ipv4 unicast + no aggregate-address 172.31.1.0/24 + """ + ) + + r1 = tgen.gears["r1"] + logger.info("Checking 172.31.1.0/24 VPN prefix is hot present on R1") + + expected = {} + test_func = partial( + topotest.router_json_cmp, + r1, + "show bgp ipv4 vpn 172.31.1.0/24 json", + expected, + exact=True, + ) + _, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5) + assertmsg = '"{}" JSON output mismatches'.format(r1.name) + assert result is None, assertmsg + + def test_memory_leak(): "Run the memory leak test and report results." tgen = get_topogen() From bf15730267628e2d47ad78cc58895bc375d1b400 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 3 Mar 2025 17:11:33 +0100 Subject: [PATCH 5/6] bgpd: fix syncs suppressed prefixes in VPN environments By using the summary-only option for aggregated prefixes, the suppressed prefixes are however exported as VPN prefixes, whereas they should not. > r1# show bgp vrf vrf1 ipv4 > [..] > *> 172.31.1.0/24 0.0.0.0 0 32768 ? > s> 172.31.1.1/32 0.0.0.0 0 32768 ? > s> 172.31.1.2/32 0.0.0.0 0 32768 ? > s> 172.31.1.3/32 0.0.0.0 0 32768 ? > [..] > r1# > > r1# show bgp ipv4 vpn > [..] > *> 172.31.1.0/24 0.0.0.0@4< 0 32768 ? > UN=0.0.0.0 EC{52:100} label=101 type=bgp, subtype=5 > *> 172.31.1.1/32 0.0.0.0@4< 0 32768 ? > UN=0.0.0.0 EC{52:100} label=101 type=bgp, subtype=5 > *> 172.31.1.2/32 0.0.0.0@4< 0 32768 ? > UN=0.0.0.0 EC{52:100} label=101 type=bgp, subtype=5 > *> 172.31.1.3/32 0.0.0.0@4< 0 32768 ? > UN=0.0.0.0 EC{52:100} label=101 type=bgp, subtype=5 > [..] > r1# Signed-off-by: Philippe Guibert --- bgpd/bgp_mplsvpn.c | 8 ++++++++ bgpd/bgp_route.c | 50 +++++++++++++++++++++++++++++++++++++++------- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 8057b3d56a..d1d4c5af68 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1696,6 +1696,14 @@ void vpn_leak_from_vrf_update(struct bgp *to_bgp, /* to */ return; } + /* Aggregate-address suppress check. */ + if (bgp_path_suppressed(path_vrf)) { + if (debug) + zlog_debug("%s: %s skipping: suppressed path will not be exported", + __func__, from_bgp->name); + return; + } + /* shallow copy */ static_attr = *path_vrf->attr; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 910fa97493..f1a6ad8c94 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -8135,14 +8135,25 @@ void bgp_aggregate_toggle_suppressed(struct bgp_aggregate *aggregate, /* We are toggling suppression back. */ if (suppress) { /* Suppress route if not suppressed already. */ - if (aggr_suppress_path(aggregate, pi)) + if (aggr_suppress_path(aggregate, pi)) { bgp_process(bgp, dest, pi, afi, safi); + if (SAFI_UNICAST == safi && + (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) + vpn_leak_from_vrf_withdraw(bgp_get_default(), bgp, + pi); + } continue; } /* Install route if there is no more suppression. */ - if (aggr_unsuppress_path(aggregate, pi)) + if (aggr_unsuppress_path(aggregate, pi)) { bgp_process(bgp, dest, pi, afi, safi); + if (SAFI_UNICAST == safi && + (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) + vpn_leak_from_vrf_update(bgp_get_default(), bgp, pi); + } } } bgp_dest_unlock_node(top); @@ -8273,8 +8284,14 @@ bool bgp_aggregate_route(struct bgp *bgp, const struct prefix *p, afi_t afi, */ if (aggregate->summary_only && AGGREGATE_MED_VALID(aggregate)) { - if (aggr_suppress_path(aggregate, pi)) + if (aggr_suppress_path(aggregate, pi)) { bgp_process(bgp, dest, pi, afi, safi); + if (SAFI_UNICAST == safi && + (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) + vpn_leak_from_vrf_withdraw(bgp_get_default(), bgp, + pi); + } } /* @@ -8289,8 +8306,14 @@ bool bgp_aggregate_route(struct bgp *bgp, const struct prefix *p, afi_t afi, if (aggregate->suppress_map_name && AGGREGATE_MED_VALID(aggregate) && aggr_suppress_map_test(bgp, aggregate, pi)) { - if (aggr_suppress_path(aggregate, pi)) + if (aggr_suppress_path(aggregate, pi)) { bgp_process(bgp, dest, pi, afi, safi); + if (SAFI_UNICAST == safi && + (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) + vpn_leak_from_vrf_withdraw(bgp_get_default(), bgp, + pi); + } } aggregate->count++; @@ -8438,8 +8461,13 @@ void bgp_aggregate_delete(struct bgp *bgp, const struct prefix *p, afi_t afi, */ if (pi->extra && pi->extra->aggr_suppressors && listcount(pi->extra->aggr_suppressors)) { - if (aggr_unsuppress_path(aggregate, pi)) + if (aggr_unsuppress_path(aggregate, pi)) { bgp_process(bgp, dest, pi, afi, safi); + if (SAFI_UNICAST == safi && + (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) + vpn_leak_from_vrf_update(bgp_get_default(), bgp, pi); + } } if (aggregate->count > 0) @@ -8645,13 +8673,21 @@ static void bgp_remove_route_from_aggregate(struct bgp *bgp, afi_t afi, return; if (aggregate->summary_only && AGGREGATE_MED_VALID(aggregate)) - if (aggr_unsuppress_path(aggregate, pi)) + if (aggr_unsuppress_path(aggregate, pi)) { bgp_process(bgp, pi->net, pi, afi, safi); + if (SAFI_UNICAST == safi && (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) + vpn_leak_from_vrf_update(bgp_get_default(), bgp, pi); + } if (aggregate->suppress_map_name && AGGREGATE_MED_VALID(aggregate) && aggr_suppress_map_test(bgp, aggregate, pi)) - if (aggr_unsuppress_path(aggregate, pi)) + if (aggr_unsuppress_path(aggregate, pi)) { bgp_process(bgp, pi->net, pi, afi, safi); + if (SAFI_UNICAST == safi && (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) + vpn_leak_from_vrf_update(bgp_get_default(), bgp, pi); + } /* * This must be called after `summary`, `suppress-map` check to avoid From f8438393ac54bdb36275445a388ad14468c68cfe Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 3 Mar 2025 17:25:04 +0100 Subject: [PATCH 6/6] topotests: add a test to configure aggregated summary-only prefix on VPN That configured aggregated prefix should be present, but all other suppressed prefixes should not be exported. Signed-off-by: Philippe Guibert --- .../bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py b/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py index f6bfed5bb0..694364b17c 100644 --- a/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py +++ b/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py @@ -608,6 +608,40 @@ def test_aggregated_suppress_aggregate_r1(): assert result is None, assertmsg +def test_aggregated_suppressed_networks_not_exported_on_r1(): + """ + Check that the suppressed networks are not exported + """ + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r1.vtysh_cmd( + """ + configure terminal + router bgp 65500 vrf vrf1 + address-family ipv4 unicast + aggregate-address 172.31.1.0/24 summary-only + """ + ) + + for prefix in ("172.31.1.1/32", "172.31.1.2/32", "172.31.1.3/32"): + logger.info(f"Checking {prefix} VPN prefix is not on R1") + + expected = {} + test_func = partial( + topotest.router_json_cmp, + r1, + f"show bgp ipv4 vpn {prefix} json", + expected, + exact=True, + ) + _, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5) + assertmsg = '"{}" JSON output mismatches'.format(r1.name) + assert result is None, assertmsg + + def test_memory_leak(): "Run the memory leak test and report results." tgen = get_topogen()