Re: [dpdk-dev] [PATCH v3 1/3] app/testpmd: fix mempool free on exit

[Ferruh Yigit <ferruh.yigit@intel.com>]

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.