pimd: Cleanup JP Agg a bit more

The J/P Aggregation + the NHT tracking code was not
playing nicely together

1) Clean up pim_upstream ref counting to keep a bit better
track of it.

2) When we delete pim_upstream zero it out to hopefully
catch issues faster in the future

3) Clean up J/P Agg source list a bit better to keep order

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

3 files changed, 45 insertions, 13 deletions

diff --git a/pimd/pim_jp_agg.c b/pimd/pim_jp_agg.c
index 8d4510324..4c1c0c7de 100644
--- a/pimd/pim_jp_agg.c
+++ b/pimd/pim_jp_agg.c
@@ -40,6 +40,8 @@ pim_jp_agg_group_list_free (struct pim_jp_agg_group *jag)
 static void
 pim_jp_agg_src_free (struct pim_jp_sources *js)
 {
+  struct pim_upstream *up = js->up;
+
   /*
    * When we are being called here, we know
    * that the neighbor is going away start
@@ -47,7 +49,11 @@ pim_jp_agg_src_free (struct pim_jp_sources *js)
    * pick this shit back up when the
    * nbr comes back alive
    */
-  join_timer_start(js->up);
+
+  up = pim_upstream_del (up, __PRETTY_FUNCTION__);
+
+  if (up)
+    join_timer_start(js->up);
   XFREE (MTYPE_PIM_JP_AGG_SOURCE, js);
 }
 
@@ -72,6 +78,12 @@ pim_jp_agg_src_cmp (void *arg1, void *arg2)
   const struct pim_jp_sources *js1 = (const struct pim_jp_sources *)arg1;
   const struct pim_jp_sources *js2 = (const struct pim_jp_sources *)arg2;
 
+  if (js1->is_join && !js2->is_join)
+    return -1;
+
+  if (!js1->is_join && js2->is_join)
+    return 1;
+
   if (js1->up->sg.src.s_addr < js2->up->sg.src.s_addr)
     return -1;
 
@@ -99,6 +111,7 @@ pim_jp_agg_clear_group (struct list *group)
     {
       for (ALL_LIST_ELEMENTS(jag->sources, snode, snnode, js))
         {
+          pim_upstream_del (js->up, __PRETTY_FUNCTION__);
           listnode_delete(jag->sources, js);
           XFREE(MTYPE_PIM_JP_AGG_SOURCE, js);
         }
@@ -126,7 +139,7 @@ pim_jp_agg_get_interface_upstream_switch_list (struct pim_rpf *rpf)
       pius = XCALLOC(MTYPE_PIM_JP_AGG_GROUP, sizeof (struct pim_iface_upstream_switch));
       pius->address.s_addr = rpf->rpf_addr.u.prefix4.s_addr;
       pius->us = list_new();
-      listnode_add (pim_ifp->upstream_switch_list, pius);
+      listnode_add_sort (pim_ifp->upstream_switch_list, pius);
     }
 
   return pius;
@@ -154,9 +167,12 @@ pim_jp_agg_remove_group (struct list *group, struct pim_upstream *up)
         break;
     }
 
-  listnode_delete(jag->sources, js);
-
-  XFREE(MTYPE_PIM_JP_AGG_SOURCE, js);
+  if (js)
+    {
+      pim_upstream_del (up, __PRETTY_FUNCTION__);
+      listnode_delete(jag->sources, js);
+      XFREE(MTYPE_PIM_JP_AGG_SOURCE, js);
+    }
 
   if (jag->sources->count == 0)
     {
@@ -185,7 +201,7 @@ pim_jp_agg_add_group (struct list *group, struct pim_upstream *up, bool is_join)
       jag->sources = list_new();
       jag->sources->cmp = pim_jp_agg_src_cmp;
       jag->sources->del = (void (*)(void *))pim_jp_agg_src_free;
-      listnode_add (group, jag);
+      listnode_add_sort (group, jag);
     }
 
   for (ALL_LIST_ELEMENTS(jag->sources, node, nnode, js))
@@ -197,11 +213,20 @@ pim_jp_agg_add_group (struct list *group, struct pim_upstream *up, bool is_join)
   if (!js)
     {
       js = XCALLOC(MTYPE_PIM_JP_AGG_SOURCE, sizeof (struct pim_jp_sources));
+      pim_upstream_ref (up, 0);
       js->up = up;
-      listnode_add (jag->sources, js);
+      js->is_join = is_join;
+      listnode_add_sort (jag->sources, js);
+    }
+  else
+    {
+      if (js->is_join != is_join)
+        {
+          listnode_delete(jag->sources, js);
+          js->is_join = is_join;
+          listnode_add_sort (jag->sources, js);
+        }
     }
-
-  js->is_join = is_join;
 }
 
 void
diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c
index 06f0c5c03..8ea9e3d68 100644
--- a/pimd/pim_upstream.c
+++ b/pimd/pim_upstream.c
@@ -142,6 +142,7 @@ pim_upstream_find_parent (struct pim_upstream *child)
 
 void pim_upstream_free(struct pim_upstream *up)
 {
+  memset (up, 0, sizeof (struct pim_upstream));
   XFREE(MTYPE_PIM_UPSTREAM, up);
   up = NULL;
 }
@@ -154,7 +155,7 @@ static void upstream_channel_oil_detach(struct pim_upstream *up)
   }
 }
 
-void
+struct pim_upstream *
 pim_upstream_del(struct pim_upstream *up, const char *name)
 {
   bool notify_msdp = false;
@@ -167,7 +168,9 @@ pim_upstream_del(struct pim_upstream *up, const char *name)
   --up->ref_count;
 
   if (up->ref_count >= 1)
-    return;
+    return up;
+  else if (up->ref_count < 0)
+    return NULL;
 
   THREAD_OFF(up->t_ka_timer);
   THREAD_OFF(up->t_rs_timer);
@@ -231,6 +234,8 @@ pim_upstream_del(struct pim_upstream *up, const char *name)
   pim_delete_tracked_nexthop (&nht_p, up, NULL);
 
   pim_upstream_free (up);
+
+  return NULL;
 }
 
 void
@@ -699,7 +704,8 @@ pim_upstream_find_or_add(struct prefix_sg *sg,
   return up;
 }
 
-static void pim_upstream_ref(struct pim_upstream *up, int flags)
+void
+pim_upstream_ref(struct pim_upstream *up, int flags)
 {
   up->flags |= flags;
   ++up->ref_count;
diff --git a/pimd/pim_upstream.h b/pimd/pim_upstream.h
index 21c78021e..126824e98 100644
--- a/pimd/pim_upstream.h
+++ b/pimd/pim_upstream.h
@@ -135,7 +135,8 @@ 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_del(struct pim_upstream *up, const char *name);
+void pim_upstream_ref (struct pim_upstream *up, int flags);
+struct pim_upstream *pim_upstream_del(struct pim_upstream *up, const char *name);
 
 int pim_upstream_evaluate_join_desired(struct pim_upstream *up);
 int pim_upstream_evaluate_join_desired_interface(struct pim_upstream *up,