From 79992e8a4257c35a1d255c24aa3b2df2f8b80b96 Mon Sep 17 00:00:00 2001 From: Mobashshera Rasool Date: Fri, 1 Oct 2021 00:36:10 -0700 Subject: [PATCH 1/3] pimd: Add a flag to decide PIM has to send Hello Problem Statement: ================== pim maintains two counters hello tx and hello rx at interface level. At present pim needs to send the hello message prior to other pim message as per RFC. This logic is getting derived from the tx hello counters. So when a new neighbor is added, tx counters are set to zero and then based on this, it is further decided to send hello in pim_hello_require function. Fix: ==== Separating the hello statistics and the logic to decide when to send hello based on a new flag. pim_ifstat_hello_sent will be used to note down the hello stats while a new flag is added to decide whether to send hello or not if it is the first packet to a neighbor. Signed-off-by: Mobashshera Rasool --- pimd/pim_hello.c | 2 +- pimd/pim_iface.c | 5 ++++- pimd/pim_iface.h | 15 +++++++++++++++ pimd/pim_neighbor.c | 2 +- pimd/pim_pim.c | 16 ++++++++++++++++ 5 files changed, 37 insertions(+), 3 deletions(-) diff --git a/pimd/pim_hello.c b/pimd/pim_hello.c index e48a4bdd4d..45ea6a9562 100644 --- a/pimd/pim_hello.c +++ b/pimd/pim_hello.c @@ -545,7 +545,7 @@ void pim_hello_require(struct interface *ifp) assert(pim_ifp); - if (pim_ifp->pim_ifstat_hello_sent) + if (PIM_IF_FLAG_TEST_HELLO_SENT(pim_ifp->flags)) return; pim_hello_restart_now(ifp); /* Send hello and restart timer */ diff --git a/pimd/pim_iface.c b/pimd/pim_iface.c index eb19cf4ddf..4b87bde281 100644 --- a/pimd/pim_iface.c +++ b/pimd/pim_iface.c @@ -290,6 +290,7 @@ static void pim_addr_change(struct interface *ifp) -- FIXME See TODO T31 */ pim_ifp->pim_ifstat_hello_sent = 0; /* reset hello counter */ + PIM_IF_FLAG_UNSET_HELLO_SENT(pim_ifp->flags); if (pim_ifp->pim_sock_fd < 0) return; pim_hello_restart_now(ifp); /* send hello and restart timer */ @@ -1697,8 +1698,10 @@ int pim_ifp_down(struct interface *ifp) } } - if (ifp->info) + if (ifp->info) { pim_if_del_vif(ifp); + pim_ifstat_reset(ifp); + } return 0; } diff --git a/pimd/pim_iface.h b/pimd/pim_iface.h index 55c278d6e2..514e158e74 100644 --- a/pimd/pim_iface.h +++ b/pimd/pim_iface.h @@ -61,6 +61,20 @@ #define PIM_I_am_DR(pim_ifp) (pim_ifp)->pim_dr_addr.s_addr == (pim_ifp)->primary_address.s_addr #define PIM_I_am_DualActive(pim_ifp) (pim_ifp)->activeactive == true +/* Macros for interface flags */ + +/* + * PIM needs to know if hello is required to send before other PIM messages + * like Join, prune, assert would go out + */ +#define PIM_IF_FLAG_HELLO_SENT (1 << 0) + +#define PIM_IF_FLAG_TEST_HELLO_SENT(flags) ((flags)&PIM_IF_FLAG_HELLO_SENT) + +#define PIM_IF_FLAG_SET_HELLO_SENT(flags) ((flags) |= PIM_IF_FLAG_HELLO_SENT) + +#define PIM_IF_FLAG_UNSET_HELLO_SENT(flags) ((flags) &= ~PIM_IF_FLAG_HELLO_SENT) + struct pim_iface_upstream_switch { struct in_addr address; struct list *us; @@ -161,6 +175,7 @@ struct pim_interface { uint32_t pim_ifstat_bsm_cfg_miss; uint32_t pim_ifstat_ucast_bsm_cfg_miss; uint32_t pim_ifstat_bsm_invalid_sz; + uint8_t flags; bool bsm_enable; /* bsm processing enable */ bool ucast_bsm_accept; /* ucast bsm processing */ diff --git a/pimd/pim_neighbor.c b/pimd/pim_neighbor.c index 571173c62a..530c2e429b 100644 --- a/pimd/pim_neighbor.c +++ b/pimd/pim_neighbor.c @@ -341,7 +341,7 @@ pim_neighbor_new(struct interface *ifp, struct in_addr source_addr, * reset the value so that we can know to hurry up and * hello */ - pim_ifp->pim_ifstat_hello_sent = 0; + PIM_IF_FLAG_UNSET_HELLO_SENT(pim_ifp->flags); pim_inet4_dump("", source_addr, src_str, sizeof(src_str)); diff --git a/pimd/pim_pim.c b/pimd/pim_pim.c index 8c38cf6c4c..5c3c4c23f4 100644 --- a/pimd/pim_pim.c +++ b/pimd/pim_pim.c @@ -455,6 +455,21 @@ void pim_ifstat_reset(struct interface *ifp) pim_ifp->pim_ifstat_hello_sendfail = 0; pim_ifp->pim_ifstat_hello_recv = 0; pim_ifp->pim_ifstat_hello_recvfail = 0; + pim_ifp->pim_ifstat_bsm_rx = 0; + pim_ifp->pim_ifstat_bsm_tx = 0; + pim_ifp->pim_ifstat_join_recv = 0; + pim_ifp->pim_ifstat_join_send = 0; + pim_ifp->pim_ifstat_prune_recv = 0; + pim_ifp->pim_ifstat_prune_send = 0; + pim_ifp->pim_ifstat_reg_recv = 0; + pim_ifp->pim_ifstat_reg_send = 0; + pim_ifp->pim_ifstat_reg_stop_recv = 0; + pim_ifp->pim_ifstat_reg_stop_send = 0; + pim_ifp->pim_ifstat_assert_recv = 0; + pim_ifp->pim_ifstat_assert_send = 0; + pim_ifp->pim_ifstat_bsm_cfg_miss = 0; + pim_ifp->pim_ifstat_ucast_bsm_cfg_miss = 0; + pim_ifp->pim_ifstat_bsm_invalid_sz = 0; } void pim_sock_reset(struct interface *ifp) @@ -707,6 +722,7 @@ int pim_hello_send(struct interface *ifp, uint16_t holdtime) } ++pim_ifp->pim_ifstat_hello_sent; + PIM_IF_FLAG_SET_HELLO_SENT(pim_ifp->flags); return 0; } From de4e7e3355b64d1ec95d55426843014d905d43df Mon Sep 17 00:00:00 2001 From: Mobashshera Rasool Date: Sun, 3 Oct 2021 21:55:46 -0700 Subject: [PATCH 2/3] pimd: pimd: hello sent counters are getting clear on new neighbor addition Problem Statement: ================== On new neighbor addition, the tx counter for hello msg is reset. Fix: ================= Do not reset the tx counter on new neighbor addition. Signed-off-by: Mobashshera Rasool --- pimd/pim_iface.c | 1 - 1 file changed, 1 deletion(-) diff --git a/pimd/pim_iface.c b/pimd/pim_iface.c index 4b87bde281..b18743e2a4 100644 --- a/pimd/pim_iface.c +++ b/pimd/pim_iface.c @@ -289,7 +289,6 @@ static void pim_addr_change(struct interface *ifp) HoldTime should be sent immediately. -- FIXME See TODO T31 */ - pim_ifp->pim_ifstat_hello_sent = 0; /* reset hello counter */ PIM_IF_FLAG_UNSET_HELLO_SENT(pim_ifp->flags); if (pim_ifp->pim_sock_fd < 0) return; From 0d16f9d8243dccdddafb956a6c19ea9e5d656f8d Mon Sep 17 00:00:00 2001 From: Mobashshera Rasool Date: Mon, 4 Oct 2021 03:30:51 -0700 Subject: [PATCH 3/3] tests: Modify the script to verify the hello stats increment Co-authored-by: Vijay Gupta Co-authored-by: Mobashshera Rasool Signed-off-by: Mobashshera Rasool --- .../test_multicast_pim_sm_topo4.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo4.py b/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo4.py index 5e29a1f1fd..2a8e29b1d4 100755 --- a/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo4.py +++ b/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo4.py @@ -954,11 +954,13 @@ def test_PIM_hello_tx_rx_p1(request): tc_name, result ) - step("verify stats not increamented on c1") + step("verify stats incremented on c1") result = verify_state_incremented(c1_state_before, c1_state_after) assert ( - result is not True - ), "Testcase{} : Failed Error: {}" "stats incremented".format(tc_name, result) + result is True + ), "Testcase{} : Failed Error: {}" "stats is not incremented".format( + tc_name, result + ) step("verify before stats on l1") l1_state_dict = { @@ -991,7 +993,7 @@ def test_PIM_hello_tx_rx_p1(request): tc_name, result ) - step("verify stats not increamented on l1") + step("verify stats not incremented on l1") result = verify_state_incremented(l1_state_before, l1_state_after) assert ( result is not True @@ -1041,10 +1043,10 @@ def test_PIM_hello_tx_rx_p1(request): tc_name, result ) - step("verify stats not increamented on c1") + step("verify stats incremented on c1") result = verify_state_incremented(c1_state_before, c1_state_after) assert ( - result is not True + result is True ), "Testcase{} : Failed Error: {}" "stats incremented".format(tc_name, result) write_test_footer(tc_name)