From 815c33c92f10c112dba83f4ed46a6eaaa55edbb4 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Thu, 27 Apr 2017 11:01:32 -0700 Subject: [PATCH 1/5] pimd: fix channel_oil and upstream RPF in sync During PIM Neighbor change/UP event, pim_scan_oil api scans all channel oil to see any rpf impacted. Instead of passing current upstream's RPF it passes current RPF as 0 and does query to rib for nexhtop (without ECMP/Rebalance). This creates inconsist RPF between Upstream and Channel oil. In Channel Oil keep backward pointer to upstream DB and fetch up's RPF and passed to channel_oil scan. Decrement channel_oil ref_count in upstream_del when decrementing up ref_count and it is not the last. Created ECMP based FIB lookup API. Testing Done: Performed following testing on tester setup: 5 x LHR, 4 x MSDP Spines, 6 Sources each sending to 1023 groups from one of the spines. Total send rate 8Mpps. Test that caused problems was to reboot every device at the same time. After fix performed 5 iterations of reboot devices and show no sign of the problem. Signed-off-by: Chirag Shah --- pimd/pim_igmpv3.c | 5 +- pimd/pim_nht.c | 295 ++++++++++++++++++++++++-------------------- pimd/pim_nht.h | 3 + pimd/pim_oil.c | 3 + pimd/pim_oil.h | 1 + pimd/pim_rp.c | 14 +-- pimd/pim_upstream.c | 5 +- pimd/pim_zebra.c | 147 ++++++++++------------ 8 files changed, 245 insertions(+), 228 deletions(-) diff --git a/pimd/pim_igmpv3.c b/pimd/pim_igmpv3.c index 8c7083d83d..86509a20c4 100644 --- a/pimd/pim_igmpv3.c +++ b/pimd/pim_igmpv3.c @@ -357,10 +357,11 @@ void igmp_source_delete(struct igmp_source *source) char source_str[INET_ADDRSTRLEN]; pim_inet4_dump("", group->group_addr, group_str, sizeof(group_str)); pim_inet4_dump("", source->source_addr, source_str, sizeof(source_str)); - zlog_debug("Deleting IGMP source %s for group %s from socket %d interface %s", + zlog_debug("Deleting IGMP source %s for group %s from socket %d interface %s c_oil ref_count %d", source_str, group_str, group->group_igmp_sock->fd, - group->group_igmp_sock->interface->name); + group->group_igmp_sock->interface->name, + source->source_channel_oil ? source->source_channel_oil->oil_ref_count : 0); } source_timer_off(group, source); diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c index 98c98cdf24..c5f8d1d826 100644 --- a/pimd/pim_nht.c +++ b/pimd/pim_nht.c @@ -147,7 +147,9 @@ pim_nexthop_cache_add (struct pim_rpf *rpf_addr) return pnc; } -/* This API is used to Register an address with Zebra */ +/* This API is used to Register an address with Zebra + ret 1 means nexthop cache is found. +*/ int pim_find_or_track_nexthop (struct prefix *addr, struct pim_upstream *up, struct rp_info *rp, @@ -167,13 +169,6 @@ pim_find_or_track_nexthop (struct prefix *addr, struct pim_upstream *up, pnc = pim_nexthop_cache_find (&rpf); if (!pnc) { - if (PIM_DEBUG_ZEBRA) - { - char buf[PREFIX2STR_BUFFER]; - prefix2str (&rpf.rpf_addr, buf, sizeof (buf)); - zlog_debug ("%s: NHT New PNC allocated for addr %s ", - __PRETTY_FUNCTION__, buf); - } pnc = pim_nexthop_cache_add (&rpf); if (pnc) pim_sendmsg_zebra_rnh (zclient, pnc, ZEBRA_NEXTHOP_REGISTER); @@ -213,8 +208,7 @@ pim_find_or_track_nexthop (struct prefix *addr, struct pim_upstream *up, { char buf[PREFIX2STR_BUFFER]; prefix2str (addr, buf, sizeof (buf)); - zlog_debug - ("%s: Add upstream %s node to pnc cached list, rpf %s", + zlog_debug ("%s: Add upstream %s node to pnc cached list, rpf %s", __PRETTY_FUNCTION__, up->sg_str, buf); } listnode_add_sort (pnc->upstream_list, up); @@ -271,11 +265,12 @@ pim_delete_tracked_nexthop (struct prefix *addr, struct pim_upstream *up, } /* Update RP nexthop info based on Nexthop update received from Zebra.*/ -static int +int pim_update_rp_nh (struct pim_nexthop_cache *pnc) { struct listnode *node = NULL; struct rp_info *rp_info = NULL; + int ret = 0; /*Traverse RP list and update each RP Nexthop info */ for (ALL_LIST_ELEMENTS_RO (pnc->rp_list, node, rp_info)) @@ -284,8 +279,8 @@ pim_update_rp_nh (struct pim_nexthop_cache *pnc) continue; //Compute PIM RPF using cached nexthop - pim_ecmp_nexthop_search (pnc, &rp_info->rp.source_nexthop, - &rp_info->rp.rpf_addr, &rp_info->group, 1); + ret = pim_ecmp_nexthop_search (pnc, &rp_info->rp.source_nexthop, + &rp_info->rp.rpf_addr, &rp_info->group, 1); if (PIM_DEBUG_TRACE) { @@ -298,7 +293,10 @@ pim_update_rp_nh (struct pim_nexthop_cache *pnc) } } - return 0; + if (ret) + return 0; + + return 1; } /* This API is used to traverse nexthop cache of RPF addr @@ -320,9 +318,9 @@ pim_resolve_upstream_nh (struct prefix *nht_p) { if (nh_node->gate.ipv4.s_addr == 0) { - nbr = - pim_neighbor_find_if (if_lookup_by_index - (nh_node->ifindex, VRF_DEFAULT)); + struct interface *ifp1 = if_lookup_by_index(nh_node->ifindex, + VRF_DEFAULT); + nbr = pim_neighbor_find_if (ifp1); if (nbr) { nh_node->gate.ipv4 = nbr->source_addr; @@ -333,9 +331,8 @@ pim_resolve_upstream_nh (struct prefix *nht_p) 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 ifindex %d ", - __PRETTY_FUNCTION__, str, str1, nh_node->ifindex); + zlog_debug ("%s: addr %s new nexthop addr %s interface %s", + __PRETTY_FUNCTION__, str, str1, ifp1->name); } } } @@ -368,18 +365,15 @@ pim_update_upstream_nh (struct pim_nexthop_cache *pnc) /* update kernel multicast forwarding cache (MFC) */ if (up->channel_oil) { - vif_index = - pim_if_find_vifindex_by_ifindex (up->rpf. - source_nexthop.interface-> - ifindex); + ifindex_t ifindex = up->rpf.source_nexthop.interface->ifindex; + vif_index = pim_if_find_vifindex_by_ifindex (ifindex); /* Pass Current selected NH vif index to mroute download */ if (vif_index) pim_scan_individual_oil (up->channel_oil, vif_index); else { if (PIM_DEBUG_ZEBRA) - zlog_debug - ("%s: NHT upstream %s channel_oil IIF %s vif_index is not valid", + zlog_debug ("%s: NHT upstream %s channel_oil IIF %s vif_index is not valid", __PRETTY_FUNCTION__, up->sg_str, up->rpf.source_nexthop.interface->name); } @@ -490,7 +484,7 @@ pim_compute_ecmp_hash (struct prefix * src, struct prefix * grp) } hash_val = jhash_2words (g, s, 101); - if (PIM_DEBUG_TRACE) + if (PIM_DEBUG_PIM_TRACE_DETAIL) { char buf[PREFIX2STR_BUFFER]; char bufg[PREFIX2STR_BUFFER]; @@ -519,49 +513,57 @@ pim_ecmp_nexthop_search (struct pim_nexthop_cache *pnc, if (!pnc || !pnc->nexthop_num || !nexthop) return -1; - if (qpim_ecmp_enable) + //Current Nexthop is VALID, check to stay on the current path. + if (nexthop->interface && nexthop->interface->info && + nexthop->mrib_nexthop_addr.u.prefix4.s_addr != + PIM_NET_INADDR_ANY) { - //User configured knob to explicitly switch to new path. + /* User configured knob to explicitly switch + to new path is disabled or current path + metric is less than nexthop update. + */ + if (qpim_ecmp_rebalance_enable == 0) { - //Current Nexthop is VALID then stay on the current path. - if (nexthop->interface && nexthop->interface->info && - nexthop->mrib_nexthop_addr.u.prefix4.s_addr != - PIM_NET_INADDR_ANY) + uint8_t curr_route_valid = 0; + //Check if current nexthop is present in new updated Nexthop list. + //If the current nexthop is not valid, candidate to choose new Nexthop. + for (nh_node = pnc->nexthop; nh_node; nh_node = nh_node->next) + curr_route_valid = (nexthop->interface->ifindex == nh_node->ifindex); + + if (curr_route_valid && + !pim_if_connected_to_source (nexthop->interface, + src->u.prefix4)) { - if (neighbor_needed - && !pim_if_connected_to_source (nexthop->interface, - src->u.prefix4)) + nbr = pim_neighbor_find (nexthop->interface, + nexthop->mrib_nexthop_addr.u.prefix4); + if (!nbr && !if_is_loopback (nexthop->interface)) { - nbr = pim_neighbor_find (nexthop->interface, - nexthop->mrib_nexthop_addr. - u.prefix4); - if (!nbr && !if_is_loopback (nexthop->interface)) + if (PIM_DEBUG_TRACE) + zlog_debug ("%s: current nexthop does not have nbr ", + __PRETTY_FUNCTION__); + } + else + { + if (PIM_DEBUG_TRACE) { - if (PIM_DEBUG_TRACE) - zlog_debug ("%s: current nexthop does not have nbr ", - __PRETTY_FUNCTION__); - } - else - { - if (PIM_DEBUG_TRACE) - { - char src_str[INET_ADDRSTRLEN]; - pim_inet4_dump ("", src->u.prefix4, src_str, - sizeof (src_str)); - char grp_str[INET_ADDRSTRLEN]; - pim_inet4_dump ("", grp->u.prefix4, grp_str, - sizeof (grp_str)); - zlog_debug - ("%s: %s %s current nexthop %d is valid, not choosing new path", - __PRETTY_FUNCTION__, src_str, grp_str, - nexthop->interface->ifindex); - } - return 0; + char src_str[INET_ADDRSTRLEN]; + pim_inet4_dump ("", src->u.prefix4, src_str, + sizeof (src_str)); + char grp_str[INET_ADDRSTRLEN]; + pim_inet4_dump ("", grp->u.prefix4, grp_str, + sizeof (grp_str)); + zlog_debug ("%s: (%s, %s) current nexthop %s is valid, skipping new path selection", + __PRETTY_FUNCTION__, src_str, grp_str, + nexthop->interface->name); } + return 0; } } } + } + if (qpim_ecmp_enable) + { //PIM ECMP flag is enable then choose ECMP path. hash_val = pim_compute_ecmp_hash (src, grp); mod_val = hash_val % pnc->nexthop_num; @@ -574,7 +576,7 @@ pim_ecmp_nexthop_search (struct pim_nexthop_cache *pnc, nh_node = nh_node->next) { first_ifindex = nh_node->ifindex; - ifp = if_lookup_by_index (first_ifindex, VRF_DEFAULT); + ifp = if_lookup_by_index(first_ifindex, VRF_DEFAULT); if (!ifp) { if (PIM_DEBUG_ZEBRA) @@ -582,8 +584,7 @@ pim_ecmp_nexthop_search (struct pim_nexthop_cache *pnc, char addr_str[INET_ADDRSTRLEN]; pim_inet4_dump ("", src->u.prefix4, addr_str, sizeof (addr_str)); - zlog_debug - ("%s %s: could not find interface for ifindex %d (address %s)", + zlog_debug ("%s %s: could not find interface for ifindex %d (address %s)", __FILE__, __PRETTY_FUNCTION__, first_ifindex, addr_str); } if (nh_iter == mod_val) @@ -598,8 +599,7 @@ pim_ecmp_nexthop_search (struct pim_nexthop_cache *pnc, char addr_str[INET_ADDRSTRLEN]; pim_inet4_dump ("", src->u.prefix4, addr_str, sizeof (addr_str)); - zlog_debug - ("%s: multicast not enabled on input interface %s (ifindex=%d, RPF for source %s)", + zlog_debug ("%s: multicast not enabled on input interface %s (ifindex=%d, RPF for source %s)", __PRETTY_FUNCTION__, ifp->name, first_ifindex, addr_str); } if (nh_iter == mod_val) @@ -617,9 +617,8 @@ pim_ecmp_nexthop_search (struct pim_nexthop_cache *pnc, if (!nbr && !if_is_loopback (ifp)) { if (PIM_DEBUG_ZEBRA) - zlog_debug - ("%s: pim nbr not found on input interface %s", - __PRETTY_FUNCTION__, ifp->name); + zlog_debug ("%s: pim nbr not found on input interface %s", + __PRETTY_FUNCTION__, ifp->name); if (nh_iter == mod_val) mod_val++; //Select nexthpath nh_iter++; @@ -639,30 +638,22 @@ pim_ecmp_nexthop_search (struct pim_nexthop_cache *pnc, nexthop->last_lookup_time = pim_time_monotonic_usec (); nexthop->nbr = nbr; found = 1; - if (PIM_DEBUG_ZEBRA) { - char buf[NEXTHOP_STRLEN]; - char buf2[PREFIX2STR_BUFFER]; - char buf3[PREFIX2STR_BUFFER]; - char buf4[PREFIX2STR_BUFFER]; + char buf[INET_ADDRSTRLEN]; + char buf2[INET_ADDRSTRLEN]; + char buf3[INET_ADDRSTRLEN]; pim_inet4_dump ("", src->u.prefix4, buf2, sizeof (buf2)); - if (grp) - pim_inet4_dump ("", grp->u.prefix4, buf3, - sizeof (buf3)); + pim_inet4_dump ("", grp->u.prefix4, buf3, sizeof (buf3)); pim_inet4_dump ("", - nexthop->mrib_nexthop_addr.u.prefix4, buf4, - sizeof (buf4)); - snprintf (buf, sizeof (buf), "%s if %u", - inet_ntoa (nh_node->gate.ipv4), nh_node->ifindex); - zlog_debug - ("%s: NHT %s %s selected nhop interface %s nhop %s (%s) mod_val:%u iter:%d ecmp_enable:%d", - __PRETTY_FUNCTION__, buf2, grp ? buf3 : " ", ifp->name, - buf, buf4, mod_val, nh_iter, qpim_ecmp_enable); + nexthop->mrib_nexthop_addr.u.prefix4, buf, + sizeof (buf)); + zlog_debug ("%s: (%s, %s) selected nhop interface %s addr %s mod_val %u iter %d ecmp %d", + __PRETTY_FUNCTION__, buf2, buf3, ifp->name, + buf, mod_val, nh_iter, qpim_ecmp_enable); } } nh_iter++; - } if (found) @@ -717,8 +708,7 @@ pim_parse_nexthop_update (int command, struct zclient *zclient, { char buf[PREFIX2STR_BUFFER]; prefix2str (&rpf.rpf_addr, buf, sizeof (buf)); - zlog_debug - ("%s: Skipping NHT update, addr %s is not in local cached DB.", + zlog_debug ("%s: Skipping NHT update, addr %s is not in local cached DB.", __PRETTY_FUNCTION__, buf); } return 0; @@ -737,16 +727,6 @@ pim_parse_nexthop_update (int command, struct zclient *zclient, metric = stream_getl (s); nexthop_num = stream_getc (s); - if (PIM_DEBUG_TRACE) - { - char buf[PREFIX2STR_BUFFER]; - prefix2str (&p, buf, sizeof (buf)); - zlog_debug - ("%s: NHT Update for %s nexthop_num %d vrf:%d upcount %d rpcount %d", - __PRETTY_FUNCTION__, buf, nexthop_num, vrf_id, - listcount (pnc->upstream_list), listcount (pnc->rp_list)); - } - if (nexthop_num) { pnc->nexthop_num = 0; //Only increment for pim enabled rpf. @@ -774,9 +754,8 @@ pim_parse_nexthop_update (int command, struct zclient *zclient, case NEXTHOP_TYPE_IPV6_IFINDEX: stream_get (&nexthop->gate.ipv6, s, 16); nexthop->ifindex = stream_getl (s); - nbr = - pim_neighbor_find_if (if_lookup_by_index - (nexthop->ifindex, VRF_DEFAULT)); + struct interface *ifp1 = if_lookup_by_index (nexthop->ifindex, VRF_DEFAULT); + nbr = pim_neighbor_find_if (ifp1); /* Overwrite with Nbr address as NH addr */ if (nbr) { @@ -786,21 +765,16 @@ pim_parse_nexthop_update (int command, struct zclient *zclient, char str[INET_ADDRSTRLEN]; pim_inet4_dump ("", nbr->source_addr, str, sizeof (str)); - zlog_debug - ("%s: NHT using pim nbr addr %s ifindex %d as rpf", - __PRETTY_FUNCTION__, str, nexthop->ifindex); + zlog_debug ("%s: NHT using pim nbr addr %s interface %s as rpf", + __PRETTY_FUNCTION__, str, ifp1->name); } } else { if (PIM_DEBUG_TRACE) { - struct interface *ifp1 = - if_lookup_by_index (nexthop->ifindex, - VRF_DEFAULT); struct pim_interface *pim_ifp = ifp1->info; - zlog_debug - ("%s: NHT pim nbr not found on interface %s nbr count:%d ", + zlog_debug ("%s: NHT pim nbr not found on interface %s nbr count:%d ", __PRETTY_FUNCTION__, ifp1->name, pim_ifp->pim_neighbor_list->count); } @@ -818,9 +792,10 @@ pim_parse_nexthop_update (int command, struct zclient *zclient, { char p_str[PREFIX2STR_BUFFER]; prefix2str (&p, p_str, sizeof (p_str)); - zlog_debug ("%s: NHT addr %s %d-nhop via %s type %d", - __PRETTY_FUNCTION__, p_str, i + 1, - inet_ntoa (nexthop->gate.ipv4), nexthop->type); + zlog_debug ("%s: NHT addr %s %d-nhop via %s type %d distance:%u metric:%u ", + __PRETTY_FUNCTION__, p_str, i + 1, + inet_ntoa (nexthop->gate.ipv4), nexthop->type, distance, + metric); } ifp = if_lookup_by_index (nexthop->ifindex, VRF_DEFAULT); @@ -829,8 +804,7 @@ pim_parse_nexthop_update (int command, struct zclient *zclient, if (PIM_DEBUG_ZEBRA) { char buf[NEXTHOP_STRLEN]; - zlog_debug - ("%s: could not find interface for ifindex %d (addr %s)", + zlog_debug ("%s: could not find interface for ifindex %d (addr %s)", __PRETTY_FUNCTION__, nexthop->ifindex, nexthop2str (nexthop, buf, sizeof (buf))); } @@ -843,8 +817,7 @@ pim_parse_nexthop_update (int command, struct zclient *zclient, if (PIM_DEBUG_ZEBRA) { char buf[NEXTHOP_STRLEN]; - zlog_debug - ("%s: multicast not enabled on input interface %s (ifindex=%d, addr %s)", + zlog_debug ("%s: multicast not enabled on input interface %s (ifindex=%d, addr %s)", __PRETTY_FUNCTION__, ifp->name, nexthop->ifindex, nexthop2str (nexthop, buf, sizeof (buf))); } @@ -887,8 +860,7 @@ pim_parse_nexthop_update (int command, struct zclient *zclient, { char buf[PREFIX2STR_BUFFER]; prefix2str (&p, buf, sizeof (buf)); - zlog_debug - ("%s: NHT Update for %s nexthop_num:%d pim nexthop_num %d vrf:%d up %d rp %d", + zlog_debug ("%s: NHT Update for %s num_nh %d num_pim_nh %d vrf:%d up %d rp %d", __PRETTY_FUNCTION__, buf, nexthop_num, pnc->nexthop_num, vrf_id, listcount (pnc->upstream_list), listcount (pnc->rp_list)); } @@ -925,10 +897,8 @@ pim_ecmp_nexthop_lookup (struct pim_nexthop *nexthop, struct in_addr addr, __PRETTY_FUNCTION__, addr_str, nexthop->last_lookup_time); } - memset (nexthop_tab, 0, - sizeof (struct pim_zlookup_nexthop) * MULTIPATH_NUM); - num_ifindex = - zclient_lookup_nexthop (nexthop_tab, MULTIPATH_NUM, addr, + memset (nexthop_tab, 0, sizeof (struct pim_zlookup_nexthop) * MULTIPATH_NUM); + num_ifindex = zclient_lookup_nexthop (nexthop_tab, MULTIPATH_NUM, addr, PIM_NEXTHOP_LOOKUP_MAX); if (num_ifindex < 1) { @@ -939,13 +909,13 @@ pim_ecmp_nexthop_lookup (struct pim_nexthop *nexthop, struct in_addr addr, return -1; } - //If PIM ECMP enable then choose ECMP path + //If PIM ECMP enable then choose ECMP path. if (qpim_ecmp_enable) { hash_val = pim_compute_ecmp_hash (src, grp); mod_val = hash_val % num_ifindex; if (PIM_DEBUG_TRACE) - zlog_debug ("%s: hash_val %u mod_val %u ", + zlog_debug ("%s: hash_val %u mod_val %u", __PRETTY_FUNCTION__, hash_val, mod_val); } @@ -960,8 +930,7 @@ pim_ecmp_nexthop_lookup (struct pim_nexthop *nexthop, struct in_addr addr, { char addr_str[INET_ADDRSTRLEN]; pim_inet4_dump ("", addr, addr_str, sizeof (addr_str)); - zlog_debug - ("%s %s: could not find interface for ifindex %d (address %s)", + zlog_debug ("%s %s: could not find interface for ifindex %d (address %s)", __FILE__, __PRETTY_FUNCTION__, first_ifindex, addr_str); } if (i == mod_val) @@ -976,8 +945,7 @@ pim_ecmp_nexthop_lookup (struct pim_nexthop *nexthop, struct in_addr addr, { char addr_str[INET_ADDRSTRLEN]; pim_inet4_dump ("", addr, addr_str, sizeof (addr_str)); - zlog_debug - ("%s: multicast not enabled on input interface %s (ifindex=%d, RPF for source %s)", + zlog_debug ("%s: multicast not enabled on input interface %s (ifindex=%d, RPF for source %s)", __PRETTY_FUNCTION__, ifp->name, first_ifindex, addr_str); } if (i == mod_val) @@ -987,8 +955,7 @@ pim_ecmp_nexthop_lookup (struct pim_nexthop *nexthop, struct in_addr addr, } if (neighbor_needed && !pim_if_connected_to_source (ifp, addr)) { - nbr = - pim_neighbor_find (ifp, nexthop_tab[i].nexthop_addr.u.prefix4); + nbr = pim_neighbor_find (ifp, nexthop_tab[i].nexthop_addr.u.prefix4); if (PIM_DEBUG_PIM_TRACE_DETAIL) zlog_debug ("ifp name: %s, pim nbr: %p", ifp->name, nbr); if (!nbr && !if_is_loopback (ifp)) @@ -1001,8 +968,7 @@ pim_ecmp_nexthop_lookup (struct pim_nexthop *nexthop, struct in_addr addr, char addr_str[INET_ADDRSTRLEN]; pim_inet4_dump ("", addr, addr_str, sizeof (addr_str)); - zlog_debug - ("%s: NBR not found on input interface %s (RPF for source %s)", + zlog_debug ("%s: NBR not found on input interface %s (RPF for source %s)", __PRETTY_FUNCTION__, ifp->name, addr_str); } continue; @@ -1018,8 +984,7 @@ pim_ecmp_nexthop_lookup (struct pim_nexthop *nexthop, struct in_addr addr, pim_addr_dump ("", &nexthop_tab[i].nexthop_addr, nexthop_str, sizeof (nexthop_str)); pim_inet4_dump ("", addr, addr_str, sizeof (addr_str)); - zlog_debug - ("%s %s: found nexthop %s for addr %s interface %s metric=%d pref=%d", + zlog_debug ("%s %s: found nhop %s for addr %s interface %s metric %d dist %d", __FILE__, __PRETTY_FUNCTION__, nexthop_str, addr_str, ifp->name, nexthop_tab[i].route_metric, nexthop_tab[i].protocol_distance); @@ -1041,3 +1006,67 @@ pim_ecmp_nexthop_lookup (struct pim_nexthop *nexthop, struct in_addr addr, else return -1; } + +int pim_ecmp_fib_lookup_if_vif_index(struct in_addr addr, + struct prefix *src, struct prefix *grp) +{ + struct pim_zlookup_nexthop nexthop_tab[MULTIPATH_NUM]; + int num_ifindex; + int vif_index; + ifindex_t first_ifindex; + uint32_t hash_val = 0, mod_val = 0; + + memset (nexthop_tab, 0, sizeof (struct pim_zlookup_nexthop) * MULTIPATH_NUM); + num_ifindex = zclient_lookup_nexthop(nexthop_tab, MULTIPATH_NUM, addr, + PIM_NEXTHOP_LOOKUP_MAX); + if (num_ifindex < 1) + { + if (PIM_DEBUG_ZEBRA) + { + char addr_str[INET_ADDRSTRLEN]; + pim_inet4_dump("", addr, addr_str, sizeof(addr_str)); + zlog_debug("%s %s: could not find nexthop ifindex for address %s", + __FILE__, __PRETTY_FUNCTION__, + addr_str); + } + return -1; + } + + //If PIM ECMP enable then choose ECMP path. + if (qpim_ecmp_enable) + { + hash_val = pim_compute_ecmp_hash (src, grp); + mod_val = hash_val % num_ifindex; + if (PIM_DEBUG_TRACE) + zlog_debug ("%s: hash_val %u mod_val %u", + __PRETTY_FUNCTION__, hash_val, mod_val); + } + + first_ifindex = nexthop_tab[mod_val].ifindex; + + if (PIM_DEBUG_ZEBRA) + { + char addr_str[INET_ADDRSTRLEN]; + pim_inet4_dump("", addr, addr_str, sizeof(addr_str)); + zlog_debug("%s %s: found nexthop ifindex=%d (interface %s) for address %s", + __FILE__, __PRETTY_FUNCTION__, + first_ifindex, ifindex2ifname(first_ifindex, VRF_DEFAULT), addr_str); + } + + vif_index = pim_if_find_vifindex_by_ifindex(first_ifindex); + + if (vif_index < 0) + { + if (PIM_DEBUG_ZEBRA) + { + char addr_str[INET_ADDRSTRLEN]; + pim_inet4_dump("", addr, addr_str, sizeof(addr_str)); + zlog_debug("%s %s: low vif_index=%d < 1 nexthop for address %s", + __FILE__, __PRETTY_FUNCTION__, + vif_index, addr_str); + } + return -2; + } + + return vif_index; +} diff --git a/pimd/pim_nht.h b/pimd/pim_nht.h index b4b2d91e47..0ccc5399c3 100644 --- a/pimd/pim_nht.h +++ b/pimd/pim_nht.h @@ -65,5 +65,8 @@ int pim_ecmp_nexthop_lookup (struct pim_nexthop *nexthop, struct in_addr addr, int neighbor_needed); void pim_sendmsg_zebra_rnh (struct zclient *zclient, struct pim_nexthop_cache *pnc, int command); +int pim_update_rp_nh (struct pim_nexthop_cache *pnc); void pim_resolve_upstream_nh (struct prefix *nht_p); +int pim_ecmp_fib_lookup_if_vif_index(struct in_addr addr, + struct prefix *src, struct prefix *grp); #endif diff --git a/pimd/pim_oil.c b/pimd/pim_oil.c index 2d4aa3febc..7f5f3970ae 100644 --- a/pimd/pim_oil.c +++ b/pimd/pim_oil.c @@ -164,6 +164,7 @@ struct channel_oil *pim_channel_oil_add(struct prefix_sg *sg, } c_oil->oil.mfcc_parent = input_vif_index; ++c_oil->oil_ref_count; + c_oil->up = pim_upstream_find(sg); //channel might be present prior to upstream return c_oil; } @@ -188,6 +189,7 @@ struct channel_oil *pim_channel_oil_add(struct prefix_sg *sg, c_oil->oil.mfcc_parent = input_vif_index; c_oil->oil_ref_count = 1; c_oil->installed = 0; + c_oil->up = pim_upstream_find(sg); listnode_add_sort(pim_channel_oil_list, c_oil); @@ -204,6 +206,7 @@ void pim_channel_oil_del(struct channel_oil *c_oil) * into pim_channel_oil_free() because the later is * called by list_delete_all_node() */ + c_oil->up = NULL; listnode_delete(pim_channel_oil_list, c_oil); hash_release (pim_channel_oil_hash, c_oil); diff --git a/pimd/pim_oil.h b/pimd/pim_oil.h index e90cd5fc19..a7bb23cd0e 100644 --- a/pimd/pim_oil.h +++ b/pimd/pim_oil.h @@ -79,6 +79,7 @@ struct channel_oil { time_t oif_creation[MAXVIFS]; uint32_t oif_flags[MAXVIFS]; struct channel_counts cc; + struct pim_upstream *up; }; extern struct list *pim_channel_oil_list; diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c index 776687d011..062ae6e808 100644 --- a/pimd/pim_rp.c +++ b/pimd/pim_rp.c @@ -281,7 +281,7 @@ pim_rp_check_interfaces (struct rp_info *rp_info) int pim_rp_new (const char *rp, const char *group_range, const char *plist) { - int result; + int result = 0; struct rp_info *rp_info; struct rp_info *rp_all; struct prefix group_all; @@ -759,7 +759,7 @@ pim_rp_g (struct in_addr group) char buf1[PREFIX2STR_BUFFER]; prefix2str (&nht_p, buf, sizeof (buf)); prefix2str (&g, buf1, sizeof (buf1)); - zlog_debug ("%s: NHT nexthop cache not found for RP %s grp %s", + zlog_debug ("%s: Nexthop cache not found for RP %s grp %s register with Zebra NHT", __PRETTY_FUNCTION__, buf, buf1); } pim_rpf_set_refresh_time (); @@ -972,8 +972,7 @@ pim_resolve_rp_nh (void) { if (nh_node->gate.ipv4.s_addr == 0) { - nbr = - pim_neighbor_find_if (if_lookup_by_index + nbr = pim_neighbor_find_if (if_lookup_by_index (nh_node->ifindex, VRF_DEFAULT)); if (nbr) { @@ -982,14 +981,15 @@ pim_resolve_rp_nh (void) { char str[PREFIX_STRLEN]; char str1[INET_ADDRSTRLEN]; + struct interface *ifp1 = if_lookup_by_index(nh_node->ifindex, + VRF_DEFAULT); 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 ifindex %d ", + zlog_debug ("%s: addr %s new nexthop addr %s interface %s", __PRETTY_FUNCTION__, str, str1, - nh_node->ifindex); + ifp1->name); } } } diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c index 5a407f9f9e..2afcd6aeea 100644 --- a/pimd/pim_upstream.c +++ b/pimd/pim_upstream.c @@ -162,8 +162,9 @@ pim_upstream_del(struct pim_upstream *up, const char *name) struct prefix nht_p; if (PIM_DEBUG_TRACE) - zlog_debug ("%s(%s): Delete %s ref count: %d, flags: %d (Pre decrement)", - __PRETTY_FUNCTION__, name, up->sg_str, up->ref_count, up->flags); + zlog_debug ("%s(%s): Delete %s ref count: %d , flags: %d c_oil ref count %d (Pre decrement)", + __PRETTY_FUNCTION__, name, up->sg_str, up->ref_count, up->flags, + up->channel_oil->oil_ref_count); --up->ref_count; diff --git a/pimd/pim_zebra.c b/pimd/pim_zebra.c index 80e7d77642..baf296623f 100644 --- a/pimd/pim_zebra.c +++ b/pimd/pim_zebra.c @@ -53,7 +53,6 @@ static struct zclient *zclient = NULL; -static int fib_lookup_if_vif_index(struct in_addr addr); /* Router-id update message from zebra. */ static int pim_router_id_update_zebra(int command, struct zclient *zclient, @@ -469,7 +468,28 @@ pim_scan_individual_oil (struct channel_oil *c_oil, int in_vif_index) if (in_vif_index) input_iface_vif_index = in_vif_index; else - input_iface_vif_index = fib_lookup_if_vif_index (vif_source); + { + struct prefix src, grp; + + src.family = AF_INET; + src.prefixlen = IPV4_MAX_BITLEN; + src.u.prefix4 = vif_source; + grp.family = AF_INET; + grp.prefixlen = IPV4_MAX_BITLEN; + grp.u.prefix4 = c_oil->oil.mfcc_mcastgrp; + + if (PIM_DEBUG_ZEBRA) + { + char source_str[INET_ADDRSTRLEN]; + char group_str[INET_ADDRSTRLEN]; + pim_inet4_dump("", c_oil->oil.mfcc_origin, source_str, sizeof(source_str)); + pim_inet4_dump("", c_oil->oil.mfcc_mcastgrp, group_str, sizeof(group_str)); + zlog_debug ("%s: channel_oil (%s, %s) upstream info is not present.", + __PRETTY_FUNCTION__, source_str, group_str); + } + input_iface_vif_index = pim_ecmp_fib_lookup_if_vif_index(vif_source, &src, &grp); + } + if (input_iface_vif_index < 1) { if (PIM_DEBUG_ZEBRA) @@ -557,12 +577,25 @@ void pim_scan_oil() struct listnode *node; struct listnode *nextnode; struct channel_oil *c_oil; + ifindex_t ifindex; + int vif_index = 0; qpim_scan_oil_last = pim_time_monotonic_sec(); ++qpim_scan_oil_events; for (ALL_LIST_ELEMENTS(pim_channel_oil_list, node, nextnode, c_oil)) - pim_scan_individual_oil (c_oil, 0); + { + if (c_oil->up && c_oil->up->rpf.source_nexthop.interface) + { + ifindex = c_oil->up->rpf.source_nexthop.interface->ifindex; + vif_index = pim_if_find_vifindex_by_ifindex (ifindex); + /* Pass Current selected NH vif index to mroute download */ + if (vif_index) + pim_scan_individual_oil (c_oil, vif_index); + } + else + pim_scan_individual_oil (c_oil, 0); + } } static int on_rpf_cache_refresh(struct thread *t) @@ -699,67 +732,6 @@ void igmp_anysource_forward_stop(struct igmp_group *group) igmp_source_forward_stop (source); } -static int fib_lookup_if_vif_index(struct in_addr addr) -{ - struct pim_zlookup_nexthop nexthop_tab[MULTIPATH_NUM]; - int num_ifindex; - int vif_index; - ifindex_t first_ifindex; - - num_ifindex = zclient_lookup_nexthop(nexthop_tab, - MULTIPATH_NUM, addr, - PIM_NEXTHOP_LOOKUP_MAX); - if (num_ifindex < 1) { - if (PIM_DEBUG_ZEBRA) - { - char addr_str[INET_ADDRSTRLEN]; - pim_inet4_dump("", addr, addr_str, sizeof(addr_str)); - zlog_debug("%s %s: could not find nexthop ifindex for address %s", - __FILE__, __PRETTY_FUNCTION__, - addr_str); - } - return -1; - } - - first_ifindex = nexthop_tab[0].ifindex; - - if (num_ifindex > 1) { - if (PIM_DEBUG_ZEBRA) - { - char addr_str[INET_ADDRSTRLEN]; - pim_inet4_dump("", addr, addr_str, sizeof(addr_str)); - zlog_debug("%s %s: FIXME ignoring multiple nexthop ifindex'es num_ifindex=%d for address %s (using only ifindex=%d)", - __FILE__, __PRETTY_FUNCTION__, - num_ifindex, addr_str, first_ifindex); - } - /* debug warning only, do not return */ - } - - if (PIM_DEBUG_ZEBRA) { - char addr_str[INET_ADDRSTRLEN]; - pim_inet4_dump("", addr, addr_str, sizeof(addr_str)); - zlog_debug("%s %s: found nexthop ifindex=%d (interface %s) for address %s", - __FILE__, __PRETTY_FUNCTION__, - first_ifindex, ifindex2ifname(first_ifindex, VRF_DEFAULT), addr_str); - } - - vif_index = pim_if_find_vifindex_by_ifindex(first_ifindex); - - if (vif_index < 0) { - if (PIM_DEBUG_ZEBRA) - { - char addr_str[INET_ADDRSTRLEN]; - pim_inet4_dump("", addr, addr_str, sizeof(addr_str)); - zlog_debug("%s %s: low vif_index=%d < 1 nexthop for address %s", - __FILE__, __PRETTY_FUNCTION__, - vif_index, addr_str); - } - return -2; - } - - return vif_index; -} - static void igmp_source_forward_reevaluate_one(struct igmp_source *source) { @@ -873,6 +845,7 @@ void igmp_source_forward_start(struct igmp_source *source) int ret = 0; struct pim_nexthop_cache out_pnc; struct pim_nexthop nexthop; + struct pim_upstream *up = NULL; if (!pim_rp_set_upstream_addr (&vif_source, source->source_addr, sg.grp)) return; @@ -883,20 +856,23 @@ void igmp_source_forward_start(struct igmp_source *source) nht_p.u.prefix4 = vif_source; memset (&out_pnc, 0, sizeof (struct pim_nexthop_cache)); + src.family = AF_INET; + src.prefixlen = IPV4_MAX_BITLEN; + src.u.prefix4 = vif_source; //RP or Src address + grp.family = AF_INET; + grp.prefixlen = IPV4_MAX_BITLEN; + grp.u.prefix4 = sg.grp; + if ((ret = pim_find_or_track_nexthop (&nht_p, NULL, NULL, &out_pnc)) == 1) { if (out_pnc.nexthop_num) { - src.family = AF_INET; - src.prefixlen = IPV4_MAX_BITLEN; - src.u.prefix4 = vif_source; //RP or Src address - grp.family = AF_INET; - grp.prefixlen = IPV4_MAX_BITLEN; - grp.u.prefix4 = sg.grp; - memset (&nexthop, 0, sizeof (nexthop)); + up = pim_upstream_find (&sg); + memset (&nexthop, 0, sizeof (struct pim_nexthop)); + if (up) + memcpy (&nexthop, &up->rpf.source_nexthop, sizeof (struct pim_nexthop)); //Compute PIM RPF using Cached nexthop - pim_ecmp_nexthop_search (&out_pnc, &nexthop, - &src, &grp, 0); + pim_ecmp_nexthop_search (&out_pnc, &nexthop, &src, &grp, 0); if (nexthop.interface) input_iface_vif_index = pim_if_find_vifindex_by_ifindex (nexthop.interface->ifindex); } @@ -914,7 +890,7 @@ void igmp_source_forward_start(struct igmp_source *source) } } else - input_iface_vif_index = fib_lookup_if_vif_index(vif_source); + input_iface_vif_index = pim_ecmp_fib_lookup_if_vif_index(vif_source, &src, &grp); if (PIM_DEBUG_ZEBRA) { @@ -1095,7 +1071,6 @@ void pim_forward_start(struct pim_ifchannel *ch) struct prefix nht_p, src, grp; int ret = 0; struct pim_nexthop_cache out_pnc; - struct pim_nexthop nexthop; /* Register addr with Zebra NHT */ nht_p.family = AF_INET; @@ -1106,8 +1081,7 @@ void pim_forward_start(struct pim_ifchannel *ch) grp.u.prefix4 = up->sg.grp; memset (&out_pnc, 0, sizeof (struct pim_nexthop_cache)); - if ((ret = - pim_find_or_track_nexthop (&nht_p, NULL, NULL, &out_pnc)) == 1) + if ((ret = pim_find_or_track_nexthop (&nht_p, NULL, NULL, &out_pnc)) == 1) { if (out_pnc.nexthop_num) { @@ -1117,11 +1091,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 (&nexthop, 0, sizeof (nexthop)); //Compute PIM RPF using Cached nexthop - pim_ecmp_nexthop_search (&out_pnc, &nexthop, &src, &grp, 0); - input_iface_vif_index = - pim_if_find_vifindex_by_ifindex (nexthop.interface->ifindex); + if (pim_ecmp_nexthop_search (&out_pnc, &up->rpf.source_nexthop, &src, &grp, 0) == 0) + input_iface_vif_index = pim_if_find_vifindex_by_ifindex (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 { @@ -1137,7 +1114,7 @@ void pim_forward_start(struct pim_ifchannel *ch) } } else - input_iface_vif_index = fib_lookup_if_vif_index (up->upstream_addr); + input_iface_vif_index = pim_ecmp_fib_lookup_if_vif_index(up->upstream_addr, &src, &grp); if (input_iface_vif_index < 1) { @@ -1153,8 +1130,10 @@ void pim_forward_start(struct pim_ifchannel *ch) } if (PIM_DEBUG_TRACE) { - zlog_debug ("%s: NHT entry %s update channel_oil vif_index %d ", - __PRETTY_FUNCTION__, up->sg_str, input_iface_vif_index); + struct interface *in_intf = pim_if_find_by_vif_index (input_iface_vif_index); + zlog_debug ("%s: Update channel_oil IIF %s VIFI %d entry %s ", + __PRETTY_FUNCTION__, in_intf ? in_intf->name : "NIL", + input_iface_vif_index, up->sg_str); } up->channel_oil = pim_channel_oil_add (&up->sg, input_iface_vif_index); if (!up->channel_oil) From 0c62a899978c96af64da7e867035d71d4882159a Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Mon, 10 Apr 2017 13:27:56 -0700 Subject: [PATCH 2/5] pimd: fix pim ecmp rebalance config write ip pim ecmp and ip pim ecmp rebalance configuration CLIs were not adding to Quagga.confg or running configuration. Added both the configuration write in Config write handler. Testing Done: Execute configuration cli and verified running config and Quagga.conf file containing both configuration. 03# show running-config Building configuration... Current configuration: ! ip multicast-routing ip pim rp 6.0.0.9 230.0.0.0/16 ip pim join-prune-interval 61 ip pim ecmp ip pim ecmp rebalance ! Signed-off-by: Chirag Shah --- pimd/pim_cmd.c | 1 + pimd/pim_vty.c | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c index d40ff0fe30..1224bc5fc8 100644 --- a/pimd/pim_cmd.c +++ b/pimd/pim_cmd.c @@ -4085,6 +4085,7 @@ DEFUN (ip_pim_ecmp_rebalance, "Enable PIM ECMP \n" "Enable PIM ECMP Rebalance\n") { + qpim_ecmp_enable = 1; qpim_ecmp_rebalance_enable = 1; return CMD_SUCCESS; diff --git a/pimd/pim_vty.c b/pimd/pim_vty.c index c8322b629a..754cf8468b 100644 --- a/pimd/pim_vty.c +++ b/pimd/pim_vty.c @@ -192,7 +192,16 @@ int pim_global_config_write(struct vty *vty) VTY_NEWLINE); ++writes; } - + if (qpim_ecmp_rebalance_enable) + { + vty_out (vty, "ip pim ecmp rebalance%s", VTY_NEWLINE); + ++writes; + } + else if (qpim_ecmp_enable) + { + vty_out (vty, "ip pim ecmp%s", VTY_NEWLINE); + ++writes; + } if (qpim_ssmpingd_list) { struct listnode *node; struct ssmpingd_sock *ss; From 9ab028b26daeeb180e2d38489c118ea119ec643d Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Wed, 3 May 2017 18:56:44 -0700 Subject: [PATCH 3/5] pimd: fix pimd crash when pim interface disabled Upon pim enabled disabled, current upstreams entries rpf update callback, needs to check if old rpf is still pim enabled otherwise do not trigger prune towards old rpf. Testing Done: Verified by disabling pim enabled rpf interface and crash is not observed upon disabling pim on rpf interface. Signed-off-by: Chirag Shah --- pimd/pim_jp_agg.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pimd/pim_jp_agg.c b/pimd/pim_jp_agg.c index ce4ddfd4a4..bbc51bd91c 100644 --- a/pimd/pim_jp_agg.c +++ b/pimd/pim_jp_agg.c @@ -127,6 +127,10 @@ pim_jp_agg_get_interface_upstream_switch_list (struct pim_rpf *rpf) struct pim_iface_upstream_switch *pius; struct listnode *node, *nnode; + /* Old interface is pim disabled */ + if (!pim_ifp) + return NULL; + for (ALL_LIST_ELEMENTS(pim_ifp->upstream_switch_list, node, nnode, pius)) { if (pius->address.s_addr == rpf->rpf_addr.u.prefix4.s_addr) @@ -323,7 +327,8 @@ pim_jp_agg_switch_interface (struct pim_rpf *orpf, */ /* send Prune(S,G) to the old upstream neighbor */ - pim_jp_agg_add_group (opius->us, up, false); + if (opius) + pim_jp_agg_add_group (opius->us, up, false); /* send Join(S,G) to the current upstream neighbor */ pim_jp_agg_add_group (npius->us, up, true); From dba88ccbb0a3b4f842671564e170755944e514e1 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Tue, 25 Apr 2017 18:56:00 -0700 Subject: [PATCH 4/5] pimd: replay IGMP static group upon if creation Upon static IGMP configured port down igmp source info is destroyed. Upon port up or quagga restart (if addr add) register IGMP sock with port, source, group. Testing Done: Configure IGMPv3 Report on Host swpx, ifdown/ifup swpx, verify ip mroute containng IGMP mroute tor-1# show ip mroute Source Group Proto Input Output TTL Uptime 30.1.1.1 225.1.1.21 IGMP swp2 swp3 1 00:00:05 Signed-off-by: Chirag Shah --- pimd/pim_iface.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/pimd/pim_iface.c b/pimd/pim_iface.c index bdad1c531c..cce87ae5fd 100644 --- a/pimd/pim_iface.c +++ b/pimd/pim_iface.c @@ -50,6 +50,8 @@ struct list *pim_ifchannel_list = NULL; static int pim_iface_vif_index[MAXVIFS]; static void pim_if_igmp_join_del_all(struct interface *ifp); +static int igmp_join_sock(const char *ifname, ifindex_t ifindex, + struct in_addr group_addr, struct in_addr source_addr); void pim_if_init (void) @@ -583,6 +585,35 @@ void pim_if_addr_add(struct connected *ifc) /* if addr new, add IGMP socket */ pim_igmp_sock_add(pim_ifp->igmp_socket_list, ifaddr, ifp); } + + /* Replay Static IGMP groups */ + if (pim_ifp->igmp_join_list) + { + struct listnode *node; + struct listnode *nextnode; + struct igmp_join *ij; + int join_fd; + + for (ALL_LIST_ELEMENTS (pim_ifp->igmp_join_list, node, nextnode, ij)) + { + /* Close socket and reopen with Source and Group */ + close(ij->sock_fd); + join_fd = igmp_join_sock(ifp->name, ifp->ifindex, ij->group_addr, ij->source_addr); + if (join_fd < 0) + { + char group_str[INET_ADDRSTRLEN]; + char source_str[INET_ADDRSTRLEN]; + pim_inet4_dump("", ij->group_addr, group_str, sizeof(group_str)); + pim_inet4_dump("", ij->source_addr, source_str, sizeof(source_str)); + zlog_warn("%s: igmp_join_sock() failure for IGMP group %s source %s on interface %s", + __PRETTY_FUNCTION__, + group_str, source_str, ifp->name); + /* warning only */ + } + else + ij->sock_fd = join_fd; + } + } } /* igmp */ if (PIM_IF_TEST_PIM(pim_ifp->options)) @@ -1506,6 +1537,9 @@ pim_if_connected_to_source (struct interface *ifp, struct in_addr src) struct connected *c; struct prefix p; + if (!ifp) + return 0; + p.family = AF_INET; p.u.prefix4 = src; p.prefixlen = IPV4_MAX_BITLEN; From 4ba87bb9e2b6f36f9504c6468ec2a465a28fe43f Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Fri, 21 Apr 2017 15:08:03 -0700 Subject: [PATCH 5/5] pimd: Fix WG/SGRpt & WG J/P processing During processing of Join/Prune, for a S,G entry, current state is SGRpt, when only *,G is received, need to clear SGRpt and add/inherit the *,G OIF to S,G so it can forward traffic to downstream where *,G is received. Upon receiving SGRpt prune remove the inherited *,G OIF. From, downstream router received *,G Prune along with SGRpt prune. Avoid sending *,G and SGRpt Prune together. Reset upstream_del reset ifchannel to NULL. Testing Done: Run failed smoke test of sending data packets, trigger SPT switchover, *,G path received SGRpt later data traffic stopped S,G ages out from LHR, sends only *,G join to upstream, verified S,G entry inherit the OIF. Upon receiving SGRpt deletes inherited oif and retains in SGRpt state. Signed-off-by: Chirag Shah --- pimd/pim_ifchannel.c | 9 ++++++--- pimd/pim_join.c | 11 ++++++----- pimd/pim_jp_agg.c | 2 +- pimd/pim_msg.c | 4 +++- pimd/pim_rp.c | 4 ++-- pimd/pim_rpf.c | 3 +-- pimd/pim_upstream.c | 18 +++++++++++------- 7 files changed, 30 insertions(+), 21 deletions(-) diff --git a/pimd/pim_ifchannel.c b/pimd/pim_ifchannel.c index f3c3e282c6..f4fe609605 100644 --- a/pimd/pim_ifchannel.c +++ b/pimd/pim_ifchannel.c @@ -145,6 +145,9 @@ void pim_ifchannel_delete(struct pim_ifchannel *ch) if (ch->upstream->flags & PIM_UPSTREAM_FLAG_MASK_SRC_IGMP) mask = PIM_OIF_FLAG_PROTO_IGMP; + /* SGRpt entry could have empty oil */ + if (ch->upstream->channel_oil) + pim_channel_del_oif (ch->upstream->channel_oil, ch->interface, mask); pim_channel_del_oif (ch->upstream->channel_oil, ch->interface, mask); /* * Do we have any S,G's that are inheriting? @@ -610,6 +613,7 @@ static int on_ifjoin_prune_pending_timer(struct thread *t) pim_ifp = ifp->info; send_prune_echo = (listcount(pim_ifp->pim_neighbor_list) > 1); + //ch->ifjoin_state transition to NOINFO ifjoin_to_noinfo(ch); /* from here ch may have been deleted */ @@ -1093,8 +1097,7 @@ void pim_ifchannel_local_membership_del(struct interface *ifp, if (!chchannel && c_oil && c_oil->oil.mfcc_ttls[pim_ifp->mroute_vif_index]) pim_channel_del_oif (c_oil, ifp, PIM_OIF_FLAG_PROTO_STAR); - if (c_oil->oil_size == 0) - pim_upstream_del (child, __PRETTY_FUNCTION__); + /* Child node removal/ref count-- will happen as part of parent' delete_no_info */ } } delete_on_noinfo(orig); @@ -1288,7 +1291,7 @@ pim_ifchannel_set_star_g_join_state (struct pim_ifchannel *ch, int eom, uint8_t if (up) { if (PIM_DEBUG_TRACE) - zlog_debug ("%s: add inherit oif to up %s ", __PRETTY_FUNCTION__, up->sg_str); + zlog_debug ("%s: clearing SGRpt flag, add inherit oif to up %s ", __PRETTY_FUNCTION__, up->sg_str); pim_channel_add_oif (up->channel_oil, ch->interface, PIM_OIF_FLAG_PROTO_STAR); } } diff --git a/pimd/pim_join.c b/pimd/pim_join.c index a9ca349102..828781a467 100644 --- a/pimd/pim_join.c +++ b/pimd/pim_join.c @@ -374,7 +374,7 @@ int pim_joinprune_send(struct pim_rpf *rpf, struct list *groups) { struct pim_jp_agg_group *group; - struct pim_interface *pim_ifp; + struct pim_interface *pim_ifp = NULL; struct pim_jp_groups *grp = NULL; struct pim_jp *msg; struct listnode *node, *nnode; @@ -395,12 +395,13 @@ int pim_joinprune_send(struct pim_rpf *rpf, return -1; } - if (!pim_ifp) { - zlog_warn("%s: multicast not enabled on interface %s", + if (!pim_ifp) + { + zlog_warn ("%s: multicast not enabled on interface %s", __PRETTY_FUNCTION__, rpf->source_nexthop.interface->name); - return -1; - } + return -1; + } if (PIM_INADDR_IS_ANY(rpf->rpf_addr.u.prefix4)) { diff --git a/pimd/pim_jp_agg.c b/pimd/pim_jp_agg.c index bbc51bd91c..46c6cbc690 100644 --- a/pimd/pim_jp_agg.c +++ b/pimd/pim_jp_agg.c @@ -348,7 +348,7 @@ pim_jp_agg_single_upstream_send (struct pim_rpf *rpf, static bool first = true; /* skip JP upstream messages if source is directly connected */ - if (!rpf->source_nexthop.interface || + if (!up || !rpf->source_nexthop.interface || pim_if_connected_to_source (rpf->source_nexthop.interface, up->sg.src)) return; diff --git a/pimd/pim_msg.c b/pimd/pim_msg.c index e19893f5da..a9e0130905 100644 --- a/pimd/pim_msg.c +++ b/pimd/pim_msg.c @@ -195,7 +195,9 @@ pim_msg_build_jp_groups (struct pim_jp_groups *grp, struct pim_jp_agg_group *sgs struct pim_rpf *rpf = pim_rp_g (source->up->sg.grp); bits = PIM_ENCODE_SPARSE_BIT | PIM_ENCODE_WC_BIT | PIM_ENCODE_RPT_BIT; stosend = rpf->rpf_addr.u.prefix4; - up = source->up; + /* Only Send SGRpt in case of *,G Join */ + if (source->is_join) + up = source->up; } else { diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c index 062ae6e808..6c4504d9b0 100644 --- a/pimd/pim_rp.c +++ b/pimd/pim_rp.c @@ -741,7 +741,7 @@ pim_rp_g (struct in_addr group) char buf1[PREFIX2STR_BUFFER]; prefix2str (&nht_p, buf, sizeof (buf)); prefix2str (&rp_info->group, buf1, sizeof (buf1)); - zlog_debug ("%s: NHT Register RP addr %s grp %s with Zebra ", + zlog_debug ("%s: NHT Register RP addr %s grp %s with Zebra", __PRETTY_FUNCTION__, buf, buf1); } memset (&pnc, 0, sizeof (struct pim_nexthop_cache)); @@ -759,7 +759,7 @@ pim_rp_g (struct in_addr group) 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 NHT", + zlog_debug ("%s: Nexthop cache not found for RP %s grp %s register with Zebra", __PRETTY_FUNCTION__, buf, buf1); } pim_rpf_set_refresh_time (); diff --git a/pimd/pim_rpf.c b/pimd/pim_rpf.c index f454ac59c1..f46ebfb979 100644 --- a/pimd/pim_rpf.c +++ b/pimd/pim_rpf.c @@ -240,8 +240,7 @@ enum pim_rpf_result pim_rpf_update(struct pim_upstream *up, struct pim_rpf *old, if (pim_ecmp_nexthop_lookup (&rpf->source_nexthop, up->upstream_addr, &src, &grp, !PIM_UPSTREAM_FLAG_TEST_FHR (up->flags) && - !PIM_UPSTREAM_FLAG_TEST_SRC_IGMP (up-> - flags))) + !PIM_UPSTREAM_FLAG_TEST_SRC_IGMP (up->flags))) { return PIM_RPF_FAILURE; } diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c index 2afcd6aeea..dd6eab9cfe 100644 --- a/pimd/pim_upstream.c +++ b/pimd/pim_upstream.c @@ -216,6 +216,7 @@ pim_upstream_del(struct pim_upstream *up, const char *name) up->sources = NULL; list_delete (up->ifchannels); + up->ifchannels = NULL; /* notice that listnode_delete() can't be moved @@ -243,7 +244,7 @@ pim_upstream_del(struct pim_upstream *up, const char *name) { char buf[PREFIX2STR_BUFFER]; prefix2str (&nht_p, buf, sizeof (buf)); - zlog_debug ("%s: Deregister upstream %s addr %s with Zebra", + zlog_debug ("%s: Deregister upstream %s addr %s with Zebra NHT", __PRETTY_FUNCTION__, up->sg_str, buf); } pim_delete_tracked_nexthop (&nht_p, up, NULL); @@ -1017,14 +1018,17 @@ static void pim_upstream_update_assert_tracking_desired(struct pim_upstream *up) struct pim_ifchannel *ch; /* scan per-interface (S,G) state */ - for (ALL_LIST_ELEMENTS(up->ifchannels, chnode, chnextnode, ch)) { - pim_ifp = ch->interface->info; - if (!pim_ifp) - continue; + for (ALL_LIST_ELEMENTS(up->ifchannels, chnode, chnextnode, ch)) + { + if (!ch->interface) + continue; + pim_ifp = ch->interface->info; + if (!pim_ifp) + continue; - pim_ifchannel_update_assert_tracking_desired(ch); + pim_ifchannel_update_assert_tracking_desired(ch); - } /* scan iface channel list */ + } /* scan iface channel list */ } /* When kat is stopped CouldRegister goes to false so we need to