diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index ac2f90e867..d1d4c5af68 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 @@ -1693,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 6c5793525d..9b3053ff25 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -8032,6 +8032,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. */ @@ -8040,6 +8043,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); + } } } @@ -8144,14 +8152,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); @@ -8282,8 +8301,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); + } } /* @@ -8298,8 +8323,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++; @@ -8447,8 +8478,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) @@ -8654,13 +8690,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 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..694364b17c 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,122 @@ 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_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_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()