ospf6d: Fix memory leaks

Free route node upon asbr redistribute route cleanup from
external_id_table route tale.
Free route node when route_remove is called and
node->info is set to null.
Decrement route node lock in route_lookup api as it
is incremented as part of node_lookup api.
use local variable for nexthop vs. malloc in zebra parse
routine.

two of the memory leaks related to nexthops per route were not freed.
two of the memory leak detected per frr service restart

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
This commit is contained in:
Chirag Shah 2017-07-13 11:39:22 -07:00
parent 0d510865ec
commit d107621dbc
9 changed files with 48 additions and 40 deletions

View File

@ -622,7 +622,8 @@ void ospf6_asbr_redistribute_remove(int type, ifindex_t ifindex,
node = route_node_lookup(ospf6->external_id_table, &prefix_id);
assert(node);
node->info = NULL;
route_unlock_node(node);
route_unlock_node(node); /* to free the lookup lock */
route_unlock_node(node); /* to free the original lock */
ospf6_route_remove(match, ospf6->external_table);
XFREE(MTYPE_OSPF6_EXTERNAL_INFO, info);

View File

@ -509,7 +509,8 @@ struct ospf6_lsa *ospf6_lsa_create(struct ospf6_lsa_header *header)
/* allocate memory for this LSA */
new_header =
(struct ospf6_lsa_header *)XMALLOC(MTYPE_OSPF6_LSA, lsa_size);
(struct ospf6_lsa_header *)XMALLOC(MTYPE_OSPF6_LSA_HEADER,
lsa_size);
/* copy LSA from original header */
memcpy(new_header, header, lsa_size);
@ -537,7 +538,7 @@ struct ospf6_lsa *ospf6_lsa_create_headeronly(struct ospf6_lsa_header *header)
/* allocate memory for this LSA */
new_header = (struct ospf6_lsa_header *)XMALLOC(
MTYPE_OSPF6_LSA, sizeof(struct ospf6_lsa_header));
MTYPE_OSPF6_LSA_HEADER, sizeof(struct ospf6_lsa_header));
/* copy LSA from original header */
memcpy(new_header, header, sizeof(struct ospf6_lsa_header));
@ -568,7 +569,7 @@ void ospf6_lsa_delete(struct ospf6_lsa *lsa)
THREAD_OFF(lsa->refresh);
/* do free */
XFREE(MTYPE_OSPF6_LSA, lsa->header);
XFREE(MTYPE_OSPF6_LSA_HEADER, lsa->header);
XFREE(MTYPE_OSPF6_LSA, lsa);
}

View File

@ -139,6 +139,8 @@ void ospf6_lsdb_add(struct ospf6_lsa *lsa, struct ospf6_lsdb *lsdb)
(*lsdb->hook_add)(lsa);
}
}
/* to free the lookup lock in node get*/
route_unlock_node(current);
ospf6_lsa_unlock(old);
}

View File

@ -34,6 +34,7 @@ DEFINE_MTYPE(OSPF6D, OSPF6_ROUTE, "OSPF6 route")
DEFINE_MTYPE(OSPF6D, OSPF6_PREFIX, "OSPF6 prefix")
DEFINE_MTYPE(OSPF6D, OSPF6_MESSAGE, "OSPF6 message")
DEFINE_MTYPE(OSPF6D, OSPF6_LSA, "OSPF6 LSA")
DEFINE_MTYPE(OSPF6D, OSPF6_LSA_HEADER, "OSPF6 LSA header")
DEFINE_MTYPE(OSPF6D, OSPF6_LSA_SUMMARY, "OSPF6 LSA summary")
DEFINE_MTYPE(OSPF6D, OSPF6_LSDB, "OSPF6 LSA database")
DEFINE_MTYPE(OSPF6D, OSPF6_VERTEX, "OSPF6 vertex")

View File

@ -33,6 +33,7 @@ DECLARE_MTYPE(OSPF6_ROUTE)
DECLARE_MTYPE(OSPF6_PREFIX)
DECLARE_MTYPE(OSPF6_MESSAGE)
DECLARE_MTYPE(OSPF6_LSA)
DECLARE_MTYPE(OSPF6_LSA_HEADER)
DECLARE_MTYPE(OSPF6_LSA_SUMMARY)
DECLARE_MTYPE(OSPF6_LSDB)
DECLARE_MTYPE(OSPF6_VERTEX)

View File

@ -178,17 +178,6 @@ void ospf6_nexthop_delete(struct ospf6_nexthop *nh)
XFREE(MTYPE_OSPF6_NEXTHOP, nh);
}
void ospf6_free_nexthops(struct list *nh_list)
{
struct ospf6_nexthop *nh;
struct listnode *node, *nnode;
if (nh_list) {
for (ALL_LIST_ELEMENTS(nh_list, node, nnode, nh))
ospf6_nexthop_delete(nh);
}
}
void ospf6_clear_nexthops(struct list *nh_list)
{
struct listnode *node;
@ -340,19 +329,29 @@ int ospf6_route_get_first_nh_index(struct ospf6_route *route)
return (-1);
}
static int ospf6_nexthop_cmp(struct ospf6_nexthop *a, struct ospf6_nexthop *b)
{
if ((a)->ifindex == (b)->ifindex &&
IN6_ARE_ADDR_EQUAL(&(a)->address, &(b)->address))
return 1;
return 0;
}
struct ospf6_route *ospf6_route_create(void)
{
struct ospf6_route *route;
route = XCALLOC(MTYPE_OSPF6_ROUTE, sizeof(struct ospf6_route));
route->nh_list = list_new();
route->nh_list->cmp = (int (*)(void *, void *))ospf6_nexthop_cmp;
route->nh_list->del = (void (*) (void *))ospf6_nexthop_delete;
return route;
}
void ospf6_route_delete(struct ospf6_route *route)
{
if (route) {
ospf6_free_nexthops(route->nh_list);
list_free(route->nh_list);
if (route->nh_list)
list_free(route->nh_list);
XFREE(MTYPE_OSPF6_ROUTE, route);
}
}
@ -439,6 +438,7 @@ struct ospf6_route *ospf6_route_lookup(struct prefix *prefix,
return NULL;
route = (struct ospf6_route *)node->info;
route_unlock_node(node); /* to free the lookup lock */
return route;
}
@ -583,6 +583,8 @@ struct ospf6_route *ospf6_route_add(struct ospf6_route *route,
SET_FLAG(old->flag, OSPF6_ROUTE_ADD);
ospf6_route_table_assert(table);
/* to free the lookup lock */
route_unlock_node(node);
return old;
}
@ -628,9 +630,10 @@ struct ospf6_route *ospf6_route_add(struct ospf6_route *route,
if (prev || next) {
if (IS_OSPF6_DEBUG_ROUTE(MEMORY))
zlog_debug(
"%s %p: route add %p: another path: prev %p, next %p",
"%s %p: route add %p: another path: prev %p, next %p node lock %u",
ospf6_route_table_name(table), (void *)table,
(void *)route, (void *)prev, (void *)next);
(void *)route, (void *)prev, (void *)next,
node->lock);
else if (IS_OSPF6_DEBUG_ROUTE(TABLE))
zlog_debug("%s: route add: another path found",
ospf6_route_table_name(table));
@ -755,9 +758,9 @@ void ospf6_route_remove(struct ospf6_route *route,
prefix2str(&route->prefix, buf, sizeof(buf));
if (IS_OSPF6_DEBUG_ROUTE(MEMORY))
zlog_debug("%s %p: route remove %p: %s",
zlog_debug("%s %p: route remove %p: %s rnode lock %u",
ospf6_route_table_name(table), (void *)table,
(void *)route, buf);
(void *)route, buf, route->rnode->lock);
else if (IS_OSPF6_DEBUG_ROUTE(TABLE))
zlog_debug("%s: route remove: %s",
ospf6_route_table_name(table), buf);
@ -768,11 +771,9 @@ void ospf6_route_remove(struct ospf6_route *route,
/* find the route to remove, making sure that the route pointer
is from the route table. */
current = node->info;
while (current && ospf6_route_is_same(current, route)) {
if (current == route)
break;
while (current && current != route)
current = current->next;
}
assert(current == route);
/* adjust doubly linked list */
@ -785,10 +786,14 @@ void ospf6_route_remove(struct ospf6_route *route,
if (route->next && route->next->rnode == node) {
node->info = route->next;
SET_FLAG(route->next->flag, OSPF6_ROUTE_BEST);
} else
node->info = NULL; /* should unlock route_node here ? */
} else {
node->info = NULL;
route->rnode = NULL;
route_unlock_node(node); /* to free the original lock */
}
}
route_unlock_node(node); /* to free the lookup lock */
table->count--;
ospf6_route_table_assert(table);
@ -935,6 +940,7 @@ struct ospf6_route_table *ospf6_route_table_create(int s, int t)
void ospf6_route_table_delete(struct ospf6_route_table *table)
{
ospf6_route_remove_all(table);
bf_free(table->idspace);
route_table_finish(table->table);
XFREE(MTYPE_OSPF6_ROUTE, table);
}
@ -1062,6 +1068,7 @@ void ospf6_route_show_detail(struct vty *vty, struct ospf6_route *route)
vty_out(vty, "Metric: %d (%d)\n", route->path.cost,
route->path.u.cost_e2);
vty_out(vty, "Nexthop count: %u\n", route->nh_list->count);
/* Nexthops */
vty_out(vty, "Nexthop:\n");
for (ALL_LIST_ELEMENTS_RO(route->nh_list, node, nh)) {

View File

@ -256,7 +256,6 @@ extern void ospf6_linkstate_prefix2str(struct prefix *prefix, char *buf,
extern struct ospf6_nexthop *ospf6_nexthop_create(void);
extern void ospf6_nexthop_delete(struct ospf6_nexthop *nh);
extern void ospf6_free_nexthops(struct list *nh_list);
extern void ospf6_clear_nexthops(struct list *nh_list);
extern int ospf6_num_nexthops(struct list *nh_list);
extern void ospf6_copy_nexthops(struct list *dst, struct list *src);

View File

@ -141,6 +141,7 @@ static struct ospf6_vertex *ospf6_vertex_create(struct ospf6_lsa *lsa)
v->options[2] = *(u_char *)(OSPF6_LSA_HEADER_END(lsa->header) + 3);
v->nh_list = list_new();
v->nh_list->del = (void (*) (void *))ospf6_nexthop_delete;
v->parent = NULL;
v->child_list = list_new();

View File

@ -214,14 +214,14 @@ static int ospf6_zebra_read_ipv6(int command, struct zclient *zclient,
struct zapi_ipv6 api;
unsigned long ifindex;
struct prefix p, src_p;
struct in6_addr *nexthop;
struct in6_addr nexthop;
if (ospf6 == NULL)
return 0;
s = zclient->ibuf;
ifindex = 0;
nexthop = NULL;
memset(&nexthop, 0, sizeof(struct in6_addr));
memset(&api, 0, sizeof(api));
/* Type, flags, message. */
@ -250,10 +250,7 @@ static int ospf6_zebra_read_ipv6(int command, struct zclient *zclient,
/* Nexthop, ifindex, distance, metric. */
if (CHECK_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP)) {
api.nexthop_num = stream_getc(s);
nexthop = (struct in6_addr *)malloc(api.nexthop_num
* sizeof(struct in6_addr));
stream_get(nexthop, s,
api.nexthop_num * sizeof(struct in6_addr));
stream_get(&nexthop, s, IPV6_MAX_BYTELEN);
}
if (CHECK_FLAG(api.message, ZAPI_MESSAGE_IFINDEX)) {
api.ifindex_num = stream_getc(s);
@ -276,9 +273,9 @@ static int ospf6_zebra_read_ipv6(int command, struct zclient *zclient,
if (IS_OSPF6_DEBUG_ZEBRA(RECV)) {
char prefixstr[PREFIX2STR_BUFFER], nexthopstr[128];
prefix2str((struct prefix *)&p, prefixstr, sizeof(prefixstr));
if (nexthop)
inet_ntop(AF_INET6, nexthop, nexthopstr,
sizeof(nexthopstr));
if (api.nexthop_num)
inet_ntop(AF_INET6, &nexthop, nexthopstr,
sizeof(nexthopstr));
else
snprintf(nexthopstr, sizeof(nexthopstr), "::");
@ -292,12 +289,10 @@ static int ospf6_zebra_read_ipv6(int command, struct zclient *zclient,
if (command == ZEBRA_REDISTRIBUTE_IPV6_ADD)
ospf6_asbr_redistribute_add(api.type, ifindex, &p,
api.nexthop_num, nexthop, api.tag);
api.nexthop_num, &nexthop, api.tag);
else
ospf6_asbr_redistribute_remove(api.type, ifindex, &p);
if (CHECK_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP))
free(nexthop);
return 0;
}