eigrpd: Cleanup memory issues on shutdown

a) EIGRP was having issues with the prefix created as part
of the topology destination.  Make this just a part of the
topology data structure instead of allocating it.

b) EIGRP was not freeing up any memory associated with
the network table.  Free it.

c) EIGRP was confusing zebra shutdown as part of the deletion
of the last eigrp data structure.  This was inappropriate it
should be part of the `I'm just shutting down`.

d) The QOBJ was not being properly freed, free it.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

diff --git a/eigrpd/eigrp_dump.c b/eigrpd/eigrp_dump.c
index 231554a0981e31af71401a56235226afc66e7562..d59adc8234b6dc018c928c63cdb28bb347404400 100644 (file)
--- a/eigrpd/eigrp_dump.c
+++ b/eigrpd/eigrp_dump.c
@@ -228,7 +228,7 @@ void show_ip_eigrp_prefix_descriptor(struct vty *vty,
 
        vty_out(vty, "%-3c", (tn->state > 0) ? 'A' : 'P');
 
-       vty_out(vty, "%pFX, ", tn->destination);
+       vty_out(vty, "%pFX, ", &tn->destination);
        vty_out(vty, "%u successors, ", (successors) ? successors->count : 0);
        vty_out(vty, "FD is %u, serno: %" PRIu64 " \n", tn->fdistance,
                tn->serno);

diff --git a/eigrpd/eigrp_fsm.c b/eigrpd/eigrp_fsm.c
index 6d8061e57232e3943d594f8c21cbffa56547c268..99b090eeb6cf18a111ec214081e42c9813cb50d2 100644 (file)
--- a/eigrpd/eigrp_fsm.c
+++ b/eigrpd/eigrp_fsm.c
@@ -403,12 +403,10 @@ int eigrp_fsm_event(struct eigrp_fsm_action_message *msg)
 {
        enum eigrp_fsm_events event = eigrp_get_fsm_event(msg);
 
-       zlog_info(
-               "EIGRP AS: %d State: %s Event: %s Network: %pI4 Packet Type: %s Reply RIJ Count: %d change: %s",
-               msg->eigrp->AS, prefix_state2str(msg->prefix->state),
-               fsm_state2str(event), &msg->prefix->destination->u.prefix4,
-               packet_type2str(msg->packet_type), msg->prefix->rij->count,
-               change2str(msg->change));
+       zlog_info("EIGRP AS: %d State: %s Event: %s Network: %pFX Packet Type: %s Reply RIJ Count: %d change: %s",
+                 msg->eigrp->AS, prefix_state2str(msg->prefix->state), fsm_state2str(event),
+                 &msg->prefix->destination, packet_type2str(msg->packet_type),
+                 msg->prefix->rij->count, change2str(msg->change));
        (*(NSM[msg->prefix->state][event].func))(msg);
 
        return 1;

diff --git a/eigrpd/eigrp_interface.c b/eigrpd/eigrp_interface.c
index 2e3beacae785d6a35422403e0619de9970f52694..73caa72b6d6fbfeb9881f9aae975ffac4a027a94 100644 (file)
--- a/eigrpd/eigrp_interface.c
+++ b/eigrpd/eigrp_interface.c
@@ -294,8 +294,7 @@ int eigrp_if_up(struct eigrp_interface *ei)
        if (pe == NULL) {
                pe = eigrp_prefix_descriptor_new();
                pe->serno = eigrp->serno;
-               pe->destination = (struct prefix *)prefix_ipv4_new();
-               prefix_copy(pe->destination, &dest_addr);
+               prefix_copy(&pe->destination, &dest_addr);
                pe->af = AF_INET;
                pe->nt = EIGRP_TOPOLOGY_TYPE_CONNECTED;
 

diff --git a/eigrpd/eigrp_network.c b/eigrpd/eigrp_network.c
index 910215cbcbea2b7c97f43cc2d8c361b187354c38..fc334cf97d22da5f2acd646247c53a4d2d1521aa 100644 (file)
--- a/eigrpd/eigrp_network.c
+++ b/eigrpd/eigrp_network.c
@@ -219,6 +219,21 @@ int eigrp_network_set(struct eigrp *eigrp, struct prefix *p)
        return 1;
 }
 
+static void eigrp_network_delete_all(struct eigrp *eigrp, struct route_table *table)
+{
+       struct route_node *rn;
+
+       for (rn = route_top(table); rn; rn = route_next(rn)) {
+               prefix_free((struct prefix **)&rn->info);
+       }
+}
+
+void eigrp_network_free(struct eigrp *eigrp, struct route_table *table)
+{
+       eigrp_network_delete_all(eigrp, table);
+       route_table_finish(table);
+}
+
 /* Check whether interface matches given network
  * returns: 1, true. 0, false
  */

diff --git a/eigrpd/eigrp_network.h b/eigrpd/eigrp_network.h
index ac5c47f6f98bbab97233b6ee8049df2c3d7c4021..1baf26b0c0ae9034a9996caba7ea58c73dc494a8 100644 (file)
--- a/eigrpd/eigrp_network.h
+++ b/eigrpd/eigrp_network.h
@@ -19,6 +19,7 @@ extern int eigrp_sock_init(struct vrf *vrf);
 extern int eigrp_if_ipmulticast(struct eigrp *, struct prefix *, unsigned int);
 extern int eigrp_network_set(struct eigrp *eigrp, struct prefix *p);
 extern int eigrp_network_unset(struct eigrp *eigrp, struct prefix *p);
+extern void eigrp_network_free(struct eigrp *eigrp, struct route_table *table);
 
 extern void eigrp_hello_timer(struct event *thread);
 extern void eigrp_if_update(struct interface *);

diff --git a/eigrpd/eigrp_packet.c b/eigrpd/eigrp_packet.c
index 189d9330cf851f9c4babf337ad9f4705c0d099d7..7560514ceca37bddb4e01d47e6165efd82f40e9d 100644 (file)
--- a/eigrpd/eigrp_packet.c
+++ b/eigrpd/eigrp_packet.c
@@ -1129,7 +1129,7 @@ uint16_t eigrp_add_internalTLV_to_stream(struct stream *s,
        uint16_t length;
 
        stream_putw(s, EIGRP_TLV_IPv4_INT);
-       switch (pe->destination->prefixlen) {
+       switch (pe->destination.prefixlen) {
        case 0:
        case 1:
        case 2:
@@ -1176,8 +1176,8 @@ uint16_t eigrp_add_internalTLV_to_stream(struct stream *s,
                stream_putw(s, length);
                break;
        default:
-               flog_err(EC_LIB_DEVELOPMENT, "%s: Unexpected prefix length: %d",
-                        __func__, pe->destination->prefixlen);
+               flog_err(EC_LIB_DEVELOPMENT, "%s: Unexpected prefix length: %d", __func__,
+                        pe->destination.prefixlen);
                return 0;
        }
        stream_putl(s, 0x00000000);
@@ -1194,15 +1194,15 @@ uint16_t eigrp_add_internalTLV_to_stream(struct stream *s,
        stream_putc(s, pe->reported_metric.tag);
        stream_putc(s, pe->reported_metric.flags);
 
-       stream_putc(s, pe->destination->prefixlen);
+       stream_putc(s, pe->destination.prefixlen);
 
-       stream_putc(s, (ntohl(pe->destination->u.prefix4.s_addr) >> 24) & 0xFF);
-       if (pe->destination->prefixlen > 8)
-               stream_putc(s, (ntohl(pe->destination->u.prefix4.s_addr) >> 16) & 0xFF);
-       if (pe->destination->prefixlen > 16)
-               stream_putc(s, (ntohl(pe->destination->u.prefix4.s_addr) >> 8) & 0xFF);
-       if (pe->destination->prefixlen > 24)
-               stream_putc(s, ntohl(pe->destination->u.prefix4.s_addr) & 0xFF);
+       stream_putc(s, (ntohl(pe->destination.u.prefix4.s_addr) >> 24) & 0xFF);
+       if (pe->destination.prefixlen > 8)
+               stream_putc(s, (ntohl(pe->destination.u.prefix4.s_addr) >> 16) & 0xFF);
+       if (pe->destination.prefixlen > 16)
+               stream_putc(s, (ntohl(pe->destination.u.prefix4.s_addr) >> 8) & 0xFF);
+       if (pe->destination.prefixlen > 24)
+               stream_putc(s, ntohl(pe->destination.u.prefix4.s_addr) & 0xFF);
 
        return length;
 }

diff --git a/eigrpd/eigrp_reply.c b/eigrpd/eigrp_reply.c
index aae89e832b0aeae409925596628a489afb4076e3..a444f3a5a936c688c00b72a78f1e3650edec56c5 100644 (file)
--- a/eigrpd/eigrp_reply.c
+++ b/eigrpd/eigrp_reply.c
@@ -61,8 +61,7 @@ void eigrp_send_reply(struct eigrp_neighbor *nbr,
                      sizeof(struct eigrp_prefix_descriptor));
        memcpy(pe2, pe, sizeof(struct eigrp_prefix_descriptor));
 
-       if (eigrp_update_prefix_apply(eigrp, ei, EIGRP_FILTER_OUT,
-                                     pe2->destination)) {
+       if (eigrp_update_prefix_apply(eigrp, ei, EIGRP_FILTER_OUT, &pe2->destination)) {
                zlog_info("REPLY SEND: Setting Metric to max");
                pe2->reported_metric.delay = EIGRP_MAX_METRIC;
        }

diff --git a/eigrpd/eigrp_structs.h b/eigrpd/eigrp_structs.h
index 93bcf07885f6baaa004dcf9eaf12850fc96dbbc9..c24067bc33385aedd4d62aea0a9be0e09a2363e0 100644 (file)
--- a/eigrpd/eigrp_structs.h
+++ b/eigrpd/eigrp_structs.h
@@ -447,7 +447,7 @@ struct eigrp_prefix_descriptor {
        uint8_t af;      // address family
        uint8_t req_action; // required action
 
-       struct prefix *destination;
+       struct prefix destination;
 
        // If network type is REMOTE_EXTERNAL, pointer will have reference to
        // its external TLV

diff --git a/eigrpd/eigrp_topology.c b/eigrpd/eigrp_topology.c
index f17be8f4b7db8e89717c353f1aee55308b8e080f..e59dcacb7cbcecc8e58432a7dbe4dd719be3252c 100644 (file)
--- a/eigrpd/eigrp_topology.c
+++ b/eigrpd/eigrp_topology.c
@@ -68,7 +68,6 @@ struct eigrp_prefix_descriptor *eigrp_prefix_descriptor_new(void)
        new->rij = list_new();
        new->entries->cmp = (int (*)(void *, void *))eigrp_route_descriptor_cmp;
        new->distance = new->fdistance = new->rdistance = EIGRP_MAX_METRIC;
-       new->destination = NULL;
 
        return new;
 }
@@ -120,12 +119,11 @@ void eigrp_prefix_descriptor_add(struct route_table *topology,
 {
        struct route_node *rn;
 
-       rn = route_node_get(topology, pe->destination);
+       rn = route_node_get(topology, &pe->destination);
        if (rn->info) {
                if (IS_DEBUG_EIGRP_EVENT)
-                       zlog_debug(
-                               "%s: %pFX Should we have found this entry in the topo table?",
-                               __func__, pe->destination);
+                       zlog_debug("%s: %pFX Should we have found this entry in the topo table?",
+                                  __func__, &pe->destination);
                route_unlock_node(rn);
        }
 
@@ -147,8 +145,7 @@ void eigrp_route_descriptor_add(struct eigrp *eigrp,
                listnode_add_sort(node->entries, entry);
                entry->prefix = node;
 
-               eigrp_zebra_route_add(eigrp, node->destination,
-                                     l, node->fdistance);
+               eigrp_zebra_route_add(eigrp, &node->destination, l, node->fdistance);
        }
 
        list_delete(&l);
@@ -168,7 +165,7 @@ void eigrp_prefix_descriptor_delete(struct eigrp *eigrp,
        if (!eigrp)
                return;
 
-       rn = route_node_lookup(table, pe->destination);
+       rn = route_node_lookup(table, &pe->destination);
        if (!rn)
                return;
 
@@ -182,8 +179,7 @@ void eigrp_prefix_descriptor_delete(struct eigrp *eigrp,
                eigrp_route_descriptor_delete(eigrp, pe, ne);
        list_delete(&pe->entries);
        list_delete(&pe->rij);
-       eigrp_zebra_route_delete(eigrp, pe->destination);
-       prefix_free(&pe->destination);
+       eigrp_zebra_route_delete(eigrp, &pe->destination);
 
        rn->info = NULL;
        route_unlock_node(rn); // Lookup above
@@ -200,7 +196,7 @@ void eigrp_route_descriptor_delete(struct eigrp *eigrp,
 {
        if (listnode_lookup(node->entries, entry) != NULL) {
                listnode_delete(node->entries, entry);
-               eigrp_zebra_route_delete(eigrp, node->destination);
+               eigrp_zebra_route_delete(eigrp, &node->destination);
                XFREE(MTYPE_EIGRP_ROUTE_DESCRIPTOR, entry);
        }
 }
@@ -462,14 +458,13 @@ void eigrp_update_routing_table(struct eigrp *eigrp,
        successors = eigrp_topology_get_successor_max(prefix, eigrp->max_paths);
 
        if (successors) {
-               eigrp_zebra_route_add(eigrp, prefix->destination, successors,
-                                     prefix->fdistance);
+               eigrp_zebra_route_add(eigrp, &prefix->destination, successors, prefix->fdistance);
                for (ALL_LIST_ELEMENTS_RO(successors, node, entry))
                        entry->flags |= EIGRP_ROUTE_DESCRIPTOR_INTABLE_FLAG;
 
                list_delete(&successors);
        } else {
-               eigrp_zebra_route_delete(eigrp, prefix->destination);
+               eigrp_zebra_route_delete(eigrp, &prefix->destination);
                for (ALL_LIST_ELEMENTS_RO(prefix->entries, node, entry))
                        entry->flags &= ~EIGRP_ROUTE_DESCRIPTOR_INTABLE_FLAG;
        }

diff --git a/eigrpd/eigrp_update.c b/eigrpd/eigrp_update.c
index 6511db2feb2b1a9014ee80f3bfc9e3d73d786fd7..7348231c3b049d2525e7603988edfae4fbf2ee25 100644 (file)
--- a/eigrpd/eigrp_update.c
+++ b/eigrpd/eigrp_update.c
@@ -128,8 +128,7 @@ static void eigrp_update_receive_GR_ask(struct eigrp *eigrp,
 
        /* iterate over all prefixes which weren't advertised by neighbor */
        for (ALL_LIST_ELEMENTS_RO(nbr_prefixes, node1, prefix)) {
-               zlog_debug("GR receive: Neighbor not advertised %pFX",
-                          prefix->destination);
+               zlog_debug("GR receive: Neighbor not advertised %pFX", &prefix->destination);
 
                fsm_msg.metrics = prefix->reported_metric;
                /* set delay to MAX */
@@ -320,9 +319,7 @@ void eigrp_update_receive(struct eigrp *eigrp, struct ip *iph,
                                /*Here comes topology information save*/
                                pe = eigrp_prefix_descriptor_new();
                                pe->serno = eigrp->serno;
-                               pe->destination =
-                                       (struct prefix *)prefix_ipv4_new();
-                               prefix_copy(pe->destination, &dest_addr);
+                               prefix_copy(&pe->destination, &dest_addr);
                                pe->af = AF_INET;
                                pe->state = EIGRP_FSM_STATE_PASSIVE;
                                pe->nt = EIGRP_TOPOLOGY_TYPE_REMOTE;
@@ -566,7 +563,7 @@ void eigrp_update_send_EOT(struct eigrp_neighbor *nbr)
                                }
                        }
                        /* Get destination address from prefix */
-                       dest_addr = pe->destination;
+                       dest_addr = &pe->destination;
 
                        /* Check if any list fits */
                        if (eigrp_update_prefix_apply(
@@ -650,7 +647,7 @@ void eigrp_update_send(struct eigrp_interface *ei)
                        has_tlv = 0;
                }
                /* Get destination address from prefix */
-               dest_addr = pe->destination;
+               dest_addr = &pe->destination;
 
                if (eigrp_update_prefix_apply(eigrp, ei, EIGRP_FILTER_OUT,
                                              dest_addr)) {
@@ -798,7 +795,7 @@ static void eigrp_update_send_GR_part(struct eigrp_neighbor *nbr)
                /*
                 * Filtering
                 */
-               dest_addr = pe->destination;
+               dest_addr = &pe->destination;
 
                if (eigrp_update_prefix_apply(eigrp, ei, EIGRP_FILTER_OUT,
                                              dest_addr)) {

diff --git a/eigrpd/eigrp_zebra.c b/eigrpd/eigrp_zebra.c
index a0eff683dbc9b92b8e75cec0db036c17751f5ccf..5b0c64ffd0981517187be1fb5598427c3cf5b664 100644 (file)
--- a/eigrpd/eigrp_zebra.c
+++ b/eigrpd/eigrp_zebra.c
@@ -105,6 +105,11 @@ void eigrp_zebra_init(void)
        zclient->zebra_connected = eigrp_zebra_connected;
 }
 
+void eigrp_zebra_stop(void)
+{
+       zclient_stop(zclient);
+       zclient_free(zclient);
+}
 
 /* Zebra route add and delete treatment. */
 static int eigrp_zebra_read_route(ZAPI_CALLBACK_ARGS)

diff --git a/eigrpd/eigrp_zebra.h b/eigrpd/eigrp_zebra.h
index 927d562ab2cdce0d818b9e5f39dba47746f4c705..723e71b7f0f5466c26ea08701675c3ce7431e5ca 100644 (file)
--- a/eigrpd/eigrp_zebra.h
+++ b/eigrpd/eigrp_zebra.h
@@ -17,6 +17,7 @@
 #include "vrf.h"
 
 extern void eigrp_zebra_init(void);
+extern void eigrp_zebra_stop(void);
 
 extern void eigrp_zebra_route_add(struct eigrp *eigrp, struct prefix *p,
                                  struct list *successors, uint32_t distance);

diff --git a/eigrpd/eigrpd.c b/eigrpd/eigrpd.c
index 9a4dd6f55ff9322906ab247749140ad4d889bb1a..981965f101b17c063ab8fd65dfd7ad0764585ad4 100644 (file)
--- a/eigrpd/eigrpd.c
+++ b/eigrpd/eigrpd.c
@@ -232,10 +232,15 @@ void eigrp_terminate(void)
 
        SET_FLAG(eigrp_om->options, EIGRP_MASTER_SHUTDOWN);
 
-       frr_each (eigrp_master_hash, &eigrp_om->eigrp, eigrp)
+       while (eigrp_master_hash_count(&eigrp_om->eigrp)) {
+               eigrp = eigrp_master_hash_first(&eigrp_om->eigrp);
                eigrp_finish(eigrp);
+       }
 
        eigrp_master_hash_fini(&eigrp_om->eigrp);
+
+       eigrp_zebra_stop();
+
        frr_fini();
 }
 
@@ -243,16 +248,6 @@ void eigrp_finish(struct eigrp *eigrp)
 {
        eigrp_finish_final(eigrp);
 
-       /* eigrp being shut-down? If so, was this the last eigrp instance? */
-       if (CHECK_FLAG(eigrp_om->options, EIGRP_MASTER_SHUTDOWN) &&
-           (eigrp_master_hash_count(&eigrp_om->eigrp) == 0)) {
-               if (zclient) {
-                       zclient_stop(zclient);
-                       zclient_free(zclient);
-               }
-               exit(0);
-       }
-
        return;
 }
 
@@ -279,6 +274,7 @@ void eigrp_finish_final(struct eigrp *eigrp)
        list_delete(&eigrp->oi_write_q);
 
        eigrp_topology_free(eigrp, eigrp->topology_table);
+       eigrp_network_free(eigrp, eigrp->networks);
 
        eigrp_nbr_delete(eigrp->neighbor_self);
 
@@ -289,6 +285,9 @@ void eigrp_finish_final(struct eigrp *eigrp)
 
        stream_free(eigrp->ibuf);
        distribute_list_delete(&eigrp->distribute_ctx);
+
+       QOBJ_UNREG(eigrp);
+
        XFREE(MTYPE_EIGRP_TOP, eigrp);
 }