pimd: Avoid deleting SGRpt entry from PP->P state

-Upon Receving SGRpt Prune message, transitioning from Prune Pending state
to NOINFO state, ifchannel entry was getting deleted in prune pending timer
expiry. This can result in SGRpt ifhchannel deleted and recreated upon receving
triggered or periodic SGRpt received from downstream.
The automation test failed as it expected (check) SGRpt entry at RP after it triggers
SPT switchover.

- While transitioning from Prune-Pending state to NOINFO(Pruned) state, Trigger
SGRpt message towards RP.

- Add/del some of the debug traces

Ticket:CM-16057
Reviewed By:CCR-6198
Testing Done:
Rerun test08 multiple times and observed passing it.

Pim-smoke with hardnode
Ran 95 tests in 11219.420s
FAILED (SKIP=10, failures=4)

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>

diff --git a/pimd/pim_ifchannel.c b/pimd/pim_ifchannel.c
index f4fe609605bcd2dbedfe2baf0e1ab2bc7804054d..04f8a22c6b9583e2c7f49ed0e863ccb735792d43 100644 (file)
--- a/pimd/pim_ifchannel.c
+++ b/pimd/pim_ifchannel.c
@@ -148,7 +148,6 @@ void pim_ifchannel_delete(struct pim_ifchannel *ch)
       /* 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?
        * Nuke from on high too.
@@ -179,6 +178,10 @@ void pim_ifchannel_delete(struct pim_ifchannel *ch)
     pim_upstream_update_join_desired(ch->upstream);
   }
 
+  /* upstream is common across ifchannels, check if upstream's
+     ifchannel list is empty before deleting upstream_del
+     ref count will take care of it.
+  */
   pim_upstream_del(ch->upstream, __PRETTY_FUNCTION__);
   ch->upstream = NULL;
 
@@ -200,6 +203,9 @@ void pim_ifchannel_delete(struct pim_ifchannel *ch)
   hash_release(pim_ifp->pim_ifchannel_hash, ch);
   listnode_delete(pim_ifchannel_list, ch);
 
+  if (PIM_DEBUG_PIM_TRACE)
+    zlog_debug ("%s: ifchannel entry %s is deleted ", __PRETTY_FUNCTION__, ch->sg_str);
+
   pim_ifchannel_free(ch);
 }
 
@@ -571,14 +577,18 @@ pim_ifchannel_add(struct interface *ifp,
 
   listnode_add_sort(up->ifchannels, ch);
 
+  if (PIM_DEBUG_PIM_TRACE)
+    zlog_debug ("%s: ifchannel %s is created ", __PRETTY_FUNCTION__, ch->sg_str);
+
   return ch;
 }
 
-static void ifjoin_to_noinfo(struct pim_ifchannel *ch)
+static void ifjoin_to_noinfo(struct pim_ifchannel *ch, uint8_t ch_del)
 {
   pim_forward_stop(ch);
   pim_ifchannel_ifjoin_switch(__PRETTY_FUNCTION__, ch, PIM_IFJOIN_NOINFO);
-  delete_on_noinfo(ch);
+  if (ch_del)
+    delete_on_noinfo(ch);
 }
 
 static int on_ifjoin_expiry_timer(struct thread *t)
@@ -589,7 +599,7 @@ static int on_ifjoin_expiry_timer(struct thread *t)
 
   ch->t_ifjoin_expiry_timer = NULL;
 
-  ifjoin_to_noinfo(ch);
+  ifjoin_to_noinfo(ch, 1);
   /* ch may have been deleted */
 
   return 0;
@@ -613,10 +623,6 @@ 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 */
-
       if (send_prune_echo)
         {
           struct pim_rpf rpf;
@@ -625,6 +631,23 @@ static int on_ifjoin_prune_pending_timer(struct thread *t)
           rpf.rpf_addr.u.prefix4 = pim_ifp->primary_address;
           pim_jp_agg_single_upstream_send(&rpf, ch->upstream, 0);
         }
+      /* If SGRpt flag is set on ifchannel, Trigger SGRpt
+         message on RP path upon prune timer expiry.
+      */
+      if (PIM_IF_FLAG_TEST_S_G_RPT (ch->flags))
+        {
+          if (ch->upstream)
+            pim_upstream_update_join_desired(ch->upstream);
+            /*
+              ch->ifjoin_state transition to NOINFO state
+              ch_del is set to 0 for not deleteing from here.
+              Holdtime expiry (ch_del set to 1) delete the entry.
+            */
+            ifjoin_to_noinfo(ch, 0);
+        }
+      else
+        ifjoin_to_noinfo(ch, 1);
+        /* from here ch may have been deleted */
     }
   else
     {
@@ -801,7 +824,7 @@ void pim_ifchannel_join_add(struct interface *ifp,
         (ch->upstream->parent->flags & PIM_UPSTREAM_FLAG_MASK_SRC_IGMP) &&
         !(ch->upstream->flags & PIM_UPSTREAM_FLAG_MASK_SRC_LHR))
       {
-        pim_upstream_ref (ch->upstream, PIM_UPSTREAM_FLAG_MASK_SRC_LHR);
+        pim_upstream_ref (ch->upstream, PIM_UPSTREAM_FLAG_MASK_SRC_LHR, __PRETTY_FUNCTION__);
         pim_upstream_keep_alive_timer_start (ch->upstream, qpim_keep_alive_time);
       }
     break;
@@ -895,8 +918,10 @@ void pim_ifchannel_prune(struct interface *ifp,
   case PIM_IFJOIN_NOINFO:
     if (source_flags & PIM_ENCODE_RPT_BIT)
       {
-       PIM_IF_FLAG_SET_S_G_RPT(ch->flags);
-       ch->ifjoin_state = PIM_IFJOIN_PRUNE_PENDING;
+       if (!(source_flags & PIM_ENCODE_WC_BIT))
+          PIM_IF_FLAG_SET_S_G_RPT(ch->flags);
+
+        ch->ifjoin_state = PIM_IFJOIN_PRUNE_PENDING;
         if (listcount(pim_ifp->pim_neighbor_list) > 1)
           jp_override_interval_msec = pim_if_jp_override_interval_msec(ifp);
         else
@@ -1306,7 +1331,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: del inherit oif from up %s", __PRETTY_FUNCTION__, up->sg_str);
+                zlog_debug ("%s: SGRpt Set, del inherit oif from up %s", __PRETTY_FUNCTION__, up->sg_str);
               pim_channel_del_oif (up->channel_oil, ch->interface, PIM_OIF_FLAG_PROTO_STAR);
             }
         }

diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c
index c5f8d1d8268d07a6c6432663e07bd0efded33fde..9165bef5663fbdd153f129eac8264ba9c9ea67bf 100644 (file)
--- a/pimd/pim_nht.c
+++ b/pimd/pim_nht.c
@@ -567,7 +567,7 @@ pim_ecmp_nexthop_search (struct pim_nexthop_cache *pnc,
       //PIM ECMP flag is enable then choose ECMP path.
       hash_val = pim_compute_ecmp_hash (src, grp);
       mod_val = hash_val % pnc->nexthop_num;
-      if (PIM_DEBUG_TRACE)
+      if (PIM_DEBUG_PIM_TRACE_DETAIL)
         zlog_debug ("%s: hash_val %u mod_val %u ",
                     __PRETTY_FUNCTION__, hash_val, mod_val);
     }
@@ -914,7 +914,7 @@ pim_ecmp_nexthop_lookup (struct pim_nexthop *nexthop, struct in_addr addr,
     {
       hash_val = pim_compute_ecmp_hash (src, grp);
       mod_val = hash_val % num_ifindex;
-      if (PIM_DEBUG_TRACE)
+      if (PIM_DEBUG_PIM_TRACE_DETAIL)
         zlog_debug ("%s: hash_val %u mod_val %u",
                     __PRETTY_FUNCTION__, hash_val, mod_val);
     }
@@ -1037,7 +1037,7 @@ int pim_ecmp_fib_lookup_if_vif_index(struct in_addr addr,
     {
       hash_val = pim_compute_ecmp_hash (src, grp);
       mod_val = hash_val % num_ifindex;
-      if (PIM_DEBUG_TRACE)
+      if (PIM_DEBUG_PIM_TRACE_DETAIL)
         zlog_debug ("%s: hash_val %u mod_val %u",
                     __PRETTY_FUNCTION__, hash_val, mod_val);
     }

diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c
index dd6eab9cfe8766bce4ce75d7ae4b8ae203d760f4..6fadfc2f29a213bd486425167f24c57391c4855e 100644 (file)
--- a/pimd/pim_upstream.c
+++ b/pimd/pim_upstream.c
@@ -78,9 +78,17 @@ pim_upstream_remove_children (struct pim_upstream *up)
   while (!list_isempty (up->sources))
     {
       child = listnode_head (up->sources);
-      child->parent = NULL;
       listnode_delete (up->sources, child);
+      if (PIM_UPSTREAM_FLAG_TEST_SRC_LHR(child->flags))
+        {
+          PIM_UPSTREAM_FLAG_UNSET_SRC_LHR(child->flags);
+          child = pim_upstream_del(child, __PRETTY_FUNCTION__);
+        }
+      if (child)
+        child->parent = NULL;
     }
+  list_delete(up->sources);
+  up->sources = NULL;
 }
 
 /*
@@ -149,10 +157,14 @@ void pim_upstream_free(struct pim_upstream *up)
 
 static void upstream_channel_oil_detach(struct pim_upstream *up)
 {
-  if (up->channel_oil) {
-    pim_channel_oil_del(up->channel_oil);
-    up->channel_oil = NULL;
-  }
+  if (up->channel_oil)
+    {
+      /* Detaching from channel_oil, channel_oil may exist post del,
+         but upstream would not keep reference of it
+       */
+      pim_channel_oil_del(up->channel_oil);
+      up->channel_oil = NULL;
+    }
 }
 
 struct pim_upstream *
@@ -163,7 +175,7 @@ pim_upstream_del(struct pim_upstream *up, const char *name)
 
   if (PIM_DEBUG_TRACE)
     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,
+                __PRETTY_FUNCTION__, name, up->sg_str, up->ref_count, up->flags,
                 up->channel_oil->oil_ref_count);
 
   --up->ref_count;
@@ -195,25 +207,11 @@ pim_upstream_del(struct pim_upstream *up, const char *name)
   }
 
   pim_upstream_remove_children (up);
-  pim_mroute_del (up->channel_oil, __PRETTY_FUNCTION__);
-  upstream_channel_oil_detach(up);
-
   if (up->sources)
-    {
-      struct listnode *node, *nnode;
-      struct pim_upstream *child;
-      for (ALL_LIST_ELEMENTS (up->sources, node, nnode, child))
-       {
-         if (PIM_UPSTREAM_FLAG_TEST_SRC_LHR(child->flags))
-           {
-             PIM_UPSTREAM_FLAG_UNSET_SRC_LHR(child->flags);
-             pim_upstream_del(child, __PRETTY_FUNCTION__);
-           }
-       }
-
-      list_delete (up->sources);
-    }
+    list_delete (up->sources);
   up->sources = NULL;
+  pim_mroute_del (up->channel_oil, __PRETTY_FUNCTION__);
+  upstream_channel_oil_detach(up);
 
   list_delete (up->ifchannels);
   up->ifchannels = NULL;
@@ -223,11 +221,10 @@ pim_upstream_del(struct pim_upstream *up, const char *name)
     into pim_upstream_free() because the later is
     called by list_delete_all_node()
   */
-  if (up->parent)
-    {
-      listnode_delete (up->parent->sources, up);
-      up->parent = NULL;
-    }
+  if (up->parent && up->parent->sources)
+    listnode_delete (up->parent->sources, up);
+  up->parent = NULL;
+
   listnode_delete (pim_upstream_list, up);
   hash_release (pim_upstream_hash, up);
 
@@ -538,13 +535,14 @@ pim_upstream_switch(struct pim_upstream *up,
 {
   enum pim_upstream_state old_state = up->join_state;
 
-  if (PIM_DEBUG_PIM_EVENTS) {
-    zlog_debug("%s: PIM_UPSTREAM_%s: (S,G) old: %s new: %s",
+  if (PIM_DEBUG_PIM_EVENTS)
+    {
+      zlog_debug ("%s: PIM_UPSTREAM_%s: (S,G) old: %s new: %s",
               __PRETTY_FUNCTION__,
               up->sg_str,
               pim_upstream_state2str (up->join_state),
               pim_upstream_state2str (new_state));
-  }
+    }
 
   up->join_state = new_state;
   if (old_state != new_state)
@@ -584,7 +582,17 @@ pim_upstream_switch(struct pim_upstream *up,
     if (old_state == PIM_UPSTREAM_JOINED)
       pim_msdp_up_join_state_changed(up);
 
-    pim_jp_agg_single_upstream_send(&up->rpf, up, 0 /* prune */);
+    /* IHR, Trigger SGRpt on *,G IIF to prune S,G from RPT */
+    if (pim_upstream_is_sg_rpt(up) && up->parent)
+      {
+        if (PIM_DEBUG_PIM_TRACE_DETAIL)
+          zlog_debug ("%s: *,G IIF %s S,G IIF %s ", __PRETTY_FUNCTION__,
+                    up->parent->rpf.source_nexthop.interface->name,
+                    up->rpf.source_nexthop.interface->name);
+        pim_jp_agg_single_upstream_send(&up->parent->rpf, up->parent, 1 /* (W,G) Join */);
+      }
+    else
+      pim_jp_agg_single_upstream_send(&up->rpf, up, 0 /* prune */);
     join_timer_stop(up);
   }
 }
@@ -717,9 +725,9 @@ pim_upstream_new (struct prefix_sg *sg,
 
   if (PIM_DEBUG_TRACE)
     {
-      zlog_debug ("%s: Created Upstream %s upstream_addr %s",
+      zlog_debug ("%s: Created Upstream %s upstream_addr %s ref count %d increment",
             __PRETTY_FUNCTION__, up->sg_str,
-            inet_ntoa (up->upstream_addr));
+            inet_ntoa (up->upstream_addr), up->ref_count);
     }
 
   return up;
@@ -750,6 +758,9 @@ pim_upstream_find_or_add(struct prefix_sg *sg,
         {
           up->flags |= flags;
           up->ref_count++;
+          if (PIM_DEBUG_TRACE)
+            zlog_debug ("%s(%s): upstream %s ref count %d increment",
+                  __PRETTY_FUNCTION__, name, up->sg_str, up->ref_count);
         }
     }
   else
@@ -759,10 +770,13 @@ pim_upstream_find_or_add(struct prefix_sg *sg,
 }
 
 void
-pim_upstream_ref(struct pim_upstream *up, int flags)
+pim_upstream_ref(struct pim_upstream *up, int flags, const char *name)
 {
   up->flags |= flags;
   ++up->ref_count;
+  if (PIM_DEBUG_TRACE)
+    zlog_debug ("%s(%s): upstream %s ref count %d increment",
+              __PRETTY_FUNCTION__, name, up->sg_str, up->ref_count);
 }
 
 struct pim_upstream *pim_upstream_add(struct prefix_sg *sg,
@@ -773,7 +787,7 @@ struct pim_upstream *pim_upstream_add(struct prefix_sg *sg,
   int found = 0;
   up = pim_upstream_find(sg);
   if (up) {
-    pim_upstream_ref(up, flags);
+    pim_upstream_ref(up, flags, name);
     found = 1;
   }
   else {
@@ -786,10 +800,11 @@ struct pim_upstream *pim_upstream_add(struct prefix_sg *sg,
         {
           char buf[PREFIX2STR_BUFFER];
           prefix2str (&up->rpf.rpf_addr, buf, sizeof (buf));
-         zlog_debug("%s(%s): %s, iif %s found: %d: ref_count: %d",
+         zlog_debug("%s(%s): %s, iif %s (%s) found: %d: ref_count: %d",
                   __PRETTY_FUNCTION__, name,
-                  up->sg_str, buf, found,
-                  up->ref_count);
+                  up->sg_str, buf, up->rpf.source_nexthop.interface ?
+                   up->rpf.source_nexthop.interface->name : "NIL" ,
+                  found, up->ref_count);
         }
       else
        zlog_debug("%s(%s): (%s) failure to create",
@@ -1660,7 +1675,7 @@ pim_upstream_sg_running (void *arg)
          if (PIM_DEBUG_TRACE)
            zlog_debug ("source reference created on kat restart %s", up->sg_str);
 
-         pim_upstream_ref(up, PIM_UPSTREAM_FLAG_MASK_SRC_STREAM);
+         pim_upstream_ref(up, PIM_UPSTREAM_FLAG_MASK_SRC_STREAM, __PRETTY_FUNCTION__);
          PIM_UPSTREAM_FLAG_SET_SRC_STREAM(up->flags);
          pim_upstream_fhr_kat_start(up);
        }

diff --git a/pimd/pim_upstream.h b/pimd/pim_upstream.h
index f1c8df35b1671dfa668c72108cd671f55bae1639..d728c6c01f9c869569e682efb13a31d710d2f41f 100644 (file)
--- a/pimd/pim_upstream.h
+++ b/pimd/pim_upstream.h
@@ -145,7 +145,7 @@ struct pim_upstream *pim_upstream_find_or_add (struct prefix_sg *sg,
 struct pim_upstream *pim_upstream_add (struct prefix_sg *sg,
                                       struct interface *ifp, int flags,
                                       const char *name);
-void pim_upstream_ref (struct pim_upstream *up, int flags);
+void pim_upstream_ref (struct pim_upstream *up, int flags, const char *name);
 struct pim_upstream *pim_upstream_del(struct pim_upstream *up, const char *name);
 
 int pim_upstream_evaluate_join_desired(struct pim_upstream *up);