From 377e4f99bbbd3d38ddd1429fb5aaef4322283ffd Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 1 Apr 2019 12:13:34 -0400 Subject: [PATCH 1/3] pimd: Remove pim_resolve_upstream_nh The pim_resolve_upstream_nh function call is no longer being used let's remove it from the code base. Signed-off-by: Donald Sharp --- pimd/pim_nht.c | 39 --------------------------------------- pimd/pim_nht.h | 1 - 2 files changed, 40 deletions(-) diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c index 9b5379384c..5e550dfe85 100644 --- a/pimd/pim_nht.c +++ b/pimd/pim_nht.c @@ -264,45 +264,6 @@ static void pim_update_rp_nh(struct pim_instance *pim, } } -/* This API is used to traverse nexthop cache of RPF addr - of upstream entry whose IPv4 nexthop address is in - unresolved state and due to event like pim neighbor - UP event if it can be resolved. -*/ -void pim_resolve_upstream_nh(struct pim_instance *pim, struct prefix *nht_p) -{ - struct nexthop *nh_node = NULL; - struct pim_nexthop_cache pnc; - struct pim_neighbor *nbr = NULL; - - memset(&pnc, 0, sizeof(struct pim_nexthop_cache)); - if (!pim_find_or_track_nexthop(pim, nht_p, NULL, NULL, &pnc)) - return; - - for (nh_node = pnc.nexthop; nh_node; nh_node = nh_node->next) { - if (nh_node->gate.ipv4.s_addr != 0) - continue; - - struct interface *ifp1 = - if_lookup_by_index(nh_node->ifindex, pim->vrf_id); - nbr = pim_neighbor_find_if(ifp1); - if (!nbr) - continue; - - nh_node->gate.ipv4 = nbr->source_addr; - if (PIM_DEBUG_PIM_NHT) { - char str[PREFIX_STRLEN]; - char str1[INET_ADDRSTRLEN]; - pim_inet4_dump("", nbr->source_addr, str1, - sizeof(str1)); - pim_addr_dump("", nht_p, str, sizeof(str)); - zlog_debug( - "%s: addr %s new nexthop addr %s interface %s", - __PRETTY_FUNCTION__, str, str1, ifp1->name); - } - } -} - /* Update Upstream nexthop info based on Nexthop update received from Zebra.*/ static int pim_update_upstream_nh_helper(struct hash_bucket *bucket, void *arg) { diff --git a/pimd/pim_nht.h b/pimd/pim_nht.h index 6eff7bbc89..e3e9f578c9 100644 --- a/pimd/pim_nht.h +++ b/pimd/pim_nht.h @@ -65,7 +65,6 @@ int pim_ecmp_nexthop_lookup(struct pim_instance *pim, struct prefix *grp, int neighbor_needed); void pim_sendmsg_zebra_rnh(struct pim_instance *pim, struct zclient *zclient, struct pim_nexthop_cache *pnc, int command); -void pim_resolve_upstream_nh(struct pim_instance *pim, struct prefix *nht_p); int pim_ecmp_fib_lookup_if_vif_index(struct pim_instance *pim, struct prefix *src, struct prefix *grp); void pim_rp_nexthop_del(struct rp_info *rp_info); From c9cd7fbc3f4e0c7722897db71220ae997de1975f Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 1 Apr 2019 12:31:28 -0400 Subject: [PATCH 2/3] pimd: Limit lookup of neighbor since we know we have one When a new pim neighbor comes up, we need to go through and mark nexthops that we have received from zebra for reachability tracking so we can refigure stuff. If we pass in the new neighbor we can limit the search to those nexthops out the interface that the new neighbor has come up. Signed-off-by: Donald Sharp --- pimd/pim_neighbor.c | 2 +- pimd/pim_rp.c | 7 +++---- pimd/pim_rp.h | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/pimd/pim_neighbor.c b/pimd/pim_neighbor.c index 436f2dec27..a63b09fc1f 100644 --- a/pimd/pim_neighbor.c +++ b/pimd/pim_neighbor.c @@ -540,7 +540,7 @@ pim_neighbor_add(struct interface *ifp, struct in_addr source_addr, Upon PIM neighbor UP, iterate all RPs and update nexthop cache with this neighbor. */ - pim_resolve_rp_nh(pim_ifp->pim); + pim_resolve_rp_nh(pim_ifp->pim, neigh); pim_rp_setup(pim_ifp->pim); diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c index b7db7d0418..4e285720a2 100644 --- a/pimd/pim_rp.c +++ b/pimd/pim_rp.c @@ -1206,14 +1206,13 @@ void pim_rp_show_information(struct pim_instance *pim, struct vty *vty, bool uj) } } -void pim_resolve_rp_nh(struct pim_instance *pim) +void pim_resolve_rp_nh(struct pim_instance *pim, struct pim_neighbor *nbr) { struct listnode *node = NULL; struct rp_info *rp_info = NULL; struct nexthop *nh_node = NULL; struct prefix nht_p; struct pim_nexthop_cache pnc; - struct pim_neighbor *nbr = NULL; for (ALL_LIST_ELEMENTS_RO(pim->rp_list, node, rp_info)) { if (rp_info->rp.rpf_addr.u.prefix4.s_addr == INADDR_NONE) @@ -1233,8 +1232,8 @@ void pim_resolve_rp_nh(struct pim_instance *pim) struct interface *ifp1 = if_lookup_by_index( nh_node->ifindex, pim->vrf_id); - nbr = pim_neighbor_find_if(ifp1); - if (!nbr) + + if (nbr->interface != ifp1) continue; nh_node->gate.ipv4 = nbr->source_addr; diff --git a/pimd/pim_rp.h b/pimd/pim_rp.h index e0f8e16be0..402ec30aba 100644 --- a/pimd/pim_rp.h +++ b/pimd/pim_rp.h @@ -68,7 +68,7 @@ struct pim_rpf *pim_rp_g(struct pim_instance *pim, struct in_addr group); void pim_rp_show_information(struct pim_instance *pim, struct vty *vty, bool uj); -void pim_resolve_rp_nh(struct pim_instance *pim); +void pim_resolve_rp_nh(struct pim_instance *pim, struct pim_neighbor *nbr); int pim_rp_list_cmp(void *v1, void *v2); struct rp_info *pim_rp_find_match_group(struct pim_instance *pim, const struct prefix *group); From ade155e146939906da26ce52f9edb1052fadd015 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 1 Apr 2019 15:41:32 -0400 Subject: [PATCH 3/3] pimd: pim_nexthop_lookup should return true/false The current reverse logic led to this code construct if (!pim_nexthop_lookup(...)) { //Do something successfull } This is backwards and will cause logic errors when people use this code. Fix to use true/false for success/failure. Signed-off-by: Donald Sharp --- pimd/pim_cmd.c | 2 +- pimd/pim_igmp_mtrace.c | 14 +++----------- pimd/pim_mroute.c | 7 +++---- pimd/pim_rpf.c | 14 +++++++------- pimd/pim_rpf.h | 4 ++-- 5 files changed, 16 insertions(+), 25 deletions(-) diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c index f9140802fb..3e61782f8d 100644 --- a/pimd/pim_cmd.c +++ b/pimd/pim_cmd.c @@ -5026,7 +5026,7 @@ DEFUN (show_ip_rib, return CMD_WARNING; } - if (pim_nexthop_lookup(vrf->info, &nexthop, addr, 0)) { + if (!pim_nexthop_lookup(vrf->info, &nexthop, addr, 0)) { vty_out(vty, "Failure querying RIB nexthop for unicast address %s\n", addr_str); diff --git a/pimd/pim_igmp_mtrace.c b/pimd/pim_igmp_mtrace.c index f51e0c0d2f..0758e2f784 100644 --- a/pimd/pim_igmp_mtrace.c +++ b/pimd/pim_igmp_mtrace.c @@ -66,16 +66,13 @@ static bool mtrace_fwd_info_weak(struct pim_instance *pim, struct pim_nexthop nexthop; struct interface *ifp_in; struct in_addr nh_addr; - int ret; char nexthop_str[INET_ADDRSTRLEN]; nh_addr.s_addr = 0; memset(&nexthop, 0, sizeof(nexthop)); - ret = pim_nexthop_lookup(pim, &nexthop, mtracep->src_addr, 1); - - if (ret != 0) { + if (!pim_nexthop_lookup(pim, &nexthop, mtracep->src_addr, 1)) { if (PIM_DEBUG_MTRACE) zlog_debug("mtrace not found neighbor"); return false; @@ -418,9 +415,7 @@ static int mtrace_un_forward_packet(struct pim_instance *pim, struct ip *ip_hdr, if (interface == NULL) { memset(&nexthop, 0, sizeof(nexthop)); - ret = pim_nexthop_lookup(pim, &nexthop, ip_hdr->ip_dst, 0); - - if (ret != 0) { + if (!pim_nexthop_lookup(pim, &nexthop, ip_hdr->ip_dst, 0)) { close(fd); if (PIM_DEBUG_MTRACE) zlog_warn( @@ -568,7 +563,6 @@ static int mtrace_send_response(struct pim_instance *pim, struct igmp_mtrace *mtracep, size_t mtrace_len) { struct pim_nexthop nexthop; - int ret; mtracep->type = PIM_IGMP_MTRACE_RESPONSE; @@ -599,9 +593,7 @@ static int mtrace_send_response(struct pim_instance *pim, } else { memset(&nexthop, 0, sizeof(nexthop)); /* TODO: should use unicast rib lookup */ - ret = pim_nexthop_lookup(pim, &nexthop, mtracep->rsp_addr, 1); - - if (ret != 0) { + if (!pim_nexthop_lookup(pim, &nexthop, mtracep->rsp_addr, 1)) { if (PIM_DEBUG_MTRACE) zlog_warn( "Dropped response qid=%ud, no route to " diff --git a/pimd/pim_mroute.c b/pimd/pim_mroute.c index bba4031744..92065ff957 100644 --- a/pimd/pim_mroute.c +++ b/pimd/pim_mroute.c @@ -513,8 +513,7 @@ static int pim_mroute_msg_wrvifwhole(int fd, struct interface *ifp, if (!PIM_UPSTREAM_FLAG_TEST_FHR(up->flags)) { // No if channel, but upstream we are at the RP. if (pim_nexthop_lookup(pim_ifp->pim, &source, - up->upstream_register, 0) - == 0) { + up->upstream_register, 0)) { pim_register_stop_send(source.interface, &sg, pim_ifp->primary_address, up->upstream_register); @@ -531,8 +530,8 @@ static int pim_mroute_msg_wrvifwhole(int fd, struct interface *ifp, } else { if (I_am_RP(pim_ifp->pim, up->sg.grp)) { if (pim_nexthop_lookup(pim_ifp->pim, &source, - up->upstream_register, 0) - == 0) + up->upstream_register, + 0)) pim_register_stop_send( source.interface, &sg, pim_ifp->primary_address, diff --git a/pimd/pim_rpf.c b/pimd/pim_rpf.c index ee145a5b51..55f788b5bb 100644 --- a/pimd/pim_rpf.c +++ b/pimd/pim_rpf.c @@ -48,8 +48,8 @@ void pim_rpf_set_refresh_time(struct pim_instance *pim) pim->last_route_change_time); } -int pim_nexthop_lookup(struct pim_instance *pim, struct pim_nexthop *nexthop, - struct in_addr addr, int neighbor_needed) +bool pim_nexthop_lookup(struct pim_instance *pim, struct pim_nexthop *nexthop, + struct in_addr addr, int neighbor_needed) { struct pim_zlookup_nexthop nexthop_tab[MULTIPATH_NUM]; struct pim_neighbor *nbr = NULL; @@ -65,7 +65,7 @@ int pim_nexthop_lookup(struct pim_instance *pim, struct pim_nexthop *nexthop, * it will never work */ if (addr.s_addr == INADDR_NONE) - return -1; + return false; if ((nexthop->last_lookup.s_addr == addr.s_addr) && (nexthop->last_lookup_time > pim->last_route_change_time)) { @@ -83,7 +83,7 @@ int pim_nexthop_lookup(struct pim_instance *pim, struct pim_nexthop *nexthop, pim->last_route_change_time, nexthop_str); } pim->nexthop_lookups_avoided++; - return 0; + return true; } else { if (PIM_DEBUG_TRACE) { char addr_str[INET_ADDRSTRLEN]; @@ -107,7 +107,7 @@ int pim_nexthop_lookup(struct pim_instance *pim, struct pim_nexthop *nexthop, zlog_warn( "%s %s: could not find nexthop ifindex for address %s", __FILE__, __PRETTY_FUNCTION__, addr_str); - return -1; + return false; } while (!found && (i < num_ifindex)) { @@ -179,9 +179,9 @@ int pim_nexthop_lookup(struct pim_instance *pim, struct pim_nexthop *nexthop, nexthop->last_lookup = addr; nexthop->last_lookup_time = pim_time_monotonic_usec(); nexthop->nbr = nbr; - return 0; + return true; } else - return -1; + return false; } static int nexthop_mismatch(const struct pim_nexthop *nh1, diff --git a/pimd/pim_rpf.h b/pimd/pim_rpf.h index a4793df667..57bb22674f 100644 --- a/pimd/pim_rpf.h +++ b/pimd/pim_rpf.h @@ -59,8 +59,8 @@ struct pim_upstream; unsigned int pim_rpf_hash_key(void *arg); bool pim_rpf_equal(const void *arg1, const void *arg2); -int pim_nexthop_lookup(struct pim_instance *pim, struct pim_nexthop *nexthop, - struct in_addr addr, int neighbor_needed); +bool pim_nexthop_lookup(struct pim_instance *pim, struct pim_nexthop *nexthop, + struct in_addr addr, int neighbor_needed); enum pim_rpf_result pim_rpf_update(struct pim_instance *pim, struct pim_upstream *up, struct pim_rpf *old, uint8_t is_new);