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

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

[Ke Zhang]

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(+)

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

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

[Ferruh Yigit]

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.

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

[Zhang, Yuying]

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

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

[Ke Zhang]

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(+)

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