From c8e624bfd5bda89545fca853540d4f2fc7e25d83 Mon Sep 17 00:00:00 2001 From: varuntumbe Date: Thu, 12 Dec 2024 17:55:37 +0530 Subject: [PATCH 1/2] topotests: Adding a test to control release of BGP labels Adding the topotest which verifies whether label beloning to corresponding chunk has been released properly or not once we remove the vpn session Signed-off-by: Varun Hegde --- .../bgp_vpnv4_ebgp_vpn_auto/__init__.py | 0 .../bgp_vpnv4_ebgp_vpn_auto/r1/frr.conf | 117 +++++++++++ .../bgp_vpnv4_ebgp_vpn_auto/r2/frr.conf | 88 ++++++++ .../bgp_vpnv4_ebgp_vpn_auto/r3/frr.conf | 32 +++ .../test_bgp_vpnv4_vpn_auto.py | 191 ++++++++++++++++++ 5 files changed, 428 insertions(+) create mode 100644 tests/topotests/bgp_vpnv4_ebgp_vpn_auto/__init__.py create mode 100644 tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r1/frr.conf create mode 100644 tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r2/frr.conf create mode 100644 tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r3/frr.conf create mode 100644 tests/topotests/bgp_vpnv4_ebgp_vpn_auto/test_bgp_vpnv4_vpn_auto.py diff --git a/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/__init__.py b/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r1/frr.conf b/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r1/frr.conf new file mode 100644 index 000000000000..7daf335aabc7 --- /dev/null +++ b/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r1/frr.conf @@ -0,0 +1,117 @@ +interface r1-eth1 vrf vrf1 + ip address 173.31.1.1/32 +! +interface r1-eth2 vrf vrf2 + ip address 173.31.1.2/32 +! +interface r1-eth3 vrf vrf3 + ip address 173.31.1.3/32 +! +interface r1-eth4 vrf vrf4 + ip address 173.31.1.4/32 +! +interface r1-eth5 vrf vrf5 + ip address 173.31.1.5/32 +! + +interface r1-eth0 + ip address 192.168.0.1/24 +! + +interface r1-eth6 + ip address 193.170.0.1/24 + +interface lo + ip address 11.11.11.11/32 +! +router ospf + ospf router-id 11.11.11.11 + network 193.170.0.0/24 area 0.0.0.0 + network 11.11.11.11/32 area 0.0.0.0 + redistribute connected +exit +! +mpls ldp + router-id 11.11.11.11 + ! + address-family ipv4 + discovery transport-address 11.11.11.11 + ! + interface r1-eth6 + exit + ! + exit-address-family + ! +exit +! +bgp route-map delay-timer 1 +router bgp 65500 + bgp router-id 192.0.2.1 + no bgp ebgp-requires-policy + neighbor 192.168.0.2 remote-as 65501 + address-family ipv4 unicast + no neighbor 192.168.0.2 activate + exit-address-family + address-family ipv4 vpn + neighbor 192.168.0.2 activate + exit-address-family +! +router bgp 65500 vrf vrf1 + bgp router-id 192.0.2.1 + address-family ipv4 unicast + redistribute connected + label vpn export auto + rd vpn export 445:1 + rt vpn both 53:1 + export vpn + import vpn + exit-address-family +! +router bgp 65500 vrf vrf2 + bgp router-id 192.0.2.1 + address-family ipv4 unicast + redistribute connected + label vpn export auto + rd vpn export 445:2 + rt vpn both 53:2 + export vpn + import vpn + exit-address-family +! +router bgp 65500 vrf vrf3 + bgp router-id 192.0.2.1 + address-family ipv4 unicast + redistribute connected + label vpn export auto + rd vpn export 445:3 + rt vpn both 53:3 + export vpn + import vpn + exit-address-family +! +router bgp 65500 vrf vrf4 + bgp router-id 192.0.2.1 + address-family ipv4 unicast + redistribute connected + label vpn export auto + rd vpn export 445:4 + rt vpn both 53:4 + export vpn + import vpn + exit-address-family +! +router bgp 65500 vrf vrf5 + bgp router-id 192.0.2.1 + address-family ipv4 unicast + redistribute connected + label vpn export auto + rd vpn export 445:5 + rt vpn both 53:5 + export vpn + import vpn + exit-address-family +! + +interface r1-eth0 + mpls bgp forwarding +! \ No newline at end of file diff --git a/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r2/frr.conf b/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r2/frr.conf new file mode 100644 index 000000000000..6facebe40e05 --- /dev/null +++ b/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r2/frr.conf @@ -0,0 +1,88 @@ +interface r2-eth1 vrf vrf1 + ip address 173.31.0.1/32 +! +interface r2-eth2 vrf vrf2 + ip address 173.31.0.2/32 +! +interface r2-eth3 vrf vrf3 + ip address 173.31.0.3/32 +! +interface r2-eth4 vrf vrf4 + ip address 173.31.0.4/32 +! +interface r2-eth5 vrf vrf5 + ip address 173.31.0.5/32 +! +interface r2-eth0 + ip address 192.168.0.2/24 +! +router bgp 65501 + bgp router-id 192.0.2.2 + no bgp ebgp-requires-policy + neighbor 192.168.0.1 remote-as 65500 + address-family ipv4 unicast + no neighbor 192.168.0.1 activate + exit-address-family + address-family ipv4 vpn + neighbor 192.168.0.1 activate + exit-address-family +! +router bgp 65501 vrf vrf1 + bgp router-id 192.0.2.2 + address-family ipv4 unicast + redistribute connected + label vpn export auto + rd vpn export 445:1 + rt vpn both 53:1 + export vpn + import vpn + exit-address-family +! +router bgp 65501 vrf vrf2 + bgp router-id 192.0.2.2 + address-family ipv4 unicast + redistribute connected + label vpn export auto + rd vpn export 445:2 + rt vpn both 53:2 + export vpn + import vpn + exit-address-family +! +router bgp 65501 vrf vrf3 + bgp router-id 192.0.2.2 + address-family ipv4 unicast + redistribute connected + label vpn export auto + rd vpn export 445:3 + rt vpn both 53:3 + export vpn + import vpn + exit-address-family +! +router bgp 65501 vrf vrf4 + bgp router-id 192.0.2.2 + address-family ipv4 unicast + redistribute connected + label vpn export auto + rd vpn export 445:4 + rt vpn both 53:4 + export vpn + import vpn + exit-address-family +! +router bgp 65501 vrf vrf5 + bgp router-id 192.0.2.2 + address-family ipv4 unicast + redistribute connected + label vpn export auto + rd vpn export 445:5 + rt vpn both 53:5 + export vpn + import vpn + exit-address-family +! + +interface r2-eth0 + mpls bgp forwarding +! diff --git a/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r3/frr.conf b/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r3/frr.conf new file mode 100644 index 000000000000..8f49cdfe0c85 --- /dev/null +++ b/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/r3/frr.conf @@ -0,0 +1,32 @@ +interface r3-eth0 + ip address 193.170.0.2/24 +! +interface lo + ip address 33.33.33.33/32 +! +interface r3-eth1 + ip address 180.170.0.2/32 +! +interface r3-eth2 + ip address 180.170.0.3/32 +! +router ospf + ospf router-id 33.33.33.33 + network 193.170.0.0/24 area 0.0.0.0 + network 33.33.33.33/32 area 0.0.0.0 + redistribute connected +exit +! +mpls ldp + router-id 33.33.33.33 + ! + address-family ipv4 + discovery transport-address 33.33.33.33 + ! + interface r3-eth0 + exit + ! + exit-address-family + ! +exit +! \ No newline at end of file diff --git a/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/test_bgp_vpnv4_vpn_auto.py b/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/test_bgp_vpnv4_vpn_auto.py new file mode 100644 index 000000000000..ed3cdca2f93c --- /dev/null +++ b/tests/topotests/bgp_vpnv4_ebgp_vpn_auto/test_bgp_vpnv4_vpn_auto.py @@ -0,0 +1,191 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# +# test_bgp_vpnv4_vpn_auto.py +# +# Copyright (c) 2024 by Varun Hegde +# + +""" + test_bgp_vpnv4_vpn_auto.py: Test the FRR BGP daemon with BGP VPN session with label export auto +""" + +import os +import sys +import json +import functools +import pytest + +# Save the Current Working Directory to find configuration files. +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +# Import topogen and topotest helpers +from lib import topotest +from lib.bgpcheck import ( + check_show_bgp_vpn_prefix_found, + check_show_bgp_vpn_prefix_not_found, +) +from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.topolog import logger + +# Required to instantiate the topology builder class. + + +pytestmark = [pytest.mark.bgpd] + + +def build_topo(tgen): + "Build function" + + # Create 3 routers. + tgen.add_router("r1") + tgen.add_router("r2") + tgen.add_router("r3") + + + for i in range(6): + switch = tgen.add_switch("s{0}".format(i)) + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r2"]) + + #create a singiluar link between R2 -- R3 + switch = tgen.add_switch("s6") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r3"]) + + for i in range(7, 9): + switch = tgen.add_switch("s{0}".format(i)) + switch.add_link(tgen.gears["r3"]) + + + +def _populate_iface(): + tgen = get_topogen() + cmds_list = [ + "ip link add vrf{} type vrf table {}", + "ip link set dev vrf{} up", + "ip link set dev r1-eth{} master vrf{}", + "echo 1 > /proc/sys/net/mpls/conf/r1-eth{}/input", + ] + cmds_list2 = [ + "ip link add vrf{} type vrf table {}", + "ip link set dev vrf{} up", + "ip link set dev r2-eth{} master vrf{}", + "echo 1 > /proc/sys/net/mpls/conf/r2-eth{}/input", + ] + + for i in range(1, 6): + for cmd in cmds_list: + input = cmd.format(i, i) + logger.info("input: " + cmd) + output = tgen.net["r1"].cmd(cmd.format(i, i)) + logger.info("output: " + output) + + for cmd in cmds_list2: + input = cmd.format(i, i) + logger.info("input: " + cmd) + output = tgen.net["r2"].cmd(cmd.format(i, i)) + logger.info("output: " + output) + +def setup_module(mod): + "Sets up the pytest environment" + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + _populate_iface() + + for rname, router in router_list.items(): + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) + + # Initialize all routers. + tgen.start_router() + + +def teardown_module(_mod): + "Teardown the pytest environment" + tgen = get_topogen() + + tgen.stop_topology() + + +def test_labelpool_release(): + """ + Check that once we remove BGP VPN sesson + label pool structure ( allocated_map ) gets released properly or not + """ + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + # Just waiting for BGP VPN session to converge + logger.info("Waiting for BGP VPN sessions to converge and label pools to get initialised") + router = tgen.gears["r1"] + + def _bgp_converge(): + output = json.loads( + router.vtysh_cmd("show bgp labelpool summary json") + ) + expected = {"ledger":5,"inUse":5,"requests":0,"labelChunks":1,"pending":0,"reconnects":1} + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bgp_converge) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to see BGP Labelpool initialised" + + + # checking the initial label pool chunk's free labels + logger.info("checking the initial label pool chunk's free labels") + expected = [{"first":80,"last":207,"size":128,"numberFree":123}] + test_func = functools.partial( + topotest.router_json_cmp, + router, + "show bgp label chunks json", + expected, + ) + + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = '"{}" JSON output mismatches'.format(router.name) + assert result is None, assertmsg + + + # Test case : check whether label got released or not + logger.info( + "Remove multiple vpn session and check whether label got released or no" + ) + router.vtysh_cmd( + """ + configure terminal + no router bgp 65500 vrf vrf1 + no router bgp 65500 vrf vrf2 + """ + ) + expected = [{"first":80,"last":207,"size":128,"numberFree":125}] + test_func = functools.partial( + topotest.router_json_cmp, + router, + "show bgp label chunks json", + expected, + ) + + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = '"{}" JSON output mismatches'.format(router.name) + assert result is None, assertmsg + + + +def test_memory_leak(): + "Run the memory leak test and report results." + tgen = get_topogen() + if not tgen.is_memleak_enabled(): + pytest.skip("Memory leak test/report is disabled") + + tgen.report_memory_leaks() + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) From d5c2f2df199819bc7497db5a5d69a569eaccc1f9 Mon Sep 17 00:00:00 2001 From: varuntumbe Date: Thu, 12 Dec 2024 17:57:02 +0530 Subject: [PATCH 2/2] bgpd: Releasing the label in bgp_delete flow Releasing the vpn label from label pool chunk using bgp_lp_release routine whenever vpn session is removed. bgp_lp_release will clear corresponding bit in the allocated map of the label pool chunk and increases nfree by 1 Signed-off-by: Philippe Guibert --- bgpd/bgp_mplsvpn.c | 29 +++++++++++++++++++++++++++++ bgpd/bgp_mplsvpn.h | 1 + bgpd/bgp_vty.c | 21 ++------------------- bgpd/bgpd.c | 3 +++ 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index ca7f73dde993..e18293c0470c 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -4074,6 +4074,35 @@ void bgp_vpn_leak_export(struct bgp *from_bgp) } } +/* It releases the label from labelpool which + * was previously assigned and unsets the flag based on reset arg + * This also used in vty to release the label and to change the allocation mode as well + */ +void bgp_vpn_release_label(struct bgp *bgp, afi_t afi, bool reset) +{ + if (!CHECK_FLAG(bgp->vpn_policy[afi].flags, BGP_VPN_POLICY_TOVPN_LABEL_AUTO)) + return; + /* + * label has previously been automatically + * assigned by labelpool: release it + * + * NB if tovpn_label == MPLS_LABEL_NONE it + * means the automatic assignment is in flight + * and therefore the labelpool callback must + * detect that the auto label is not needed. + */ + if (bgp->vpn_policy[afi].tovpn_label == MPLS_LABEL_NONE) + return; + if (CHECK_FLAG(bgp->vpn_policy[afi].flags, BGP_VPN_POLICY_TOVPN_LABEL_PER_NEXTHOP)) + return; + + bgp_lp_release(LP_TYPE_VRF, &bgp->vpn_policy[afi], bgp->vpn_policy[afi].tovpn_label); + bgp->vpn_policy[afi].tovpn_label = MPLS_LABEL_NONE; + + if (reset) + UNSET_FLAG(bgp->vpn_policy[afi].flags, BGP_VPN_POLICY_TOVPN_LABEL_AUTO); +} + /* The nexthops values are compared to * find in the tree the appropriate cache entry */ diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h index 39fed667818a..18639fc69b23 100644 --- a/bgpd/bgp_mplsvpn.h +++ b/bgpd/bgp_mplsvpn.h @@ -352,6 +352,7 @@ extern void vpn_handle_router_id_update(struct bgp *bgp, bool withdraw, bool is_config); extern void bgp_vpn_leak_unimport(struct bgp *from_bgp); extern void bgp_vpn_leak_export(struct bgp *from_bgp); +extern void bgp_vpn_release_label(struct bgp *bgp, afi_t afi, bool reset); extern bool bgp_mplsvpn_path_uses_valid_mpls_label(struct bgp_path_info *pi); extern int diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 6ff94129dcf5..296ebaf0ac72 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -9941,26 +9941,9 @@ DEFPY (af_label_vpn_export, UNSET_FLAG(bgp->vpn_policy[afi].flags, BGP_VPN_POLICY_TOVPN_LABEL_MANUAL_REG); - } else if (CHECK_FLAG(bgp->vpn_policy[afi].flags, - BGP_VPN_POLICY_TOVPN_LABEL_AUTO)) { + } else if (CHECK_FLAG(bgp->vpn_policy[afi].flags, BGP_VPN_POLICY_TOVPN_LABEL_AUTO)) /* release any previous auto label */ - if (bgp->vpn_policy[afi].tovpn_label != MPLS_LABEL_NONE) { - - /* - * label has previously been automatically - * assigned by labelpool: release it - * - * NB if tovpn_label == MPLS_LABEL_NONE it - * means the automatic assignment is in flight - * and therefore the labelpool callback must - * detect that the auto label is not needed. - */ - - bgp_lp_release(LP_TYPE_VRF, - &bgp->vpn_policy[afi], - bgp->vpn_policy[afi].tovpn_label); - } - } + bgp_vpn_release_label(bgp, afi, false); if (yes) { if (label_auto) { diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index dccac3eceb19..0791919287b1 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -4006,6 +4006,9 @@ int bgp_delete(struct bgp *bgp) bgp_vpn_leak_unimport(bgp); + bgp_vpn_release_label(bgp, AFI_IP, true); + bgp_vpn_release_label(bgp, AFI_IP6, true); + hook_call(bgp_inst_delete, bgp); FOREACH_AFI_SAFI (afi, safi)