diff options
| -rw-r--r-- | bgpd/bgp_bfd.c | 2 | ||||
| -rw-r--r-- | ospfd/ospf_nsm.c | 9 | ||||
| -rw-r--r-- | ospfd/ospf_te.c | 4 | ||||
| -rw-r--r-- | ospfd/ospf_vty.c | 2 | ||||
| -rw-r--r-- | tests/topotests/bgp_tcp_mss/bgp_vrf_tcp_mss.json | 222 | ||||
| -rw-r--r-- | tests/topotests/bgp_tcp_mss/test_bgp_vrf_tcp_mss.py | 766 | ||||
| -rw-r--r-- | tests/topotests/lib/bgp.py | 53 | ||||
| -rwxr-xr-x | tools/frr-reload.py | 590 | ||||
| -rw-r--r-- | tools/valgrind.supp | 8 | ||||
| -rw-r--r-- | zebra/main.c | 2 | ||||
| -rw-r--r-- | zebra/table_manager.c | 6 | ||||
| -rw-r--r-- | zebra/zebra_router.c | 3 |
12 files changed, 1335 insertions, 332 deletions
diff --git a/bgpd/bgp_bfd.c b/bgpd/bgp_bfd.c index f23e6b2e9b..ed54f42b0a 100644 --- a/bgpd/bgp_bfd.c +++ b/bgpd/bgp_bfd.c @@ -163,7 +163,7 @@ void bgp_peer_bfd_update_source(struct peer *p) return; /* Figure out the correct source to use. */ - if (CHECK_FLAG(p->flags, PEER_FLAG_UPDATE_SOURCE)) + if (CHECK_FLAG(p->flags, PEER_FLAG_UPDATE_SOURCE) && p->update_source) source = p->update_source; else source = p->su_local; diff --git a/ospfd/ospf_nsm.c b/ospfd/ospf_nsm.c index 865e7d37a8..e3cf1cfc8b 100644 --- a/ospfd/ospf_nsm.c +++ b/ospfd/ospf_nsm.c @@ -76,10 +76,11 @@ static int ospf_inactivity_timer(struct thread *thread) */ if (!OSPF_GR_IS_ACTIVE_HELPER(nbr)) OSPF_NSM_EVENT_SCHEDULE(nbr, NSM_InactivityTimer); - else if (IS_DEBUG_OSPF_GR) { - zlog_debug( - "%s, Acting as HELPER for this neighbour, So restart the dead timer", - __func__); + else { + if (IS_DEBUG_OSPF_GR) + zlog_debug( + "%s, Acting as HELPER for this neighbour, So restart the dead timer", + __func__); OSPF_NSM_TIMER_ON(nbr->t_inactivity, ospf_inactivity_timer, nbr->v_inactivity); } diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c index 03fa572859..c5d1079e91 100644 --- a/ospfd/ospf_te.c +++ b/ospfd/ospf_te.c @@ -2171,7 +2171,7 @@ static int ospf_te_parse_te(struct ls_ted *ted, struct ospf_lsa *lsa) if ((len == 0) || (ntohs(tlvh->type) != TE_TLV_LINK)) return 0; - sum = 0; + sum = sizeof(struct tlv_header); /* Browse sub-TLV and fulfill Link State Attributes */ for (tlvh = TLV_DATA(tlvh); sum < len; tlvh = TLV_HDR_NEXT(tlvh)) { uint32_t val32, tab32[2]; @@ -2377,7 +2377,7 @@ static int ospf_te_delete_te(struct ls_ted *ted, struct ospf_lsa *lsa) if (ntohs(tlvh->type) == TE_TLV_ROUTER_ADDR) tlvh = TLV_HDR_NEXT(tlvh); len = TLV_BODY_SIZE(tlvh); - sum = 0; + sum = sizeof(struct tlv_header); /* Browse sub-TLV to find Link ID */ for (tlvh = TLV_DATA(tlvh); sum < len; tlvh = TLV_HDR_NEXT(tlvh)) { diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index e7f18ae79c..5b9519ea59 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -4350,7 +4350,7 @@ static void show_ip_ospf_neighbor_sub(struct vty *vty, char buf[PREFIX_STRLEN]; char timebuf[OSPF_TIME_DUMP_SIZE]; json_object *json_neighbor = NULL, *json_neigh_array = NULL; - struct timeval res; + struct timeval res = {.tv_sec = 0, .tv_usec = 0}; long time_val = 0; char uptime[OSPF_TIME_DUMP_SIZE]; diff --git a/tests/topotests/bgp_tcp_mss/bgp_vrf_tcp_mss.json b/tests/topotests/bgp_tcp_mss/bgp_vrf_tcp_mss.json new file mode 100644 index 0000000000..17cee03fa3 --- /dev/null +++ b/tests/topotests/bgp_tcp_mss/bgp_vrf_tcp_mss.json @@ -0,0 +1,222 @@ +{ + "address_types": [ + "ipv4", + "ipv6" + ], + "ipv4base": "20.20.20.0", + "ipv4mask": 24, + "ipv6base": "fd00::", + "ipv6mask": 64, + "link_ip_start": { + "ipv4": "10.10.10.0", + "v4mask": 24, + "ipv6": "fd00::", + "v6mask": 64 + }, + "lo_prefix": { + "ipv4": "1.0.", + "v4mask": 24, + "ipv6": "2001:db8:f::", + "v6mask": 128 + }, + "routers": { + "r1": { + "links": { + "r2-link1": { + "ipv4": "auto", + "ipv6": "auto", + "vrf": "RED" + } + }, + "vrfs": [ + { + "name": "RED", + "id": "1" + } + ], + "bgp": [ + { + "local_as": "100", + "vrf": "RED", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r2": { + "dest_link": { + "r1-link1": {} + } + } + }, + "redistribute": [ + { + "redist_type": "static" + } + ] + } + }, + "ipv6": { + "unicast": { + "neighbor": { + "r2": { + "dest_link": { + "r1-link1": {} + } + } + }, + "redistribute": [ + { + "redist_type": "static" + } + ] + } + } + } + } + ] + }, + "r2": { + "links": { + "r1-link1": { + "ipv4": "auto", + "ipv6": "auto", + "vrf": "RED" + }, + "r3-link1": { + "ipv4": "auto", + "ipv6": "auto" + } + }, + "vrfs": [ + { + "name": "RED", + "id": "1" + } + ], + "bgp": [ + { + "local_as": "200", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r3": { + "dest_link": { + "r2-link1": {} + } + } + }, + "redistribute": [ + { + "redist_type": "static" + } + ] + } + }, + "ipv6": { + "unicast": { + "neighbor": { + "r3": { + "dest_link": { + "r2-link1": {} + } + } + }, + "redistribute": [ + { + "redist_type": "static" + } + ] + } + } + } + }, + { + "local_as": "200", + "vrf": "RED", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r1": { + "dest_link": { + "r2-link1": {} + } + } + }, + "redistribute": [ + { + "redist_type": "static" + } + ] + } + }, + "ipv6": { + "unicast": { + "neighbor": { + "r1": { + "dest_link": { + "r2-link1": {} + } + } + }, + "redistribute": [ + { + "redist_type": "static" + } + ] + } + } + } + } + ] + }, + "r3": { + "links": { + "r2-link1": { + "ipv4": "auto", + "ipv6": "auto" + } + }, + "bgp": [ + { + "local_as": "300", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r2": { + "dest_link": { + "r3-link1": {} + } + } + }, + "redistribute": [ + { + "redist_type": "static" + } + ] + } + }, + "ipv6": { + "unicast": { + "neighbor": { + "r2": { + "dest_link": { + "r3-link1": {} + } + } + }, + "redistribute": [ + { + "redist_type": "static" + } + ] + } + } + } + } + ] + } + } +}
\ No newline at end of file diff --git a/tests/topotests/bgp_tcp_mss/test_bgp_vrf_tcp_mss.py b/tests/topotests/bgp_tcp_mss/test_bgp_vrf_tcp_mss.py new file mode 100644 index 0000000000..c48bd8a439 --- /dev/null +++ b/tests/topotests/bgp_tcp_mss/test_bgp_vrf_tcp_mss.py @@ -0,0 +1,766 @@ +#!/usr/bin/env python + +# +# bgp_tcp_mss.py +# Part of NetDEF Topology Tests +# +# Copyright (c) 2021 by +# Shreenidhi A R <rshreenidhi@vmware.com> +# +# Permission to use, copy, modify, and/or distribute this software +# for any purpose with or without fee is hereby granted, provided +# that the above copyright notice and this permission notice appear +# in all copies. +# +# THE SOFTWARE IS PROVIDED "AS IS" AND NETDEF DISCLAIMS ALL WARRANTIES +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL NETDEF BE LIABLE FOR +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY +# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, +# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS +# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE +# OF THIS SOFTWARE. +# + +""" +bgp_tcp_mss_vrf.py: + +Need to verify if the tcp-mss value is reflected in the TCP session and in VRF. +""" + +import os +import sys +import json +import pytest +import functools +import platform +import socket +import subprocess + +# add after imports, before defining classes or functions: +pytestmark = [pytest.mark.bgpd] + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +from lib import topotest +from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.topojson import build_config_from_json +from lib.topolog import logger +import time +from lib.bgp import ( + clear_bgp, + clear_bgp_and_verify, + create_router_bgp, + modify_as_number, + verify_as_numbers, + verify_bgp_convergence, + verify_bgp_rib, + verify_bgp_timers_and_functionality, + verify_router_id, + verify_tcp_mss +) +from lib.common_config import ( + kill_router_daemons, + start_router_daemons, + addKernelRoute, + apply_raw_config, + check_address_types, + create_prefix_lists, + create_route_maps, + create_static_routes, + required_linux_kernel_version, + reset_config_on_routers, + start_topology, + step, + verify_admin_distance_for_static_routes, + verify_bgp_community, + verify_fib_routes, + verify_rib, + write_test_footer, + write_test_header +) + +pytestmark = [pytest.mark.bgpd] +# Global variables +NETWORK1_1 = {"ipv4": "1.1.1.1/32", "ipv6": "1::1/128"} +NETWORK1_2 = {"ipv4": "1.1.1.2/32", "ipv6": "1::2/128"} +NETWORK2_1 = {"ipv4": "2.1.1.1/32", "ipv6": "2::1/128"} +NETWORK2_2 = {"ipv4": "2.1.1.2/32", "ipv6": "2::2/128"} +NETWORK3_1 = {"ipv4": "3.1.1.1/32", "ipv6": "3::1/128"} +NETWORK3_2 = {"ipv4": "3.1.1.2/32", "ipv6": "3::2/128"} +NETWORK4_1 = {"ipv4": "4.1.1.1/32", "ipv6": "4::1/128"} +NETWORK4_2 = {"ipv4": "4.1.1.2/32", "ipv6": "4::2/128"} +NETWORK5_1 = {"ipv4": "5.1.1.1/32", "ipv6": "5::1/128"} +NETWORK5_2 = {"ipv4": "5.1.1.2/32", "ipv6": "5::2/128"} + +NEXT_HOP_IP = {"ipv4": "Null0", "ipv6": "Null0"} + +## File name +TCPDUMP_FILE="test_tcp_packet_test.txt" + +def setup_module(mod): + """ + Sets up the pytest environment + + * `mod`: module name + """ + global topo,TCPDUMP_FILE + + # Required linux kernel version for this suite to run. + result = required_linux_kernel_version("4.15") + if result is not True: + pytest.skip("Kernel requirements are not met") + + testsuite_run_time = time.asctime(time.localtime(time.time())) + step("Testsuite start time: {}".format(testsuite_run_time)) + step("=" * 40) + + step("Running setup_module to create topology") + + # This function initiates the topology build with Topogen... + json_file = "{}/bgp_vrf_tcp_mss.json".format(CWD) + tgen = Topogen(json_file, mod.__name__) + global topo + topo = tgen.json_topo + # ... and here it calls Mininet initialization functions. + # Starting topology, create tmp files which are loaded to routers + # to start deamons and then start routers + start_topology(tgen) + # Creating configuration from JSON + build_config_from_json(tgen, topo) + + global ADDR_TYPES + global BGP_CONVERGENCE + ADDR_TYPES = check_address_types() + BGP_CONVERGENCE = verify_bgp_convergence(tgen, topo) + assert BGP_CONVERGENCE is True, "setup_module :Failed \n Error: {}".format( + BGP_CONVERGENCE + ) + + step("Running setup_module() done") + +def teardown_module(): + """Teardown the pytest environment""" + + step("Running teardown_module to delete topology") + + tgen = get_topogen() + + # Stop toplogy and Remove tmp files + tgen.stop_topology() + + step( + "Testsuite end time: {}".format(time.asctime(time.localtime(time.time()))) + ) + step("=" * 40) + + +##################################################### +# +# Testcases +# +##################################################### + +def test_bgp_vrf_tcp_mss(request): + tgen = get_topogen() + tc_name = request.node.name + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + step("Verify the router failures") + if tgen.routers_have_failure(): + check_router_status(tgen) + + step("Configuring 5 static Routes in Router R3 with NULL0 as Next hop") + for addr_type in ADDR_TYPES: + static_routes_input = { + "r3": { + "static_routes": [ + { + "network": [NETWORK1_1[addr_type]] + [NETWORK1_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK2_1[addr_type]] + [NETWORK2_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK3_1[addr_type]] + [NETWORK3_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK4_1[addr_type]] + [NETWORK4_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK5_1[addr_type]] + [NETWORK5_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + ] + } + } + result = create_static_routes(tgen, static_routes_input) + assert result is True, "Testcase {} : Failed \n Error: {}".format( + tc_name, result + ) + + step("Verify the static Routes in R3 on default VRF") + for addr_type in ADDR_TYPES: + static_routes_input = { + "r3": { + "static_routes": [ + { + "network": [NETWORK1_1[addr_type]] + [NETWORK1_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK2_1[addr_type]] + [NETWORK2_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK3_1[addr_type]] + [NETWORK3_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK4_1[addr_type]] + [NETWORK4_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK5_1[addr_type]] + [NETWORK5_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + ] + } + } + dut = "r3" + result = verify_bgp_rib(tgen, addr_type, dut, static_routes_input) + assert result is True, "Testcase {} :Failed \n Error {}".format(tc_name, result) + + step("Verify the static Routes in R2 on default VRF") + for addr_type in ADDR_TYPES: + static_routes_input = { + "r3": { + "static_routes": [ + { + "network": [NETWORK1_1[addr_type]] + [NETWORK1_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK2_1[addr_type]] + [NETWORK2_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK3_1[addr_type]] + [NETWORK3_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK4_1[addr_type]] + [NETWORK4_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + { + "network": [NETWORK5_1[addr_type]] + [NETWORK5_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + }, + ] + } + } + dut = "r2" + result = verify_bgp_rib(tgen, addr_type, dut, static_routes_input) + assert result is True, "Testcase {} :Failed \n Error {}".format(tc_name, result) + + step("importing default vrf on R2 under VRF RED Address Family") + for addr_type in ADDR_TYPES: + input_import_vrf = { + "r2": { + "bgp": [ + { + "local_as": 200, + "vrf": "RED", + "address_family": { + addr_type: {"unicast": {"import": {"vrf": "default"}}} + }, + } + ] + } + } + result = create_router_bgp(tgen, topo, input_import_vrf) + assert result is True, "Testcase {} : Failed \n Error: {}".format( + tc_name, result + ) + + step("Verify the static Routes in R2 on RED VRF") + for addr_type in ADDR_TYPES: + static_routes_input = { + "r3": { + "static_routes": [ + { + "network": [NETWORK1_1[addr_type]] + [NETWORK1_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + "vrf": "RED", + }, + { + "network": [NETWORK2_1[addr_type]] + [NETWORK2_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + "vrf": "RED", + }, + { + "network": [NETWORK3_1[addr_type]] + [NETWORK3_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + "vrf": "RED", + }, + { + "network": [NETWORK4_1[addr_type]] + [NETWORK4_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + "vrf": "RED", + }, + { + "network": [NETWORK5_1[addr_type]] + [NETWORK5_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + "vrf": "RED", + }, + ] + } + } + dut = "r2" + result = verify_bgp_rib(tgen, addr_type, dut, static_routes_input) + assert result is True, "Testcase {} :Failed \n Error {}".format(tc_name, result) + + step("Verify the static Routes in R1 on RED VRF") + for addr_type in ADDR_TYPES: + static_routes_input = { + "r3": { + "static_routes": [ + { + "network": [NETWORK1_1[addr_type]] + [NETWORK1_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + "vrf": "RED", + }, + { + "network": [NETWORK2_1[addr_type]] + [NETWORK2_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + "vrf": "RED", + }, + { + "network": [NETWORK3_1[addr_type]] + [NETWORK3_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + "vrf": "RED", + }, + { + "network": [NETWORK4_1[addr_type]] + [NETWORK4_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + "vrf": "RED", + }, + { + "network": [NETWORK5_1[addr_type]] + [NETWORK5_2[addr_type]], + "next_hop": NEXT_HOP_IP[addr_type], + "vrf": "RED", + }, + ] + } + } + dut = "r1" + result = verify_bgp_rib(tgen, addr_type, dut, static_routes_input) + assert result is True, "Testcase {} :Failed \n Error {}".format(tc_name, result) + + step("Enabling tcp-mss 150 on Router R1 in VRF RED") + TCP_MSS = 150 + raw_config = { + "r1": { + "raw_config": [ + "router bgp {} vrf {}".format( + topo["routers"]["r1"]["bgp"][0]["local_as"], + topo["routers"]["r1"]["bgp"][0]["vrf"], + ), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r1-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r1-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + }, + } + + result = apply_raw_config(tgen, raw_config) + assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) + + step("Clearing BGP on R1 and R2 ") + for addr_type in ADDR_TYPES: + clear_bgp(tgen, addr_type, "r1", vrf=topo["routers"]["r1"]["bgp"][0]["vrf"]) + clear_bgp(tgen, addr_type, "r2", vrf=topo["routers"]["r2"]["bgp"][1]["vrf"]) + + step("Verify the BGP Convergence at R1 & R2 after Clear BGP") + r1_convergence = verify_bgp_convergence(tgen, topo, dut="r1") + assert ( + r1_convergence is True + ), "BGP convergence after Clear BGP :Failed \n Error: {}".format(r1_convergence) + r2_convergence = verify_bgp_convergence(tgen, topo, dut="r2") + assert ( + r2_convergence is True + ), "BGP convergence after Clear BGP :Failed \n Error: {}".format(r2_convergence) + + step("Verify the TCP-MSS value on both Router R1 and R2") + for addr_type in ADDR_TYPES: + dut = "r1" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r2"]["links"]["r1-link1"][addr_type].split("/")[0], + TCP_MSS, + "RED", + ) + assert tcp_mss_result is True, " TCP-MSS mismatch :Failed \n Error: {}".format( + tcp_mss_result + ) + + + + + step("Enabling tcp-mss 500 between R2 and R3 of VRF Default") + TCP_MSS = 500 + raw_config = { + "r2": { + "raw_config": [ + "router bgp {} ".format(topo["routers"]["r2"]["bgp"][0]["local_as"]), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r3"]["links"]["r2-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r3"]["links"]["r2-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + }, + "r3": { + "raw_config": [ + "router bgp {} ".format(topo["routers"]["r3"]["bgp"][0]["local_as"]), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r3-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r3-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + }, + } + result = apply_raw_config(tgen, raw_config) + assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) + + + + step("Clear BGP at router R2 and R3") + for addr_type in ADDR_TYPES: + clear_bgp(tgen, topo, "r2", addr_type) + clear_bgp(tgen, topo, "r3", addr_type) + + step("Verify the BGP Convergence at R2 & R3 after Clear BGP") + r1_convergence = verify_bgp_convergence(tgen, topo, dut="r2") + assert ( + r1_convergence is True + ), "BGP convergence after Clear BGP :Failed \n Error: {}".format(r2_convergence) + r2_convergence = verify_bgp_convergence(tgen, topo, dut="r3") + assert ( + r2_convergence is True + ), "BGP convergence after Clear BGP :Failed \n Error: {}".format(r2_convergence) + + step("Verify the TCP-MSS value on both Router R2 and R3") + for addr_type in ADDR_TYPES: + dut = "r2" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r3"]["links"]["r2-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ) + assert tcp_mss_result is True, " TCP-MSS mismatch :Failed \n Error: {}".format( + tcp_mss_result + ) + + dut = "r3" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r2"]["links"]["r3-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ) + assert tcp_mss_result is True, " TCP-MSS mismatch :Failed \n Error: {}".format( + tcp_mss_result + ) + + step("Removing tcp-mss 150 between R1 and R2 of VRF RED ") + TCP_MSS = 150 + raw_config = { + "r1": { + "raw_config": [ + "router bgp {} vrf {}".format( + topo["routers"]["r1"]["bgp"][0]["local_as"], + topo["routers"]["r1"]["bgp"][0]["vrf"], + ), + "no neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r1-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "no neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r1-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + } + } + result = apply_raw_config(tgen, raw_config) + assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) + raw_config = { + "r2": { + "raw_config": [ + "router bgp {} vrf {}".format( + topo["routers"]["r2"]["bgp"][0]["local_as"], + topo["routers"]["r2"]["bgp"][1]["vrf"], + ), + "no neighbor {} tcp-mss {}".format( + topo["routers"]["r1"]["links"]["r2-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "no neighbor {} tcp-mss {}".format( + topo["routers"]["r1"]["links"]["r2-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + } + } + result = apply_raw_config(tgen, raw_config) + assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) + + step("Verify the TCP-MSS value cleared on both Router R1 and R2") + for addr_type in ADDR_TYPES: + dut = "r1" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r2"]["links"]["r1-link1"][addr_type].split("/")[0], + TCP_MSS, + "RED", + ) + assert ( + tcp_mss_result is not True + ), " TCP-MSS mismatch :Failed \n Error: {}".format(tcp_mss_result) + + dut = "r2" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r1"]["links"]["r2-link1"]["ipv4"].split("/")[0], + TCP_MSS, + "RED", + ) + assert ( + tcp_mss_result is not True + ), " TCP-MSS mismatch :Failed \n Error: {}".format(tcp_mss_result) + + + step("Removing tcp-mss 500 between R2 and R3 of VRF Default ") + TCP_MSS = 500 + raw_config = { + "r2": { + "raw_config": [ + "router bgp {} ".format(topo["routers"]["r2"]["bgp"][0]["local_as"]), + "no neighbor {} tcp-mss {}".format( + topo["routers"]["r3"]["links"]["r2-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "no neighbor {} tcp-mss {}".format( + topo["routers"]["r3"]["links"]["r2-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + } + } + result = apply_raw_config(tgen, raw_config) + assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) + + raw_config = { + "r3": { + "raw_config": [ + "router bgp {} ".format(topo["routers"]["r3"]["bgp"][0]["local_as"]), + "no neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r3-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "no neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r3-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + } + } + result = apply_raw_config(tgen, raw_config) + assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) + + step("Verify the TCP-MSS value got cleared on both Router R2 and R3") + for addr_type in ADDR_TYPES: + dut = "r2" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r3"]["links"]["r2-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ) + assert ( + tcp_mss_result is not True + ), " TCP-MSS mismatch :Failed \n Error: {}".format(tcp_mss_result) + + dut = "r3" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r2"]["links"]["r3-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ) + assert ( + tcp_mss_result is not True + ), " TCP-MSS mismatch :Failed \n Error: {}".format(tcp_mss_result) + + step("Configuring different TCP-MSS R2 and R3 ") + TCP_MSS = 500 + raw_config = { + "r2": { + "raw_config": [ + "router bgp {}".format(topo["routers"]["r2"]["bgp"][0]["local_as"]), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r3"]["links"]["r2-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r3"]["links"]["r2-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + } + } + result = apply_raw_config(tgen, raw_config) + assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) + TCP_MSS = 300 + raw_config = { + "r3": { + "raw_config": [ + "router bgp {} ".format(topo["routers"]["r3"]["bgp"][0]["local_as"]), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r3-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r3-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + } + } + result = apply_raw_config(tgen, raw_config) + assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) + + step("Verify the TCP-MSS value on both Router R2 and R3") + for addr_type in ADDR_TYPES: + TCP_MSS = 500 + dut = "r2" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r3"]["links"]["r2-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ) + assert tcp_mss_result is True, " TCP-MSS mismatch :Failed \n Error: {}".format( + tcp_mss_result + ) + + TCP_MSS = 300 + dut = "r3" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r2"]["links"]["r3-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ) + assert tcp_mss_result is True, " TCP-MSS mismatch :Failed \n Error: {}".format( + tcp_mss_result + ) + + step("Configure TCP_MSS > MTU on R2 and R3 and it should be 1460 ") + TCP_MSS = 4096 + REF_TCP_MSS = 1460 + raw_config = { + "r2": { + "raw_config": [ + "router bgp {} ".format(topo["routers"]["r2"]["bgp"][0]["local_as"]), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r3"]["links"]["r2-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r3"]["links"]["r2-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + } + } + result = apply_raw_config(tgen, raw_config) + assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) + raw_config = { + "r3": { + "raw_config": [ + "router bgp {} ".format(topo["routers"]["r3"]["bgp"][0]["local_as"]), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r3-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ), + "neighbor {} tcp-mss {}".format( + topo["routers"]["r2"]["links"]["r3-link1"]["ipv6"].split("/")[0], + TCP_MSS, + ), + ] + } + } + result = apply_raw_config(tgen, raw_config) + assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) + + step("Restarting BGP Daemon on R3") + + kill_router_daemons(tgen, "r3", ["bgpd"]) + start_router_daemons(tgen, "r3", ["bgpd"]) + + step( + "Verify the configured TCP-MSS 4096 value on restarting both Daemon both Router R2 and R3 " + ) + for addr_type in ADDR_TYPES: + TCP_MSS = 4096 + dut = "r2" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r3"]["links"]["r2-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ) + assert tcp_mss_result is True, " TCP-MSS mismatch :Failed \n Error: {}".format( + tcp_mss_result + ) + dut = "r3" + tcp_mss_result = verify_tcp_mss( + tgen, + dut, + topo["routers"]["r2"]["links"]["r3-link1"]["ipv4"].split("/")[0], + TCP_MSS, + ) + assert tcp_mss_result is True, " TCP-MSS mismatch :Failed \n Error: {}".format( + tcp_mss_result + ) + write_test_footer(tc_name) + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) diff --git a/tests/topotests/lib/bgp.py b/tests/topotests/lib/bgp.py index 556240bfb5..458ae4b054 100644 --- a/tests/topotests/lib/bgp.py +++ b/tests/topotests/lib/bgp.py @@ -4428,3 +4428,56 @@ def verify_evpn_routes( logger.debug("Exiting lib API: {}".format(sys._getframe().f_code.co_name)) return False +def verify_tcp_mss(tgen, dut, neighbour, configured_tcp_mss, vrf=None): + """ + This api is used to verify the tcp-mss value assigned to a neigbour of DUT + + Parameters + ---------- + * `tgen` : topogen object + * `dut`: device under test + * `neighbour`:neigbout IP address + * `configured_tcp_mss`:The TCP-MSS value to be verified + * `vrf`:vrf + + Usage + ----- + result = verify_tcp_mss(tgen, dut,neighbour,configured_tcp_mss) + Returns + ------- + errormsg(str) or True + """ + + logger.debug("Entering lib API: {}".format(sys._getframe().f_code.co_name)) + rnode = tgen.routers()[dut] + if vrf: + cmd = "show bgp vrf {} neighbors {} json".format(vrf, neighbour) + else: + cmd = "show bgp neighbors {} json".format(neighbour) + + # Execute the command + show_vrf_stats = run_frr_cmd(rnode, cmd, isjson=True) + + # Verify TCP-MSS on router + logger.info("Verify that no core is observed") + if tgen.routers_have_failure(): + errormsg = "Core observed while running CLI: %s" % (cmd) + return errormsg + else: + if configured_tcp_mss == show_vrf_stats.get(neighbour).get( + "bgpTcpMssConfigured" + ): + logger.debug( + "Configured TCP - MSS Found: {}".format(sys._getframe().f_code.co_name) + ) + return True + else: + logger.debug( + "TCP-MSS Mismatch ,configured {} expecting {}".format( + show_vrf_stats.get(neighbour).get("bgpTcpMssConfigured"), + configured_tcp_mss, + ) + ) + return "TCP-MSS Mismatch" + logger.debug("Exiting lib API: {}".format(sys._getframe().f_code.co_name)) + return False diff --git a/tools/frr-reload.py b/tools/frr-reload.py index da51c231d1..8762b956b6 100755 --- a/tools/frr-reload.py +++ b/tools/frr-reload.py @@ -172,7 +172,6 @@ class Vtysh(object): class Context(object): - """ A Context object represents a section of frr configuration such as: ! @@ -234,7 +233,6 @@ def get_normalized_mac_ip_line(line): class Config(object): - """ A frr configuration is stored in a Config object. A Config object contains a dictionary of Context objects where the Context keys @@ -265,22 +263,20 @@ class Config(object): if ":" in line: line = get_normalized_mac_ip_line(line) - """ - vrf static routes can be added in two ways. The old way is: - - "ip route x.x.x.x/x y.y.y.y vrf <vrfname>" - - but it's rendered in the configuration as the new way:: - - vrf <vrf-name> - ip route x.x.x.x/x y.y.y.y - exit-vrf - - this difference causes frr-reload to not consider them a - match and delete vrf static routes incorrectly. - fix the old way to match new "show running" output so a - proper match is found. - """ + # vrf static routes can be added in two ways. The old way is: + # + # "ip route x.x.x.x/x y.y.y.y vrf <vrfname>" + # + # but it's rendered in the configuration as the new way:: + # + # vrf <vrf-name> + # ip route x.x.x.x/x y.y.y.y + # exit-vrf + # + # this difference causes frr-reload to not consider them a + # match and delete vrf static routes incorrectly. + # fix the old way to match new "show running" output so a + # proper match is found. if ( line.startswith("ip route ") or line.startswith("ipv6 route ") ) and " vrf " in line: @@ -326,14 +322,12 @@ class Config(object): """ Return the lines read in from the configuration """ - return "\n".join(self.lines) def get_contexts(self): """ Return the parsed context as strings for display, log etc. """ - for (_, ctx) in sorted(iteritems(self.contexts)): print(str(ctx) + "\n") @@ -341,18 +335,15 @@ class Config(object): """ Save the provided key and lines as a context """ - if not key: return - """ - IP addresses specified in "network" statements, "ip prefix-lists" - etc. can differ in the host part of the specification the user - provides and what the running config displays. For example, user - can specify 11.1.1.1/24, and the running config displays this as - 11.1.1.0/24. Ensure we don't do a needless operation for such - lines. IS-IS & OSPFv3 have no "network" support. - """ + # IP addresses specified in "network" statements, "ip prefix-lists" + # etc. can differ in the host part of the specification the user + # provides and what the running config displays. For example, user can + # specify 11.1.1.1/24, and the running config displays this as + # 11.1.1.0/24. Ensure we don't do a needless operation for such lines. + # IS-IS & OSPFv3 have no "network" support. re_key_rt = re.match(r"(ip|ipv6)\s+route\s+([A-Fa-f:.0-9/]+)(.*)$", key[0]) if re_key_rt: addr = re_key_rt.group(2) @@ -430,10 +421,8 @@ class Config(object): newlines.append(line) lines = newlines - """ - More fixups in user specification and what running config shows. - "null0" in routes must be replaced by Null0. - """ + # More fixups in user specification and what running config shows. + # "null0" in routes must be replaced by Null0. if ( key[0].startswith("ip route") or key[0].startswith("ipv6 route") @@ -441,10 +430,8 @@ class Config(object): ): key[0] = re.sub(r"\s+null0(\s*$)", " Null0", key[0]) - """ - Similar to above, but when the static is in a vrf, it turns into a - blackhole nexthop for both null0 and Null0. Fix it accordingly - """ + # Similar to above, but when the static is in a vrf, it turns into a + # blackhole nexthop for both null0 and Null0. Fix it accordingly if lines and key[0].startswith("vrf "): newlines = [] for line in lines: @@ -474,71 +461,69 @@ class Config(object): def load_contexts(self): """ Parse the configuration and create contexts for each appropriate block - """ - """ The end of a context is flagged via the 'end' keyword: -! -interface swp52 - ipv6 nd suppress-ra - link-detect -! -end -router bgp 10 - bgp router-id 10.0.0.1 - bgp log-neighbor-changes - no bgp default ipv4-unicast - neighbor EBGP peer-group - neighbor EBGP advertisement-interval 1 - neighbor EBGP timers connect 10 - neighbor 2001:40:1:4::6 remote-as 40 - neighbor 2001:40:1:8::a remote-as 40 -! -end - address-family ipv6 - neighbor IBGPv6 activate - neighbor 2001:10::2 peer-group IBGPv6 - neighbor 2001:10::3 peer-group IBGPv6 - exit-address-family -! -end - address-family evpn - neighbor LEAF activate - advertise-all-vni - vni 10100 - rd 65000:10100 - route-target import 10.1.1.1:10100 - route-target export 10.1.1.1:10100 - exit-vni - exit-address-family -! -end -router ospf - ospf router-id 10.0.0.1 - log-adjacency-changes detail - timers throttle spf 0 50 5000 -! -end + ! + interface swp52 + ipv6 nd suppress-ra + link-detect + ! + end + router bgp 10 + bgp router-id 10.0.0.1 + bgp log-neighbor-changes + no bgp default ipv4-unicast + neighbor EBGP peer-group + neighbor EBGP advertisement-interval 1 + neighbor EBGP timers connect 10 + neighbor 2001:40:1:4::6 remote-as 40 + neighbor 2001:40:1:8::a remote-as 40 + ! + end + address-family ipv6 + neighbor IBGPv6 activate + neighbor 2001:10::2 peer-group IBGPv6 + neighbor 2001:10::3 peer-group IBGPv6 + exit-address-family + ! + end + address-family evpn + neighbor LEAF activate + advertise-all-vni + vni 10100 + rd 65000:10100 + route-target import 10.1.1.1:10100 + route-target export 10.1.1.1:10100 + exit-vni + exit-address-family + ! + end + router ospf + ospf router-id 10.0.0.1 + log-adjacency-changes detail + timers throttle spf 0 50 5000 + ! + end + + The code assumes that its working on the output from the "vtysh -m" + command. That provides the appropriate markers to signify end of + a context. This routine uses that to build the contexts for the + config. + + There are single line contexts such as "log file /media/node/zebra.log" + and multi-line contexts such as "router ospf" and subcontexts + within a context such as "address-family" within "router bgp" + In each of these cases, the first line of the context becomes the + key of the context. So "router bgp 10" is the key for the non-address + family part of bgp, "router bgp 10, address-family ipv6 unicast" is + the key for the subcontext and so on. + + This dictionary contains a tree of all commands that we know start a + new multi-line context. All other commands are treated either as + commands inside a multi-line context or as single-line contexts. This + dictionary should be updated whenever a new node is added to FRR. """ - - # The code assumes that its working on the output from the "vtysh -m" - # command. That provides the appropriate markers to signify end of - # a context. This routine uses that to build the contexts for the - # config. - # - # There are single line contexts such as "log file /media/node/zebra.log" - # and multi-line contexts such as "router ospf" and subcontexts - # within a context such as "address-family" within "router bgp" - # In each of these cases, the first line of the context becomes the - # key of the context. So "router bgp 10" is the key for the non-address - # family part of bgp, "router bgp 10, address-family ipv6 unicast" is - # the key for the subcontext and so on. - - # This dictionary contains a tree of all commands that we know start a - # new multi-line context. All other commands are treated either as - # commands inside a multi-line context or as single-line contexts. This - # dictionary should be updated whenever a new node is added to FRR. ctx_keywords = { "router bgp ": { "address-family ": { @@ -788,16 +773,12 @@ def check_for_exit_vrf(lines_to_add, lines_to_del): return (lines_to_add, lines_to_del) -""" -This method handles deletion of bgp peer group config. -The objective is to delete config lines related to peers -associated with the peer-group and move the peer-group -config line to the end of the lines_to_del list. -""" - - def delete_move_lines(lines_to_add, lines_to_del): - + """ + This function handles deletion of bgp peer group config. The objective is + to delete config lines related to peers associated with the peer-group and + move the peer-group config line to the end of the lines_to_del list. + """ del_dict = dict() # Stores the lines to move to the end of the pending list. lines_to_del_to_del = [] @@ -805,54 +786,51 @@ def delete_move_lines(lines_to_add, lines_to_del): lines_to_del_to_app = [] found_pg_del_cmd = False - """ - When "neighbor <pg_name> peer-group" under a bgp instance is removed, - it also deletes the associated peer config. Any config line below no form of - peer-group related to a peer are errored out as the peer no longer exists. - To cleanup peer-group and associated peer(s) configs: - - Remove all the peers config lines from the pending list (lines_to_del list). - - Move peer-group deletion line to the end of the pending list, to allow - removal of any of the peer-group specific configs. - - Create a dictionary of config context (i.e. router bgp vrf x). - Under each context node, create a dictionary of a peer-group name. - Append a peer associated to the peer-group into a list under a peer-group node. - Remove all of the peer associated config lines from the pending list. - Append peer-group deletion line to end of the pending list. - - Example: - neighbor underlay peer-group - neighbor underlay remote-as external - neighbor underlay advertisement-interval 0 - neighbor underlay timers 3 9 - neighbor underlay timers connect 10 - neighbor swp1 interface peer-group underlay - neighbor swp1 advertisement-interval 0 - neighbor swp1 timers 3 9 - neighbor swp1 timers connect 10 - neighbor swp2 interface peer-group underlay - neighbor swp2 advertisement-interval 0 - neighbor swp2 timers 3 9 - neighbor swp2 timers connect 10 - neighbor swp3 interface peer-group underlay - neighbor uplink1 interface remote-as internal - neighbor uplink1 advertisement-interval 0 - neighbor uplink1 timers 3 9 - neighbor uplink1 timers connect 10 - - New order: - "router bgp 200 no bgp bestpath as-path multipath-relax" - "router bgp 200 no neighbor underlay advertisement-interval 0" - "router bgp 200 no neighbor underlay timers 3 9" - "router bgp 200 no neighbor underlay timers connect 10" - "router bgp 200 no neighbor uplink1 advertisement-interval 0" - "router bgp 200 no neighbor uplink1 timers 3 9" - "router bgp 200 no neighbor uplink1 timers connect 10" - "router bgp 200 no neighbor underlay remote-as external" - "router bgp 200 no neighbor uplink1 interface remote-as internal" - "router bgp 200 no neighbor underlay peer-group" - - """ + # When "neighbor <pg_name> peer-group" under a bgp instance is removed, + # it also deletes the associated peer config. Any config line below no form of + # peer-group related to a peer are errored out as the peer no longer exists. + # To cleanup peer-group and associated peer(s) configs: + # - Remove all the peers config lines from the pending list (lines_to_del list). + # - Move peer-group deletion line to the end of the pending list, to allow + # removal of any of the peer-group specific configs. + # + # Create a dictionary of config context (i.e. router bgp vrf x). + # Under each context node, create a dictionary of a peer-group name. + # Append a peer associated to the peer-group into a list under a peer-group node. + # Remove all of the peer associated config lines from the pending list. + # Append peer-group deletion line to end of the pending list. + # + # Example: + # neighbor underlay peer-group + # neighbor underlay remote-as external + # neighbor underlay advertisement-interval 0 + # neighbor underlay timers 3 9 + # neighbor underlay timers connect 10 + # neighbor swp1 interface peer-group underlay + # neighbor swp1 advertisement-interval 0 + # neighbor swp1 timers 3 9 + # neighbor swp1 timers connect 10 + # neighbor swp2 interface peer-group underlay + # neighbor swp2 advertisement-interval 0 + # neighbor swp2 timers 3 9 + # neighbor swp2 timers connect 10 + # neighbor swp3 interface peer-group underlay + # neighbor uplink1 interface remote-as internal + # neighbor uplink1 advertisement-interval 0 + # neighbor uplink1 timers 3 9 + # neighbor uplink1 timers connect 10 + + # New order: + # "router bgp 200 no bgp bestpath as-path multipath-relax" + # "router bgp 200 no neighbor underlay advertisement-interval 0" + # "router bgp 200 no neighbor underlay timers 3 9" + # "router bgp 200 no neighbor underlay timers connect 10" + # "router bgp 200 no neighbor uplink1 advertisement-interval 0" + # "router bgp 200 no neighbor uplink1 timers 3 9" + # "router bgp 200 no neighbor uplink1 timers connect 10" + # "router bgp 200 no neighbor underlay remote-as external" + # "router bgp 200 no neighbor uplink1 interface remote-as internal" + # "router bgp 200 no neighbor underlay peer-group" for (ctx_keys, line) in lines_to_del: if ( @@ -860,37 +838,33 @@ def delete_move_lines(lines_to_add, lines_to_del): and line and line.startswith("neighbor ") ): - """ - When 'neighbor <peer> remote-as <>' is removed it deletes the peer, - there might be a peer associated config which also needs to be removed - prior to peer. - Append the 'neighbor <peer> remote-as <>' to the lines_to_del. - Example: - - neighbor uplink1 interface remote-as internal - neighbor uplink1 advertisement-interval 0 - neighbor uplink1 timers 3 9 - neighbor uplink1 timers connect 10 - - Move to end: - neighbor uplink1 advertisement-interval 0 - neighbor uplink1 timers 3 9 - neighbor uplink1 timers connect 10 - ... - - neighbor uplink1 interface remote-as internal - - """ + # When 'neighbor <peer> remote-as <>' is removed it deletes the peer, + # there might be a peer associated config which also needs to be removed + # prior to peer. + # Append the 'neighbor <peer> remote-as <>' to the lines_to_del. + # Example: + # + # neighbor uplink1 interface remote-as internal + # neighbor uplink1 advertisement-interval 0 + # neighbor uplink1 timers 3 9 + # neighbor uplink1 timers connect 10 + + # Move to end: + # neighbor uplink1 advertisement-interval 0 + # neighbor uplink1 timers 3 9 + # neighbor uplink1 timers connect 10 + # ... + # + # neighbor uplink1 interface remote-as internal + # # 'no neighbor peer [interface] remote-as <>' nb_remoteas = "neighbor (\S+) .*remote-as (\S+)" re_nb_remoteas = re.search(nb_remoteas, line) if re_nb_remoteas: lines_to_del_to_app.append((ctx_keys, line)) - """ - {'router bgp 65001': {'PG': [], 'PG1': []}, - 'router bgp 65001 vrf vrf1': {'PG': [], 'PG1': []}} - """ + # {'router bgp 65001': {'PG': [], 'PG1': []}, + # 'router bgp 65001 vrf vrf1': {'PG': [], 'PG1': []}} if ctx_keys[0] not in del_dict: del_dict[ctx_keys[0]] = dict() # find 'no neighbor <pg_name> peer-group' @@ -905,10 +879,8 @@ def delete_move_lines(lines_to_add, lines_to_del): if found_pg_del_cmd == False: return (lines_to_add, lines_to_del) - """ - {'router bgp 65001': {'PG': ['10.1.1.2'], 'PG1': ['10.1.1.21']}, - 'router bgp 65001 vrf vrf1': {'PG': ['10.1.1.2'], 'PG1': ['10.1.1.21']}} - """ + # {'router bgp 65001': {'PG': ['10.1.1.2'], 'PG1': ['10.1.1.21']}, + # 'router bgp 65001 vrf vrf1': {'PG': ['10.1.1.2'], 'PG1': ['10.1.1.21']}} for (ctx_keys, line) in lines_to_del: if ( ctx_keys[0].startswith("router bgp") @@ -982,25 +954,22 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del): if ctx_keys[0].startswith("router bgp") and line: if line.startswith("neighbor "): - """ - BGP changed how it displays swpX peers that are part of peer-group. Older - versions of frr would display these on separate lines: - neighbor swp1 interface - neighbor swp1 peer-group FOO - - but today we display via a single line - neighbor swp1 interface peer-group FOO - - This change confuses frr-reload.py so check to see if we are deleting - neighbor swp1 interface peer-group FOO - - and adding - neighbor swp1 interface - neighbor swp1 peer-group FOO - - If so then chop the del line and the corresponding add lines - """ - + # BGP changed how it displays swpX peers that are part of peer-group. Older + # versions of frr would display these on separate lines: + # neighbor swp1 interface + # neighbor swp1 peer-group FOO + # + # but today we display via a single line + # neighbor swp1 interface peer-group FOO + # + # This change confuses frr-reload.py so check to see if we are deleting + # neighbor swp1 interface peer-group FOO + # + # and adding + # neighbor swp1 interface + # neighbor swp1 peer-group FOO + # + # If so then chop the del line and the corresponding add lines re_swpx_int_peergroup = re.search( "neighbor (\S+) interface peer-group (\S+)", line ) @@ -1052,12 +1021,10 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del): lines_to_add_to_del.append((ctx_keys, swpx_interface)) lines_to_add_to_del.append((tmp_ctx_keys, swpx_peergroup)) - """ - Changing the bfd timers on neighbors is allowed without doing - a delete/add process. Since doing a "no neighbor blah bfd ..." - will cause the peer to bounce unnecessarily, just skip the delete - and just do the add. - """ + # Changing the bfd timers on neighbors is allowed without doing + # a delete/add process. Since doing a "no neighbor blah bfd + # ..." will cause the peer to bounce unnecessarily, just skip + # the delete and just do the add. re_nbr_bfd_timers = re.search( r"neighbor (\S+) bfd (\S+) (\S+) (\S+)", line ) @@ -1081,16 +1048,14 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del): if found_add_bfd_nbr: lines_to_del_to_del.append((ctx_keys, line)) - """ - Neighbor changes of route-maps need to be accounted for in that we - do not want to do a `no route-map...` `route-map ....` when changing - a route-map. This is bad mojo as that we will send/receive - data we don't want. - Additionally we need to ensure that if we have different afi/safi - variants that they actually match and if we are going from a very - old style command such that the neighbor command is under the - `router bgp ..` node that we need to handle that appropriately - """ + # Neighbor changes of route-maps need to be accounted for in + # that we do not want to do a `no route-map...` `route-map + # ....` when changing a route-map. This is bad mojo as that we + # will send/receive data we don't want. Additionally we need + # to ensure that if we have different afi/safi variants that + # they actually match and if we are going from a very old style + # command such that the neighbor command is under the `router + # bgp ..` node that we need to handle that appropriately re_nbr_rm = re.search("neighbor(.*)route-map(.*)(in|out)$", line) if re_nbr_rm: adjust_for_bgp_node = 0 @@ -1128,29 +1093,27 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del): if save_line == dl_line: lines_to_del_to_del.append((ctx_keys_dl, save_line)) - """ - We changed how we display the neighbor interface command. Older - versions of frr would display the following: - neighbor swp1 interface - neighbor swp1 remote-as external - neighbor swp1 capability extended-nexthop - - but today we display via a single line - neighbor swp1 interface remote-as external - - and capability extended-nexthop is no longer needed because we - automatically enable it when the neighbor is of type interface. - - This change confuses frr-reload.py so check to see if we are deleting - neighbor swp1 interface remote-as (external|internal|ASNUM) - - and adding - neighbor swp1 interface - neighbor swp1 remote-as (external|internal|ASNUM) - neighbor swp1 capability extended-nexthop - - If so then chop the del line and the corresponding add lines - """ + # We changed how we display the neighbor interface command. Older + # versions of frr would display the following: + # neighbor swp1 interface + # neighbor swp1 remote-as external + # neighbor swp1 capability extended-nexthop + # + # but today we display via a single line + # neighbor swp1 interface remote-as external + # + # and capability extended-nexthop is no longer needed because we + # automatically enable it when the neighbor is of type interface. + # + # This change confuses frr-reload.py so check to see if we are deleting + # neighbor swp1 interface remote-as (external|internal|ASNUM) + # + # and adding + # neighbor swp1 interface + # neighbor swp1 remote-as (external|internal|ASNUM) + # neighbor swp1 capability extended-nexthop + # + # If so then chop the del line and the corresponding add lines re_swpx_int_remoteas = re.search( "neighbor (\S+) interface remote-as (\S+)", line ) @@ -1186,15 +1149,13 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del): lines_to_add_to_del.append((ctx_keys, swpx_interface)) lines_to_add_to_del.append((tmp_ctx_keys, swpx_remoteas)) - """ - We made the 'bgp bestpath as-path multipath-relax' command - automatically assume 'no-as-set' since the lack of this option caused - weird routing problems. When the running config is shown in - releases with this change, the no-as-set keyword is not shown as it - is the default. This causes frr-reload to unnecessarily unapply - this option only to apply it back again, causing unnecessary session - resets. - """ + # We made the 'bgp bestpath as-path multipath-relax' command + # automatically assume 'no-as-set' since the lack of this option + # caused weird routing problems. When the running config is shown + # in releases with this change, the no-as-set keyword is not shown + # as it is the default. This causes frr-reload to unnecessarily + # unapply this option only to apply it back again, causing + # unnecessary session resets. if "multipath-relax" in line: re_asrelax_new = re.search( "^bgp\s+bestpath\s+as-path\s+multipath-relax$", line @@ -1207,25 +1168,21 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del): lines_to_del_to_del.append((ctx_keys, line)) lines_to_add_to_del.append((ctx_keys, old_asrelax_cmd)) - """ - If we are modifying the BGP table-map we need to avoid a del/add and - instead modify the table-map in place via an add. This is needed to - avoid installing all routes in the RIB the second the 'no table-map' - is issued. - """ + # If we are modifying the BGP table-map we need to avoid a del/add + # and instead modify the table-map in place via an add. This is + # needed to avoid installing all routes in the RIB the second the + # 'no table-map' is issued. if line.startswith("table-map"): found_table_map = line_exist(lines_to_add, ctx_keys, "table-map", False) if found_table_map: lines_to_del_to_del.append((ctx_keys, line)) - """ - More old-to-new config handling. ip import-table no longer accepts - distance, but we honor the old syntax. But 'show running' shows only - the new syntax. This causes an unnecessary 'no import-table' followed - by the same old 'ip import-table' which causes perturbations in - announced routes leading to traffic blackholes. Fix this issue. - """ + # More old-to-new config handling. ip import-table no longer accepts + # distance, but we honor the old syntax. But 'show running' shows only + # the new syntax. This causes an unnecessary 'no import-table' followed + # by the same old 'ip import-table' which causes perturbations in + # announced routes leading to traffic blackholes. Fix this issue. re_importtbl = re.search("^ip\s+import-table\s+(\d+)$", ctx_keys[0]) if re_importtbl: table_num = re_importtbl.group(1) @@ -1236,17 +1193,16 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del): ) lines_to_add_to_del.append((ctx[0], None)) - """ - ip/ipv6 prefix-lists and access-lists can be specified without a seq number. - However, the running config always adds 'seq x', where x is a number - incremented by 5 for every element of the prefix/access list. - So, ignore such lines as well. Sample prefix-list and acces-list lines: - ip prefix-list PR-TABLE-2 seq 5 permit 20.8.2.0/24 le 32 - ip prefix-list PR-TABLE-2 seq 10 permit 20.8.2.0/24 le 32 - ipv6 prefix-list vrfdev6-12 permit 2000:9:2::/64 gt 64 - access-list FOO seq 5 permit 2.2.2.2/32 - ipv6 access-list BAR seq 5 permit 2:2:2::2/128 - """ + # ip/ipv6 prefix-lists and access-lists can be specified without a seq + # number. However, the running config always adds 'seq x', where x is + # a number incremented by 5 for every element of the prefix/access + # list. So, ignore such lines as well. Sample prefix-list and + # acces-list lines: + # ip prefix-list PR-TABLE-2 seq 5 permit 20.8.2.0/24 le 32 + # ip prefix-list PR-TABLE-2 seq 10 permit 20.8.2.0/24 le 32 + # ipv6 prefix-list vrfdev6-12 permit 2000:9:2::/64 gt 64 + # access-list FOO seq 5 permit 2.2.2.2/32 + # ipv6 access-list BAR seq 5 permit 2:2:2::2/128 re_acl_pfxlst = re.search( "^(ip |ipv6 |)(prefix-list|access-list)(\s+\S+\s+)(seq \d+\s+)(permit|deny)(.*)$", ctx_keys[0], @@ -1265,12 +1221,9 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del): lines_to_del_to_del.append((ctx_keys, None)) lines_to_add_to_del.append(((tmpline,), None)) found = True - """ - If prefix-lists or access-lists are being deleted and - not added (see comment above), add command with 'no' to - lines_to_add and remove from lines_to_del to improve - scaling performance. - """ + # If prefix-lists or access-lists are being deleted and not added + # (see comment above), add command with 'no' to lines_to_add and + # remove from lines_to_del to improve scaling performance. if found is False: add_cmd = ("no " + ctx_keys[0],) lines_to_add.append((add_cmd, None)) @@ -1302,16 +1255,12 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del): lines_to_add, ctx_keys, route_target_both_line ) - """ - If the running configs has - route-target import 1:1 - route-target export 1:1 - - and the config we are reloading against has - route-target both 1:1 - - then we can ignore deleting the import/export and ignore adding the 'both' - """ + # If the running configs has + # route-target import 1:1 + # route-target export 1:1 + # and the config we are reloading against has + # route-target both 1:1 + # then we can ignore deleting the import/export and ignore adding the 'both' if found_route_target_export_line and found_route_target_both_line: lines_to_del_to_del.append((ctx_keys, route_target_import_line)) lines_to_del_to_del.append((ctx_keys, route_target_export_line)) @@ -1333,23 +1282,21 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del): lines_to_del_to_del.append((ctx_keys, line)) lines_to_add_to_del.append((ctx_keys, line)) else: - """ - We have commands that used to be displayed in the global part - of 'router bgp' that are now displayed under 'address-family ipv4 unicast' - - # old way - router bgp 64900 - neighbor ISL advertisement-interval 0 - - vs. - - # new way - router bgp 64900 - address-family ipv4 unicast - neighbor ISL advertisement-interval 0 - - Look to see if we are deleting it in one format just to add it back in the other - """ + # We have commands that used to be displayed in the global part + # of 'router bgp' that are now displayed under 'address-family ipv4 unicast' + # + # # old way + # router bgp 64900 + # neighbor ISL advertisement-interval 0 + # + # vs. + # + # # new way + # router bgp 64900 + # address-family ipv4 unicast + # neighbor ISL advertisement-interval 0 + # + # Look to see if we are deleting it in one format just to add it back in the other if ( ctx_keys[0].startswith("router bgp") and len(ctx_keys) > 1 @@ -1382,19 +1329,22 @@ def ignore_unconfigurable_lines(lines_to_add, lines_to_del): for (ctx_keys, line) in lines_to_del: - if ( - ctx_keys[0].startswith("frr version") - or ctx_keys[0].startswith("frr defaults") - or ctx_keys[0].startswith("username") - or ctx_keys[0].startswith("password") - or ctx_keys[0].startswith("line vty") - or - # This is technically "no"able but if we did so frr-reload would - # stop working so do not let the user shoot themselves in the foot - # by removing this. - ctx_keys[0].startswith("service integrated-vtysh-config") + # The integrated-vtysh-config one is technically "no"able but if we did + # so frr-reload would stop working so do not let the user shoot + # themselves in the foot by removing this. + if any( + [ + ctx_keys[0].startswith(x) + for x in [ + "frr version", + "frr defaults", + "username", + "password", + "line vty", + "service integrated-vtysh-config", + ] + ] ): - log.info('"%s" cannot be removed' % (ctx_keys[-1],)) lines_to_del_to_del.append((ctx_keys, line)) diff --git a/tools/valgrind.supp b/tools/valgrind.supp index 88f46bf575..da3d4a8d6d 100644 --- a/tools/valgrind.supp +++ b/tools/valgrind.supp @@ -78,3 +78,11 @@ ... fun:yang_module_load } +{ + <libyang2 lys_compile_type_range> + Memcheck:Leak + ... + fun:lys_compile_type_range + ... + fun:yang_module_load +} diff --git a/zebra/main.c b/zebra/main.c index e4363bd899..038022ceb2 100644 --- a/zebra/main.c +++ b/zebra/main.c @@ -182,8 +182,6 @@ static void sigint(void) SET_FLAG(zvrf->flags, ZEBRA_VRF_RETAIN); } } - if (zrouter.lsp_process_q) - work_queue_free_and_null(&zrouter.lsp_process_q); vrf_terminate(); diff --git a/zebra/table_manager.c b/zebra/table_manager.c index 82d6a0a6a2..ffc7a48eb9 100644 --- a/zebra/table_manager.c +++ b/zebra/table_manager.c @@ -72,7 +72,8 @@ void table_manager_enable(struct zebra_vrf *zvrf) if (zvrf->tbl_mgr) return; - if (!vrf_is_backend_netns() && zvrf_id(zvrf) != VRF_DEFAULT) { + if (!vrf_is_backend_netns() + && strcmp(zvrf_name(zvrf), VRF_DEFAULT_NAME)) { struct zebra_vrf *def = zebra_vrf_lookup_by_id(VRF_DEFAULT); if (def) @@ -284,7 +285,8 @@ void table_manager_disable(struct zebra_vrf *zvrf) { if (!zvrf->tbl_mgr) return; - if (!vrf_is_backend_netns() && zvrf_id(zvrf) != VRF_DEFAULT) { + if (!vrf_is_backend_netns() + && strcmp(zvrf_name(zvrf), VRF_DEFAULT_NAME)) { zvrf->tbl_mgr = NULL; return; } diff --git a/zebra/zebra_router.c b/zebra/zebra_router.c index a80c573855..4ce756c953 100644 --- a/zebra/zebra_router.c +++ b/zebra/zebra_router.c @@ -235,6 +235,9 @@ void zebra_router_terminate(void) RB_FOREACH_SAFE (zrt, zebra_router_table_head, &zrouter.tables, tmp) zebra_router_free_table(zrt); + if (zrouter.lsp_process_q) + work_queue_free_and_null(&zrouter.lsp_process_q); + work_queue_free_and_null(&zrouter.ribq); meta_queue_free(zrouter.mq); |