pimd: Drop local SA reference when the upstream SG is deleted

This is done irrespective of the reason for del and is intended as a
catchall for cases (unclear which ones) where the RP can drop the SG
without KAT expiry.

Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>

diff --git a/pimd/pim_msdp.c b/pimd/pim_msdp.c
index d327984132f6aed7aae668fc14140e3baff396fb..c90a7e1e2ad56c827498f03a6c0a50bf99bc3aec 100644 (file)
--- a/pimd/pim_msdp.c
+++ b/pimd/pim_msdp.c
@@ -124,7 +124,9 @@ pim_msdp_sa_upstream_del(struct pim_msdp_sa *sa)
   sa->up = NULL;
   if (PIM_UPSTREAM_FLAG_TEST_SRC_MSDP(up->flags)) {
     PIM_UPSTREAM_FLAG_UNSET_SRC_MSDP(up->flags);
+    sa->flags |= PIM_MSDP_SAF_UP_DEL_IN_PROG;
     pim_upstream_del(up, __PRETTY_FUNCTION__);
+    sa->flags &= ~PIM_MSDP_SAF_UP_DEL_IN_PROG;
   }
 
   if (PIM_DEBUG_MSDP_EVENTS) {
@@ -234,7 +236,7 @@ pim_msdp_sa_new(struct prefix_sg *sg, struct in_addr rp)
   sa = XCALLOC(MTYPE_PIM_MSDP_SA, sizeof(*sa));
   if (!sa) {
     zlog_err("%s: PIM XCALLOC(%zu) failure",
-             __PRETTY_FUNCTION__, sizeof(*sa));
+            __PRETTY_FUNCTION__, sizeof(*sa));
     return NULL;
   }
 
@@ -477,6 +479,50 @@ pim_msdp_sa_local_del(struct prefix_sg *sg)
   }
 }
 
+/* we need to be very cautious with this API as SA del too can trigger an
+ * upstream del and we will get stuck in a simple loop */
+static void
+pim_msdp_sa_local_del_on_up_del(struct prefix_sg *sg)
+{
+  struct pim_msdp_sa *sa;
+
+  sa = pim_msdp_sa_find(sg);
+  if (sa) {
+    if (PIM_DEBUG_MSDP_INTERNAL) {
+      zlog_debug("MSDP local sa %s del on up del", sa->sg_str);
+    }
+
+    /* if there is no local reference escape */
+    if (!(sa->flags & PIM_MSDP_SAF_LOCAL)) {
+      if (PIM_DEBUG_MSDP_INTERNAL) {
+        zlog_debug("MSDP local sa %s del; no local ref", sa->sg_str);
+      }
+      return;
+    }
+
+    if (sa->flags & PIM_MSDP_SAF_UP_DEL_IN_PROG) {
+      /* MSDP is the one that triggered the upstream del. if this happens
+       * we most certainly have a bug in the PIM upstream state machine. We
+       * will not have a local reference unless the KAT is running. And if the
+       * KAT is running there MUST be an additional source-stream reference to
+       * the flow. Accounting for such cases requires lot of changes; perhaps
+       * address this in the next release? - XXX  */
+      zlog_err("MSDP sa %s SPT teardown is causing the local entry to be removed", sa->sg_str);
+      return;
+    }
+
+    /* we are dropping the sa on upstream del we should not have an
+     * upstream reference */
+    if (sa->up) {
+      if (PIM_DEBUG_MSDP_INTERNAL) {
+        zlog_debug("MSDP local sa %s del; up non-NULL", sa->sg_str);
+      }
+      sa->up = NULL;
+    }
+    pim_msdp_sa_deref(sa, PIM_MSDP_SAF_LOCAL);
+  }
+}
+
 /* Local SA qualification needs to be re-evaluated when -
  * 1. KAT is started or stopped
  * 2. on RP changes
@@ -579,7 +625,7 @@ pim_msdp_up_join_state_changed(struct pim_upstream *xg_up)
   }
 }
 
-void
+static void
 pim_msdp_up_xg_del(struct prefix_sg *sg)
 {
   struct listnode *sanode;
@@ -605,6 +651,19 @@ pim_msdp_up_xg_del(struct prefix_sg *sg)
   }
 }
 
+void
+pim_msdp_up_del(struct prefix_sg *sg)
+{
+  if (PIM_DEBUG_MSDP_INTERNAL) {
+      zlog_debug("MSDP up %s del", pim_str_sg_dump(sg));
+  }
+  if (sg->src.s_addr == INADDR_ANY) {
+    pim_msdp_up_xg_del(sg);
+  } else {
+    pim_msdp_sa_local_del_on_up_del(sg);
+  }
+}
+
 /* sa hash and peer list helpers */
 static unsigned int
 pim_msdp_sa_hash_key_make(void *p)

diff --git a/pimd/pim_msdp.h b/pimd/pim_msdp.h
index 80d928a05d6e9bda9e7f13acd8c21766a5bc5276..33c1d88a45592e343d1ba832cc71e4df3928593d 100644 (file)
--- a/pimd/pim_msdp.h
+++ b/pimd/pim_msdp.h
@@ -69,8 +69,9 @@ enum pim_msdp_sa_flags {
    * checks) */
   PIM_MSDP_SAF_PEER = (1 << 1),
   PIM_MSDP_SAF_REF = (PIM_MSDP_SAF_LOCAL | PIM_MSDP_SAF_PEER),
-  PIM_MSDP_SAF_STALE = (1 << 2) /* local entries can get kicked out on
+  PIM_MSDP_SAF_STALE = (1 << 2), /* local entries can get kicked out on
                                  * misc pim events such as RP change */
+  PIM_MSDP_SAF_UP_DEL_IN_PROG = (1 << 3)
 };
 
 struct pim_msdp_sa {
@@ -223,7 +224,7 @@ void pim_msdp_sa_local_del(struct prefix_sg *sg);
 void pim_msdp_i_am_rp_changed(void);
 bool pim_msdp_peer_rpf_check(struct pim_msdp_peer *mp, struct in_addr rp);
 void pim_msdp_up_join_state_changed(struct pim_upstream *xg_up);
-void pim_msdp_up_xg_del(struct prefix_sg *sg);
+void pim_msdp_up_del(struct prefix_sg *sg);
 enum pim_msdp_err pim_msdp_mg_mbr_add(const char *mesh_group_name, struct in_addr mbr_ip);
 enum pim_msdp_err pim_msdp_mg_mbr_del(const char *mesh_group_name, struct in_addr mbr_ip);
 enum pim_msdp_err pim_msdp_mg_src_del(const char *mesh_group_name);

diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c
index ec985076df48528283c450d644b5c581190fcdbc..d823934e8cd5f8b059335e4d153d33bd26e204a5 100644 (file)
--- a/pimd/pim_upstream.c
+++ b/pimd/pim_upstream.c
@@ -187,8 +187,10 @@ pim_upstream_del(struct pim_upstream *up, const char *name)
     }
   }
 
-  if (up->sg.src.s_addr != INADDR_ANY)
+  if (up->sg.src.s_addr != INADDR_ANY) {
     wheel_remove_item (pim_upstream_sg_wheel, up);
+    notify_msdp = true;
+  }
 
   pim_upstream_remove_children (up);
   pim_mroute_del (up->channel_oil);
@@ -212,7 +214,7 @@ pim_upstream_del(struct pim_upstream *up, const char *name)
   hash_release (pim_upstream_hash, up);
 
   if (notify_msdp) {
-    pim_msdp_up_xg_del(&up->sg);
+    pim_msdp_up_del(&up->sg);
   }
   pim_upstream_free(up);
 }