DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] app/testpmd: fix issue with memory leaks when quit testpmd
@ 2022-03-01  2:06 Ke Zhang
  2022-03-04 16:43 ` Ferruh Yigit
  2022-03-14  5:52 ` [PATCH v2] " Ke Zhang
  0 siblings, 2 replies; 13+ messages in thread
From: Ke Zhang @ 2022-03-01  2:06 UTC (permalink / raw)
  To: xiaoyun.li, aman.deep.singh, yuying.zhang, dev; +Cc: Ke Zhang

when dpdk is compiled in ASan, there is a memory leaks after
quit testpmd if set mcast_addr, this patch fix this issue.

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/testpmd.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index fe2ce19f99..fa7f80fdf7 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2742,6 +2742,8 @@ start_port(portid_t pid)
 			continue;
 		}
 
+		port->mc_addr_pool = NULL;
+
 		if (port->need_reconfig > 0) {
 			struct rte_eth_conf dev_conf;
 			int k;
@@ -3065,6 +3067,16 @@ stop_port(portid_t pid)
 		if (eth_dev_stop_mp(pi) != 0)
 			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
 				pi);
+		/*
+		 * free the pool of multicast addresses. If it is NULL,
+		 * it means there is no mc addr.Make sure the mc_addr_pool
+		 * is NULL at port init.
+		 */
+		if (port->mc_addr_pool != NULL) {
+			free(port->mc_addr_pool);
+			port->mc_addr_pool = NULL;
+		}
+		port->mc_addr_nb = 0;
 
 		if (port->port_status == RTE_PORT_HANDLING)
 			port->port_status = RTE_PORT_STOPPED;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] app/testpmd: fix issue with memory leaks when quit testpmd
  2022-03-01  2:06 [PATCH] app/testpmd: fix issue with memory leaks when quit testpmd Ke Zhang
@ 2022-03-04 16:43 ` Ferruh Yigit
  2022-03-08  6:05   ` Zhang, Ke1X
  2022-03-14  5:52 ` [PATCH v2] " Ke Zhang
  1 sibling, 1 reply; 13+ messages in thread
From: Ferruh Yigit @ 2022-03-04 16:43 UTC (permalink / raw)
  To: Ke Zhang, xiaoyun.li, aman.deep.singh, yuying.zhang, dev

On 3/1/2022 2:06 AM, Ke Zhang wrote:
> when dpdk is compiled in ASan, there is a memory leaks after
> quit testpmd if set mcast_addr, this patch fix this issue.
> 
> 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/testpmd.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index fe2ce19f99..fa7f80fdf7 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2742,6 +2742,8 @@ start_port(portid_t pid)
>   			continue;
>   		}
>   
> +		port->mc_addr_pool = NULL;
> +

User can do consecutive start / stop / start / ..

Won't user expect the previous allocated and set MAC address
to be valid after this?

What about to free the 'port->mc_addr_pool' in close()
instead of stop()?

>   		if (port->need_reconfig > 0) {
>   			struct rte_eth_conf dev_conf;
>   			int k;
> @@ -3065,6 +3067,16 @@ stop_port(portid_t pid)
>   		if (eth_dev_stop_mp(pi) != 0)
>   			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
>   				pi);
> +		/*
> +		 * free the pool of multicast addresses. If it is NULL,
> +		 * it means there is no mc addr.Make sure the mc_addr_pool
> +		 * is NULL at port init.
> +		 */
> +		if (port->mc_addr_pool != NULL) {
> +			free(port->mc_addr_pool);
> +			port->mc_addr_pool = NULL;
> +		}
> +		port->mc_addr_nb = 0;

will it work to use 'port->mc_addr_nb' to detect if the
'port->mc_addr_pool' pointer is valid

if (port->mc_addr_nb) {
   free(port->mc_addr_pool);
   port->mc_addr_pool = NULL;
   port->mc_addr_nb = 0;
}

>   
>   		if (port->port_status == RTE_PORT_HANDLING)
>   			port->port_status = RTE_PORT_STOPPED;


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] app/testpmd: fix issue with memory leaks when quit testpmd
  2022-03-04 16:43 ` Ferruh Yigit
@ 2022-03-08  6:05   ` Zhang, Ke1X
  0 siblings, 0 replies; 13+ messages in thread
From: Zhang, Ke1X @ 2022-03-08  6:05 UTC (permalink / raw)
  To: Yigit, Ferruh, Li, Xiaoyun, Singh, Aman Deep, Zhang, Yuying, dev



> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Saturday, March 5, 2022 12:44 AM
> To: Zhang, Ke1X <ke1x.zhang@intel.com>; Li, Xiaoyun
> <xiaoyun.li@intel.com>; Singh, Aman Deep <aman.deep.singh@intel.com>;
> Zhang, Yuying <yuying.zhang@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH] app/testpmd: fix issue with memory leaks when quit
> testpmd
> 
> On 3/1/2022 2:06 AM, Ke Zhang wrote:
> > when dpdk is compiled in ASan, there is a memory leaks after quit
> > testpmd if set mcast_addr, this patch fix this issue.
> >
> > 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/testpmd.c | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > fe2ce19f99..fa7f80fdf7 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2742,6 +2742,8 @@ start_port(portid_t pid)
> >   			continue;
> >   		}
> >
> > +		port->mc_addr_pool = NULL;
> > +
> 
> User can do consecutive start / stop / start / ..
> 
> Won't user expect the previous allocated and set MAC address to be valid
> after this?
> 
> What about to free the 'port->mc_addr_pool' in close() instead of stop()?
>

Thanks for your comments,  I checked the code in dpdk, when start, the resource associated with port
Will be allocated, when stop, the resource will be free from hardware and software struct.For example,
In the function ice_dev_stop(), the rx and tx resource will be free, and also in start, resource like tx,
 it will be allocated and request hardware to allocate the resource using ice_aqc_opc_add_txqs admQ
command, and the mc_addr_pool is a port resource. So what's your opinion about this ?
 
> >   		if (port->need_reconfig > 0) {
> >   			struct rte_eth_conf dev_conf;
> >   			int k;
> > @@ -3065,6 +3067,16 @@ stop_port(portid_t pid)
> >   		if (eth_dev_stop_mp(pi) != 0)
> >   			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for
> port %u\n",
> >   				pi);
> > +		/*
> > +		 * free the pool of multicast addresses. If it is NULL,
> > +		 * it means there is no mc addr.Make sure the mc_addr_pool
> > +		 * is NULL at port init.
> > +		 */
> > +		if (port->mc_addr_pool != NULL) {
> > +			free(port->mc_addr_pool);
> > +			port->mc_addr_pool = NULL;
> > +		}
> > +		port->mc_addr_nb = 0;
> 
> will it work to use 'port->mc_addr_nb' to detect if the 'port->mc_addr_pool'
> pointer is valid
> 
> if (port->mc_addr_nb) {
>    free(port->mc_addr_pool);
>    port->mc_addr_pool = NULL;
>    port->mc_addr_nb = 0;
> }
> 

Yes , it is a better way using " if (port->mc_addr_nb) ", I will modify my code.

> >
> >   		if (port->port_status == RTE_PORT_HANDLING)
> >   			port->port_status = RTE_PORT_STOPPED;


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] app/testpmd: fix issue with memory leaks when quit testpmd
  2022-03-01  2:06 [PATCH] app/testpmd: fix issue with memory leaks when quit testpmd Ke Zhang
  2022-03-04 16:43 ` Ferruh Yigit
@ 2022-03-14  5:52 ` Ke Zhang
  2022-03-14  9:10   ` David Marchand
                     ` (2 more replies)
  1 sibling, 3 replies; 13+ messages in thread
From: Ke Zhang @ 2022-03-14  5:52 UTC (permalink / raw)
  To: xiaoyun.li, aman.deep.singh, yuying.zhang, dev; +Cc: Ke Zhang

when dpdk is compiled in ASan, there is a memory leaks after
quit testpmd if set mcast_addr, this patch fix this issue.

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/testpmd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index fe2ce19f99..f7e18aee25 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3136,6 +3136,12 @@ close_port(portid_t pid)
 			continue;
 		}
 
+		if (port->mc_addr_nb != 0) {
+			/* free the pool of multicast addresses. */
+			free(port->mc_addr_pool);
+			port->mc_addr_pool = NULL;
+		}
+
 		if (is_proc_primary()) {
 			port_flow_flush(pi);
 			port_flex_item_flush(pi);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] app/testpmd: fix issue with memory leaks when quit testpmd
  2022-03-14  5:52 ` [PATCH v2] " Ke Zhang
@ 2022-03-14  9:10   ` David Marchand
  2022-03-15 10:06   ` Zhang, Yuying
  2022-03-25  8:35   ` [PATCH v3] " Ke Zhang
  2 siblings, 0 replies; 13+ messages in thread
From: David Marchand @ 2022-03-14  9:10 UTC (permalink / raw)
  To: Ke Zhang
  Cc: Xiaoyun Li, Singh, Aman Deep, Yuying Zhang, dev, Yigit, Ferruh,
	Thomas Monjalon

On Mon, Mar 14, 2022 at 6:59 AM Ke Zhang <ke1x.zhang@intel.com> wrote:
>
> when dpdk is compiled in ASan, there is a memory leaks after
> quit testpmd if set mcast_addr, this patch fix this issue.

The memory leak is present regardless of ASan being compiled in.
Plus, afaiu, the issue happens too when closing a port.

I'd rather rephrase like:
"""
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
>

This patch needs a Fixes: tag.


> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> ---
>  app/test-pmd/testpmd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index fe2ce19f99..f7e18aee25 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3136,6 +3136,12 @@ close_port(portid_t pid)
>                         continue;
>                 }
>
> +               if (port->mc_addr_nb != 0) {
> +                       /* free the pool of multicast addresses. */
> +                       free(port->mc_addr_pool);
> +                       port->mc_addr_pool = NULL;
> +               }
> +

Nit: I would introduce a helper in app/test-pmd/config.c, for example,
mcast_addr_pool_destroy.


>                 if (is_proc_primary()) {
>                         port_flow_flush(pi);
>                         port_flex_item_flush(pi);
> --
> 2.25.1
>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v2] app/testpmd: fix issue with memory leaks when quit testpmd
  2022-03-14  5:52 ` [PATCH v2] " Ke Zhang
  2022-03-14  9:10   ` David Marchand
@ 2022-03-15 10:06   ` Zhang, Yuying
  2022-03-25  8:35   ` [PATCH v3] " Ke Zhang
  2 siblings, 0 replies; 13+ messages in thread
From: Zhang, Yuying @ 2022-03-15 10:06 UTC (permalink / raw)
  To: Zhang, Ke1X, Li, Xiaoyun, Singh, Aman Deep, dev

Hi Ke,

> -----Original Message-----
> From: Zhang, Ke1X <ke1x.zhang@intel.com>
> Sent: Monday, March 14, 2022 1:53 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 v2] app/testpmd: fix issue with memory leaks when quit
> testpmd
> 
> when dpdk is compiled in ASan, there is a memory leaks after quit testpmd if
> set mcast_addr, this patch fix this issue.
> 
> 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/testpmd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> fe2ce19f99..f7e18aee25 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3136,6 +3136,12 @@ close_port(portid_t pid)
>  			continue;
>  		}
> 
> +		if (port->mc_addr_nb != 0) {
> +			/* free the pool of multicast addresses. */
> +			free(port->mc_addr_pool);
> +			port->mc_addr_pool = NULL;
> +		}
> +

Is port->mc_addr_pool located in shared memory and may it be freed in primary process?
BTW,  you can write a function in config.c such as mcast_addr_pool_extend() and reference in close_port().

>  		if (is_proc_primary()) {
>  			port_flow_flush(pi);
>  			port_flex_item_flush(pi);
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3] app/testpmd: fix issue with memory leaks when quit testpmd
  2022-03-14  5:52 ` [PATCH v2] " Ke Zhang
  2022-03-14  9:10   ` David Marchand
  2022-03-15 10:06   ` Zhang, Yuying
@ 2022-03-25  8:35   ` Ke Zhang
  2022-04-04 15:34     ` Zhang, Yuying
  2 siblings, 1 reply; 13+ messages in thread
From: Ke Zhang @ 2022-03-25  8:35 UTC (permalink / raw)
  To: xiaoyun.li, aman.deep.singh, yuying.zhang, dev; +Cc: 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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v3] app/testpmd: fix issue with memory leaks when quit testpmd
  2022-03-25  8:35   ` [PATCH v3] " Ke Zhang
@ 2022-04-04 15:34     ` Zhang, Yuying
  2022-06-08 12:06       ` Ferruh Yigit
  0 siblings, 1 reply; 13+ messages in thread
From: Zhang, Yuying @ 2022-04-04 15:34 UTC (permalink / raw)
  To: Zhang, Ke1X, Li, Xiaoyun, Singh, Aman Deep, dev

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] app/testpmd: fix issue with memory leaks when quit testpmd
  2022-04-04 15:34     ` Zhang, Yuying
@ 2022-06-08 12:06       ` Ferruh Yigit
  0 siblings, 0 replies; 13+ messages in thread
From: Ferruh Yigit @ 2022-06-08 12:06 UTC (permalink / raw)
  To: Zhang, Yuying, Zhang, Ke1X, Li, Xiaoyun, Singh, Aman Deep; +Cc: dev

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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v2] app/testpmd: fix issue with memory leaks when quit testpmd
  2022-03-14  5:47 Ke Zhang
@ 2022-03-15 10:00 ` Zhang, Yuying
  0 siblings, 0 replies; 13+ messages in thread
From: Zhang, Yuying @ 2022-03-15 10:00 UTC (permalink / raw)
  To: Zhang, Ke1X, Li, Xiaoyun, Singh, Aman Deep, dev

Hi Ke,

-----Original Message-----
From: Zhang, Ke1X <ke1x.zhang@intel.com> 
Sent: 2022年3月14日 13:47
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 v2] app/testpmd: fix issue with memory leaks when quit testpmd

when dpdk is compiled in ASan, there is a memory leaks after quit testpmd if set mcast_addr, this patch fix this issue.

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/testpmd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index fe2ce19f99..f7e18aee25 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3136,6 +3136,12 @@ close_port(portid_t pid)
 			continue;
 		}
 
+		if (port->mc_addr_nb != 0) {
+			/* free the pool of multicast addresses. */
+			free(port->mc_addr_pool);
+			port->mc_addr_pool = NULL;
+		}
+
Is port->mc_addr_pool located in shared memory and may it be freed in primary process?
BTW,  you can write a function in config.c such as mcast_addr_pool_extend() and reference in close_port().
 		if (is_proc_primary()) {
 			port_flow_flush(pi);
 			port_flex_item_flush(pi);
--
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] app/testpmd: fix issue with memory leaks when quit testpmd
@ 2022-03-14  5:47 Ke Zhang
  2022-03-15 10:00 ` Zhang, Yuying
  0 siblings, 1 reply; 13+ messages in thread
From: Ke Zhang @ 2022-03-14  5:47 UTC (permalink / raw)
  To: xiaoyun.li, aman.deep.singh, yuying.zhang, dev; +Cc: Ke Zhang

when dpdk is compiled in ASan, there is a memory leaks after
quit testpmd if set mcast_addr, this patch fix this issue.

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/testpmd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index fe2ce19f99..f7e18aee25 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3136,6 +3136,12 @@ close_port(portid_t pid)
 			continue;
 		}
 
+		if (port->mc_addr_nb != 0) {
+			/* free the pool of multicast addresses. */
+			free(port->mc_addr_pool);
+			port->mc_addr_pool = NULL;
+		}
+
 		if (is_proc_primary()) {
 			port_flow_flush(pi);
 			port_flex_item_flush(pi);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] app/testpmd: fix issue with memory leaks when quit testpmd
@ 2022-03-14  5:34 Ke Zhang
  0 siblings, 0 replies; 13+ messages in thread
From: Ke Zhang @ 2022-03-14  5:34 UTC (permalink / raw)
  To: xiaoyun.li, aman.deep.singh, yuying.zhang, dev; +Cc: Ke Zhang

when dpdk is compiled in ASan, there is a memory leaks after
quit testpmd if set mcast_addr, this patch fix this issue.

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/testpmd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index fe2ce19f99..f7e18aee25 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3136,6 +3136,12 @@ close_port(portid_t pid)
 			continue;
 		}
 
+		if (port->mc_addr_nb != 0) {
+			/* free the pool of multicast addresses. */
+			free(port->mc_addr_pool);
+			port->mc_addr_pool = NULL;
+		}
+
 		if (is_proc_primary()) {
 			port_flow_flush(pi);
 			port_flex_item_flush(pi);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] app/testpmd: fix issue with memory leaks when quit testpmd
@ 2022-03-14  5:17 Ke Zhang
  0 siblings, 0 replies; 13+ messages in thread
From: Ke Zhang @ 2022-03-14  5:17 UTC (permalink / raw)
  To: xiaoyun.li, aman.deep.singh, yuying.zhang, dev; +Cc: Ke Zhang

when dpdk is compiled in ASan, there is a memory leaks after
quit testpmd if set mcast_addr, this patch fix this issue.

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/testpmd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index fe2ce19f99..f7e18aee25 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3136,6 +3136,12 @@ close_port(portid_t pid)
 			continue;
 		}
 
+		if (port->mc_addr_nb != 0) {
+			/* free the pool of multicast addresses. */
+			free(port->mc_addr_pool);
+			port->mc_addr_pool = NULL;
+		}
+
 		if (is_proc_primary()) {
 			port_flow_flush(pi);
 			port_flex_item_flush(pi);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-06-08 12:06 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01  2:06 [PATCH] app/testpmd: fix issue with memory leaks when quit testpmd Ke Zhang
2022-03-04 16:43 ` Ferruh Yigit
2022-03-08  6:05   ` Zhang, Ke1X
2022-03-14  5:52 ` [PATCH v2] " Ke Zhang
2022-03-14  9:10   ` David Marchand
2022-03-15 10:06   ` Zhang, Yuying
2022-03-25  8:35   ` [PATCH v3] " Ke Zhang
2022-04-04 15:34     ` Zhang, Yuying
2022-06-08 12:06       ` Ferruh Yigit
2022-03-14  5:17 [PATCH v2] " Ke Zhang
2022-03-14  5:34 Ke Zhang
2022-03-14  5:47 Ke Zhang
2022-03-15 10:00 ` Zhang, Yuying

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).