ripd: simplify cleaning up of routing instance

* Call rip_clean() only when RIP is configured, this way we can
  remove one indentation level from this function.
* rip_redistribute_clean() is only called on shutdown, so there's
  no need to call rip_redistribute_withdraw() there since the RIP
  table is already cleaned up elsewhere.
* There's no need to clean up the "rip->neighbor" nodes manually before
  calling route_table_finish().
* Deallocate the rip structure only at the end of the function. This
  prepares the ground for the next commits where all global variables
  will be moved to the rip structure.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

diff --git a/ripd/rip_main.c b/ripd/rip_main.c
index 5db9c4b7e9e0250d9240a9404d5163de9db210e6..e6373664bf19e23bc1f4e2c3bccb543c5aa22201 100644 (file)
--- a/ripd/rip_main.c
+++ b/ripd/rip_main.c
@@ -81,7 +81,8 @@ static void sigint(void)
 {
        zlog_notice("Terminating on signal");
 
-       rip_clean();
+       if (rip)
+               rip_clean();
 
        rip_zclient_stop();
        frr_fini();

diff --git a/ripd/rip_zebra.c b/ripd/rip_zebra.c
index fff8681775a8c997f94b37272956d7b063cb6fae..684614fb470aaaff9611d7ab2fbe2ec6d617a321 100644 (file)
--- a/ripd/rip_zebra.c
+++ b/ripd/rip_zebra.c
@@ -180,9 +180,6 @@ void rip_redistribute_clean(void)
                                                VRF_DEFAULT);
 
                vrf_bitmap_unset(zclient->redist[AFI_IP][i], VRF_DEFAULT);
-
-               /* Remove the routes from RIP table. */
-               rip_redistribute_withdraw(i);
        }
 }
 

diff --git a/ripd/ripd.c b/ripd/ripd.c
index 0ce5324057cf849c30483021df4013e40dc54d60..f1c92ac7b9c52aa6adc4572ce48acb6d6528808e 100644 (file)
--- a/ripd/ripd.c
+++ b/ripd/ripd.c
@@ -2634,9 +2634,6 @@ void rip_redistribute_withdraw(int type)
        struct rip_info *rinfo = NULL;
        struct list *list = NULL;
 
-       if (!rip)
-               return;
-
        for (rp = route_top(rip->table); rp; rp = route_next(rp))
                if ((list = rp->info) != NULL) {
                        rinfo = listgetdata(listhead(list));
@@ -2696,6 +2693,12 @@ int rip_create(int socket)
        rip->table = route_table_init();
        rip->neighbor = route_table_init();
 
+       /* Distribute list install. */
+       rip->distribute_ctx =
+               distribute_list_ctx_create(vrf_lookup_by_id(VRF_DEFAULT));
+       distribute_list_add_hook(rip->distribute_ctx, rip_distribute_update);
+       distribute_list_delete_hook(rip->distribute_ctx, rip_distribute_update);
+
        /* Make output stream. */
        rip->obuf = stream_new(1500);
 
@@ -2705,13 +2708,7 @@ int rip_create(int socket)
        /* Create read and timer thread. */
        rip_event(RIP_READ, rip->sock);
        rip_event(RIP_UPDATE_EVENT, 1);
-       /* Distribute list install. */
-       rip->distribute_ctx = distribute_list_ctx_create(
-                                        vrf_lookup_by_id(VRF_DEFAULT));
-       distribute_list_add_hook(rip->distribute_ctx,
-                                rip_distribute_update);
-       distribute_list_delete_hook(rip->distribute_ctx,
-                                   rip_distribute_update);
+
        return 0;
 }
 
@@ -3325,65 +3322,54 @@ static void rip_distribute_update_all_wrapper(struct access_list *notused)
 /* Delete all added rip route. */
 void rip_clean(void)
 {
-       int i;
        struct route_node *rp;
-       struct rip_info *rinfo = NULL;
-       struct list *list = NULL;
-       struct listnode *listnode = NULL;
 
-       if (rip) {
-               /* Clear RIP routes */
-               for (rp = route_top(rip->table); rp; rp = route_next(rp))
-                       if ((list = rp->info) != NULL) {
-                               rinfo = listgetdata(listhead(list));
-                               if (rip_route_rte(rinfo))
-                                       rip_zebra_ipv4_delete(rp);
+       /* Clear RIP routes */
+       for (rp = route_top(rip->table); rp; rp = route_next(rp)) {
+               struct rip_info *rinfo;
+               struct list *list;
+               struct listnode *listnode;
 
-                               for (ALL_LIST_ELEMENTS_RO(list, listnode,
-                                                         rinfo)) {
-                                       RIP_TIMER_OFF(rinfo->t_timeout);
-                                       RIP_TIMER_OFF(rinfo->t_garbage_collect);
-                                       rip_info_free(rinfo);
-                               }
-                               list_delete(&list);
-                               rp->info = NULL;
-                               route_unlock_node(rp);
-                       }
-
-               /* Cancel RIP related timers. */
-               RIP_TIMER_OFF(rip->t_update);
-               RIP_TIMER_OFF(rip->t_triggered_update);
-               RIP_TIMER_OFF(rip->t_triggered_interval);
+               if ((list = rp->info) == NULL)
+                       continue;
 
-               /* Cancel read thread. */
-               THREAD_READ_OFF(rip->t_read);
+               rinfo = listgetdata(listhead(list));
+               if (rip_route_rte(rinfo))
+                       rip_zebra_ipv4_delete(rp);
 
-               /* Close RIP socket. */
-               if (rip->sock >= 0) {
-                       close(rip->sock);
-                       rip->sock = -1;
+               for (ALL_LIST_ELEMENTS_RO(list, listnode, rinfo)) {
+                       RIP_TIMER_OFF(rinfo->t_timeout);
+                       RIP_TIMER_OFF(rinfo->t_garbage_collect);
+                       rip_info_free(rinfo);
                }
+               list_delete(&list);
+               rp->info = NULL;
+               route_unlock_node(rp);
+       }
 
-               stream_free(rip->obuf);
+       /* Cancel RIP related timers. */
+       RIP_TIMER_OFF(rip->t_update);
+       RIP_TIMER_OFF(rip->t_triggered_update);
+       RIP_TIMER_OFF(rip->t_triggered_interval);
 
-               /* RIP neighbor configuration. */
-               for (rp = route_top(rip->neighbor); rp; rp = route_next(rp))
-                       if (rp->info) {
-                               rp->info = NULL;
-                               route_unlock_node(rp);
-                       }
+       /* Cancel read thread. */
+       THREAD_READ_OFF(rip->t_read);
 
-               for (i = 0; i < ZEBRA_ROUTE_MAX; i++)
-                       if (rip->route_map[i].name)
-                               free(rip->route_map[i].name);
+       /* Close RIP socket. */
+       if (rip->sock >= 0) {
+               close(rip->sock);
+               rip->sock = -1;
+       }
 
-               route_table_finish(rip->table);
-               route_table_finish(rip->neighbor);
+       stream_free(rip->obuf);
 
-               distribute_list_delete(&rip->distribute_ctx);
-               XFREE(MTYPE_RIP, rip);
-               rip = NULL;
-       }
+       for (int i = 0; i < ZEBRA_ROUTE_MAX; i++)
+               if (rip->route_map[i].name)
+                       free(rip->route_map[i].name);
+
+       route_table_finish(rip->table);
+       route_table_finish(rip->neighbor);
+       distribute_list_delete(&rip->distribute_ctx);
 
        rip_clean_network();
        rip_passive_nondefault_clean();
@@ -3391,6 +3377,8 @@ void rip_clean(void)
        rip_interfaces_clean();
        rip_distance_reset();
        rip_redistribute_clean();
+
+       XFREE(MTYPE_RIP, rip);
 }
 
 static void rip_if_rmap_update(struct if_rmap *if_rmap)