From d319a3abbf6e6c310b6b6a6891ebe1957592f9a9 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Wed, 25 May 2016 14:47:00 +0100 Subject: [PATCH] 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 --- ripd/rip_interface.c | 129 ++++++++++++++++++++----------------------- ripd/ripd.c | 4 +- ripd/ripd.h | 4 +- 3 files changed, 65 insertions(+), 72 deletions(-) diff --git a/ripd/rip_interface.c b/ripd/rip_interface.c index 7bdcf46a102b..240c9688b4cd 100644 --- a/ripd/rip_interface.c +++ b/ripd/rip_interface.c @@ -110,6 +110,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) @@ -117,19 +119,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; } @@ -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 diff --git a/ripd/ripd.c b/ripd/ripd.c index 0beb0e6285ab..1d3eb4c501ee 100644 --- a/ripd/ripd.c +++ b/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 (); diff --git a/ripd/ripd.h b/ripd/ripd.h index dbed342db736..5e87fcd579ba 100644 --- a/ripd/ripd.h +++ b/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);