From ba4555c646f68cbcda8b075abb7fee74ca06a2d6 Mon Sep 17 00:00:00 2001 From: "Barry A. Trent" Date: Thu, 26 Sep 2024 14:49:19 -0700 Subject: [PATCH 1/2] pimd: fix autorp CLI bugs Signed-off-by: Barry A. Trent --- pimd/pim_autorp.c | 3 ++- pimd/pim_cmd.c | 2 +- pimd/pim_cmd_common.c | 8 ++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pimd/pim_autorp.c b/pimd/pim_autorp.c index 8f3b8de3cd95..1f4d0c65af67 100644 --- a/pimd/pim_autorp.c +++ b/pimd/pim_autorp.c @@ -851,6 +851,7 @@ void pim_autorp_add_candidate_rp_plist(struct pim_instance *pim, snprintf(rp->grplist, sizeof(rp->grplist), "%s", plist); /* A new group prefix list implies that any previous group prefix is now invalid */ memset(&(rp->grp), 0, sizeof(rp->grp)); + rp->grp.family = AF_INET; pim_autorp_new_announcement(pim); } @@ -1155,7 +1156,7 @@ void pim_autorp_show_autorp(struct vty *vty, struct pim_instance *pim, table = ttable_dump(tt, "\n"); vty_out(vty, "%s\n", table); - XFREE(MTYPE_TMP, table); + XFREE(MTYPE_TMP_TTABLE, table); } ttable_del(tt); diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c index aa7fc0d81f6a..934da2d53e67 100644 --- a/pimd/pim_cmd.c +++ b/pimd/pim_cmd.c @@ -4609,7 +4609,7 @@ DEFPY (pim_autorp_announce_rp, "Prefix list\n" "List name\n") { - return pim_process_autorp_candidate_rp_cmd(vty, no, rpaddr_str, grp, + return pim_process_autorp_candidate_rp_cmd(vty, no, rpaddr_str, (grp_str ? grp : NULL), plist); } diff --git a/pimd/pim_cmd_common.c b/pimd/pim_cmd_common.c index be7460d0fbeb..02ddea8252d6 100644 --- a/pimd/pim_cmd_common.c +++ b/pimd/pim_cmd_common.c @@ -639,9 +639,9 @@ int pim_process_autorp_candidate_rp_cmd(struct vty *vty, bool no, char grpstr[64]; if (no) { - if (!is_default_prefix((const struct prefix *)grp) || plist) { + if ((grp && !is_default_prefix((const struct prefix *)grp)) || plist) { /* If any single values are set, only destroy those */ - if (!is_default_prefix((const struct prefix *)grp)) { + if (grp && !is_default_prefix((const struct prefix *)grp)) { snprintfrr(xpath, sizeof(xpath), "%s/candidate-rp-list[rp-address='%s']/group", FRR_PIM_AUTORP_XPATH, rpaddr_str); @@ -663,12 +663,12 @@ int pim_process_autorp_candidate_rp_cmd(struct vty *vty, bool no, nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); } } else { - if (!is_default_prefix((const struct prefix *)grp) || plist) { + if ((grp && !is_default_prefix((const struct prefix *)grp)) || plist) { snprintfrr(xpath, sizeof(xpath), "%s/candidate-rp-list[rp-address='%s']", FRR_PIM_AUTORP_XPATH, rpaddr_str); nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL); - if (!is_default_prefix((const struct prefix *)grp)) { + if (grp && !is_default_prefix((const struct prefix *)grp)) { snprintfrr(xpath, sizeof(xpath), "%s/candidate-rp-list[rp-address='%s']/group", FRR_PIM_AUTORP_XPATH, rpaddr_str); From 41fa1541632660e8eab92c3b3b786007c5cbe67b Mon Sep 17 00:00:00 2001 From: "Barry A. Trent" Date: Fri, 27 Sep 2024 12:59:12 -0700 Subject: [PATCH 2/2] tests: enhance autorp topotest Signed-off-by: Barry A. Trent --- tests/topotests/pim_autorp/test_pim_autorp.py | 154 ++++++++++++++++-- 1 file changed, 144 insertions(+), 10 deletions(-) diff --git a/tests/topotests/pim_autorp/test_pim_autorp.py b/tests/topotests/pim_autorp/test_pim_autorp.py index 5aecce942e7b..ad618af29e3d 100644 --- a/tests/topotests/pim_autorp/test_pim_autorp.py +++ b/tests/topotests/pim_autorp/test_pim_autorp.py @@ -11,12 +11,18 @@ import os import sys import pytest +from functools import partial # pylint: disable=C0413 # Import topogen and topotest helpers +from lib import topotest from lib.topogen import Topogen, get_topogen from lib.topolog import logger -from lib.pim import scapy_send_autorp_raw_packet, verify_pim_rp_info, verify_pim_rp_info_is_empty +from lib.pim import ( + scapy_send_autorp_raw_packet, + verify_pim_rp_info, + verify_pim_rp_info_is_empty, +) from lib.common_config import step, write_test_header from time import sleep @@ -55,6 +61,7 @@ def build_topo(tgen): switch.add_link(tgen.gears["r1"]) switch.add_link(tgen.gears["r2"]) + def setup_module(mod): logger.info("PIM AutoRP basic functionality:\n {}".format(TOPOLOGY)) @@ -87,6 +94,7 @@ def teardown_module(mod): tgen = get_topogen() tgen.stop_topology() + def test_pim_autorp_discovery_single_rp(request): "Test PIM AutoRP Discovery with single RP" tgen = get_topogen() @@ -106,13 +114,25 @@ def test_pim_autorp_discovery_single_rp(request): scapy_send_autorp_raw_packet(tgen, "r1", "r1-eth0", data) step("Verify rp-info from AutoRP packet") - result = verify_pim_rp_info(tgen, None, "r2", "224.0.0.0/4", "r2-eth0", "10.10.76.1", "AutoRP", False, "ipv4", True) + result = verify_pim_rp_info( + tgen, + None, + "r2", + "224.0.0.0/4", + "r2-eth0", + "10.10.76.1", + "AutoRP", + False, + "ipv4", + True, + ) assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result) step("Verify AutoRP configuration times out") result = verify_pim_rp_info_is_empty(tgen, "r2") assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result) + def test_pim_autorp_discovery_multiple_rp(request): "Test PIM AutoRP Discovery with multiple RP's" tgen = get_topogen() @@ -132,9 +152,31 @@ def test_pim_autorp_discovery_multiple_rp(request): scapy_send_autorp_raw_packet(tgen, "r1", "r1-eth0", data) step("Verify rp-info from AutoRP packet") - result = verify_pim_rp_info(tgen, None, "r2", "224.0.0.0/8", "r2-eth0", "10.10.76.1", "AutoRP", False, "ipv4", True) + result = verify_pim_rp_info( + tgen, + None, + "r2", + "224.0.0.0/8", + "r2-eth0", + "10.10.76.1", + "AutoRP", + False, + "ipv4", + True, + ) assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result) - result = verify_pim_rp_info(tgen, None, "r2", "225.0.0.0/8", "r2-eth0", "10.10.76.3", "AutoRP", False, "ipv4", True) + result = verify_pim_rp_info( + tgen, + None, + "r2", + "225.0.0.0/8", + "r2-eth0", + "10.10.76.3", + "AutoRP", + False, + "ipv4", + True, + ) assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result) @@ -156,7 +198,18 @@ def test_pim_autorp_discovery_static(request): rnode.cmd("vtysh -c 'conf t' -c 'router pim' -c 'rp 10.10.76.3 224.0.0.0/4'") step("Verify static rp-info from r2") - result = verify_pim_rp_info(tgen, None, "r2", "224.0.0.0/4", "r2-eth0", "10.10.76.3", "Static", False, "ipv4", True) + result = verify_pim_rp_info( + tgen, + None, + "r2", + "224.0.0.0/4", + "r2-eth0", + "10.10.76.3", + "Static", + False, + "ipv4", + True, + ) assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result) step("Send AutoRP packet from r1 to r2") @@ -165,10 +218,87 @@ def test_pim_autorp_discovery_static(request): scapy_send_autorp_raw_packet(tgen, "r1", "r1-eth0", data) step("Verify rp-info from AutoRP packet") - result = verify_pim_rp_info(tgen, None, "r2", "224.0.0.0/4", "r2-eth0", "10.10.76.1", "AutoRP", False, "ipv4", True) + result = verify_pim_rp_info( + tgen, + None, + "r2", + "224.0.0.0/4", + "r2-eth0", + "10.10.76.1", + "AutoRP", + False, + "ipv4", + True, + ) assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result) +def test_pim_autorp_announce_cli(request): + "Test PIM AutoRP Announcement CLI commands" + tgen = get_topogen() + tc_name = request.node.name + write_test_header(tc_name) + + if tgen.routers_have_failure(): + pytest.skip("skipped because of router(s) failure") + + step("Add AutoRP announcement configuration to r1") + r1 = tgen.routers()["r1"] + r1.vtysh_cmd( + """ + conf + router pim + autorp announce holdtime 90 + autorp announce interval 120 + autorp announce scope 5 + autorp announce 10.2.3.4 225.0.0.0/24 +""" + ) + + expected = { + "discoveryEnabled": True, + "announce": { + "scope": 5, + "interval": 120, + "holdtime": 90, + "rpList": [ + {"rpAddress": "10.2.3.4", "group": "225.0.0.0/24", "prefixList": ""} + ], + }, + } + + test_func = partial( + topotest.router_json_cmp, r1, "show ip pim autorp json", expected + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = '"{}" JSON output mismatches'.format(r1.name) + assert result is None, assertmsg + + r1.vtysh_cmd( + """ + conf + router pim + autorp announce 10.2.3.4 group-list ListA +""" + ) + expected = { + "discoveryEnabled": True, + "announce": { + "scope": 5, + "interval": 120, + "holdtime": 90, + "rpList": [{"rpAddress": "10.2.3.4", "group": "", "prefixList": "ListA"}], + }, + } + + test_func = partial( + topotest.router_json_cmp, r1, "show ip pim autorp json", expected + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = '"{}" JSON output mismatches'.format(r1.name) + assert result is None, assertmsg + + def test_pim_autorp_announce_group(request): "Test PIM AutoRP Announcement with a single group" tgen = get_topogen() @@ -180,17 +310,21 @@ def test_pim_autorp_announce_group(request): step("Add candidate RP configuration to r1") rnode = tgen.routers()["r1"] - rnode.cmd("vtysh -c 'conf t' -c 'router pim' -c 'send-rp-announce 10.10.76.1 224.0.0.0/4'") + rnode.cmd( + "vtysh -c 'conf t' -c 'router pim' -c 'send-rp-announce 10.10.76.1 224.0.0.0/4'" + ) step("Verify Announcement sent data") # TODO: Verify AutoRP mapping agent receives candidate RP announcement # Mapping agent is not yet implemented - #sleep(10) + # sleep(10) step("Change AutoRP Announcement packet parameters") - rnode.cmd("vtysh -c 'conf t' -c 'router pim' -c 'send-rp-announce scope 8 interval 10 holdtime 60'") + rnode.cmd( + "vtysh -c 'conf t' -c 'router pim' -c 'send-rp-announce scope 8 interval 10 holdtime 60'" + ) step("Verify Announcement sent data") # TODO: Verify AutoRP mapping agent receives updated candidate RP announcement # Mapping agent is not yet implemented - #sleep(10) + # sleep(10) def test_memory_leak():