diff mbox series

[v3] app/testpmd: fix issue with memory leaks when quit testpmd

Message ID 20220325083555.422962-1-ke1x.zhang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [v3] app/testpmd: fix issue with memory leaks when quit testpmd |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Functional fail Functional Testing issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS

Commit Message

Zhang, Ke1X March 25, 2022, 8:35 a.m. UTC 
  A multicast address pool is allocated for a port when
using mcast_addr testpmd commands.

When closing a port or stopping testpmd, this pool was
not freed, resulting in a leak.This issue has been caught
using ASan.

Free this pool when closing the port.

Error info as following:
ERROR: LeakSanitizer: detected memory leaksDirect leak of
       192 byte(s)
0 0x7f6a2e0aeffe in __interceptor_realloc
	(/lib/x86_64-linux-gnu/libasan.so.5+0x10dffe)
1 0x565361eb340f in mcast_addr_pool_extend
	../app/test-pmd/config.c:5162
2 0x565361eb3556 in mcast_addr_pool_append
	../app/test-pmd/config.c:5180
3 0x565361eb3aae in mcast_addr_add
	../app/test-pmd/config.c:5243

Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
---
 app/test-pmd/config.c  | 19 +++++++++++++++++++
 app/test-pmd/testpmd.c |  1 +
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 21 insertions(+)

Comments

Zhang, Yuying April 4, 2022, 3:34 p.m. UTC | #1 
Hi Ke,

> -----Original Message-----
> From: Zhang, Ke1X <ke1x.zhang@intel.com>
> Sent: Friday, March 25, 2022 4:36 PM
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Singh, Aman Deep
> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>;
> dev@dpdk.org
> Cc: Zhang, Ke1X <ke1x.zhang@intel.com>
> Subject: [PATCH v3] app/testpmd: fix issue with memory leaks when quit
> testpmd
> 
> A multicast address pool is allocated for a port when using mcast_addr
> testpmd commands.
> 
> When closing a port or stopping testpmd, this pool was not freed, resulting in
> a leak.This issue has been caught using ASan.
> 
> Free this pool when closing the port.
> 
> Error info as following:
> ERROR: LeakSanitizer: detected memory leaksDirect leak of
>        192 byte(s)
> 0 0x7f6a2e0aeffe in __interceptor_realloc
> 	(/lib/x86_64-linux-gnu/libasan.so.5+0x10dffe)
> 1 0x565361eb340f in mcast_addr_pool_extend
> 	../app/test-pmd/config.c:5162
> 2 0x565361eb3556 in mcast_addr_pool_append
> 	../app/test-pmd/config.c:5180
> 3 0x565361eb3aae in mcast_addr_add
> 	../app/test-pmd/config.c:5243
> 
> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>

Acked-by: Yuying Zhang <yuying.zhang@intel.com>

> ---
>  app/test-pmd/config.c  | 19 +++++++++++++++++++  app/test-
> pmd/testpmd.c |  1 +  app/test-pmd/testpmd.h |  1 +
>  3 files changed, 21 insertions(+)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> cc8e7aa138..a6fa9be3ef 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -5978,6 +5978,25 @@ mcast_addr_pool_remove(struct rte_port *port,
> uint32_t addr_idx)
>  		sizeof(struct rte_ether_addr) * (port->mc_addr_nb -
> addr_idx));  }
> 
> +int
> +mcast_addr_pool_destroy(portid_t port_id) {
> +	struct rte_port *port;
> +
> +	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
> +	    port_id == (portid_t)RTE_PORT_ALL)
> +		return -EINVAL;
> +	port = &ports[port_id];
> +
> +	if (port->mc_addr_nb != 0) {
> +		/* free the pool of multicast addresses. */
> +		free(port->mc_addr_pool);
> +		port->mc_addr_pool = NULL;
> +		port->mc_addr_nb = 0;
> +	}
> +	return 0;
> +}
> +
>  static int
>  eth_port_multicast_addr_list_set(portid_t port_id)  { diff --git a/app/test-
> pmd/testpmd.c b/app/test-pmd/testpmd.c index fe2ce19f99..1861a02c2f
> 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3137,6 +3137,7 @@ close_port(portid_t pid)
>  		}
> 
>  		if (is_proc_primary()) {
> +			mcast_addr_pool_destroy(pi);
>  			port_flow_flush(pi);
>  			port_flex_item_flush(pi);
>  			rte_eth_dev_close(pi);
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 31f766c965..daa3c08317 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -965,6 +965,7 @@ int port_flow_create(portid_t port_id,  int
> port_action_handle_query(portid_t port_id, uint32_t id);  void
> update_age_action_context(const struct rte_flow_action *actions,
>  		     struct port_flow *pf);
> +int mcast_addr_pool_destroy(portid_t port_id);
>  int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);  int
> port_flow_flush(portid_t port_id);  int port_flow_dump(portid_t port_id,
> bool dump_all,
> --
> 2.25.1
Ferruh Yigit June 8, 2022, 12:06 p.m. UTC | #2 
On 4/4/2022 4:34 PM, Zhang, Yuying wrote:
> Hi Ke,
> 
>> -----Original Message-----
>> From: Zhang, Ke1X <ke1x.zhang@intel.com>
>> Sent: Friday, March 25, 2022 4:36 PM
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Singh, Aman Deep
>> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>;
>> dev@dpdk.org
>> Cc: Zhang, Ke1X <ke1x.zhang@intel.com>
>> Subject: [PATCH v3] app/testpmd: fix issue with memory leaks when quit
>> testpmd
>>
>> A multicast address pool is allocated for a port when using mcast_addr
>> testpmd commands.
>>
>> When closing a port or stopping testpmd, this pool was not freed, resulting in
>> a leak.This issue has been caught using ASan.
>>
>> Free this pool when closing the port.
>>
>> Error info as following:
>> ERROR: LeakSanitizer: detected memory leaksDirect leak of
>>         192 byte(s)
>> 0 0x7f6a2e0aeffe in __interceptor_realloc
>> 	(/lib/x86_64-linux-gnu/libasan.so.5+0x10dffe)
>> 1 0x565361eb340f in mcast_addr_pool_extend
>> 	../app/test-pmd/config.c:5162
>> 2 0x565361eb3556 in mcast_addr_pool_append
>> 	../app/test-pmd/config.c:5180
>> 3 0x565361eb3aae in mcast_addr_add
>> 	../app/test-pmd/config.c:5243
>>
>> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> 
> Acked-by: Yuying Zhang <yuying.zhang@intel.com>
> 

     Fixes: 8fff667578a7 ("app/testpmd: new command to add/remove 
multicast MAC addresses")
     Cc: stable@dpdk.org

Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com>

Applied to dpdk-next-net/main, thanks.


<...>

>> pmd/testpmd.c b/app/test-pmd/testpmd.c index fe2ce19f99..1861a02c2f
>> 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -3137,6 +3137,7 @@ close_port(portid_t pid)
>>   		}
>>
>>   		if (is_proc_primary()) {
>> +			mcast_addr_pool_destroy(pi);
>>   			port_flow_flush(pi);
>>   			port_flex_item_flush(pi);
>>   			rte_eth_dev_close(pi);

'rte_eth_dev_close()' may fail, like if the port is not stopped, in that 
case mcast pool will be freed although port is not closed.
This can be prevented by saving existing mcast poll and restore it if 
close failed, but I think it doesn't worth the complexity it brings, so 
OK to continue as it is.
diff mbox series

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index cc8e7aa138..a6fa9be3ef 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -5978,6 +5978,25 @@  mcast_addr_pool_remove(struct rte_port *port, uint32_t addr_idx)
 		sizeof(struct rte_ether_addr) * (port->mc_addr_nb - addr_idx));
 }
 
+int
+mcast_addr_pool_destroy(portid_t port_id)
+{
+	struct rte_port *port;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+	port = &ports[port_id];
+
+	if (port->mc_addr_nb != 0) {
+		/* free the pool of multicast addresses. */
+		free(port->mc_addr_pool);
+		port->mc_addr_pool = NULL;
+		port->mc_addr_nb = 0;
+	}
+	return 0;
+}
+
 static int
 eth_port_multicast_addr_list_set(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index fe2ce19f99..1861a02c2f 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3137,6 +3137,7 @@  close_port(portid_t pid)
 		}
 
 		if (is_proc_primary()) {
+			mcast_addr_pool_destroy(pi);
 			port_flow_flush(pi);
 			port_flex_item_flush(pi);
 			rte_eth_dev_close(pi);
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 31f766c965..daa3c08317 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -965,6 +965,7 @@  int port_flow_create(portid_t port_id,
 int port_action_handle_query(portid_t port_id, uint32_t id);
 void update_age_action_context(const struct rte_flow_action *actions,
 		     struct port_flow *pf);
+int mcast_addr_pool_destroy(portid_t port_id);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, bool dump_all,