On Sat, Apr 16, 2022 at 3:14 AM Chengwen Feng <fengcheng...@huawei.com> wrote:
>
> The 'eth_xstats' should be freed after setup telemetry dictionary. This
> patch fixes it.
>
> Fixes: c190daedb9b1 ("ethdev: add telemetry callbacks")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Chengwen Feng <fengcheng...@huawei.com>
> ---
> lib/ethdev/rte_ethdev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 615383bde2..df20433c2d 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -5585,6 +5585,7 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
> for (i = 0; i < num_xstats; i++)
> rte_tel_data_add_dict_u64(d, xstat_names[i].name,
> eth_xstats[i].value);
> + free(eth_xstats);
> return 0;
> }
>
We need some minimal testing for telemetry commands.
It could be a test automatically calling all available /ethdev/
commands on a running testpmd.
This test could be really simple, not even checking what is returned.
It would just try every command sequentially with no parameter first,
then with port 0 and finally with port 1.
Coupled with ASan in the CI, this current issue would have been caught.
For example, I tested manually with testpmd + net/null ports:
==3825787==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 1040 byte(s) in 1 object(s) allocated from:
#0 0x7f7048a2d91f in __interceptor_malloc (/usr/lib64/libasan.so.6+0xae91f)
#1 0x32859e9 in eth_dev_handle_port_xstats
(/home/dmarchan/dpdk/build-gcc-asan/app/dpdk-testpmd+0x32859e9)
#2 0x3346ac9 in perform_command
(/home/dmarchan/dpdk/build-gcc-asan/app/dpdk-testpmd+0x3346ac9)
#3 0x3347a8e in client_handler
(/home/dmarchan/dpdk/build-gcc-asan/app/dpdk-testpmd+0x3347a8e)
#4 0x7f7045751b19 in start_thread
/usr/src/debug/glibc-2.34-25.fc35.x86_64/nptl/pthread_create.c:443
Opinions?
--
David Marchand
