diff mbox series

[v3,1/3] app/testpmd: fix mempool free on exit

Message ID 12ce667683e47d94bb0debea9af481b87a3fba6f.1554406293.git.shahafs@mellanox.com (mailing list archive)
State Superseded, archived
Headers
Series DMA map anonymous memory to eth devices |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Shahaf Shuler April 4, 2019, 7:34 p.m. UTC 
  Allocated mempools were never free. it is bad practice.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 app/test-pmd/testpmd.c | 25 +++++++++++++++++++------
 app/test-pmd/testpmd.h |  2 ++
 2 files changed, 21 insertions(+), 6 deletions(-)

Comments

Ferruh Yigit April 5, 2019, 2:41 p.m. UTC | #1 
On 4/4/2019 8:34 PM, Shahaf Shuler wrote:
> Allocated mempools were never free. it is bad practice.

+1

> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
<...>

> @@ -835,7 +837,7 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
>  /*
>   * Configuration initialisation done once at init time.
>   */
> -static void
> +static struct rte_mempool *
>  mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
>  		 unsigned int socket_id)
>  {
> @@ -904,6 +906,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
>  			rte_exit(EXIT_FAILURE, "Invalid mempool creation mode\n");
>  		}
>  	}
> +	return rte_mp;
>  
>  err:
>  	if (rte_mp == NULL) {
> @@ -913,6 +916,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
>  	} else if (verbose_level > 0) {
>  		rte_mempool_dump(stdout, rte_mp);
>  	}
> +	return NULL;

This return never reached, because a few lines above there is:
  if (rte_mp == NULL) {
     rte_exit(EXIT_FAILURE ...

And for above "return rte_mp;" case, it skips "if (verbose_level > 0)" checks
because of an early return.

So what do you think remove above "return rte_mp;", and move here, instead of
NULL return?

<...>

> @@ -264,6 +264,8 @@ extern struct fwd_engine ieee1588_fwd_engine;
>  
>  extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
>  
> +extern struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];

There is no other .c file using 'mempools', can drop the extern.
diff mbox series

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 40c873b972..5c68eb9ec6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -188,6 +188,8 @@  struct fwd_engine * fwd_engines[] = {
 	NULL,
 };
 
+struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+
 struct fwd_config cur_fwd_config;
 struct fwd_engine *cur_fwd_eng = &io_fwd_engine; /**< IO mode by default. */
 uint32_t retry_enabled;
@@ -835,7 +837,7 @@  setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
 /*
  * Configuration initialisation done once at init time.
  */
-static void
+static struct rte_mempool *
 mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 		 unsigned int socket_id)
 {
@@ -904,6 +906,7 @@  mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			rte_exit(EXIT_FAILURE, "Invalid mempool creation mode\n");
 		}
 	}
+	return rte_mp;
 
 err:
 	if (rte_mp == NULL) {
@@ -913,6 +916,7 @@  mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 	} else if (verbose_level > 0) {
 		rte_mempool_dump(stdout, rte_mp);
 	}
+	return NULL;
 }
 
 /*
@@ -1130,14 +1134,18 @@  init_config(void)
 		uint8_t i;
 
 		for (i = 0; i < num_sockets; i++)
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool,
-					 socket_ids[i]);
+			mempools[i] = mbuf_pool_create(mbuf_data_size,
+						       nb_mbuf_per_pool,
+						       socket_ids[i]);
 	} else {
 		if (socket_num == UMA_NO_CONFIG)
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool, 0);
+			mempools[0] = mbuf_pool_create(mbuf_data_size,
+						       nb_mbuf_per_pool, 0);
 		else
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool,
-						 socket_num);
+			mempools[socket_num] = mbuf_pool_create
+							(mbuf_data_size,
+							 nb_mbuf_per_pool,
+							 socket_num);
 	}
 
 	init_port_config();
@@ -2396,6 +2404,7 @@  pmd_test_exit(void)
 	struct rte_device *device;
 	portid_t pt_id;
 	int ret;
+	int i;
 
 	if (test_done == 0)
 		stop_packet_forwarding();
@@ -2449,6 +2458,10 @@  pmd_test_exit(void)
 			return;
 		}
 	}
+	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++) {
+		if (mempools[i])
+			rte_mempool_free(mempools[i]);
+	}
 
 	printf("\nBye...\n");
 }
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a45988ebc5..84ce8ffa2f 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -264,6 +264,8 @@  extern struct fwd_engine ieee1588_fwd_engine;
 
 extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
 
+extern struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+
 /**
  * Forwarding Configuration
  *