Skip to content
Permalink
Browse files

pimd: Figure neighbors -vs- paths when doing RPF

When we are looking up a RPF with a ecmp path, there
are situations where we are failing to find a path change
because we were not considering the actual number of neighbors
we have available to us at the start of the loop.

Example:

Suppose 2 way ecmp with a neighbor on each path.  We have
multiple upstreams that are strewn across both paths.
If we loose a pim neighbor on one of the paths we would
initiate a rescan of the upstreams.  If the neighbor
we lost happened to be the last ecmp path we rescanned
we would not successfully find a new path and leave
the upstream stranded.

This code change looks at the number of available neighbors
that we have -vs- the number of paths we have and chooses
the smaller of the two for figuring out what to do.

There probably exist other failure scenarios as well that
I am missing here and quite frankly the current code muddies
the water between a RPF lookup failure -vs- a RPF lookup succeeded
and there are no paths.  Further work is needed here imo.

Additionally this idea of a pim_ecmp_nexthop_lookup and
pim_ecmp_nexthop_search is bogus.  They are the same function and
should be merged at some point in time.

Ticket: CM-21599
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
  • Loading branch information...
donaldsharp committed Jul 6, 2018
1 parent 57695eb commit 1996712744f009ed6d4281f7686429dab3f75960
Showing with 61 additions and 10 deletions.
  1. +61 −10 pimd/pim_nht.c
@@ -430,12 +430,14 @@ int pim_ecmp_nexthop_search(struct pim_instance *pim,
struct pim_nexthop *nexthop, struct prefix *src,
struct prefix *grp, int neighbor_needed)
{
struct pim_neighbor *nbr = NULL;
struct pim_neighbor *nbrs[MULTIPATH_NUM], *nbr;
struct interface *ifps[MULTIPATH_NUM];
struct nexthop *nh_node = NULL;
ifindex_t first_ifindex;
struct interface *ifp = NULL;
uint32_t hash_val = 0, mod_val = 0;
uint8_t nh_iter = 0, found = 0;
uint32_t i, num_nbrs = 0;

if (!pnc || !pnc->nexthop_num || !nexthop)
return 0;
@@ -499,16 +501,40 @@ int pim_ecmp_nexthop_search(struct pim_instance *pim,
}
}
}

/*
* Look up all interfaces and neighbors,
* store for later usage
*/
for (nh_node = pnc->nexthop, i = 0; nh_node;
nh_node = nh_node->next, i++) {
ifps[i] = if_lookup_by_index(nh_node->ifindex, pim->vrf_id);
if (ifps[i]) {
nbrs[i] = pim_neighbor_find(ifps[i],
nh_node->gate.ipv4);
if (nbrs[i] || pim_if_connected_to_source(ifps[i],
src->u.prefix4))
num_nbrs++;
}
}
if (pim->ecmp_enable) {
uint32_t consider = pnc->nexthop_num;

if (neighbor_needed && num_nbrs < consider)
consider = num_nbrs;

if (consider == 0)
return 0;

// PIM ECMP flag is enable then choose ECMP path.
hash_val = pim_compute_ecmp_hash(src, grp);
mod_val = hash_val % pnc->nexthop_num;
mod_val = hash_val % consider;
}

for (nh_node = pnc->nexthop; nh_node && (found == 0);
nh_node = nh_node->next) {
first_ifindex = nh_node->ifindex;
ifp = if_lookup_by_index(first_ifindex, pim->vrf_id);
ifp = ifps[nh_iter];
if (!ifp) {
if (PIM_DEBUG_PIM_NHT) {
char addr_str[INET_ADDRSTRLEN];
@@ -544,7 +570,7 @@ int pim_ecmp_nexthop_search(struct pim_instance *pim,

if (neighbor_needed
&& !pim_if_connected_to_source(ifp, src->u.prefix4)) {
nbr = pim_neighbor_find(ifp, nh_node->gate.ipv4);
nbr = nbrs[nh_iter];
if (!nbr && !if_is_loopback(ifp)) {
if (PIM_DEBUG_PIM_NHT)
zlog_debug(
@@ -784,13 +810,14 @@ int pim_ecmp_nexthop_lookup(struct pim_instance *pim,
int neighbor_needed)
{
struct pim_zlookup_nexthop nexthop_tab[MULTIPATH_NUM];
struct pim_neighbor *nbr = NULL;
struct pim_neighbor *nbrs[MULTIPATH_NUM], *nbr = NULL;
int num_ifindex;
struct interface *ifp;
struct interface *ifps[MULTIPATH_NUM], *ifp;
int first_ifindex;
int found = 0;
uint8_t i = 0;
uint32_t hash_val = 0, mod_val = 0;
uint32_t num_nbrs = 0;

if (PIM_DEBUG_PIM_NHT) {
char addr_str[INET_ADDRSTRLEN];
@@ -816,19 +843,44 @@ int pim_ecmp_nexthop_lookup(struct pim_instance *pim,
return 0;
}

/*
* Look up all interfaces and neighbors,
* store for later usage
*/
for (i = 0; i < num_ifindex; i++) {
ifps[i] = if_lookup_by_index(nexthop_tab[i].ifindex,
pim->vrf_id);
if (ifps[i]) {
nbrs[i] = pim_neighbor_find(
ifps[i], nexthop_tab[i].nexthop_addr.u.prefix4);
if (nbrs[i] || pim_if_connected_to_source(ifps[i],
addr))
num_nbrs++;
}
}

// If PIM ECMP enable then choose ECMP path.
if (pim->ecmp_enable) {
uint32_t consider = num_ifindex;

if (neighbor_needed && num_nbrs < consider)
consider = num_nbrs;

if (consider == 0)
return 0;

hash_val = pim_compute_ecmp_hash(src, grp);
mod_val = hash_val % num_ifindex;
mod_val = hash_val % consider;
if (PIM_DEBUG_PIM_NHT_DETAIL)
zlog_debug("%s: hash_val %u mod_val %u",
__PRETTY_FUNCTION__, hash_val, mod_val);
}

i = 0;
while (!found && (i < num_ifindex)) {
first_ifindex = nexthop_tab[i].ifindex;

ifp = if_lookup_by_index(first_ifindex, pim->vrf_id);
ifp = ifps[i];
if (!ifp) {
if (PIM_DEBUG_PIM_NHT) {
char addr_str[INET_ADDRSTRLEN];
@@ -863,8 +915,7 @@ int pim_ecmp_nexthop_lookup(struct pim_instance *pim,
continue;
}
if (neighbor_needed && !pim_if_connected_to_source(ifp, addr)) {
nbr = pim_neighbor_find(
ifp, nexthop_tab[i].nexthop_addr.u.prefix4);
nbr = nbrs[i];
if (PIM_DEBUG_PIM_NHT_DETAIL)
zlog_debug("ifp name: %s(%s), pim nbr: %p",
ifp->name, pim->vrf->name, nbr);

0 comments on commit 1996712

Please sign in to comment.
You can’t perform that action at this time.