ripd: split-horizon default differed between rip_interface_new and _r…

…eset

* rip_interface.c: Default for split_horizon_default differed between
  rip_interface_new and rip_interface_reset, causing at least some issues
  after interface events. See patchwork FRRouting#604. Fix, and consolidate code.

  (rip_interface_{reset,clean}) rename these to 'interface', as that's more
  appropriate.  Spin the ri specific bodies of these functions out to
  rip_interface_{reset,clean} helpers.  Factor out the overlaps, so
  rip_interface_reset uses rip_interface_clean.

  (rip_interface_new) just use rip_interface_reset.

* ripd.h: Update for (rip_interface_{reset,clean})

Reported by xufeng zhang, with a suggested fix on which this commit expands.
See patchwork FRRouting#604.  This commit addresses only the split-horizon
discrepency, issue #2.  The other issue they reported, #1, is not addressed,
though suggested fix seems inappropriate.

Cc: [email protected]

ripd/rip_interface.c

@@ -110,26 +110,18 @@ ipv4_multicast_leave (int sock,
   return ret;
 }
 
+static void rip_interface_reset (struct rip_interface *);
+
 /* Allocate new RIP's interface configuration. */
 static struct rip_interface *
 rip_interface_new (void)
 {
   struct rip_interface *ri;
 
   ri = XCALLOC (MTYPE_RIP_INTERFACE, sizeof (struct rip_interface));
-
-  /* Default authentication type is simple password for Cisco
-     compatibility. */
-  ri->auth_type = RIP_NO_AUTH;
-  ri->md5_auth_len = RIP_AUTH_MD5_COMPAT_SIZE;
-
-  /* Set default split-horizon behavior.  If the interface is Frame
-     Relay or SMDS is enabled, the default value for split-horizon is
-     off.  But currently Zebra does detect Frame Relay or SMDS
-     interface.  So all interface is set to split horizon.  */
-  ri->split_horizon_default = RIP_SPLIT_HORIZON;
-  ri->split_horizon = ri->split_horizon_default;
-
+
+  rip_interface_reset (ri);
+
   return ri;
 }
 
@@ -497,81 +489,82 @@ rip_interface_delete (int command, struct zclient *zclient,
   return 0;
 }
 
-void
-rip_interface_clean (void)
+static void
+rip_interface_clean (struct rip_interface *ri)
 {
-  struct listnode *node;
-  struct interface *ifp;
-  struct rip_interface *ri;
+  ri->enable_network = 0;
+  ri->enable_interface = 0;
+  ri->running = 0;
 
-  for (ALL_LIST_ELEMENTS_RO (iflist, node, ifp))
+  if (ri->t_wakeup)
     {
-      ri = ifp->info;
-
-      ri->enable_network = 0;
-      ri->enable_interface = 0;
-      ri->running = 0;
-
-      if (ri->t_wakeup)
-	{
-	  thread_cancel (ri->t_wakeup);
-	  ri->t_wakeup = NULL;
-	}
+      thread_cancel (ri->t_wakeup);
+      ri->t_wakeup = NULL;
     }
 }
 
 void
-rip_interface_reset (void)
+rip_interfaces_clean (void)
 {
   struct listnode *node;
   struct interface *ifp;
-  struct rip_interface *ri;
 
   for (ALL_LIST_ELEMENTS_RO (iflist, node, ifp))
-    {
-      ri = ifp->info;
-
-      ri->enable_network = 0;
-      ri->enable_interface = 0;
-      ri->running = 0;
+    rip_interface_clean (ifp->info);
+}
 
-      ri->ri_send = RI_RIP_UNSPEC;
-      ri->ri_receive = RI_RIP_UNSPEC;
+static void
+rip_interface_reset (struct rip_interface *ri)
+{
+  /* Default authentication type is simple password for Cisco
+     compatibility. */
+  ri->auth_type = RIP_NO_AUTH;
+  ri->md5_auth_len = RIP_AUTH_MD5_COMPAT_SIZE;
 
-      ri->auth_type = RIP_NO_AUTH;
+  /* Set default split-horizon behavior.  If the interface is Frame
+     Relay or SMDS is enabled, the default value for split-horizon is
+     off.  But currently Zebra does detect Frame Relay or SMDS
+     interface.  So all interface is set to split horizon.  */
+  ri->split_horizon_default = RIP_SPLIT_HORIZON;
+  ri->split_horizon = ri->split_horizon_default;
 
-      if (ri->auth_str)
-	{
-	  free (ri->auth_str);
-	  ri->auth_str = NULL;
-	}
-      if (ri->key_chain)
-	{
-	  free (ri->key_chain);
-	  ri->key_chain = NULL;
-	}
+  ri->ri_send = RI_RIP_UNSPEC;
+  ri->ri_receive = RI_RIP_UNSPEC;
+
+  if (ri->auth_str)
+    {
+      free (ri->auth_str);
+      ri->auth_str = NULL;
+    }
+  if (ri->key_chain)
+    {
+      free (ri->key_chain);
+      ri->key_chain = NULL;
+    }
 
-      ri->split_horizon = RIP_NO_SPLIT_HORIZON;
-      ri->split_horizon_default = RIP_NO_SPLIT_HORIZON;
+  ri->list[RIP_FILTER_IN] = NULL;
+  ri->list[RIP_FILTER_OUT] = NULL;
 
-      ri->list[RIP_FILTER_IN] = NULL;
-      ri->list[RIP_FILTER_OUT] = NULL;
+  ri->prefix[RIP_FILTER_IN] = NULL;
+  ri->prefix[RIP_FILTER_OUT] = NULL;
+
+  ri->recv_badpackets = 0;
+  ri->recv_badroutes = 0;
+  ri->sent_updates = 0;
 
-      ri->prefix[RIP_FILTER_IN] = NULL;
-      ri->prefix[RIP_FILTER_OUT] = NULL;
-
-      if (ri->t_wakeup)
-	{
-	  thread_cancel (ri->t_wakeup);
-	  ri->t_wakeup = NULL;
-	}
+  ri->passive = 0;
+
+  rip_interface_clean (ri);
+}
 
-      ri->recv_badpackets = 0;
-      ri->recv_badroutes = 0;
-      ri->sent_updates = 0;
+void
+rip_interfaces_reset (void)
+{
+  struct listnode *node;
+  struct interface *ifp;
 
-      ri->passive = 0;
-    }
+  for (ALL_LIST_ELEMENTS_RO (iflist, node, ifp))
+    rip_interface_reset (ifp->info);
 }
 
 int

ripd/ripd.c

@@ -4025,7 +4025,7 @@ rip_clean (void)
   rip_clean_network ();
   rip_passive_nondefault_clean ();
   rip_offset_clean ();
-  rip_interface_clean ();
+  rip_interfaces_clean ();
   rip_distance_reset ();
   rip_redistribute_clean ();
 }
@@ -4049,7 +4049,7 @@ rip_reset (void)
 
   distribute_list_reset ();
 
-  rip_interface_reset ();
+  rip_interfaces_reset ();
   rip_distance_reset ();
 
   rip_zclient_reset ();

ripd/ripd.h

@@ -381,8 +381,8 @@ extern void rip_init (void);
 extern void rip_reset (void);
 extern void rip_clean (void);
 extern void rip_clean_network (void);
-extern void rip_interface_clean (void);
-extern void rip_interface_reset (void);
+extern void rip_interfaces_clean (void);
+extern void rip_interfaces_reset (void);
 extern void rip_passive_nondefault_clean (void);
 extern void rip_if_init (void);
 extern void rip_if_down_all (void);