ligang/vpp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

stats: fix memory leakage when adding / deleting interfaces

Browse Source 
This fixes two leaks in registering errors in the stats segment.
- The error name created by vlib_register_errors() was not freed.
- Duplicate error names (when interface readded) was added to the vector.

This fix also adds memory usage statistics for the statistics segment
as /mem/statseg/{used, total}

Change-Id: Ife98d5fc5baef5bdae426a5a1eef428af2b9ab8a
Type: fix
Signed-off-by: Ole Troan <ot@cisco.com>
This commit is contained in:
Ole Troan
2019-06-16 12:33:51 +02:00
committed by Neale Ranns
parent ae8819f0a4
commit 92e3082199
7 changed files with 94 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/vlib/error.c
View File

@@ -166,7 +166,7 @@ vlib_register_errors (vlib_main_t * vm,
{
error_name = format (0, "/err/%v/%s%c", n->name, error_strings[i], 0);
/* Note: error_name consumed by the following call */
vlib_stats_register_error_index (error_name, em->counters,
vlib_stats_register_error_index (oldheap, error_name, em->counters,
n->error_heap_index + i);
}
}

4
src/vlib/stat_weak_inlines.h
View File

@@ -32,10 +32,10 @@ void
vlib_stats_pop_heap (void *notused, void *notused2, u32 i, int type)
{
};
void vlib_stats_register_error_index (u8 *, u64 *, u64)
void vlib_stats_register_error_index (void *, u8 *, u64 *, u64)
__attribute__ ((weak));
void
vlib_stats_register_error_index (u8 * notused, u64 * notused2, u64 notused3)
vlib_stats_register_error_index (void * notused, u8 * notused2, u64 * notused3, u64 notused4)
{
};

68
src/vpp/stats/stat_segment.c
View File

@@ -22,6 +22,7 @@
#undef HAVE_MEMFD_CREATE
#include <vppinfra/linux/syscall.h>
#include <vpp-api/client/stat_client.h>
#include <vppinfra/mheap.h>
stat_segment_main_t stat_segment_main;
@@ -58,14 +59,16 @@ vlib_stats_push_heap (void *old)
return clib_mem_set_heap (sm->heap);
}
/* Name to vector index hash */
static u32
lookup_or_create_hash_index (void *oldheap, char *name, u32 next_vector_index)
lookup_or_create_hash_index (u8 * name, u32 next_vector_index)
{
stat_segment_main_t *sm = &stat_segment_main;
u32 index;
hash_pair_t *hp;
/* Must be called in the context of the main heap */
ASSERT (clib_mem_get_heap != sm->heap);
hp = hash_get_pair (sm->directory_vector_by_name, name);
if (!hp)
{
@@ -106,7 +109,7 @@ vlib_stats_pop_heap (void *cm_arg, void *oldheap, u32 cindex,
cm->stat_segment_name ? cm->stat_segment_name : cm->name;
u32 next_vector_index = vec_len (sm->directory_vector);
clib_mem_set_heap (oldheap); /* Exit stats segment */
u32 vector_index = lookup_or_create_hash_index (oldheap, stat_segment_name,
u32 vector_index = lookup_or_create_hash_index ((u8 *) stat_segment_name,
next_vector_index);
/* Back to stats segment */
clib_mem_set_heap (sm->heap); /* Re-enter stat segment */
@@ -153,7 +156,8 @@ vlib_stats_pop_heap (void *cm_arg, void *oldheap, u32 cindex,
}
void
vlib_stats_register_error_index (u8 * name, u64 * em_vec, u64 index)
vlib_stats_register_error_index (void *oldheap, u8 * name, u64 * em_vec,
u64 index)
{
stat_segment_main_t *sm = &stat_segment_main;
stat_segment_shared_header_t *shared_header = sm->shared_header;
@@ -162,17 +166,32 @@ vlib_stats_register_error_index (u8 * name, u64 * em_vec, u64 index)
ASSERT (shared_header);
vlib_stat_segment_lock ();
u32 next_vector_index = vec_len (sm->directory_vector);
clib_mem_set_heap (oldheap); /* Exit stats segment */
memcpy (e.name, name, vec_len (name));
e.name[vec_len (name)] = '\0';
e.type = STAT_DIR_TYPE_ERROR_INDEX;
e.offset = index;
e.offset_vector = 0;
vec_add1 (sm->directory_vector, e);
u32 vector_index = lookup_or_create_hash_index (name,
next_vector_index);
/* Warn clients to refresh any pointers they might be holding */
shared_header->directory_offset =
stat_segment_offset (shared_header, sm->directory_vector);
/* Back to stats segment */
clib_mem_set_heap (sm->heap); /* Re-enter stat segment */
if (next_vector_index == vector_index)
{
memcpy (e.name, name, vec_len (name));
e.name[vec_len (name)] = '\0';
e.type = STAT_DIR_TYPE_ERROR_INDEX;
e.offset = index;
e.offset_vector = 0;
vec_add1 (sm->directory_vector, e);
/* Warn clients to refresh any pointers they might be holding */
shared_header->directory_offset =
stat_segment_offset (shared_header, sm->directory_vector);
}
else
{
vec_free (name);
}
vlib_stat_segment_unlock ();
}
@@ -298,6 +317,12 @@ vlib_map_stat_segment_init (void)
clib_mem_set_heap (oldheap);
/* Total shared memory size */
clib_mem_usage_t usage;
mheap_usage (sm->heap, &usage);
sm->directory_vector[STAT_COUNTER_MEM_STATSEG_TOTAL].value =
usage.bytes_total;
return 0;
}
@@ -581,6 +606,12 @@ do_stat_segment_updates (stat_segment_main_t * sm)
sm->directory_vector[STAT_COUNTER_LAST_STATS_CLEAR].value =
vm->node_main.time_last_runtime_stats_clear;
/* Stats segment memory heap counter */
clib_mem_usage_t usage;
mheap_usage (sm->heap, &usage);
sm->directory_vector[STAT_COUNTER_MEM_STATSEG_USED].value =
usage.bytes_used;
if (sm->node_counters_enabled)
update_node_counters (sm);
@@ -707,11 +738,17 @@ stat_segment_register_gauge (u8 * name, stat_segment_update_fn update_fn,
stat_segment_shared_header_t *shared_header = sm->shared_header;
void *oldheap;
stat_segment_directory_entry_t e;
u32 index;
stat_segment_gauges_pool_t *gauge;
ASSERT (shared_header);
u32 next_vector_index = vec_len (sm->directory_vector);
u32 vector_index = lookup_or_create_hash_index (name,
next_vector_index);
if (vector_index < next_vector_index) /* Already registered */
return clib_error_return (0, "%v is alreadty registered", name);
oldheap = vlib_stats_push_heap (NULL);
vlib_stat_segment_lock ();
@@ -719,7 +756,6 @@ stat_segment_register_gauge (u8 * name, stat_segment_update_fn update_fn,
e.type = STAT_DIR_TYPE_SCALAR_INDEX;
memcpy (e.name, name, vec_len (name));
index = vec_len (sm->directory_vector);
vec_add1 (sm->directory_vector, e);
shared_header->directory_offset =
@@ -732,7 +768,7 @@ stat_segment_register_gauge (u8 * name, stat_segment_update_fn update_fn,
pool_get (sm->gauges, gauge);
gauge->fn = update_fn;
gauge->caller_index = caller_index;
gauge->directory_index = index;
gauge->directory_index = next_vector_index;
return NULL;
}

6
src/vpp/stats/stat_segment.h
View File

@@ -36,6 +36,8 @@ typedef enum
STAT_COUNTER_NODE_SUSPENDS,
STAT_COUNTER_INTERFACE_NAMES,
STAT_COUNTER_NODE_NAMES,
STAT_COUNTER_MEM_STATSEG_TOTAL,
STAT_COUNTER_MEM_STATSEG_USED,
STAT_COUNTERS
} stat_segment_counter_t;
@@ -53,7 +55,9 @@ typedef enum
_(NODE_CALLS, COUNTER_VECTOR_SIMPLE, calls, /sys/node) \
_(NODE_SUSPENDS, COUNTER_VECTOR_SIMPLE, suspends, /sys/node) \
_(INTERFACE_NAMES, NAME_VECTOR, names, /if) \
_(NODE_NAMES, NAME_VECTOR, names, /sys/node)
_(NODE_NAMES, NAME_VECTOR, names, /sys/node) \
_(MEM_STATSEG_TOTAL, SCALAR_INDEX, total, /mem/statseg) \
_(MEM_STATSEG_USED, SCALAR_INDEX, used, /mem/statseg)
typedef struct
{

15
src/vppinfra/mem_dlmalloc.c
View File

@@ -383,6 +383,21 @@ clib_mem_usage (clib_mem_usage_t * u)
clib_warning ("unimp");
}
void
mheap_usage (void *heap, clib_mem_usage_t * usage)
{
struct dlmallinfo mi = mspace_mallinfo (heap);
/* TODO: Fill in some more values */
usage->object_count = 0;
usage->bytes_total = mi.arena;
usage->bytes_overhead = 0;
usage->bytes_max = 0;
usage->bytes_used = mi.uordblks;
usage->bytes_free = mi.fordblks;
usage->bytes_free_reclaimed = 0;
}
/* Call serial number for debugger breakpoints. */
uword clib_mem_validate_serial = 0;

1
src/vppinfra/mheap.h
View File

@@ -90,6 +90,7 @@ int test_mheap_main (unformat_input_t * input);
/* Format mheap data structures as string. */
u8 *format_mheap (u8 * s, va_list * va);
void *mheap_alloc_with_lock (void *memory, uword size, int locked);
void mheap_usage (void *v, clib_mem_usage_t * usage);
#endif /* USE_DLMALLOC */

19
test/test_stats_client.py
View File

@@ -1,7 +1,7 @@
#!/usr/bin/env python2.7
import unittest
import time
import psutil
from vpp_papi.vpp_stats import VPPStats
@@ -42,6 +42,23 @@ class StatsClientTestCase(VppTestCase):
"is not equal to "
"ending client side file descriptor count: %s" % (
initial_fds, ending_fds))
@unittest.skip("Manual only")
def test_mem_leak(self):
def loop():
print('Running loop')
for i in range(50):
rv = self.vapi.papi.tap_create_v2(id=i, use_random_mac=1)
self.assertEqual(rv.retval, 0)
rv = self.vapi.papi.tap_delete_v2(sw_if_index=rv.sw_if_index)
self.assertEqual(rv.retval, 0)
before = self.statistics.get_counter('/mem/statseg/used')
loop()
self.vapi.cli("memory-trace on stats-segment")
for j in range(100):
loop()
print(self.vapi.cli("show memory stats-segment verbose"))
print('AFTER', before, self.statistics.get_counter('/mem/statseg/used'))
if __name__ == '__main__':
unittest.main(testRunner=VppTestRunner)