pimd: Separate the register and upstream join states on the FHR

On the FHR upstream-join-state is not particularly relevant as we
don't need to send upstream JPs for the SG. So that field was being
overloaded with the register-state. However some of the events that
triggered changes to the JoinDesired macro were accidentally overwriting
the state with join info (instead of treating it as register info)
confusing the register state machine.

To make the PIM RFC macros' implemention simple I have separated out
the register-state. And upstream->state now solely describes the
upstream-join-state independent of the role of the PIM router.

Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Ticket: CM-14700
Testing Done: verified pim-register state-machine with separate and
combined FHR/RP routers. Also ran pim-smoke.

diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index 016ba3e4512b9d2e233f2b14e5f4a31938c142df..4183175f8626b7149433550717cdb4fb4885f001 100644 (file)
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -1667,6 +1667,7 @@ static void pim_show_upstream(struct vty *vty, u_char uj)
     char rs_timer[10];
     char ka_timer[10];
     char msdp_reg_timer[10];
+    char state_str[PIM_REG_STATE_STR_LEN];
 
     pim_inet4_dump("<src?>", up->sg.src, src_str, sizeof(src_str));
     pim_inet4_dump("<grp?>", up->sg.grp, grp_str, sizeof(grp_str));
@@ -1690,6 +1691,11 @@ static void pim_show_upstream(struct vty *vty, u_char uj)
     pim_time_timer_to_hhmmss (ka_timer, sizeof (ka_timer), up->t_ka_timer);
     pim_time_timer_to_hhmmss (msdp_reg_timer, sizeof (msdp_reg_timer), up->t_msdp_reg_timer);
 
+    if (pim_if_connected_to_source (up->rpf.source_nexthop.interface, up->sg.src))
+      strcpy (state_str, pim_upstream_state2str (up->reg_state));
+    else
+      strcpy (state_str, pim_upstream_state2str (up->join_state));
+
     if (uj) {
       json_object_object_get_ex(json, grp_str, &json_group);
 
@@ -1703,7 +1709,9 @@ static void pim_show_upstream(struct vty *vty, u_char uj)
       json_object_string_add(json_row, "inboundInterface", up->rpf.source_nexthop.interface->name);
       json_object_string_add(json_row, "source", src_str);
       json_object_string_add(json_row, "group", grp_str);
-      json_object_string_add(json_row, "state", pim_upstream_state2str (up->join_state));
+      json_object_string_add(json_row, "state", state_str);
+      json_object_string_add(json_row, "joinState", pim_upstream_state2str (up->join_state));
+      json_object_string_add(json_row, "regState", pim_reg_state2str (up->reg_state, state_str));
       json_object_string_add(json_row, "upTime", uptime);
       json_object_string_add(json_row, "joinTimer", join_timer);
       json_object_string_add(json_row, "resetTimer", rs_timer);
@@ -1717,7 +1725,7 @@ static void pim_show_upstream(struct vty *vty, u_char uj)
               up->rpf.source_nexthop.interface->name,
               src_str,
               grp_str,
-              pim_upstream_state2str (up->join_state),
+              state_str,
               uptime,
               join_timer,
               rs_timer,

diff --git a/pimd/pim_jp_agg.c b/pimd/pim_jp_agg.c
index 67ddf05d0666c2ad371fce709eaa6a015f343d83..8d4510324c32ac636fcee390dbb3c644a3f0cc6b 100644 (file)
--- a/pimd/pim_jp_agg.c
+++ b/pimd/pim_jp_agg.c
@@ -249,6 +249,10 @@ pim_jp_agg_single_upstream_send (struct pim_rpf *rpf,
 
   static bool first = true;
 
+  /* skip JP upstream messages if source is directly connected */
+  if (pim_if_connected_to_source (rpf->source_nexthop.interface, up->sg.src))
+      return;
+
   if (first)
     {
       groups = list_new();

diff --git a/pimd/pim_mroute.c b/pimd/pim_mroute.c
index 334e0ce06e19c5e5a578f6982cfe29d65456a7c3..3f483a25addc231e85568e8b44ad3b1f304377c2 100644 (file)
--- a/pimd/pim_mroute.c
+++ b/pimd/pim_mroute.c
@@ -189,7 +189,7 @@ pim_mroute_msg_nocache (int fd, struct interface *ifp, const struct igmpmsg *msg
   up->channel_oil->cc.pktcnt++;
   PIM_UPSTREAM_FLAG_SET_FHR(up->flags);
   pim_channel_add_oif (up->channel_oil, pim_regiface, PIM_OIF_FLAG_PROTO_PIM);
-  up->join_state = PIM_UPSTREAM_JOINED;
+  up->reg_state = PIM_UPSTREAM_JOINED;
 
   return 0;
 }
@@ -453,7 +453,7 @@ pim_mroute_msg_wrvifwhole (int fd, struct interface *ifp, const char *buf)
       up->channel_oil = oil;
       up->channel_oil->cc.pktcnt++;
       pim_channel_add_oif (up->channel_oil, pim_regiface, PIM_OIF_FLAG_PROTO_PIM);
-      up->join_state = PIM_UPSTREAM_JOINED;
+      up->reg_state = PIM_UPSTREAM_JOINED;
       pim_upstream_inherited_olist (up);
 
       // Send the packet to the RP

diff --git a/pimd/pim_register.c b/pimd/pim_register.c
index 29b4e8a63b2169893d28d5622e2a89f9bd6a1417..2d4d296598791db590a3df2a2816074b6a6c3f6e 100644 (file)
--- a/pimd/pim_register.c
+++ b/pimd/pim_register.c
@@ -122,19 +122,19 @@ pim_register_stop_recv (uint8_t *buf, int buf_size)
     zlog_debug ("Received Register stop for %s",
                upstream->sg_str);
 
-  switch (upstream->join_state)
+  switch (upstream->reg_state)
     {
     case PIM_UPSTREAM_NOTJOINED:
     case PIM_UPSTREAM_PRUNE:
       return 0;
       break;
     case PIM_UPSTREAM_JOINED:
-      upstream->join_state = PIM_UPSTREAM_PRUNE;
+      upstream->reg_state = PIM_UPSTREAM_PRUNE;
       pim_channel_del_oif (upstream->channel_oil, pim_regiface, PIM_OIF_FLAG_PROTO_PIM);
       pim_upstream_start_register_stop_timer (upstream, 0);
       break;
     case PIM_UPSTREAM_JOIN_PENDING:
-      upstream->join_state = PIM_UPSTREAM_PRUNE;
+      upstream->reg_state = PIM_UPSTREAM_PRUNE;
       pim_upstream_start_register_stop_timer (upstream, 0);
       return 0;
       break;

diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c
index 4187635b199c2cdedc1910d4cef498cd0ddf3e8d..68652f67fdc3d7da8db74cc65612c81c1fed5088 100644 (file)
--- a/pimd/pim_upstream.c
+++ b/pimd/pim_upstream.c
@@ -509,6 +509,7 @@ pim_upstream_switch(struct pim_upstream *up,
             PIM_UPSTREAM_FLAG_SET_FHR(up->flags);
             if (!old_fhr && PIM_UPSTREAM_FLAG_TEST_SRC_STREAM(up->flags))
               {
+                up->reg_state = PIM_UPSTREAM_JOINED;
                 pim_upstream_keep_alive_timer_start (up, qpim_keep_alive_time);
                pim_channel_add_oif (up->channel_oil, pim_regiface, PIM_OIF_FLAG_PROTO_PIM);
               }
@@ -602,6 +603,7 @@ pim_upstream_new (struct prefix_sg *sg,
   up->t_rs_timer                 = NULL;
   up->t_msdp_reg_timer           = NULL;
   up->join_state                 = 0;
+  up->reg_state                  = 0;
   up->state_transition           = pim_time_monotonic_sec();
   up->channel_oil                = NULL;
   up->sptbit                     = PIM_UPSTREAM_SPTBIT_FALSE;
@@ -958,7 +960,7 @@ static void pim_upstream_fhr_kat_expiry(struct pim_upstream *up)
   /* remove regiface from the OIL if it is there*/
   pim_channel_del_oif (up->channel_oil, pim_regiface, PIM_OIF_FLAG_PROTO_PIM);
   /* move to "not-joined" */
-  up->join_state = PIM_UPSTREAM_NOTJOINED;
+  up->reg_state = PIM_UPSTREAM_NOTJOINED;
   PIM_UPSTREAM_FLAG_UNSET_FHR(up->flags);
 }
 
@@ -972,9 +974,9 @@ static void pim_upstream_fhr_kat_start(struct pim_upstream *up)
       zlog_debug ("kat started on %s; set fhr reg state to joined", up->sg_str);
 
     PIM_UPSTREAM_FLAG_SET_FHR(up->flags);
-    if (up->join_state == PIM_UPSTREAM_NOTJOINED) {
+    if (up->reg_state == PIM_UPSTREAM_NOTJOINED) {
       pim_channel_add_oif (up->channel_oil, pim_regiface, PIM_OIF_FLAG_PROTO_PIM);
-      up->join_state = PIM_UPSTREAM_JOINED;
+      up->reg_state = PIM_UPSTREAM_JOINED;
     }
   }
 }
@@ -1211,6 +1213,29 @@ pim_upstream_state2str (enum pim_upstream_state join_state)
   return "Unknown";
 }
 
+const char *
+pim_reg_state2str (enum pim_upstream_state join_state, char *state_str)
+{
+  switch (join_state)
+    {
+    case PIM_UPSTREAM_NOTJOINED:
+      strcpy (state_str, "NoInfo");
+      break;
+    case PIM_UPSTREAM_JOINED:
+      strcpy (state_str, "Joined");
+      break;
+    case PIM_UPSTREAM_JOIN_PENDING:
+      strcpy (state_str, "JoinPending");
+      break;
+    case PIM_UPSTREAM_PRUNE:
+      strcpy (state_str, "Prune");
+      break;
+    default:
+      strcpy (state_str, "Unknown");
+    }
+  return state_str;
+}
+
 static int
 pim_upstream_register_stop_timer (struct thread *t)
 {
@@ -1226,13 +1251,13 @@ pim_upstream_register_stop_timer (struct thread *t)
     {
       zlog_debug ("%s: (S,G)=%s upstream register stop timer %s",
                  __PRETTY_FUNCTION__, up->sg_str,
-                  pim_upstream_state2str(up->join_state));
+                  pim_upstream_state2str(up->reg_state));
     }
 
-  switch (up->join_state)
+  switch (up->reg_state)
     {
     case PIM_UPSTREAM_JOIN_PENDING:
-      up->join_state = PIM_UPSTREAM_JOINED;
+      up->reg_state = PIM_UPSTREAM_JOINED;
       pim_channel_add_oif (up->channel_oil, pim_regiface, PIM_OIF_FLAG_PROTO_PIM);
       break;
     case PIM_UPSTREAM_JOINED:
@@ -1246,7 +1271,7 @@ pim_upstream_register_stop_timer (struct thread *t)
                        __PRETTY_FUNCTION__, up->rpf.source_nexthop.interface->name);
          return 0;
        }
-      up->join_state = PIM_UPSTREAM_JOIN_PENDING;
+      up->reg_state = PIM_UPSTREAM_JOIN_PENDING;
       pim_upstream_start_register_stop_timer (up, 1);
 
       if (((up->channel_oil->cc.lastused/100) > PIM_KEEPALIVE_PERIOD) &&

diff --git a/pimd/pim_upstream.h b/pimd/pim_upstream.h
index 7cdf73759d3c20ccb2f73afd1eee16ffb0485288..3f0abc16c2779790a6b3083432bec098aa8629fe 100644 (file)
--- a/pimd/pim_upstream.h
+++ b/pimd/pim_upstream.h
@@ -88,6 +88,7 @@ struct pim_upstream {
   struct list             *sources;
 
   enum pim_upstream_state  join_state;
+  enum pim_upstream_state  reg_state;
   enum pim_upstream_sptbit sptbit;
 
   int                      ref_count;
@@ -163,6 +164,8 @@ void pim_upstream_send_join (struct pim_upstream *up);
 void pim_upstream_switch (struct pim_upstream *up, enum pim_upstream_state new_state);
 
 const char *pim_upstream_state2str (enum pim_upstream_state join_state);
+#define PIM_REG_STATE_STR_LEN 12
+const char *pim_reg_state2str (enum pim_upstream_state state, char *state_str);
 
 int pim_upstream_inherited_olist_decide (struct pim_upstream *up);
 int pim_upstream_inherited_olist (struct pim_upstream *up);