bgpd: prevent routes loop through itself

Some BGP updates received by BGP invite local router to
install a route through itself. The system will not do it, and
the route should be considered as not valid at the earliest.

This case is detected on the zebra, and this detection prevents
from trying to install this route to the local system. However,
the nexthop tracking mechanism is called, and acts as if the route
was valid, which is not the case.

By detecting in BGP that use case, we avoid installing the invalid
routes.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This commit is contained in:
Philippe Guibert 2021-06-30 10:52:29 +02:00
parent 507559a089
commit 654a5978f6
7 changed files with 26 additions and 24 deletions

View File

@ -2536,7 +2536,7 @@ static int install_evpn_route_entry_in_vrf(struct bgp *bgp_vrf,
/* Gateway IP nexthop should be resolved */
if (attr.evpn_overlay.type == OVERLAY_INDEX_GATEWAY_IP) {
if (bgp_find_or_add_nexthop(bgp_vrf, bgp_vrf, afi, safi, pi,
NULL, 0))
NULL, 0, NULL))
bgp_path_info_set_flag(dest, pi, BGP_PATH_VALID);
else {
if (BGP_DEBUG(nht, NHT)) {

View File

@ -110,9 +110,9 @@ int bgp_peer_reg_with_nht(struct peer *peer)
&& !CHECK_FLAG(peer->bgp->flags, BGP_FLAG_DISABLE_NH_CONNECTED_CHK))
connected = 1;
return bgp_find_or_add_nexthop(peer->bgp, peer->bgp,
family2afi(peer->su.sa.sa_family),
SAFI_UNICAST, NULL, peer, connected);
return bgp_find_or_add_nexthop(
peer->bgp, peer->bgp, family2afi(peer->su.sa.sa_family),
SAFI_UNICAST, NULL, peer, connected, NULL);
}
static void peer_xfer_stats(struct peer *peer_dst, struct peer *peer_src)

View File

@ -845,7 +845,7 @@ leak_update(struct bgp *bgp, /* destination bgp instance */
* 'connected' parameter?
*/
nh_valid = bgp_find_or_add_nexthop(
bgp, bgp_nexthop, afi, safi, bpi, NULL, 0);
bgp, bgp_nexthop, afi, safi, bpi, NULL, 0, p);
if (debug)
zlog_debug("%s: nexthop is %svalid (in vrf %s)",
@ -931,7 +931,7 @@ leak_update(struct bgp *bgp, /* destination bgp instance */
* 'connected' parameter?
*/
nh_valid = bgp_find_or_add_nexthop(bgp, bgp_nexthop, afi, safi,
new, NULL, 0);
new, NULL, 0, p);
if (debug)
zlog_debug("%s: nexthop is %svalid (in vrf %s)",

View File

@ -160,7 +160,8 @@ void bgp_unlink_nexthop_by_peer(struct peer *peer)
*/
int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
afi_t afi, safi_t safi, struct bgp_path_info *pi,
struct peer *peer, int connected)
struct peer *peer, int connected,
const struct prefix *orig_prefix)
{
struct bgp_nexthop_cache_head *tree = NULL;
struct bgp_nexthop_cache *bnc;
@ -192,6 +193,16 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
if (make_prefix(afi, pi, &p) < 0)
return 1;
if (!is_bgp_static_route && orig_prefix
&& prefix_same(&p, orig_prefix)) {
if (BGP_DEBUG(nht, NHT)) {
zlog_debug(
"%s(%pFX): prefix loops through itself",
__func__, &p);
}
return 0;
}
srte_color = pi->attr->srte_color;
} else if (peer) {
/*

View File

@ -42,7 +42,8 @@ extern void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id);
extern int bgp_find_or_add_nexthop(struct bgp *bgp_route,
struct bgp *bgp_nexthop, afi_t a,
safi_t safi, struct bgp_path_info *p,
struct peer *peer, int connected);
struct peer *peer, int connected,
const struct prefix *orig_prefix);
/**
* bgp_unlink_nexthop() - Unlink the nexthop object from the path structure.

View File

@ -4103,7 +4103,8 @@ int bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
nh_afi = BGP_ATTR_NH_AFI(afi, pi->attr);
if (bgp_find_or_add_nexthop(bgp, bgp_nexthop, nh_afi,
safi, pi, NULL, connected)
safi, pi, NULL, connected,
p)
|| CHECK_FLAG(peer->flags, PEER_FLAG_IS_RFAPI_HD))
bgp_path_info_set_flag(dest, pi,
BGP_PATH_VALID);
@ -4244,7 +4245,7 @@ int bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
nh_afi = BGP_ATTR_NH_AFI(afi, new->attr);
if (bgp_find_or_add_nexthop(bgp, bgp, nh_afi, safi, new, NULL,
connected)
connected, p)
|| CHECK_FLAG(peer->flags, PEER_FLAG_IS_RFAPI_HD))
bgp_path_info_set_flag(dest, new, BGP_PATH_VALID);
else {
@ -5683,7 +5684,7 @@ void bgp_static_update(struct bgp *bgp, const struct prefix *p,
if (bgp_find_or_add_nexthop(bgp, bgp_nexthop,
afi, safi, pi, NULL,
0))
0, p))
bgp_path_info_set_flag(dest, pi,
BGP_PATH_VALID);
else {
@ -5735,7 +5736,8 @@ void bgp_static_update(struct bgp *bgp, const struct prefix *p,
/* Nexthop reachability check. */
if (CHECK_FLAG(bgp->flags, BGP_FLAG_IMPORT_CHECK)
&& (safi == SAFI_UNICAST || safi == SAFI_LABELED_UNICAST)) {
if (bgp_find_or_add_nexthop(bgp, bgp, afi, safi, new, NULL, 0))
if (bgp_find_or_add_nexthop(bgp, bgp, afi, safi, new, NULL, 0,
p))
bgp_path_info_set_flag(dest, new, BGP_PATH_VALID);
else {
if (BGP_DEBUG(nht, NHT)) {

View File

@ -1134,11 +1134,6 @@ def test_bgp_with_loopback_with_same_subnet_p1(request):
dut = "r1"
protocol = "bgp"
for addr_type in ADDR_TYPES:
result = verify_rib(tgen, addr_type, dut, input_dict_r1, protocol=protocol)
assert result is True, "Testcase {} :Failed \n Error: {}".format(
tc_name, result
)
result = verify_fib_routes(tgen, addr_type, dut, input_dict_r1, expected=False)
assert result is not True, "Testcase {} : Failed \n"
"Expected behavior: routes should not present in fib \n"
@ -1156,13 +1151,6 @@ def test_bgp_with_loopback_with_same_subnet_p1(request):
dut = "r3"
protocol = "bgp"
for addr_type in ADDR_TYPES:
result = verify_rib(
tgen, addr_type, dut, input_dict_r3, protocol=protocol, fib=None
)
assert result is True, "Testcase {} :Failed \n Error: {}".format(
tc_name, result
)
result = verify_fib_routes(tgen, addr_type, dut, input_dict_r1, expected=False)
assert result is not True, "Testcase {} : Failed \n"
"Expected behavior: routes should not present in fib \n"