Re: [PATCH v2 1/5] ethdev: check that device supports deferred start

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 12/14/24 21:07, Stephen Hemminger wrote:
> The check for supporting deferred start should be handled at
> the ethdev level for all devices.

It is a good idea to check it on ethdev level.

Strictly speaking presence of queue start/stop callback does not mean
support for deferred start right now. It is possible to use stop/start
without deferred start feature.

However, such check is much better than nothing since deferred start
definitely requires queue start callback.

It would be good to clarify it in the documentation.
doc/guides/nics/features.rst does not mention deferred start at all.
In fact, I don't mind to couple deferred start to queue start/stop
features.

One nit below.

Anyway:
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>   lib/ethdev/rte_ethdev.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 6413c54e3b..7768058f6d 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -2264,6 +2264,11 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
>   	if (rx_conf != NULL)
>   		rx_offloads |= rx_conf->offloads;
>   
> +	/* Deferred start requires that device supports queue start */
> +	if (rx_conf != NULL && rx_conf->rx_deferred_start &&
> +	    *dev->dev_ops->rx_queue_start == NULL)
> +		return -ENOTSUP;

Wouldn't it be useful to add some kind of logging to simplify
debugging in this case.

> +
>   	/* Ensure that we have one and only one source of Rx buffers */
>   	if ((mp != NULL) +
>   	    (rx_conf != NULL && rx_conf->rx_nseg > 0) +
> @@ -2575,6 +2580,11 @@ rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
>   		return -EINVAL;
>   	}
>   
> +	/* Deferred start requires that device supports queue start */
> +	if (tx_conf != NULL && tx_conf->tx_deferred_start &&
> +	    *dev->dev_ops->tx_queue_start == NULL)
> +		return -ENOTSUP;
> +
>   	ret = rte_eth_dev_info_get(port_id, &dev_info);
>   	if (ret != 0)
>   		return ret;