From 43763b11d0d51b60809cddc4722daae359a4d1a9 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 2 Apr 2019 09:40:41 -0400 Subject: [PATCH] pimd: Tracking of RPF is *separate* from the lookup Start the separation of tracking a Destination from the act of looking it up. The cojoining of these two concepts led to a bunch of code that had to think about both problems leading to weird situations and code paths. Simplify the code by making pim_ecmp_nexthop_search a static function and we only ever call pim_ecmp_nexthop_lookup when we need to do a RPF(). pim_ecmp_nexthop_lookup will now attempt to find a stored pnc and if it finds one it will report on the answer from it. Signed-off-by: Donald Sharp --- pimd/pim_cmd.c | 15 +------ pimd/pim_nht.c | 31 ++++++++++---- pimd/pim_nht.h | 4 -- pimd/pim_rp.c | 88 ++++++++++---------------------------- pimd/pim_rpf.c | 18 ++------ pimd/pim_zebra.c | 107 +++++++++-------------------------------------- 6 files changed, 68 insertions(+), 195 deletions(-) diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c index 1a30904e2..8d0ea12b8 100644 --- a/pimd/pim_cmd.c +++ b/pimd/pim_cmd.c @@ -4257,7 +4257,6 @@ DEFUN (show_ip_pim_nexthop_lookup, "Source/RP address\n" "Multicast Group address\n") { - struct pim_nexthop_cache *pnc = NULL; struct prefix nht_p; int result = 0; struct in_addr src_addr, grp_addr; @@ -4269,7 +4268,6 @@ DEFUN (show_ip_pim_nexthop_lookup, char grp_str[PREFIX_STRLEN]; int idx = 2; struct vrf *vrf = pim_cmd_lookup_vrf(vty, argv, argc, &idx); - struct pim_rpf rpf; if (!vrf) return CMD_WARNING; @@ -4315,18 +4313,7 @@ DEFUN (show_ip_pim_nexthop_lookup, grp.u.prefix4 = grp_addr; memset(&nexthop, 0, sizeof(nexthop)); - memset(&rpf, 0, sizeof(struct pim_rpf)); - rpf.rpf_addr.family = AF_INET; - rpf.rpf_addr.prefixlen = IPV4_MAX_BITLEN; - rpf.rpf_addr.u.prefix4 = vif_source; - - pnc = pim_nexthop_cache_find(vrf->info, &rpf); - if (pnc) - result = pim_ecmp_nexthop_search(vrf->info, pnc, &nexthop, - &nht_p, &grp, 0); - else - result = pim_ecmp_nexthop_lookup(vrf->info, &nexthop, &nht_p, - &grp, 0); + result = pim_ecmp_nexthop_lookup(vrf->info, &nexthop, &nht_p, &grp, 0); if (!result) { vty_out(vty, diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c index 31051cf73..0ec239cea 100644 --- a/pimd/pim_nht.c +++ b/pimd/pim_nht.c @@ -158,7 +158,8 @@ int pim_find_or_track_nexthop(struct pim_instance *pim, struct prefix *addr, hash_get(pnc->upstream_hash, up, hash_alloc_intern); if (CHECK_FLAG(pnc->flags, PIM_NEXTHOP_VALID)) { - memcpy(out_pnc, pnc, sizeof(struct pim_nexthop_cache)); + if (out_pnc) + memcpy(out_pnc, pnc, sizeof(struct pim_nexthop_cache)); return 1; } @@ -256,10 +257,9 @@ static void pim_update_rp_nh(struct pim_instance *pim, continue; // Compute PIM RPF using cached nexthop - if (!pim_ecmp_nexthop_search(pim, pnc, - &rp_info->rp.source_nexthop, - &rp_info->rp.rpf_addr, &rp_info->group, - 1)) + if (!pim_ecmp_nexthop_lookup(pim, &rp_info->rp.source_nexthop, + &rp_info->rp.rpf_addr, + &rp_info->group, 1)) pim_rp_nexthop_del(rp_info); } } @@ -347,10 +347,11 @@ uint32_t pim_compute_ecmp_hash(struct prefix *src, struct prefix *grp) return hash_val; } -int pim_ecmp_nexthop_search(struct pim_instance *pim, - struct pim_nexthop_cache *pnc, - struct pim_nexthop *nexthop, struct prefix *src, - struct prefix *grp, int neighbor_needed) +static int pim_ecmp_nexthop_search(struct pim_instance *pim, + struct pim_nexthop_cache *pnc, + struct pim_nexthop *nexthop, + struct prefix *src, struct prefix *grp, + int neighbor_needed) { struct pim_neighbor *nbrs[MULTIPATH_NUM], *nbr = NULL; struct interface *ifps[MULTIPATH_NUM]; @@ -734,8 +735,10 @@ int pim_ecmp_nexthop_lookup(struct pim_instance *pim, struct pim_nexthop *nexthop, struct prefix *src, struct prefix *grp, int neighbor_needed) { + struct pim_nexthop_cache *pnc; struct pim_zlookup_nexthop nexthop_tab[MULTIPATH_NUM]; struct pim_neighbor *nbrs[MULTIPATH_NUM], *nbr = NULL; + struct pim_rpf rpf; int num_ifindex; struct interface *ifps[MULTIPATH_NUM], *ifp; int first_ifindex; @@ -753,6 +756,16 @@ int pim_ecmp_nexthop_lookup(struct pim_instance *pim, nexthop->last_lookup_time); } + memset(&rpf, 0, sizeof(struct pim_rpf)); + rpf.rpf_addr.family = AF_INET; + rpf.rpf_addr.prefixlen = IPV4_MAX_BITLEN; + rpf.rpf_addr.u.prefix4 = src->u.prefix4; + + pnc = pim_nexthop_cache_find(pim, &rpf); + if (pnc) + return pim_ecmp_nexthop_search(pim, pnc, nexthop, src, grp, + neighbor_needed); + memset(nexthop_tab, 0, sizeof(struct pim_zlookup_nexthop) * MULTIPATH_NUM); num_ifindex = diff --git a/pimd/pim_nht.h b/pimd/pim_nht.h index e3e9f578c..8f68af565 100644 --- a/pimd/pim_nht.h +++ b/pimd/pim_nht.h @@ -56,10 +56,6 @@ void pim_delete_tracked_nexthop(struct pim_instance *pim, struct prefix *addr, struct pim_nexthop_cache *pim_nexthop_cache_find(struct pim_instance *pim, struct pim_rpf *rpf); uint32_t pim_compute_ecmp_hash(struct prefix *src, struct prefix *grp); -int pim_ecmp_nexthop_search(struct pim_instance *pim, - struct pim_nexthop_cache *pnc, - struct pim_nexthop *nexthop, struct prefix *src, - struct prefix *grp, int neighbor_needed); int pim_ecmp_nexthop_lookup(struct pim_instance *pim, struct pim_nexthop *nexthop, struct prefix *src, struct prefix *grp, int neighbor_needed); diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c index e7cf208f7..14643743a 100644 --- a/pimd/pim_rp.c +++ b/pimd/pim_rp.c @@ -418,7 +418,6 @@ int pim_rp_new(struct pim_instance *pim, const char *rp, char buffer[BUFSIZ]; struct prefix nht_p; struct prefix temp; - struct pim_nexthop_cache pnc; struct route_node *rn; struct pim_upstream *up; struct listnode *upnode; @@ -567,20 +566,12 @@ int pim_rp_new(struct pim_instance *pim, const char *rp, pim_rp_check_interfaces(pim, rp_all); pim_rp_refresh_group_to_rp_mapping(pim); - memset(&pnc, 0, sizeof(struct pim_nexthop_cache)); - if (pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_all, - &pnc)) { - if (!pim_ecmp_nexthop_search( - pim, &pnc, - &rp_all->rp.source_nexthop, &nht_p, - &rp_all->group, 1)) - return PIM_RP_NO_PATH; - } else { - if (!pim_ecmp_nexthop_lookup( - pim, &rp_all->rp.source_nexthop, - &nht_p, &rp_all->group, 1)) - return PIM_RP_NO_PATH; - } + pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_all, + NULL); + if (!pim_ecmp_nexthop_lookup(pim, + &rp_all->rp.source_nexthop, + &nht_p, &rp_all->group, 1)) + return PIM_RP_NO_PATH; return PIM_SUCCESS; } @@ -664,17 +655,10 @@ int pim_rp_new(struct pim_instance *pim, const char *rp, __PRETTY_FUNCTION__, buf, buf1); } - memset(&pnc, 0, sizeof(struct pim_nexthop_cache)); - if (pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, &pnc)) { - if (!pim_ecmp_nexthop_search(pim, &pnc, - &rp_info->rp.source_nexthop, - &nht_p, &rp_info->group, 1)) - return PIM_RP_NO_PATH; - } else { - if (!pim_ecmp_nexthop_lookup(pim, &rp_info->rp.source_nexthop, - &nht_p, &rp_info->group, 1)) - return PIM_RP_NO_PATH; - } + pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, NULL); + if (!pim_ecmp_nexthop_lookup(pim, &rp_info->rp.source_nexthop, &nht_p, + &rp_info->group, 1)) + return PIM_RP_NO_PATH; return PIM_SUCCESS; } @@ -826,7 +810,6 @@ void pim_rp_setup(struct pim_instance *pim) struct listnode *node; struct rp_info *rp_info; struct prefix nht_p; - struct pim_nexthop_cache pnc; for (ALL_LIST_ELEMENTS_RO(pim->rp_list, node, rp_info)) { if (rp_info->rp.rpf_addr.u.prefix4.s_addr == INADDR_NONE) @@ -835,27 +818,13 @@ void pim_rp_setup(struct pim_instance *pim) nht_p.family = AF_INET; nht_p.prefixlen = IPV4_MAX_BITLEN; nht_p.u.prefix4 = rp_info->rp.rpf_addr.u.prefix4; - memset(&pnc, 0, sizeof(struct pim_nexthop_cache)); - if (pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, &pnc)) - pim_ecmp_nexthop_search(pim, &pnc, - &rp_info->rp.source_nexthop, - &nht_p, &rp_info->group, 1); - else { - if (PIM_DEBUG_PIM_NHT_RP) { - char buf[PREFIX2STR_BUFFER]; - prefix2str(&nht_p, buf, sizeof(buf)); + pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, NULL); + if (!pim_ecmp_nexthop_lookup(pim, &rp_info->rp.source_nexthop, + &nht_p, &rp_info->group, 1)) + if (PIM_DEBUG_PIM_NHT_RP) zlog_debug( - "%s: NHT Local Nexthop not found for RP %s ", - __PRETTY_FUNCTION__, buf); - } - if (!pim_ecmp_nexthop_lookup(pim, - &rp_info->rp.source_nexthop, - &nht_p, &rp_info->group, 1)) - if (PIM_DEBUG_PIM_NHT_RP) - zlog_debug( - "Unable to lookup nexthop for rp specified"); - } + "Unable to lookup nexthop for rp specified"); } } @@ -986,7 +955,7 @@ struct pim_rpf *pim_rp_g(struct pim_instance *pim, struct in_addr group) if (rp_info) { struct prefix nht_p; - struct pim_nexthop_cache pnc; + /* Register addr with Zebra NHT */ nht_p.family = AF_INET; nht_p.prefixlen = IPV4_MAX_BITLEN; @@ -1000,26 +969,11 @@ struct pim_rpf *pim_rp_g(struct pim_instance *pim, struct in_addr group) "%s: NHT Register RP addr %s grp %s with Zebra", __PRETTY_FUNCTION__, buf, buf1); } - memset(&pnc, 0, sizeof(struct pim_nexthop_cache)); - if (pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, &pnc)) - pim_ecmp_nexthop_search(pim, &pnc, - &rp_info->rp.source_nexthop, - &nht_p, &rp_info->group, 1); - else { - if (PIM_DEBUG_PIM_NHT_RP) { - char buf[PREFIX2STR_BUFFER]; - char buf1[PREFIX2STR_BUFFER]; - prefix2str(&nht_p, buf, sizeof(buf)); - prefix2str(&g, buf1, sizeof(buf1)); - zlog_debug( - "%s: Nexthop cache not found for RP %s grp %s register with Zebra", - __PRETTY_FUNCTION__, buf, buf1); - } - pim_rpf_set_refresh_time(pim); - (void)pim_ecmp_nexthop_lookup( - pim, &rp_info->rp.source_nexthop, &nht_p, - &rp_info->group, 1); - } + + pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, NULL); + pim_rpf_set_refresh_time(pim); + (void)pim_ecmp_nexthop_lookup(pim, &rp_info->rp.source_nexthop, + &nht_p, &rp_info->group, 1); return (&rp_info->rp); } diff --git a/pimd/pim_rpf.c b/pimd/pim_rpf.c index 55f788b5b..8cc8b7481 100644 --- a/pimd/pim_rpf.c +++ b/pimd/pim_rpf.c @@ -201,7 +201,6 @@ enum pim_rpf_result pim_rpf_update(struct pim_instance *pim, struct pim_rpf *rpf = &up->rpf; struct pim_rpf saved; struct prefix nht_p; - struct pim_nexthop_cache pnc; struct prefix src, grp; bool neigh_needed = true; @@ -232,23 +231,14 @@ enum pim_rpf_result pim_rpf_update(struct pim_instance *pim, grp.family = AF_INET; grp.prefixlen = IPV4_MAX_BITLEN; grp.u.prefix4 = up->sg.grp; - memset(&pnc, 0, sizeof(struct pim_nexthop_cache)); if ((up->sg.src.s_addr == INADDR_ANY && I_am_RP(pim, up->sg.grp)) || PIM_UPSTREAM_FLAG_TEST_FHR(up->flags)) neigh_needed = FALSE; - if (pim_find_or_track_nexthop(pim, &nht_p, up, NULL, &pnc)) { - if (pnc.nexthop_num) { - if (!pim_ecmp_nexthop_search(pim, &pnc, - &up->rpf.source_nexthop, - &src, &grp, neigh_needed)) - return PIM_RPF_FAILURE; - } - } else { - if (!pim_ecmp_nexthop_lookup(pim, &rpf->source_nexthop, &src, - &grp, neigh_needed)) - return PIM_RPF_FAILURE; - } + pim_find_or_track_nexthop(pim, &nht_p, up, NULL, NULL); + if (!pim_ecmp_nexthop_lookup(pim, &rpf->source_nexthop, &src, &grp, + neigh_needed)) + return PIM_RPF_FAILURE; rpf->rpf_addr.family = AF_INET; rpf->rpf_addr.u.prefix4 = pim_rpf_find_rpf_addr(up); diff --git a/pimd/pim_zebra.c b/pimd/pim_zebra.c index 70f099b0d..4ef158496 100644 --- a/pimd/pim_zebra.c +++ b/pimd/pim_zebra.c @@ -929,7 +929,6 @@ void igmp_source_forward_start(struct pim_instance *pim, if (!source->source_channel_oil) { struct in_addr vif_source; struct prefix nht_p, src, grp; - struct pim_nexthop_cache out_pnc; struct pim_nexthop nexthop; struct pim_upstream *up = NULL; @@ -955,7 +954,6 @@ void igmp_source_forward_start(struct pim_instance *pim, nht_p.family = AF_INET; nht_p.prefixlen = IPV4_MAX_BITLEN; nht_p.u.prefix4 = vif_source; - memset(&out_pnc, 0, sizeof(struct pim_nexthop_cache)); src.family = AF_INET; src.prefixlen = IPV4_MAX_BITLEN; @@ -964,44 +962,21 @@ void igmp_source_forward_start(struct pim_instance *pim, grp.prefixlen = IPV4_MAX_BITLEN; grp.u.prefix4 = sg.grp; - if (pim_find_or_track_nexthop(pim, &nht_p, NULL, NULL, - &out_pnc)) { - if (out_pnc.nexthop_num) { - up = pim_upstream_find(pim, &sg); - memset(&nexthop, 0, sizeof(nexthop)); - if (up) - memcpy(&nexthop, - &up->rpf.source_nexthop, - sizeof(struct pim_nexthop)); - pim_ecmp_nexthop_search(pim, &out_pnc, - &nexthop, - &src, &grp, 0); - if (nexthop.interface) - input_iface_vif_index = + up = pim_upstream_find(pim, &sg); + if (up) { + memcpy(&nexthop, &up->rpf.source_nexthop, + sizeof(struct pim_nexthop)); + pim_ecmp_nexthop_lookup(pim, &nexthop, &src, + &grp, 0); + if (nexthop.interface) + input_iface_vif_index = pim_if_find_vifindex_by_ifindex( - pim, - nexthop.interface->ifindex); - } else { - if (PIM_DEBUG_ZEBRA) { - char buf1[INET_ADDRSTRLEN]; - char buf2[INET_ADDRSTRLEN]; - - pim_inet4_dump("", - nht_p.u.prefix4, buf1, - sizeof(buf1)); - pim_inet4_dump("", - grp.u.prefix4, buf2, - sizeof(buf2)); - zlog_debug( - "%s: NHT Nexthop not found for addr %s grp %s", - __PRETTY_FUNCTION__, buf1, - buf2); - } - } + pim, + nexthop.interface->ifindex); } else input_iface_vif_index = - pim_ecmp_fib_lookup_if_vif_index(pim, &src, - &grp); + pim_ecmp_fib_lookup_if_vif_index( + pim, &src, &grp); if (PIM_DEBUG_ZEBRA) { char buf2[INET_ADDRSTRLEN]; @@ -1209,7 +1184,6 @@ void pim_forward_start(struct pim_ifchannel *ch) */ if ((up->upstream_addr.s_addr != INADDR_ANY) && (!up->channel_oil)) { struct prefix nht_p, src, grp; - struct pim_nexthop_cache out_pnc; /* Register addr with Zebra NHT */ nht_p.family = AF_INET; @@ -1218,55 +1192,14 @@ void pim_forward_start(struct pim_ifchannel *ch) grp.family = AF_INET; grp.prefixlen = IPV4_MAX_BITLEN; grp.u.prefix4 = up->sg.grp; - memset(&out_pnc, 0, sizeof(struct pim_nexthop_cache)); - - if (pim_find_or_track_nexthop(pim, &nht_p, NULL, NULL, - &out_pnc)) { - if (out_pnc.nexthop_num) { - src.family = AF_INET; - src.prefixlen = IPV4_MAX_BITLEN; - src.u.prefix4 = - up->upstream_addr; // RP or Src address - grp.family = AF_INET; - grp.prefixlen = IPV4_MAX_BITLEN; - grp.u.prefix4 = up->sg.grp; - // Compute PIM RPF using Cached nexthop - if (pim_ecmp_nexthop_search( - pim, &out_pnc, - &up->rpf.source_nexthop, &src, &grp, - 0)) - input_iface_vif_index = - pim_if_find_vifindex_by_ifindex( - pim, - up->rpf.source_nexthop - .interface->ifindex); - else { - if (PIM_DEBUG_TRACE) - zlog_debug( - "%s: Nexthop selection failed for %s ", - __PRETTY_FUNCTION__, - up->sg_str); - } - } else { - if (PIM_DEBUG_ZEBRA) { - char buf1[INET_ADDRSTRLEN]; - char buf2[INET_ADDRSTRLEN]; - pim_inet4_dump("", - nht_p.u.prefix4, buf1, - sizeof(buf1)); - pim_inet4_dump("", - grp.u.prefix4, buf2, - sizeof(buf2)); - zlog_debug( - "%s: NHT pnc is NULL for addr %s grp %s", - __PRETTY_FUNCTION__, buf1, - buf2); - } - } - } else - input_iface_vif_index = - pim_ecmp_fib_lookup_if_vif_index(pim, &src, - &grp); + src.family = AF_INET; + src.prefixlen = IPV4_MAX_BITLEN; + src.u.prefix4 = up->sg.src; + + if (pim_ecmp_nexthop_lookup(pim, &up->rpf.source_nexthop, &src, + &grp, 0)) + input_iface_vif_index = pim_if_find_vifindex_by_ifindex( + pim, up->rpf.source_nexthop.interface->ifindex); if (input_iface_vif_index < 1) { if (PIM_DEBUG_PIM_TRACE) { -- 2.39.5