[PATCH 0/5] centralize deferred start checks

[PATCH 0/5] centralize deferred start checks

[Stephen Hemminger]

Recent zxdh driver review raised the question of why should
drivers have to check rx_conf for deferred start support.
This can be better handled across all drivers at ethdev level.

Stephen Hemminger (5):
  ethdev: check that device supports deferred start
  net/dpaa: remove unnecessary deferred start check
  net/dpaa2: remove unneeded deferred start check
  net/enetfec: remove unneeded deferred start check
  net/virtio: remove unneeded deferred start check

 drivers/net/dpaa/dpaa_ethdev.c    | 10 ----------
 drivers/net/dpaa2/dpaa2_ethdev.c  | 14 --------------
 drivers/net/enetfec/enet_ethdev.c | 12 ------------
 drivers/net/virtio/virtio_rxtx.c  | 10 ----------
 lib/ethdev/rte_ethdev.c           | 10 ++++++++++
 5 files changed, 10 insertions(+), 46 deletions(-)

-- 
2.45.2

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

[Stephen Hemminger]

The check for supporting deferred start should be handled at
the ethdev level for all devices.

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;
+
 	/* 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;
-- 
2.45.2

[PATCH 2/5] net/dpaa: remove unnecessary deferred start check

[Stephen Hemminger]

This check is now done in ethdev layer.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/dpaa/dpaa_ethdev.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index e8d34e5898..ce4bbbb8f0 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -1089,11 +1089,6 @@ int dpaa_eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 		return -rte_errno;
 	}
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		DPAA_PMD_ERR("%p:Rx deferred start not supported", (void *)dev);
-		return -EINVAL;
-	}
 	rxq->nb_desc = UINT16_MAX;
 	rxq->offloads = rx_conf->offloads;
 
@@ -1399,11 +1394,6 @@ int dpaa_eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		DPAA_PMD_ERR("%p:Tx deferred start not supported", (void *)dev);
-		return -EINVAL;
-	}
 	txq->nb_desc = UINT16_MAX;
 	txq->offloads = tx_conf->offloads;
 
-- 
2.45.2

[PATCH 3/5] net/dpaa2: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/dpaa2/dpaa2_ethdev.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index a9bce854c3..8624bba0ce 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -726,13 +726,6 @@ dpaa2_dev_rx_queue_setup(struct rte_eth_dev *dev,
 		DPAA2_PMD_WARN("To use Normal buffers, run 'export DPNI_NORMAL_BUF=1' before running dynamic_dpl.sh script");
 	}
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		DPAA2_PMD_ERR("%s:Rx deferred start not supported",
-			dev->data->name);
-		return -EINVAL;
-	}
-
 	if (!priv->bp_list || priv->bp_list->mp != mb_pool) {
 		if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
 			ret = rte_dpaa2_bpid_info_init(mb_pool);
@@ -889,13 +882,6 @@ dpaa2_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		DPAA2_PMD_ERR("%s:Tx deferred start not supported",
-			dev->data->name);
-		return -EINVAL;
-	}
-
 	dpaa2_q->nb_desc = UINT16_MAX;
 	dpaa2_q->offloads = tx_conf->offloads;
 
-- 
2.45.2

[PATCH 4/5] net/enetfec: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/enetfec/enet_ethdev.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/net/enetfec/enet_ethdev.c b/drivers/net/enetfec/enet_ethdev.c
index 91c0f60490..d8c81f4a70 100644
--- a/drivers/net/enetfec/enet_ethdev.c
+++ b/drivers/net/enetfec/enet_ethdev.c
@@ -377,12 +377,6 @@ enetfec_tx_queue_setup(struct rte_eth_dev *dev,
 		sizeof(struct bufdesc);
 	unsigned int dsize_log2 = rte_fls_u64(dsize) - 1;
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		ENETFEC_PMD_ERR("Tx deferred start not supported");
-		return -EINVAL;
-	}
-
 	/* allocate transmit queue */
 	txq = rte_zmalloc(NULL, sizeof(*txq), RTE_CACHE_LINE_SIZE);
 	if (txq == NULL) {
@@ -456,12 +450,6 @@ enetfec_rx_queue_setup(struct rte_eth_dev *dev,
 			sizeof(struct bufdesc);
 	unsigned int dsize_log2 = rte_fls_u64(dsize) - 1;
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		ENETFEC_PMD_ERR("Rx deferred start not supported");
-		return -EINVAL;
-	}
-
 	if (queue_idx >= ENETFEC_MAX_Q) {
 		ENETFEC_PMD_ERR("Invalid queue id %" PRIu16 ", max %d",
 			queue_idx, ENETFEC_MAX_Q);
-- 
2.45.2

[PATCH 5/5] net/virtio: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/virtio/virtio_rxtx.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index b67f063b31..6ae3a6eb12 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -654,11 +654,6 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if (rx_conf->rx_deferred_start) {
-		PMD_INIT_LOG(ERR, "Rx deferred start is not supported");
-		return -EINVAL;
-	}
-
 	buf_size = virtio_rx_mem_pool_buf_size(mp);
 	if (!virtio_rx_check_scatter(hw->max_rx_pkt_len, buf_size,
 				     hw->rx_ol_scatter, &error)) {
@@ -819,11 +814,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if (tx_conf->tx_deferred_start) {
-		PMD_INIT_LOG(ERR, "Tx deferred start is not supported");
-		return -EINVAL;
-	}
-
 	if (nb_desc == 0 || nb_desc > vq->vq_nentries)
 		nb_desc = vq->vq_nentries;
 	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
-- 
2.45.2

[PATCH v2 0/5] add check for deferred start to ethdev

[Stephen Hemminger]

Recent zxdh driver review raised the question of why should
drivers have to check rx_conf for deferred start support.
This can be better handled across all drivers at ethdev level.

v2 - annotate now unused conf pointer in enetfec

Stephen Hemminger (5):
  ethdev: check that device supports deferred start
  net/dpaa: remove unnecessary deferred start check
  net/dpaa2: remove unneeded deferred start check
  net/enetfec: remove unneeded deferred start check
  net/virtio: remove unneeded deferred start check

 drivers/net/dpaa/dpaa_ethdev.c    | 10 ----------
 drivers/net/dpaa2/dpaa2_ethdev.c  | 14 --------------
 drivers/net/enetfec/enet_ethdev.c | 16 ++--------------
 drivers/net/virtio/virtio_rxtx.c  | 10 ----------
 lib/ethdev/rte_ethdev.c           | 10 ++++++++++
 5 files changed, 12 insertions(+), 48 deletions(-)

-- 
2.45.2

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

[Stephen Hemminger]

The check for supporting deferred start should be handled at
the ethdev level for all devices.

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;
+
 	/* 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;
-- 
2.45.2

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

[Andrew Rybchenko]

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;

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

[Stephen Hemminger]

On Sun, 15 Dec 2024 11:56:55 +0300
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote:

> 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.

Right, there are drivers that define the callback but have no logic
in place to do deferred start. They just ignore the flag.
Drivers with this odditiy are: 
	ark, atlantic, cxgbe, enic, hinic, ipn3ke, nfb, nfp, ntnic
This patch set won't change that.

There are also some drivers which claim to support queue start/stop
in the documentation, but there is no functions:
   virtio, mana, netvsc, mlx4, vmxnet3
Will fix that in next version of this series.

> 
> 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.

It is a bug that the drivers that do queue start/stop and don't
implement deferred start. There is no hardware reason to not support
it, just missing feature during driver development.

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

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

[Andrew Rybchenko]

On 12/16/24 21:58, Stephen Hemminger wrote:
> On Sun, 15 Dec 2024 11:56:55 +0300
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote:
> 
>> 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.
> 
> Right, there are drivers that define the callback but have no logic
> in place to do deferred start. They just ignore the flag.
> Drivers with this odditiy are:
> 	ark, atlantic, cxgbe, enic, hinic, ipn3ke, nfb, nfp, ntnic
> This patch set won't change that.
> 
> There are also some drivers which claim to support queue start/stop
> in the documentation, but there is no functions:
>     virtio, mana, netvsc, mlx4, vmxnet3
> Will fix that in next version of this series.
> 
>>
>> 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.
> 
> It is a bug that the drivers that do queue start/stop and don't
> implement deferred start. There is no hardware reason to not support
> it, just missing feature during driver development.

I agree. Please, don't forget to add rx_deferred_start and
tx_deferred_start to queue start/stop features in
doc/guides/nics/features.rst.

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

[PATCH v2 2/5] net/dpaa: remove unnecessary deferred start check

[Stephen Hemminger]

This check is now done in ethdev layer.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/dpaa/dpaa_ethdev.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index e8d34e5898..ce4bbbb8f0 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -1089,11 +1089,6 @@ int dpaa_eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 		return -rte_errno;
 	}
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		DPAA_PMD_ERR("%p:Rx deferred start not supported", (void *)dev);
-		return -EINVAL;
-	}
 	rxq->nb_desc = UINT16_MAX;
 	rxq->offloads = rx_conf->offloads;
 
@@ -1399,11 +1394,6 @@ int dpaa_eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		DPAA_PMD_ERR("%p:Tx deferred start not supported", (void *)dev);
-		return -EINVAL;
-	}
 	txq->nb_desc = UINT16_MAX;
 	txq->offloads = tx_conf->offloads;
 
-- 
2.45.2

[PATCH v2 3/5] net/dpaa2: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/dpaa2/dpaa2_ethdev.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index a9bce854c3..8624bba0ce 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -726,13 +726,6 @@ dpaa2_dev_rx_queue_setup(struct rte_eth_dev *dev,
 		DPAA2_PMD_WARN("To use Normal buffers, run 'export DPNI_NORMAL_BUF=1' before running dynamic_dpl.sh script");
 	}
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		DPAA2_PMD_ERR("%s:Rx deferred start not supported",
-			dev->data->name);
-		return -EINVAL;
-	}
-
 	if (!priv->bp_list || priv->bp_list->mp != mb_pool) {
 		if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
 			ret = rte_dpaa2_bpid_info_init(mb_pool);
@@ -889,13 +882,6 @@ dpaa2_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		DPAA2_PMD_ERR("%s:Tx deferred start not supported",
-			dev->data->name);
-		return -EINVAL;
-	}
-
 	dpaa2_q->nb_desc = UINT16_MAX;
 	dpaa2_q->offloads = tx_conf->offloads;
 
-- 
2.45.2

[PATCH v2 4/5] net/enetfec: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/enetfec/enet_ethdev.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/net/enetfec/enet_ethdev.c b/drivers/net/enetfec/enet_ethdev.c
index 91c0f60490..9835532595 100644
--- a/drivers/net/enetfec/enet_ethdev.c
+++ b/drivers/net/enetfec/enet_ethdev.c
@@ -366,7 +366,7 @@ enetfec_tx_queue_setup(struct rte_eth_dev *dev,
 			uint16_t queue_idx,
 			uint16_t nb_desc,
 			unsigned int socket_id __rte_unused,
-			const struct rte_eth_txconf *tx_conf)
+			const struct rte_eth_txconf *tx_conf __rte_unused)
 {
 	struct enetfec_private *fep = dev->data->dev_private;
 	unsigned int i;
@@ -377,12 +377,6 @@ enetfec_tx_queue_setup(struct rte_eth_dev *dev,
 		sizeof(struct bufdesc);
 	unsigned int dsize_log2 = rte_fls_u64(dsize) - 1;
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		ENETFEC_PMD_ERR("Tx deferred start not supported");
-		return -EINVAL;
-	}
-
 	/* allocate transmit queue */
 	txq = rte_zmalloc(NULL, sizeof(*txq), RTE_CACHE_LINE_SIZE);
 	if (txq == NULL) {
@@ -443,7 +437,7 @@ enetfec_rx_queue_setup(struct rte_eth_dev *dev,
 			uint16_t queue_idx,
 			uint16_t nb_rx_desc,
 			unsigned int socket_id __rte_unused,
-			const struct rte_eth_rxconf *rx_conf,
+			const struct rte_eth_rxconf *rx_conf __rte_unused,
 			struct rte_mempool *mb_pool)
 {
 	struct enetfec_private *fep = dev->data->dev_private;
@@ -456,12 +450,6 @@ enetfec_rx_queue_setup(struct rte_eth_dev *dev,
 			sizeof(struct bufdesc);
 	unsigned int dsize_log2 = rte_fls_u64(dsize) - 1;
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		ENETFEC_PMD_ERR("Rx deferred start not supported");
-		return -EINVAL;
-	}
-
 	if (queue_idx >= ENETFEC_MAX_Q) {
 		ENETFEC_PMD_ERR("Invalid queue id %" PRIu16 ", max %d",
 			queue_idx, ENETFEC_MAX_Q);
-- 
2.45.2

[PATCH v2 5/5] net/virtio: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/virtio/virtio_rxtx.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index b67f063b31..6ae3a6eb12 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -654,11 +654,6 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if (rx_conf->rx_deferred_start) {
-		PMD_INIT_LOG(ERR, "Rx deferred start is not supported");
-		return -EINVAL;
-	}
-
 	buf_size = virtio_rx_mem_pool_buf_size(mp);
 	if (!virtio_rx_check_scatter(hw->max_rx_pkt_len, buf_size,
 				     hw->rx_ol_scatter, &error)) {
@@ -819,11 +814,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if (tx_conf->tx_deferred_start) {
-		PMD_INIT_LOG(ERR, "Tx deferred start is not supported");
-		return -EINVAL;
-	}
-
 	if (nb_desc == 0 || nb_desc > vq->vq_nentries)
 		nb_desc = vq->vq_nentries;
 	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
-- 
2.45.2

[PATCH v3 0/6] queue start/stop and deferred checks

[Stephen Hemminger]

Recent zxdh driver review raised the question of why should
drivers have to check rx_conf for deferred start support.
This can be better handled across all drivers at ethdev level.

Also found some drivers were incorrect in feature flags about
handling of queue start/stop.

v3 - fix doc about queue start/stop for some drivers

Stephen Hemminger (6):
  ethdev: check that device supports deferred start
  doc: fix feature flags for queue start/stop
  net/dpaa: remove unnecessary deferred start check
  net/dpaa2: remove unneeded deferred start check
  net/enetfec: remove unneeded deferred start check
  net/virtio: remove unneeded deferred start check

 doc/guides/nics/features/gve.ini     |  1 +
 doc/guides/nics/features/mana.ini    |  1 -
 doc/guides/nics/features/netvsc.ini  |  1 -
 doc/guides/nics/features/virtio.ini  |  1 -
 doc/guides/nics/features/vmxnet3.ini |  1 -
 drivers/net/dpaa/dpaa_ethdev.c       | 10 ----------
 drivers/net/dpaa2/dpaa2_ethdev.c     | 14 --------------
 drivers/net/enetfec/enet_ethdev.c    | 16 ++--------------
 drivers/net/virtio/virtio_rxtx.c     | 10 ----------
 lib/ethdev/rte_ethdev.c              | 16 ++++++++++++++++
 10 files changed, 19 insertions(+), 52 deletions(-)

-- 
2.45.2

[PATCH v3 1/6] ethdev: check that device supports deferred start

[Stephen Hemminger]

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 | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 6413c54e3b..7678fa47b6 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -2264,6 +2264,14 @@ 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) {
+		RTE_ETHDEV_LOG_LINE(ERR,
+			    "Deferred start requested, but driver does not support 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) +
@@ -2575,6 +2583,14 @@ 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) {
+		RTE_ETHDEV_LOG_LINE(ERR,
+			    "Deferred start requested, but driver does not support queue start");
+		return -ENOTSUP;
+	}
+
 	ret = rte_eth_dev_info_get(port_id, &dev_info);
 	if (ret != 0)
 		return ret;
-- 
2.45.2

[PATCH v3 2/6] doc: fix feature flags for queue start/stop

[Stephen Hemminger]

Several drivers have mismatch between the ethdev ops for queue
start/stop and the documentation. The gve driver does implement
queue start/stop support. The mana, netvsc, virtio and vmxnet3
drivers do not implement the rx_queue_start callback.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/nics/features/gve.ini     | 1 +
 doc/guides/nics/features/mana.ini    | 1 -
 doc/guides/nics/features/netvsc.ini  | 1 -
 doc/guides/nics/features/virtio.ini  | 1 -
 doc/guides/nics/features/vmxnet3.ini | 1 -
 5 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/doc/guides/nics/features/gve.ini b/doc/guides/nics/features/gve.ini
index 8dfa229bb0..f18b829eda 100644
--- a/doc/guides/nics/features/gve.ini
+++ b/doc/guides/nics/features/gve.ini
@@ -6,6 +6,7 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
+Queue start/stop     = Y
 MTU update           = Y
 TSO                  = Y
 RSS hash             = Y
diff --git a/doc/guides/nics/features/mana.ini b/doc/guides/nics/features/mana.ini
index 42fd3327d2..db555ffe27 100644
--- a/doc/guides/nics/features/mana.ini
+++ b/doc/guides/nics/features/mana.ini
@@ -8,7 +8,6 @@ Speed capabilities   = P
 Link status          = P
 Removal event        = Y
 Rx interrupt         = Y
-Queue start/stop     = Y
 RSS hash             = Y
 L3 checksum offload  = Y
 L4 checksum offload  = Y
diff --git a/doc/guides/nics/features/netvsc.ini b/doc/guides/nics/features/netvsc.ini
index de8c698184..32df77a03e 100644
--- a/doc/guides/nics/features/netvsc.ini
+++ b/doc/guides/nics/features/netvsc.ini
@@ -7,7 +7,6 @@
 Speed capabilities   = P
 Link status          = Y
 Free Tx mbuf on demand = Y
-Queue start/stop     = Y
 Scattered Rx         = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
index a5eab4932f..f0a1a564f9 100644
--- a/doc/guides/nics/features/virtio.ini
+++ b/doc/guides/nics/features/virtio.ini
@@ -8,7 +8,6 @@ Speed capabilities   = P
 Link status          = Y
 Link status event    = Y
 Rx interrupt         = Y
-Queue start/stop     = Y
 Scattered Rx         = P
 Promiscuous mode     = Y
 Allmulticast mode    = Y
diff --git a/doc/guides/nics/features/vmxnet3.ini b/doc/guides/nics/features/vmxnet3.ini
index 971695fc4a..749887d642 100644
--- a/doc/guides/nics/features/vmxnet3.ini
+++ b/doc/guides/nics/features/vmxnet3.ini
@@ -7,7 +7,6 @@
 Speed capabilities   = P
 Link status          = Y
 Link status event    = Y
-Queue start/stop     = Y
 MTU update           = Y
 LRO                  = Y
 TSO                  = Y
-- 
2.45.2

[PATCH v3 3/6] net/dpaa: remove unnecessary deferred start check

[Stephen Hemminger]

This check is now done in ethdev layer.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/dpaa/dpaa_ethdev.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index e8d34e5898..ce4bbbb8f0 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -1089,11 +1089,6 @@ int dpaa_eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 		return -rte_errno;
 	}
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		DPAA_PMD_ERR("%p:Rx deferred start not supported", (void *)dev);
-		return -EINVAL;
-	}
 	rxq->nb_desc = UINT16_MAX;
 	rxq->offloads = rx_conf->offloads;
 
@@ -1399,11 +1394,6 @@ int dpaa_eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		DPAA_PMD_ERR("%p:Tx deferred start not supported", (void *)dev);
-		return -EINVAL;
-	}
 	txq->nb_desc = UINT16_MAX;
 	txq->offloads = tx_conf->offloads;
 
-- 
2.45.2

[PATCH v3 4/6] net/dpaa2: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/dpaa2/dpaa2_ethdev.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index a9bce854c3..8624bba0ce 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -726,13 +726,6 @@ dpaa2_dev_rx_queue_setup(struct rte_eth_dev *dev,
 		DPAA2_PMD_WARN("To use Normal buffers, run 'export DPNI_NORMAL_BUF=1' before running dynamic_dpl.sh script");
 	}
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		DPAA2_PMD_ERR("%s:Rx deferred start not supported",
-			dev->data->name);
-		return -EINVAL;
-	}
-
 	if (!priv->bp_list || priv->bp_list->mp != mb_pool) {
 		if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
 			ret = rte_dpaa2_bpid_info_init(mb_pool);
@@ -889,13 +882,6 @@ dpaa2_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		DPAA2_PMD_ERR("%s:Tx deferred start not supported",
-			dev->data->name);
-		return -EINVAL;
-	}
-
 	dpaa2_q->nb_desc = UINT16_MAX;
 	dpaa2_q->offloads = tx_conf->offloads;
 
-- 
2.45.2

[PATCH v3 5/6] net/enetfec: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/enetfec/enet_ethdev.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/net/enetfec/enet_ethdev.c b/drivers/net/enetfec/enet_ethdev.c
index 91c0f60490..9835532595 100644
--- a/drivers/net/enetfec/enet_ethdev.c
+++ b/drivers/net/enetfec/enet_ethdev.c
@@ -366,7 +366,7 @@ enetfec_tx_queue_setup(struct rte_eth_dev *dev,
 			uint16_t queue_idx,
 			uint16_t nb_desc,
 			unsigned int socket_id __rte_unused,
-			const struct rte_eth_txconf *tx_conf)
+			const struct rte_eth_txconf *tx_conf __rte_unused)
 {
 	struct enetfec_private *fep = dev->data->dev_private;
 	unsigned int i;
@@ -377,12 +377,6 @@ enetfec_tx_queue_setup(struct rte_eth_dev *dev,
 		sizeof(struct bufdesc);
 	unsigned int dsize_log2 = rte_fls_u64(dsize) - 1;
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		ENETFEC_PMD_ERR("Tx deferred start not supported");
-		return -EINVAL;
-	}
-
 	/* allocate transmit queue */
 	txq = rte_zmalloc(NULL, sizeof(*txq), RTE_CACHE_LINE_SIZE);
 	if (txq == NULL) {
@@ -443,7 +437,7 @@ enetfec_rx_queue_setup(struct rte_eth_dev *dev,
 			uint16_t queue_idx,
 			uint16_t nb_rx_desc,
 			unsigned int socket_id __rte_unused,
-			const struct rte_eth_rxconf *rx_conf,
+			const struct rte_eth_rxconf *rx_conf __rte_unused,
 			struct rte_mempool *mb_pool)
 {
 	struct enetfec_private *fep = dev->data->dev_private;
@@ -456,12 +450,6 @@ enetfec_rx_queue_setup(struct rte_eth_dev *dev,
 			sizeof(struct bufdesc);
 	unsigned int dsize_log2 = rte_fls_u64(dsize) - 1;
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		ENETFEC_PMD_ERR("Rx deferred start not supported");
-		return -EINVAL;
-	}
-
 	if (queue_idx >= ENETFEC_MAX_Q) {
 		ENETFEC_PMD_ERR("Invalid queue id %" PRIu16 ", max %d",
 			queue_idx, ENETFEC_MAX_Q);
-- 
2.45.2

[PATCH v3 6/6] net/virtio: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/virtio/virtio_rxtx.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index b67f063b31..6ae3a6eb12 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -654,11 +654,6 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if (rx_conf->rx_deferred_start) {
-		PMD_INIT_LOG(ERR, "Rx deferred start is not supported");
-		return -EINVAL;
-	}
-
 	buf_size = virtio_rx_mem_pool_buf_size(mp);
 	if (!virtio_rx_check_scatter(hw->max_rx_pkt_len, buf_size,
 				     hw->rx_ol_scatter, &error)) {
@@ -819,11 +814,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if (tx_conf->tx_deferred_start) {
-		PMD_INIT_LOG(ERR, "Tx deferred start is not supported");
-		return -EINVAL;
-	}
-
 	if (nb_desc == 0 || nb_desc > vq->vq_nentries)
 		nb_desc = vq->vq_nentries;
 	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
-- 
2.45.2

Re: [PATCH v3 0/6] queue start/stop and deferred checks

[Stephen Hemminger]

On Mon, 16 Dec 2024 11:11:32 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Recent zxdh driver review raised the question of why should
> drivers have to check rx_conf for deferred start support.
> This can be better handled across all drivers at ethdev level.
> 
> Also found some drivers were incorrect in feature flags about
> handling of queue start/stop.
> 
> v3 - fix doc about queue start/stop for some drivers
> 
> Stephen Hemminger (6):
>   ethdev: check that device supports deferred start
>   doc: fix feature flags for queue start/stop
>   net/dpaa: remove unnecessary deferred start check
>   net/dpaa2: remove unneeded deferred start check
>   net/enetfec: remove unneeded deferred start check
>   net/virtio: remove unneeded deferred start check

I am deferring this patchset for now, need ACK from some of the drivers impacted first

[PATCH v2 0/6] fix the handling of deferred start in ethdev

[Stephen Hemminger]

There is a inconsistency and lack of documentation around the deferred
start configuration flag. The queue deferred start only makes sense if
the driver has a way to start the queue later via ethdev ops.
Therefore move the check into the ethdev queue configuration handling.
Fix the driver documentation where the support of deferred start
does not match what the driver implements.

Stephen Hemminger (6):
  ethdev: check that device supports deferred start
  doc: fix feature flags for queue start/stop
  net/dpaa: remove unnecessary deferred start check
  net/dpaa2: remove unneeded deferred start check
  net/enetfec: remove unneeded deferred start check
  net/virtio: remove unneeded deferred start check

v2 - rebase and cleanup wording

 doc/guides/nics/features.rst         |  1 +
 doc/guides/nics/features/gve.ini     |  1 +
 doc/guides/nics/features/mana.ini    |  1 -
 doc/guides/nics/features/netvsc.ini  |  1 -
 doc/guides/nics/features/virtio.ini  |  1 -
 doc/guides/nics/features/vmxnet3.ini |  1 -
 drivers/net/dpaa/dpaa_ethdev.c       | 10 ----------
 drivers/net/dpaa2/dpaa2_ethdev.c     | 14 --------------
 drivers/net/enetfec/enet_ethdev.c    | 16 ++--------------
 drivers/net/virtio/virtio_rxtx.c     | 10 ----------
 lib/ethdev/rte_ethdev.c              | 14 ++++++++++++++
 11 files changed, 18 insertions(+), 52 deletions(-)

-- 
2.47.2

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

[Stephen Hemminger]

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) {
+		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;
-- 
2.47.2

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

[Bruce Richardson]

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

[PATCH v2 2/6] doc: fix feature flags for queue start/stop

[Stephen Hemminger]

Several drivers have mismatch between the ethdev ops for queue
start/stop and the documentation. The gve driver does implement
queue start/stop support. The mana, netvsc, virtio and vmxnet3
drivers do not implement the rx_queue_start callback.

Fixes: 410333da8b5f ("net/mana: start/stop Tx queues")
Fixes: beff6d8e8e2e ("net/netvsc: add documentation")
Fixes: 9db3f52126fb ("doc: generate NIC overview table from ini files")
Fixes: 24595041937e ("net/vmxnet3: update feature doc")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/nics/features.rst         | 1 +
 doc/guides/nics/features/gve.ini     | 1 +
 doc/guides/nics/features/mana.ini    | 1 -
 doc/guides/nics/features/netvsc.ini  | 1 -
 doc/guides/nics/features/virtio.ini  | 1 -
 doc/guides/nics/features/vmxnet3.ini | 1 -
 6 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 8bd448139e..a075c057ec 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -149,6 +149,7 @@ Queue start/stop
 ----------------
 
 Supports starting/stopping a specific Rx/Tx queue of a port.
+This is required for use deferred start configuration option.
 
 * **[implements] eth_dev_ops**: ``rx_queue_start``, ``rx_queue_stop``, ``tx_queue_start``,
   ``tx_queue_stop``.
diff --git a/doc/guides/nics/features/gve.ini b/doc/guides/nics/features/gve.ini
index 8dfa229bb0..f18b829eda 100644
--- a/doc/guides/nics/features/gve.ini
+++ b/doc/guides/nics/features/gve.ini
@@ -6,6 +6,7 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
+Queue start/stop     = Y
 MTU update           = Y
 TSO                  = Y
 RSS hash             = Y
diff --git a/doc/guides/nics/features/mana.ini b/doc/guides/nics/features/mana.ini
index 42fd3327d2..db555ffe27 100644
--- a/doc/guides/nics/features/mana.ini
+++ b/doc/guides/nics/features/mana.ini
@@ -8,7 +8,6 @@ Speed capabilities   = P
 Link status          = P
 Removal event        = Y
 Rx interrupt         = Y
-Queue start/stop     = Y
 RSS hash             = Y
 L3 checksum offload  = Y
 L4 checksum offload  = Y
diff --git a/doc/guides/nics/features/netvsc.ini b/doc/guides/nics/features/netvsc.ini
index de8c698184..32df77a03e 100644
--- a/doc/guides/nics/features/netvsc.ini
+++ b/doc/guides/nics/features/netvsc.ini
@@ -7,7 +7,6 @@
 Speed capabilities   = P
 Link status          = Y
 Free Tx mbuf on demand = Y
-Queue start/stop     = Y
 Scattered Rx         = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
index a5eab4932f..f0a1a564f9 100644
--- a/doc/guides/nics/features/virtio.ini
+++ b/doc/guides/nics/features/virtio.ini
@@ -8,7 +8,6 @@ Speed capabilities   = P
 Link status          = Y
 Link status event    = Y
 Rx interrupt         = Y
-Queue start/stop     = Y
 Scattered Rx         = P
 Promiscuous mode     = Y
 Allmulticast mode    = Y
diff --git a/doc/guides/nics/features/vmxnet3.ini b/doc/guides/nics/features/vmxnet3.ini
index 971695fc4a..749887d642 100644
--- a/doc/guides/nics/features/vmxnet3.ini
+++ b/doc/guides/nics/features/vmxnet3.ini
@@ -7,7 +7,6 @@
 Speed capabilities   = P
 Link status          = Y
 Link status event    = Y
-Queue start/stop     = Y
 MTU update           = Y
 LRO                  = Y
 TSO                  = Y
-- 
2.47.2

[PATCH v2 3/6] net/dpaa: remove unnecessary deferred start check

[Stephen Hemminger]

This check is now done in ethdev layer.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/dpaa/dpaa_ethdev.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index e8d34e5898..ce4bbbb8f0 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -1089,11 +1089,6 @@ int dpaa_eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 		return -rte_errno;
 	}
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		DPAA_PMD_ERR("%p:Rx deferred start not supported", (void *)dev);
-		return -EINVAL;
-	}
 	rxq->nb_desc = UINT16_MAX;
 	rxq->offloads = rx_conf->offloads;
 
@@ -1399,11 +1394,6 @@ int dpaa_eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		DPAA_PMD_ERR("%p:Tx deferred start not supported", (void *)dev);
-		return -EINVAL;
-	}
 	txq->nb_desc = UINT16_MAX;
 	txq->offloads = tx_conf->offloads;
 
-- 
2.47.2

[PATCH v2 4/6] net/dpaa2: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/dpaa2/dpaa2_ethdev.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index a9bce854c3..8624bba0ce 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -726,13 +726,6 @@ dpaa2_dev_rx_queue_setup(struct rte_eth_dev *dev,
 		DPAA2_PMD_WARN("To use Normal buffers, run 'export DPNI_NORMAL_BUF=1' before running dynamic_dpl.sh script");
 	}
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		DPAA2_PMD_ERR("%s:Rx deferred start not supported",
-			dev->data->name);
-		return -EINVAL;
-	}
-
 	if (!priv->bp_list || priv->bp_list->mp != mb_pool) {
 		if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
 			ret = rte_dpaa2_bpid_info_init(mb_pool);
@@ -889,13 +882,6 @@ dpaa2_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		DPAA2_PMD_ERR("%s:Tx deferred start not supported",
-			dev->data->name);
-		return -EINVAL;
-	}
-
 	dpaa2_q->nb_desc = UINT16_MAX;
 	dpaa2_q->offloads = tx_conf->offloads;
 
-- 
2.47.2

[PATCH v2 5/6] net/enetfec: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/enetfec/enet_ethdev.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/net/enetfec/enet_ethdev.c b/drivers/net/enetfec/enet_ethdev.c
index 0d27b63953..7c2926409e 100644
--- a/drivers/net/enetfec/enet_ethdev.c
+++ b/drivers/net/enetfec/enet_ethdev.c
@@ -366,7 +366,7 @@ enetfec_tx_queue_setup(struct rte_eth_dev *dev,
 			uint16_t queue_idx,
 			uint16_t nb_desc,
 			unsigned int socket_id __rte_unused,
-			const struct rte_eth_txconf *tx_conf)
+			const struct rte_eth_txconf *tx_conf __rte_unused)
 {
 	struct enetfec_private *fep = dev->data->dev_private;
 	unsigned int i;
@@ -377,12 +377,6 @@ enetfec_tx_queue_setup(struct rte_eth_dev *dev,
 		sizeof(struct bufdesc);
 	unsigned int dsize_log2 = rte_fls_u64(dsize) - 1;
 
-	/* Tx deferred start is not supported */
-	if (tx_conf->tx_deferred_start) {
-		ENETFEC_PMD_ERR("Tx deferred start not supported");
-		return -EINVAL;
-	}
-
 	/* allocate transmit queue */
 	txq = rte_zmalloc(NULL, sizeof(*txq), RTE_CACHE_LINE_SIZE);
 	if (txq == NULL) {
@@ -442,7 +436,7 @@ enetfec_rx_queue_setup(struct rte_eth_dev *dev,
 			uint16_t queue_idx,
 			uint16_t nb_rx_desc,
 			unsigned int socket_id __rte_unused,
-			const struct rte_eth_rxconf *rx_conf,
+			const struct rte_eth_rxconf *rx_conf __rte_unused,
 			struct rte_mempool *mb_pool)
 {
 	struct enetfec_private *fep = dev->data->dev_private;
@@ -455,12 +449,6 @@ enetfec_rx_queue_setup(struct rte_eth_dev *dev,
 			sizeof(struct bufdesc);
 	unsigned int dsize_log2 = rte_fls_u64(dsize) - 1;
 
-	/* Rx deferred start is not supported */
-	if (rx_conf->rx_deferred_start) {
-		ENETFEC_PMD_ERR("Rx deferred start not supported");
-		return -EINVAL;
-	}
-
 	if (queue_idx >= ENETFEC_MAX_Q) {
 		ENETFEC_PMD_ERR("Invalid queue id %" PRIu16 ", max %d",
 			queue_idx, ENETFEC_MAX_Q);
-- 
2.47.2

[PATCH v2 6/6] net/virtio: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/virtio/virtio_rxtx.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index b67f063b31..6ae3a6eb12 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -654,11 +654,6 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if (rx_conf->rx_deferred_start) {
-		PMD_INIT_LOG(ERR, "Rx deferred start is not supported");
-		return -EINVAL;
-	}
-
 	buf_size = virtio_rx_mem_pool_buf_size(mp);
 	if (!virtio_rx_check_scatter(hw->max_rx_pkt_len, buf_size,
 				     hw->rx_ol_scatter, &error)) {
@@ -819,11 +814,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if (tx_conf->tx_deferred_start) {
-		PMD_INIT_LOG(ERR, "Tx deferred start is not supported");
-		return -EINVAL;
-	}
-
 	if (nb_desc == 0 || nb_desc > vq->vq_nentries)
 		nb_desc = vq->vq_nentries;
 	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
-- 
2.47.2

Re: [PATCH v2 0/6] fix the handling of deferred start in ethdev

[Bruce Richardson]

On Fri, Feb 14, 2025 at 09:38:53AM -0800, Stephen Hemminger wrote:
> There is a inconsistency and lack of documentation around the deferred
> start configuration flag. The queue deferred start only makes sense if
> the driver has a way to start the queue later via ethdev ops.
> Therefore move the check into the ethdev queue configuration handling.
> Fix the driver documentation where the support of deferred start
> does not match what the driver implements.
> 
> Stephen Hemminger (6):
>   ethdev: check that device supports deferred start
>   doc: fix feature flags for queue start/stop
>   net/dpaa: remove unnecessary deferred start check
>   net/dpaa2: remove unneeded deferred start check
>   net/enetfec: remove unneeded deferred start check
>   net/virtio: remove unneeded deferred start check
> 
> v2 - rebase and cleanup wording
> 

This seems a good idea to do.

Series-acked-by: Bruce Richardson <bruce.richardson@intel.com>

>  doc/guides/nics/features.rst         |  1 +
>  doc/guides/nics/features/gve.ini     |  1 +
>  doc/guides/nics/features/mana.ini    |  1 -
>  doc/guides/nics/features/netvsc.ini  |  1 -
>  doc/guides/nics/features/virtio.ini  |  1 -
>  doc/guides/nics/features/vmxnet3.ini |  1 -
>  drivers/net/dpaa/dpaa_ethdev.c       | 10 ----------
>  drivers/net/dpaa2/dpaa2_ethdev.c     | 14 --------------
>  drivers/net/enetfec/enet_ethdev.c    | 16 ++--------------
>  drivers/net/virtio/virtio_rxtx.c     | 10 ----------
>  lib/ethdev/rte_ethdev.c              | 14 ++++++++++++++
>  11 files changed, 18 insertions(+), 52 deletions(-)
> 
> -- 
> 2.47.2
>