From 78872311a0347b9d6953c965aa3dd9d412c812b9 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 28 Apr 2022 17:01:35 +0200 Subject: [PATCH 1/2] bgpd: fix VRF leaking with 'no bgp network import-check' BGP static routes are defined using the network statement, e.g.: > router bgp XXX > address-family ipv4 unicast > network 192.168.0.0/24 When "no bgp network import-check" is set, it is impossible to successfully import the static routes into the BGP VPN table. The prefix is present in the table but is not marked as valid. This issue applies regardless of whether or not routes are present in the router's RIB. Always mark as valid the nexthops of BGP static routes when "no bgp network import-check" is set. Signed-off-by: Louis Scalbert Signed-off-by: Philippe Guibert (cherry picked from commit 45bf46441a2e0b02650a1d162367c357b220c7b1) --- bgpd/bgp_mplsvpn.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index ed74e0f08bcd..bdbb4a2a5b9d 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1011,9 +1011,11 @@ 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; bool nh_valid; bpi_ultimate = bgp_get_imported_bpi_ultimate(source_bpi); + table = bgp_dest_table(bpi_ultimate->net); if (bpi->extra && bpi->extra->vrfleak && bpi->extra->vrfleak->bgp_orig) bgp_nexthop = bpi->extra->vrfleak->bgp_orig; @@ -1029,7 +1031,17 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn, is_pi_family_evpn(bpi_ultimate) || CHECK_FLAG(bpi_ultimate->flags, BGP_PATH_ACCEPT_OWN)) nh_valid = true; - else + else if (bpi_ultimate->type == ZEBRA_ROUTE_BGP && + bpi_ultimate->sub_type == BGP_ROUTE_STATIC && table && + (table->safi == SAFI_UNICAST || + table->safi == SAFI_LABELED_UNICAST) && + !CHECK_FLAG(bgp_nexthop->flags, BGP_FLAG_IMPORT_CHECK)) { + /* if the route is defined with the "network " command + * and "no bgp network import-check" is set, + * then mark the nexthop as valid. + */ + nh_valid = true; + } else /* * TBD do we need to do anything about the * 'connected' parameter? From 01bd36f70eca7699f2cddafb6ed830810eebf292 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 25 Jan 2024 14:26:56 +0100 Subject: [PATCH 2/2] topotests: vpnv4 route leaking with no import-check Test vpnv4 route leaking with no import-check Signed-off-by: Louis Scalbert (cherry picked from commit 4bbfade7d6115370ffaa89634b02bec8534bf037) --- .../topotests/bgp_l3vpn_to_bgp_vrf/customize.py | 9 +++++++++ .../topotests/bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf | 13 +++++++++++++ .../bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py | 16 ++++++++++++---- .../bgp_l3vpn_to_bgp_vrf/scripts/scale_down.py | 2 +- 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/customize.py b/tests/topotests/bgp_l3vpn_to_bgp_vrf/customize.py index 0ac535030846..23ab90794c92 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/customize.py +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/customize.py @@ -161,6 +161,15 @@ def ltemplatePreRouterStartHook(): logger.info( "setup {0} vrf {0}-cust1, {0}-eth4. enabled mpls input.".format(rtr) ) + # configure cust4 VRFs & MPLS + cmds = [ + "ip link add {0}-cust4 type vrf table 30", + "ip link set dev {0}-cust4 up", + ] + rtr = "r1" + for cmd in cmds: + cc.doCmd(tgen, rtr, cmd.format(rtr)) + logger.info("setup {0} vrf {0}-cust3 and{0}-cust4.".format(rtr)) # configure cust2 VRFs & MPLS rtrs = ["r4"] cmds = [ diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf b/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf index 72211fee7f10..b389eb1013de 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf @@ -50,6 +50,19 @@ router bgp 5227 vrf r1-cust1 export vpn exit-address-family +router bgp 5227 vrf r1-cust4 + no bgp network import-check + bgp router-id 192.168.1.1 + + address-family ipv4 unicast + network 172.16.0.0/24 + + rd vpn export 10:14 + rt vpn export 52:100 + + import vpn + export vpn + exit-address-family ! end diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py index 1e2758c1c962..3ab9b3f46e4b 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py @@ -59,12 +59,20 @@ {"p": "5.1.1.0/24", "n": "99.0.0.1"}, {"p": "6.0.1.0/24", "n": "99.0.0.1"}, {"p": "6.0.2.0/24", "n": "99.0.0.1"}, + {"p": "172.16.0.0/24", "n": "0.0.0.0", "bp": True}, {"p": "99.0.0.1/32", "n": "192.168.1.2"}, ] bgpribRequireUnicastRoutes( "r1", "ipv4", "r1-cust1", "Customer 1 routes in r1 vrf", want_r1_cust1_routes ) +want_r1_cust4_routes = [ + {"p": "172.16.0.0/24", "n": "0.0.0.0", "bp": True}, +] +bgpribRequireUnicastRoutes( + "r1", "ipv4", "r1-cust4", "Customer 4 routes in r1 vrf", want_r1_cust4_routes +) + want_r3_cust1_routes = [ {"p": "5.1.0.0/24", "n": "99.0.0.2"}, {"p": "5.1.1.0/24", "n": "99.0.0.2"}, @@ -667,7 +675,7 @@ luCommand( "ce1", 'vtysh -c "show bgp ipv4 uni"', - "12 routes and 12", + "13 routes and 13", "wait", "Local and remote routes", 10, @@ -689,7 +697,7 @@ luCommand( "ce2", 'vtysh -c "show bgp ipv4 uni"', - "12 routes and 15", + "13 routes and 16", "wait", "Local and remote routes", 10, @@ -721,7 +729,7 @@ luCommand( "ce3", 'vtysh -c "show bgp ipv4 uni"', - "12 routes and 13", + "13 routes and 14", "wait", "Local and remote routes", 10, @@ -743,7 +751,7 @@ luCommand( "ce4", 'vtysh -c "show bgp vrf ce4-cust2 ipv4 uni"', - "12 routes and 14", + "13 routes and 15", "wait", "Local and remote routes", 10, diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/scale_down.py b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/scale_down.py index 36be926227d0..43a5245d0fb2 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/scale_down.py +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/scale_down.py @@ -49,7 +49,7 @@ luCommand( rtr, 'vtysh -c "show bgp ipv4 uni" | grep Display', - " 12 route", + " 13 route", "wait", "BGP routes removed", wait,