From b36576e44c94594824e632e8ca954f2527d55b45 Mon Sep 17 00:00:00 2001 From: Anuradha Karuppiah Date: Fri, 15 Nov 2019 11:49:59 -0800 Subject: [PATCH] pimd: RPF change to unreachable was leaving a stale entry in the jp-agg list This was causing pimd to crash later; call-stack - (gdb) bt context=) at lib/sigevent.c:254 group=group@entry=0x7ffffa9797e0) at pimd/pim_rp.c:207 grp=grp@entry=0x7ffffa9799fe, sgs=sgs@entry=0x560ac069edb0, size=52) at pimd/pim_msg.c:200 groups=) at pimd/pim_join.c:562 at pimd/pim_neighbor.c:288 at lib/thread.c:1599 at lib/libfrr.c:1024 envp=) at pimd/pim_main.c:162 (gdb) fr 4 group=group@entry=0x7ffffa9797e0) at pimd/pim_rp.c:207 207 pimd/pim_rp.c: No such file or directory. (gdb) fr 6 grp=grp@entry=0x7ffffa9799fe, sgs=sgs@entry=0x560ac069edb0, size=52) at pimd/pim_msg.c:200 200 pimd/pim_msg.c: No such file or directory. (gdb) p source->up->sg_str $1 = '\000' , (gdb) This problem can manifest in the following event sequence - 1. upstream RPF neighbor is resolved 2. upstream RPF neighbor becomes unresolved (but upstream entry stays on the jp-agg list) 3. upstream entry is removed on the next old-neighbor jp-agg-list processing the stale entry is accessed resulting in the crash. Signed-off-by: Anuradha Karuppiah --- pimd/pim_jp_agg.c | 3 ++- pimd/pim_nht.c | 18 ++++++++++-------- pimd/pim_rp.c | 4 +++- pimd/pim_rpf.c | 9 ++++----- pimd/pim_upstream.c | 4 +++- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/pimd/pim_jp_agg.c b/pimd/pim_jp_agg.c index 243316b433..e1473bfe3b 100644 --- a/pimd/pim_jp_agg.c +++ b/pimd/pim_jp_agg.c @@ -323,7 +323,8 @@ void pim_jp_agg_switch_interface(struct pim_rpf *orpf, struct pim_rpf *nrpf, 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); + if (npius) + pim_jp_agg_add_group(npius->us, up, true); } diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c index 2fbde20075..bd8424b3e4 100644 --- a/pimd/pim_nht.c +++ b/pimd/pim_nht.c @@ -447,15 +447,16 @@ static int pim_update_upstream_nh_helper(struct hash_bucket *bucket, void *arg) old.source_nexthop.interface = up->rpf.source_nexthop.interface; rpf_result = pim_rpf_update(pim, up, &old, __func__); - if (rpf_result == PIM_RPF_FAILURE) { - pim_upstream_rpf_clear(pim, up); - return HASHWALK_CONTINUE; - } - /* update kernel multicast forwarding cache (MFC) */ - pim_upstream_mroute_iif_update(up->channel_oil, __func__); + /* update kernel multicast forwarding cache (MFC); if the + * RPF nbr is now unreachable the MFC has already been updated + * by pim_rpf_clear + */ + if (rpf_result != PIM_RPF_FAILURE) + pim_upstream_mroute_iif_update(up->channel_oil, __func__); - if (rpf_result == PIM_RPF_CHANGED) + if (rpf_result == PIM_RPF_CHANGED || + (rpf_result == PIM_RPF_FAILURE && old.source_nexthop.interface)) pim_zebra_upstream_rpf_changed(pim, up, &old); @@ -464,7 +465,8 @@ static int pim_update_upstream_nh_helper(struct hash_bucket *bucket, void *arg) __PRETTY_FUNCTION__, up->sg_str, pim->vrf->name, old.source_nexthop.interface ? old.source_nexthop.interface->name : "Unknown", - up->rpf.source_nexthop.interface->name); + up->rpf.source_nexthop.interface + ? up->rpf.source_nexthop.interface->name : "Unknown"); } return HASHWALK_CONTINUE; diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c index 5eb035f98c..bf42d03604 100644 --- a/pimd/pim_rp.c +++ b/pimd/pim_rp.c @@ -390,7 +390,9 @@ void pim_upstream_update(struct pim_instance *pim, struct pim_upstream *up) if (up->rpf.source_nexthop.interface && up->channel_oil) pim_upstream_mroute_iif_update(up->channel_oil, __func__); - if (rpf_result == PIM_RPF_CHANGED) + if (rpf_result == PIM_RPF_CHANGED || + (rpf_result == PIM_RPF_FAILURE && + old_rpf.source_nexthop.interface)) pim_zebra_upstream_rpf_changed(pim, up, &old_rpf); pim_zebra_update_all_interfaces(pim); diff --git a/pimd/pim_rpf.c b/pimd/pim_rpf.c index 1472d69f56..1eb5006b94 100644 --- a/pimd/pim_rpf.c +++ b/pimd/pim_rpf.c @@ -215,6 +215,10 @@ enum pim_rpf_result pim_rpf_update(struct pim_instance *pim, saved.source_nexthop = rpf->source_nexthop; saved.rpf_addr = rpf->rpf_addr; + if (old) { + old->source_nexthop = saved.source_nexthop; + old->rpf_addr = saved.rpf_addr; + } nht_p.family = AF_INET; nht_p.prefixlen = IPV4_MAX_BITLEN; @@ -287,11 +291,6 @@ enum pim_rpf_result pim_rpf_update(struct pim_instance *pim, || saved.source_nexthop .interface != rpf->source_nexthop.interface) { - /* return old rpf to caller ? */ - if (old) { - old->source_nexthop = saved.source_nexthop; - old->rpf_addr = saved.rpf_addr; - } return PIM_RPF_CHANGED; } diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c index b28af54a30..53da58e8fe 100644 --- a/pimd/pim_upstream.c +++ b/pimd/pim_upstream.c @@ -1772,7 +1772,9 @@ void pim_upstream_find_new_rpf(struct pim_instance *pim) __PRETTY_FUNCTION__, up->sg_str); old.source_nexthop.interface = up->rpf.source_nexthop.interface; rpf_result = pim_rpf_update(pim, up, &old, __func__); - if (rpf_result == PIM_RPF_CHANGED) + if (rpf_result == PIM_RPF_CHANGED || + (rpf_result == PIM_RPF_FAILURE && + old.source_nexthop.interface)) pim_zebra_upstream_rpf_changed(pim, up, &old); /* update kernel multicast forwarding cache (MFC) */ pim_upstream_mroute_iif_update(up->channel_oil, __func__);