ripd: split-horizon default differed between rip_interface_new and _reset

* 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 #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 #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: xufeng.zhang@windriver.com

diff --git a/ripd/rip_interface.c b/ripd/rip_interface.c
index 09b35d00b986f665e9ac977b18d32a9783ec5900..604343be896e3c18fe16c0cd591d0cf2900c3c13 100644 (file)
--- a/ripd/rip_interface.c
+++ b/ripd/rip_interface.c
@@ -112,6 +112,8 @@ 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)
@@ -119,19 +121,9 @@ 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;
 }
 
@@ -503,81 +495,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 (vrf_iflist (VRF_DEFAULT), 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 (vrf_iflist (VRF_DEFAULT), 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 (vrf_iflist (VRF_DEFAULT), node, ifp))
+    rip_interface_reset (ifp->info);
 }
 
 int

diff --git a/ripd/ripd.c b/ripd/ripd.c
index 3a8cd80e7a2ec1acc5e880388cb9d730bfe2cfb5..698de7e8174cf9eac930cc69b935f86d110a49ba 100644 (file)
--- a/ripd/ripd.c
+++ b/ripd/ripd.c
@@ -4030,7 +4030,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 ();
 }
@@ -4054,7 +4054,7 @@ rip_reset (void)
 
   distribute_list_reset ();
 
-  rip_interface_reset ();
+  rip_interfaces_reset ();
   rip_distance_reset ();
 
   rip_zclient_reset ();

diff --git a/ripd/ripd.h b/ripd/ripd.h
index 7c77b26d41de48708bea67f09bc2a1605d38fb79..588da1d5f7b8d394a77309d33cfbf2d6aa15057f 100644 (file)
--- a/ripd/ripd.h
+++ b/ripd/ripd.h
@@ -383,8 +383,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);