diff mbox series

[v2,1/6] ethdev: check that device supports deferred start

Message ID 20250214174224.79142-2-stephen@networkplumber.org (mailing list archive)
State Accepted
Delegated to: Stephen Hemminger
Headers
Series fix the handling of deferred start in ethdev |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger Feb. 14, 2025, 5:38 p.m. UTC 
The check for supporting deferred start should be handled at
the ethdev level for all devices.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 lib/ethdev/rte_ethdev.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Bruce Richardson Feb. 17, 2025, 5:12 p.m. UTC | #1 
On Fri, Feb 14, 2025 at 09:38:54AM -0800, Stephen Hemminger wrote:
> The check for supporting deferred start should be handled at
> the ethdev level for all devices.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>  lib/ethdev/rte_ethdev.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index c4079bb924..5493b3b9f8 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -2274,6 +2274,13 @@ 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 && rx_conf->rx_deferred_start && *dev->dev_ops->rx_queue_start == NULL) {

I must admit to being a little confused by the initial "*" at the beginning
of the function pointer check, but Stephen has correctly pointed out that
this style seems to be used everywhere in DPDK. Can someone perhaps just
explain why we check for "*fn_ptr == NULL", rather than just "fn_ptr == NULL".
Testing with code in gdb shows that both work fine, though interestingly
gdb itself gives and error when you interactively request a dereference of
a null fn pointer, i.e. have the extra initial "*".

/Bruce
diff mbox series

Patch

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index c4079bb924..5493b3b9f8 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -2274,6 +2274,13 @@  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 && rx_conf->rx_deferred_start && *dev->dev_ops->rx_queue_start == NULL) {
+		RTE_ETHDEV_LOG_LINE(ERR,
+			    "Deferred start requested, but driver does not support Rx queue start");
+		return -ENOTSUP;
+	}
+
 	/* Ensure that we have one and only one source of Rx buffers */
 	if ((mp != NULL) +
 	    (rx_conf != NULL && rx_conf->rx_nseg > 0) +
@@ -2585,6 +2592,13 @@  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 && tx_conf->tx_deferred_start && *dev->dev_ops->tx_queue_start == NULL) {
+		RTE_ETHDEV_LOG_LINE(ERR,
+			    "Deferred start requested, but driver does not support Tx queue start");
+		return -ENOTSUP;
+	}
+
 	ret = rte_eth_dev_info_get(port_id, &dev_info);
 	if (ret != 0)
 		return ret;