Re: [PATCH net-next 6/8] netdevsim: add queue management API support

[Stanislav Fomichev <stfomichev@gmail.com>]

On 01/03, Jakub Kicinski wrote:
> Add queue management API support. We need a way to reset queues
> to test NAPI reordering, the queue management API provides a
> handy scaffolding for that.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  drivers/net/netdevsim/netdev.c    | 135 +++++++++++++++++++++++++++---
>  drivers/net/netdevsim/netdevsim.h |   2 +
>  2 files changed, 125 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index e1bd3c1563b7..86614292314a 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
> @@ -359,25 +359,24 @@ static int nsim_poll(struct napi_struct *napi, int budget)
>  	return done;
>  }
>  
> -static int nsim_create_page_pool(struct nsim_rq *rq)
> +static int nsim_create_page_pool(struct page_pool **p, struct napi_struct *napi)
>  {
> -	struct page_pool_params p = {
> +	struct page_pool_params params = {
>  		.order = 0,
>  		.pool_size = NSIM_RING_SIZE,
>  		.nid = NUMA_NO_NODE,
> -		.dev = &rq->napi.dev->dev,
> -		.napi = &rq->napi,
> +		.dev = &napi->dev->dev,
> +		.napi = napi,
>  		.dma_dir = DMA_BIDIRECTIONAL,
> -		.netdev = rq->napi.dev,
> +		.netdev = napi->dev,
>  	};
> +	struct page_pool *pool;
>  
> -	rq->page_pool = page_pool_create(&p);
> -	if (IS_ERR(rq->page_pool)) {
> -		int err = PTR_ERR(rq->page_pool);
> +	pool = page_pool_create(&params);
> +	if (IS_ERR(pool))
> +		return PTR_ERR(pool);
>  
> -		rq->page_pool = NULL;
> -		return err;
> -	}
> +	*p = pool;
>  	return 0;
>  }
>  
> @@ -396,7 +395,7 @@ static int nsim_init_napi(struct netdevsim *ns)
>  	for (i = 0; i < dev->num_rx_queues; i++) {
>  		rq = ns->rq[i];
>  
> -		err = nsim_create_page_pool(rq);
> +		err = nsim_create_page_pool(&rq->page_pool, &rq->napi);
>  		if (err)
>  			goto err_pp_destroy;
>  	}
> @@ -613,6 +612,117 @@ static void nsim_queue_free(struct nsim_rq *rq)
>  	kfree(rq);
>  }
>  
> +/* Queue reset mode is controled by ns->rq_reset_mode.
> + * - normal - new NAPI new pool (old NAPI enabled when new added)
> + * - mode 1 - allocate new pool (NAPI is only disabled / enabled)
> + * - mode 2 - new NAPI new pool (old NAPI removed before new added)
> + * - mode 3 - new NAPI new pool (old NAPI disabled when new added)
> + */
> +struct nsim_queue_mem {
> +	struct nsim_rq *rq;
> +	struct page_pool *pp;
> +};
> +
> +static int
> +nsim_queue_mem_alloc(struct net_device *dev, void *per_queue_mem, int idx)
> +{
> +	struct nsim_queue_mem *qmem = per_queue_mem;
> +	struct netdevsim *ns = netdev_priv(dev);
> +	int err;
> +
> +	if (ns->rq_reset_mode > 3)
> +		return -EINVAL;
> +
> +	if (ns->rq_reset_mode == 1)
> +		return nsim_create_page_pool(&qmem->pp, &ns->rq[idx]->napi);
> +
> +	qmem->rq = nsim_queue_alloc();
> +	if (!qmem->rq)
> +		return -ENOMEM;
> +
> +	err = nsim_create_page_pool(&qmem->rq->page_pool, &qmem->rq->napi);
> +	if (err)
> +		goto err_free;
> +
> +	if (!ns->rq_reset_mode)
> +		netif_napi_add_config(dev, &qmem->rq->napi, nsim_poll, idx);
> +
> +	return 0;
> +
> +err_free:
> +	nsim_queue_free(qmem->rq);
> +	return err;
> +}
> +
> +static void nsim_queue_mem_free(struct net_device *dev, void *per_queue_mem)
> +{
> +	struct nsim_queue_mem *qmem = per_queue_mem;
> +	struct netdevsim *ns = netdev_priv(dev);
> +

[..]

> +	if (qmem->pp)
> +		page_pool_destroy(qmem->pp);

nit: page_pool_destroy handles NULL arg, so no need for 'if (qmem->pp)',
but probably not worth it to respin?

Acked-by: Stanislav Fomichev <sdf@fomichev.me>