From efb2aeae7b0d565f919bcd77345b78a9bd91e297 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 24 Mar 2025 08:07:02 -0400 Subject: [PATCH] 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 --- eigrpd/eigrp_dump.c | 2 +- eigrpd/eigrp_fsm.c | 10 ++++------ eigrpd/eigrp_interface.c | 3 +-- eigrpd/eigrp_network.c | 15 +++++++++++++++ eigrpd/eigrp_network.h | 1 + eigrpd/eigrp_packet.c | 22 +++++++++++----------- eigrpd/eigrp_reply.c | 3 +-- eigrpd/eigrp_structs.h | 2 +- eigrpd/eigrp_topology.c | 23 +++++++++-------------- eigrpd/eigrp_update.c | 13 +++++-------- eigrpd/eigrp_zebra.c | 5 +++++ eigrpd/eigrp_zebra.h | 1 + eigrpd/eigrpd.c | 21 ++++++++++----------- 13 files changed, 65 insertions(+), 56 deletions(-) diff --git a/eigrpd/eigrp_dump.c b/eigrpd/eigrp_dump.c index 231554a098..d59adc8234 100644 --- 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 6d8061e572..99b090eeb6 100644 --- 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 2e3beacae7..73caa72b6d 100644 --- 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 910215cbcb..fc334cf97d 100644 --- 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 ac5c47f6f9..1baf26b0c0 100644 --- 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 189d9330cf..7560514cec 100644 --- 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 aae89e832b..a444f3a5a9 100644 --- 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 93bcf07885..c24067bc33 100644 --- 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 f17be8f4b7..e59dcacb7c 100644 --- 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 6511db2feb..7348231c3b 100644 --- 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 a0eff683db..5b0c64ffd0 100644 --- 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 927d562ab2..723e71b7f0 100644 --- 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 9a4dd6f55f..981965f101 100644 --- 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); } -- 2.39.5