Merge branch 'frr/pull/548' ("Pim dev 3.0 defect fixes")

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

7 files changed, 112 insertions, 65 deletions

diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index d5dbe4f7a..80decc34c 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -5075,11 +5075,16 @@ DEFUN (interface_ip_pim_hello,
 
   pim_ifp = ifp->info;
 
-  if (!pim_ifp) {
-    vty_out(vty, "Pim not enabled on this interface%s", VTY_NEWLINE);
-    return CMD_WARNING;
-  }
+  if (!pim_ifp)
+    {
+      if (!pim_cmd_interface_add(ifp))
+        {
+          vty_out(vty, "Could not enable PIM SM on interface%s", VTY_NEWLINE);
+          return CMD_WARNING;
+        }
+    }
 
+  pim_ifp = ifp->info;
   pim_ifp->pim_hello_period = strtol(argv[idx_time]->arg, NULL, 10);
 
   if (argc == idx_hold + 1)
diff --git a/pimd/pim_ifchannel.c b/pimd/pim_ifchannel.c
index f4fe60960..c5953e560 100644
--- 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, bool 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, true);
   /* 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, false);
+        }
+      else
+        ifjoin_to_noinfo(ch, true);
+        /* 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_igmp.c b/pimd/pim_igmp.c
index ee88e7d8e..dd5f7e77e 100644
--- a/pimd/pim_igmp.c
+++ b/pimd/pim_igmp.c
@@ -39,6 +39,7 @@
 #include "pim_zebra.h"
 
 static void group_timer_off(struct igmp_group *group);
+static int pim_igmp_general_query(struct thread *t);
 
 /* This socket is used for TXing IGMP packets only, IGMP RX happens
  * in pim_mroute_msg()
@@ -172,8 +173,11 @@ static int pim_igmp_other_querier_expire(struct thread *t)
   /*
     We are the current querier, then
     re-start sending general queries.
+    RFC 2236 - sec 7 Other Querier
+    present timer expired (Send General
+    Query, Set Gen. Query. timer)
   */
-  pim_igmp_general_query_on(igmp);
+  pim_igmp_general_query(t);
 
   return 0;
 }
@@ -497,8 +501,6 @@ int pim_igmp_packet(struct igmp_sock *igmp, char *buf, size_t len)
   return -1;
 }
 
-static int pim_igmp_general_query(struct thread *t);
-
 void pim_igmp_general_query_on(struct igmp_sock *igmp)
 {
   struct pim_interface *pim_ifp;
diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c
index c5f8d1d82..9165bef56 100644
--- 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_register.c b/pimd/pim_register.c
index 8dc179c14..e6145a142 100644
--- a/pimd/pim_register.c
+++ b/pimd/pim_register.c
@@ -346,7 +346,7 @@ pim_register_recv (struct interface *ifp,
 	  zlog_debug("%s: Sending register-Stop to %s and dropping mr. packet",
 	    __func__, "Sender");
 	/* Drop Packet Silently */
-	return 1;
+	return 0;
       }
     }
 
@@ -408,5 +408,5 @@ pim_register_recv (struct interface *ifp,
     pim_register_stop_send (ifp, &sg, dest_addr, src_addr);
   }
 
-  return 1;
+  return 0;
 }
diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c
index dd6eab9cf..6fadfc2f2 100644
--- 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 f1c8df35b..d728c6c01 100644
--- 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);