Skip to content

Commit

Permalink
bgpd: fix prefix same as nexthop in label per nexthop
Browse files Browse the repository at this point in the history
When a prefix is imported using the "network" command under a vrf, which
is a connected prefix, and in the context of label allocation per
nexthop:

..
>router bgp 1 vrf vrf1
> address-family ipv4 unicast
>  redistribute static
>  network 172.16.0.1/32  <--- connected network
>  network 192.168.106.0/29
>  label vpn export auto
>  label vpn export allocation-mode per-nexthop
..

We encounter an MPLS entry where the nexthop is the prefix itself:

> 18             BGP   172.16.0.1     -

Actually, when using the "network" command, a bnc context is used, but
it is filled by using the prefix itself instead of the nexthop for other
BGP updates. Consequently, when picking up the original nexthop for
label allocation, the function behaves incorrectly. Instead ensure that
the nexthop type of bnc->nexthop is not a nexthop_ifindex; otherwise
fallback to the per vrf label.

Update topotests.

Signed-off-by: Loïc Sang <[email protected]>
  • Loading branch information
Loïc Sang committed Oct 25, 2024
1 parent 47cdfbd commit 6975228
Show file tree
Hide file tree
Showing 3 changed files with 348 additions and 8 deletions.
9 changes: 9 additions & 0 deletions bgpd/bgp_mplsvpn.c
Original file line number Diff line number Diff line change
Expand Up @@ -1598,6 +1598,15 @@ vpn_leak_from_vrf_get_per_nexthop_label(afi_t afi, struct bgp_path_info *pi,
return bgp_mplsvpn_get_vpn_label(&from_bgp->vpn_policy[afi]);
}

if (is_bgp_static_route && pi->nexthop->nexthop->type == NEXTHOP_TYPE_IFINDEX) {
/* "network" imported prefixes from vrf
* fallback to per-vrf label.
*/

bgp_mplsvpn_path_nh_label_unlink(pi);
return bgp_mplsvpn_get_vpn_label(&from_bgp->vpn_policy[afi]);
}

return _vpn_leak_from_vrf_get_per_nexthop_label(pi, to_bgp, from_bgp,
afi);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,10 +347,37 @@ def check_show_bgp_vpn_prefix_found(router, ipversion, prefix, rd):
return topotest.json_cmp(output, expected)


def check_show_mpls_table_entry_label_found(router, inlabel, interface):
output = json.loads(router.vtysh_cmd("show mpls table {} json".format(inlabel)))
def check_show_mpls_table_entry_label_found_nexthop(
expected_router, get_router, network, nexthop
):
label = get_mpls_label(get_router, network)
if label < 0:
return False

output = json.loads(
expected_router.vtysh_cmd("show mpls table {} json".format(label))
)
expected = {
"inLabel": label,
"installed": True,
"nexthops": [{"nexthop": nexthop}],
}
return topotest.json_cmp(output, expected)


def check_show_mpls_table_entry_label_found(
expected_router, get_router, interface, network=None, label=None
):
if not label:
label = get_mpls_label(get_router, network)
if label < 0:
return False

output = json.loads(
expected_router.vtysh_cmd("show mpls table {} json".format(label))
)
expected = {
"inLabel": inlabel,
"inLabel": label,
"installed": True,
"nexthops": [{"interface": interface}],
}
Expand Down Expand Up @@ -395,6 +422,17 @@ def mpls_entry_get_interface(router, label):
return outgoing_interface


def get_mpls_label(router, network):
label = router.vtysh_cmd(
"show ip route vrf vrf1 %s json" % network,
isjson=True,
)
label = label.get(f"{network}", [{}])[0].get("nexthops", [{}])[0]
label = int(label.get("labels", [-1])[0])

return label


def test_protocols_convergence():
"""
Assert that all protocols have converged
Expand Down Expand Up @@ -719,7 +757,11 @@ def test_changing_default_label_value():
"r1, mpls table, check that MPLS entry with inLabel set to 222 has vrf1 interface"
)
test_func = functools.partial(
check_show_mpls_table_entry_label_found, router, 222, "vrf1"
check_show_mpls_table_entry_label_found,
tgen.gears["r1"],
router,
"vrf1",
label=222,
)
success, _ = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
assert success, "r1, mpls entry with label 222 not found"
Expand Down Expand Up @@ -837,6 +879,126 @@ def test_reconfigure_allocation_mode_nexthop():
mpls_table_check(router, label_list=label_list)


def test_network_command():
"""
Test with network declaration
"""
tgen = get_topogen()
if tgen.routers_have_failure():
pytest.skip(tgen.errors)
logger.info("Disabling redistribute static")
tgen.gears["r1"].vtysh_cmd(
"configure terminal\nrouter bgp 65500 vrf vrf1\naddress-family ipv4 unicast\nno redistribute static\n",
isjson=False,
)
logger.info("Checking BGP VPNv4 labels on r2")
for p in ["172.31.0.24/32", "172.31.0.15/32"]:
test_func = functools.partial(
check_show_bgp_vpn_prefix_not_found, tgen.gears["r2"], "ipv4", p, "444:1"
)
success, _ = topotest.run_and_expect(test_func, None, count=10, wait=3)
assert success, "network %s should not present on r2" % p

logger.info("Use network command for host networks declared in static instead")
tgen.gears["r1"].vtysh_cmd(
"configure terminal\nrouter bgp 65500 vrf vrf1\naddress-family ipv4 unicast\nnetwork 172.31.0.14/32\n",
isjson=False,
)
tgen.gears["r1"].vtysh_cmd(
"configure terminal\nrouter bgp 65500 vrf vrf1\naddress-family ipv4 unicast\nnetwork 172.31.0.15/32\n",
isjson=False,
)
logger.info("Checking BGP VPNv4 labels on r2")
bgp_vpnv4_table_check(tgen.gears["r2"], group=["172.31.0.12/32", "172.31.0.15/32"])
bgp_vpnv4_table_check(tgen.gears["r2"], group=["172.31.0.14/32"])
mpls_table_check(tgen.gears["r1"], whitelist=["192.0.2.14"])

logger.info(" Remove network to 172.31.0.14/32")
tgen.gears["r1"].vtysh_cmd(
"configure terminal\nrouter bgp 65500 vrf vrf1\naddress-family ipv4 unicast\nno network 172.31.0.14/32\n",
isjson=False,
)
test_func = functools.partial(
check_show_bgp_vpn_prefix_not_found,
tgen.gears["r2"],
"ipv4",
"172.31.0.14/32",
"444:1",
)
success, _ = topotest.run_and_expect(test_func, None, count=10, wait=3)
assert success, "network 172.31.0.14/32 should not present on r2"
mpls_table_check(tgen.gears["r1"], blacklist=["192.0.2.14"])

logger.info("Disabling redistribute connected and enabling redistribute static")
tgen.gears["r1"].vtysh_cmd(
"configure terminal\nrouter bgp 65500 vrf vrf1\naddress-family ipv4 unicast\n"
"redistribute static\n no redistribute connected",
isjson=False,
)
logger.info("Use network command for connect network 192.168.255.0/24 in vrf")
tgen.gears["r1"].vtysh_cmd(
"configure terminal\nrouter bgp 65500 vrf vrf1\naddress-family ipv4 unicast\n"
"network 192.168.255.0/24\n",
isjson=False,
)
logger.info("Checking BGP VPNv4 labels on r2")
bgp_vpnv4_table_check(tgen.gears["r2"], group=["192.168.255.0/24"])
logger.info("Checking no mpls entry associated to 192.168.255.0/24")
mpls_table_check(tgen.gears["r1"], blacklist=["192.168.255.0"])
logger.info("Checking 192.168.255.0/24 fallback to vrf")
test_func = functools.partial(
check_show_mpls_table_entry_label_found,
tgen.gears["r1"],
tgen.gears["r2"],
"vrf1",
network="192.168.255.0/24",
)
success, _ = topotest.run_and_expect(test_func, None, count=10, wait=3)
assert success, "192.168.255.0/24 does not fallback to vrf"

logger.info(
"Use network command for statically routed network 192.168.3.0/24 in vrf"
)
tgen.gears["r1"].vtysh_cmd(
"configure terminal\nvrf vrf1\n" "ip route 192.168.3.0/24 192.0.2.11\n"
)
tgen.gears["r1"].vtysh_cmd(
"configure terminal\nrouter bgp 65500 vrf vrf1\naddress-family ipv4 unicast\n"
"network 192.168.3.0/24\n",
isjson=False,
)
logger.info("Checking 192.168.3.0/24 route on r2")
test_func = functools.partial(
check_show_mpls_table_entry_label_found_nexthop,
tgen.gears["r1"],
tgen.gears["r2"],
"192.168.3.0/24",
"192.0.2.11",
)
success, _ = topotest.run_and_expect(test_func, None, count=10, wait=3)
assert success, "label from r2 is not present on r1 for 192.168.3.0/24"

# diagnostic
logger.info("Dumping label nexthop table")
tgen.gears["r1"].vtysh_cmd("show bgp vrf vrf1 label-nexthop detail", isjson=False)
logger.info("Dumping bgp network import-check-table")
tgen.gears["r1"].vtysh_cmd(
"show bgp vrf vrf1 import-check-table detail", isjson=False
)

logger.info("Restoring 172.31.0.14 prefix on r1")
tgen.gears["r1"].vtysh_cmd(
"configure terminal\nrouter bgp 65500 vrf vrf1\naddress-family ipv4 unicast\nnetwork 172.31.0.14/32\n",
isjson=False,
)
logger.info("Restoring redistribute connected")
tgen.gears["r1"].vtysh_cmd(
"configure terminal\nrouter bgp 65500 vrf vrf1\naddress-family ipv4 unicast\n"
"no redistribute static\n redistribute connected",
isjson=False,
)


def test_memory_leak():
"Run the memory leak test and report results."
tgen = get_topogen()
Expand Down
Loading

0 comments on commit 6975228

Please sign in to comment.