Re: [dpdk-dev] [PATCH v2] ethdev: change queue release callback

[Thomas Monjalon <thomas@monjalon.net>]

15/09/2021 15:02, Xueming Li:
> To align with other eth device queue configuration callbacks, cleanup
> queue release callback API by changing RX and TX queue release callback
> API parameter from queue object to device and queue index.

Please split the explanation in multiple sentences,
so it allows taking a breath while reading :)
A good method is to start with explaining the status:
	most callbacks use queue ID
	queue release callbacks use queue object
Then explain what is done:
	queue release callbacks are changed to use queue ID
	all drivers are adapted
	empty callbacks are removed in some drivers

> This patch allows NULL callback to avoid defining empty callbacks.
> 
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Cc: "Singh, Aman Deep" <aman.deep.singh@intel.com>
> ---
> v2: included new NFP PMD driver
> 
> This patch is a preparation of shared Rx queue feature since the rxq
> object is subject to be used by PMD as shared rxq object which is shared
> among rx queues in same share group:
>   https://mails.dpdk.org/archives/dev/2021-July/215575.html

This note does not fully explain why using the queue ID helps.


> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> -typedef void (*eth_queue_release_t)(void *queue);
> +typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev,
> +				    uint16_t rx_queue_id);

> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> -		RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_release, -ENOTSUP);

Why removing this check?
It is changing the behaviour.

> -
> +		if (dev->dev_ops->rx_queue_release != NULL)
> +			for (i = nb_queues; i < old_nb_queues; i++)
> +				(*dev->dev_ops->rx_queue_release)(dev, i);
>  		rxq = dev->data->rx_queues;
> -
> -		for (i = nb_queues; i < old_nb_queues; i++)
> -			(*dev->dev_ops->rx_queue_release)(rxq[i]);
>  		rxq = rte_realloc(rxq, sizeof(rxq[0]) * nb_queues,
>  				RTE_CACHE_LINE_SIZE);