Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: fix for the validity and the presence of prefixes in the BGP VPN table. #17370

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+871 −136
Open

bgpd: fix for the validity and the presence of prefixes in the BGP VPN table. #17370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0713e6a
bgpd: merge parent and source_bpi in leak_update
louis-6wind Nov 14, 2024
dc0cfb6
bgpd: check nexthop vrf usability
louis-6wind Nov 6, 2024
78ef052
bgpd: recheck leak nexthop validity
louis-6wind Nov 5, 2024
9fc6bce
bgpd: do not leak if origin vrf is not usable
louis-6wind Nov 6, 2024
4ab0543
tests: update route_leak_basic tests
louis-6wind Nov 6, 2024
567cb1e
tests: fix no vrf1 in bgp_bmp r2
louis-6wind Nov 14, 2024
ce6e95c
tests: check bgp vpn table in bgp_vpnv4_route_leak_basic
louis-6wind Nov 6, 2024
7712a51
tests: bgp_vpnv4_route_leak_basic add del static prefix
louis-6wind Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 37 additions & 9 deletions bgpd/bgp_mplsvpn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,7 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn,
struct bgp_path_info *bpi_ultimate;
struct bgp *bgp_nexthop;
struct bgp_table *table;
struct interface *ifp;
bool nh_valid;

bpi_ultimate = bgp_get_imported_bpi_ultimate(source_bpi);
Expand All @@ -1062,6 +1063,15 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn,
else
bgp_nexthop = bgp_orig;

/* The nexthop is invalid if its VRF does not exist */
if (bgp_nexthop->vrf_id == VRF_UNKNOWN)
return false;

/* The nexthop is invalid if its VRF interface is down*/
ifp = if_get_vrf_loopback(bgp_nexthop->vrf_id);
if (ifp && !if_is_up(ifp))
return false;

/*
* No nexthop tracking for redistributed routes, for
* EVPN-imported routes that get leaked, or for routes
Expand Down Expand Up @@ -1126,8 +1136,8 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn,
struct bgp_path_info *bpi;
struct bgp_path_info *new;
struct bgp_path_info_extra *extra;
struct bgp_path_info *parent = source_bpi;
struct bgp_labels bgp_labels = {};
struct bgp *bgp_nexthop;
bool labelssame;
uint8_t i;

Expand Down Expand Up @@ -1159,8 +1169,7 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn,
* match parent
*/
for (bpi = bgp_dest_get_bgp_path_info(bn); bpi; bpi = bpi->next) {
if (bpi->extra && bpi->extra->vrfleak &&
bpi->extra->vrfleak->parent == parent)
if (bpi->extra && bpi->extra->vrfleak && bpi->extra->vrfleak->parent == source_bpi)
break;
}

Expand All @@ -1174,6 +1183,16 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn,
labelssame = bgp_path_info_labels_same(bpi, bgp_labels.label,
bgp_labels.num_labels);

bgp_nexthop = bpi->extra->vrfleak->bgp_orig ?: bgp_orig;
if (bgp_nexthop->vrf_id == VRF_UNKNOWN) {
if (debug) {
zlog_debug("%s: ->%s(s_flags: 0x%x b_flags: 0x%x): %pFX: Found route, origin VRF does not exist, not leaking",
__func__, to_bgp->name_pretty, source_bpi->flags,
bpi->flags, p);
}
return NULL;
}

if (CHECK_FLAG(source_bpi->flags, BGP_PATH_REMOVED)
&& CHECK_FLAG(bpi->flags, BGP_PATH_REMOVED)) {
if (debug) {
Expand All @@ -1185,9 +1204,11 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn,
return NULL;
}

if (attrhash_cmp(bpi->attr, new_attr) && labelssame
&& !CHECK_FLAG(bpi->flags, BGP_PATH_REMOVED)) {

if (attrhash_cmp(bpi->attr, new_attr) && labelssame &&
!CHECK_FLAG(bpi->flags, BGP_PATH_REMOVED) &&
leak_update_nexthop_valid(to_bgp, bn, new_attr, afi, safi, source_bpi, bpi,
bgp_orig, p,
debug) == !!CHECK_FLAG(bpi->flags, BGP_PATH_VALID)) {
bgp_attr_unintern(&new_attr);
if (debug)
zlog_debug(
Expand Down Expand Up @@ -1274,6 +1295,14 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn,
return NULL;
}

if (bgp_orig->vrf_id == VRF_UNKNOWN) {
if (debug) {
zlog_debug("%s: ->%s(s_flags: 0x%x): %pFX: New route, origin VRF does not exist, not leaking",
__func__, to_bgp->name_pretty, source_bpi->flags, p);
}
return NULL;
}

new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_IMPORTED, 0,
to_bgp->peer_self, new_attr, bn);

Expand All @@ -1297,9 +1326,8 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn,
if (bgp_labels.num_labels)
new->extra->labels = bgp_labels_intern(&bgp_labels);

new->extra->vrfleak->parent = bgp_path_info_lock(parent);
bgp_dest_lock_node(
(struct bgp_dest *)parent->net);
new->extra->vrfleak->parent = bgp_path_info_lock(source_bpi);
bgp_dest_lock_node((struct bgp_dest *)source_bpi->net);

new->extra->vrfleak->bgp_orig = bgp_lock(bgp_orig);

Expand Down
7 changes: 7 additions & 0 deletions tests/topotests/bgp_bmp/test_bgp_bmp_1.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ def setup_module(mod):
"tcpdump -nni r1-eth0 -s 0 -w {} &".format(pcap_file), stdout=None
)

tgen.net["r2"].cmd(
"""
ip link add vrf1 type vrf table 10
ip link set vrf1 up
"""
)

for rname, router in tgen.routers().items():
logger.info("Loading router %s" % rname)
router.load_frr_config(
Expand Down
10 changes: 10 additions & 0 deletions tests/topotests/bgp_vpnv4_route_leak_basic/r1/frr.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
vrf DONNA
ip route 172.16.3.0/24 10.0.0.254
exit-vrf
!
int dummy0
ip address 10.0.4.1/24
no shut
Expand Down Expand Up @@ -28,6 +32,9 @@ ip router-id 10.0.4.1
!
router bgp 99
no bgp ebgp-requires-policy
! 10.0.4.254 peer session will not be established
! it is there just to activate the ipv4 vpn table
neighbor 10.0.4.254 remote-as external
address-family ipv4 unicast
redistribute connected
rd vpn export 10.0.4.1:1
Expand All @@ -36,11 +43,14 @@ router bgp 99
export vpn
import vpn
!
address-family ipv4 vpn
neighbor 10.0.4.254 activate
!
router bgp 99 vrf DONNA
no bgp ebgp-requires-policy
address-family ipv4 unicast
redistribute connected
network 172.16.3.0/24
label vpn export 101
rd vpn export 10.0.4.1:1
rt vpn export 10.0.4.1:101
Expand Down
155 changes: 155 additions & 0 deletions tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_add_zita.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
{
"routerId": "10.0.4.1",
"localAS": 99,
"routes": {
"routeDistinguishers": {
"10.0.4.1:1": {
"10.0.0.0/24": [
{
"valid": true,
"pathFrom": "external",
"path": "",
"origin": "incomplete",
"announceNexthopSelf": true,
"nhVrfName": "DONNA",
"nexthops": [
{
"ip": "0.0.0.0",
"hostname": "r1",
"afi": "ipv4",
"used": true
}
]
}
],
"10.0.1.0/24": [
{
"valid": true,
"pathFrom": "external",
"path": "",
"origin": "incomplete",
"announceNexthopSelf": true,
"nhVrfName": "EVA",
"nexthops": [
{
"ip": "0.0.0.0",
"hostname": "r1",
"afi": "ipv4",
"used": true
}
]
}
],
"10.0.2.0/24": [
{
"valid": true,
"pathFrom": "external",
"path": "",
"origin": "incomplete",
"announceNexthopSelf": true,
"nhVrfName": "DONNA",
"nexthops": [
{
"ip": "0.0.0.0",
"hostname": "r1",
"afi": "ipv4",
"used": true
}
]
}
],
"10.0.3.0/24": [
{
"valid": true,
"pathFrom": "external",
"path": "",
"origin": "incomplete",
"announceNexthopSelf": true,
"nhVrfName": "EVA",
"nexthops": [
{
"ip": "0.0.0.0",
"hostname": "r1",
"afi": "ipv4",
"used": true
}
]
}
],
"10.0.4.0/24": [
{
"valid": true,
"pathFrom": "external",
"path": "",
"origin": "incomplete",
"announceNexthopSelf": true,
"nhVrfName": "default",
"nexthops": [
{
"ip": "0.0.0.0",
"hostname": "r1",
"afi": "ipv4",
"used": true
}
]
}
],
"172.16.3.0/24": [
{
"valid": true,
"pathFrom": "external",
"path": "",
"origin": "IGP",
"announceNexthopSelf": true,
"nhVrfName": "DONNA",
"nexthops": [
{
"ip": "0.0.0.0",
"hostname": "r1",
"afi": "ipv4",
"used": true
}
]
}
],
"172.16.3.0/24": [
{
"valid": true,
"pathFrom": "external",
"path": "",
"origin": "IGP",
"announceNexthopSelf": true,
"nhVrfName": "DONNA",
"nexthops": [
{
"ip": "0.0.0.0",
"hostname": "r1",
"afi": "ipv4",
"used": true
}
]
}
],
"172.16.101.0/24": [
{
"valid": null,
"pathFrom": "external",
"path": "",
"origin": "IGP",
"announceNexthopSelf": true,
"nhVrfName": "ZITA",
"nexthops": [
{
"ip": "0.0.0.0",
"hostname": "r1",
"afi": "ipv4",
"used": true
}
]
}
]
}
}
}
}

Loading
Loading