* Re: [PATCH] net/ixgbe: fix min Rx/Tx descriptors
2025-02-21 8:23 [PATCH] net/ixgbe: fix min Rx/Tx descriptors Mingjin Ye
@ 2025-02-21 14:33 ` Bruce Richardson
2025-02-21 14:36 ` Bruce Richardson
2025-02-25 9:12 ` [PATCH v2] " Mingjin Ye
2 siblings, 0 replies; 5+ messages in thread
From: Bruce Richardson @ 2025-02-21 14:33 UTC (permalink / raw)
To: Mingjin Ye; +Cc: dev, stable, Anatoly Burakov, Vladimir Medvedkin
On Fri, Feb 21, 2025 at 08:23:18AM +0000, Mingjin Ye wrote:
> The minimum free packet threshold (tx_free_thresh) and the minimum RS bit
> threshold (tx_rs_thresh) both have a default value of 32. Therefore, the
> default minimum number of ring descriptors value is 64.
>
> Fixes: dee5f1fd5fc7 ("ixgbe: get queue info and descriptor limits")
> Cc: stable@dpdk.org
>
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
> doc/guides/nics/ixgbe.rst | 2 +-
> drivers/net/intel/ixgbe/ixgbe_rxtx.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
> index c5c6a6c34b..10a0cdd270 100644
> --- a/doc/guides/nics/ixgbe.rst
> +++ b/doc/guides/nics/ixgbe.rst
> @@ -76,7 +76,7 @@ Scattered packets are not supported in this mode.
> If an incoming packet is greater than the maximum acceptable length of one "mbuf" data size (by default, the size is 2 KB),
> vPMD for RX would be disabled.
>
> -By default, IXGBE_MAX_RING_DESC is set to 8192 and RTE_PMD_IXGBE_RX_MAX_BURST is set to 32.
> +By default, IXGBE_MAX_RING_DESC is set to 8192 and RTE_PMD_IXGBE_RX_MAX_BURST is set to 64.
>
Was this part of the patch included by mistake? The RX_MAX_BURST value is
still set to 32 in the code.
However, in terms of doc updates, I believe the text above this point is
incorrect. It states that scattered packets are not supported in vector
drivers, which is no longer correct, as we support them for vector Rx.
Similarly, I think the pre-conditions for use of burst/vector mode are
incorrect compared to what is checked in the code.
> Windows Prerequisites and Pre-conditions
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> diff --git a/drivers/net/intel/ixgbe/ixgbe_rxtx.h b/drivers/net/intel/ixgbe/ixgbe_rxtx.h
> index 278f665108..54569c7ade 100644
> --- a/drivers/net/intel/ixgbe/ixgbe_rxtx.h
> +++ b/drivers/net/intel/ixgbe/ixgbe_rxtx.h
> @@ -26,7 +26,7 @@
> * descriptors should meet the following condition:
> * (num_ring_desc * sizeof(rx/tx descriptor)) % 128 == 0
> */
> -#define IXGBE_MIN_RING_DESC 32
> +#define IXGBE_MIN_RING_DESC 64
> #define IXGBE_MAX_RING_DESC 8192
>
> #define RTE_PMD_IXGBE_TX_MAX_BURST 32
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net/ixgbe: fix min Rx/Tx descriptors
2025-02-21 8:23 [PATCH] net/ixgbe: fix min Rx/Tx descriptors Mingjin Ye
2025-02-21 14:33 ` Bruce Richardson
@ 2025-02-21 14:36 ` Bruce Richardson
2025-02-25 9:12 ` [PATCH v2] " Mingjin Ye
2 siblings, 0 replies; 5+ messages in thread
From: Bruce Richardson @ 2025-02-21 14:36 UTC (permalink / raw)
To: Mingjin Ye; +Cc: dev, stable, Anatoly Burakov, Vladimir Medvedkin
On Fri, Feb 21, 2025 at 08:23:18AM +0000, Mingjin Ye wrote:
> The minimum free packet threshold (tx_free_thresh) and the minimum RS bit
> threshold (tx_rs_thresh) both have a default value of 32. Therefore, the
> default minimum number of ring descriptors value is 64.
>
> Fixes: dee5f1fd5fc7 ("ixgbe: get queue info and descriptor limits")
> Cc: stable@dpdk.org
>
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
> doc/guides/nics/ixgbe.rst | 2 +-
> drivers/net/intel/ixgbe/ixgbe_rxtx.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
> index c5c6a6c34b..10a0cdd270 100644
> --- a/doc/guides/nics/ixgbe.rst
> +++ b/doc/guides/nics/ixgbe.rst
> @@ -76,7 +76,7 @@ Scattered packets are not supported in this mode.
> If an incoming packet is greater than the maximum acceptable length of one "mbuf" data size (by default, the size is 2 KB),
> vPMD for RX would be disabled.
>
> -By default, IXGBE_MAX_RING_DESC is set to 8192 and RTE_PMD_IXGBE_RX_MAX_BURST is set to 32.
> +By default, IXGBE_MAX_RING_DESC is set to 8192 and RTE_PMD_IXGBE_RX_MAX_BURST is set to 64.
>
> Windows Prerequisites and Pre-conditions
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> diff --git a/drivers/net/intel/ixgbe/ixgbe_rxtx.h b/drivers/net/intel/ixgbe/ixgbe_rxtx.h
> index 278f665108..54569c7ade 100644
> --- a/drivers/net/intel/ixgbe/ixgbe_rxtx.h
> +++ b/drivers/net/intel/ixgbe/ixgbe_rxtx.h
> @@ -26,7 +26,7 @@
> * descriptors should meet the following condition:
> * (num_ring_desc * sizeof(rx/tx descriptor)) % 128 == 0
> */
> -#define IXGBE_MIN_RING_DESC 32
> +#define IXGBE_MIN_RING_DESC 64
> #define IXGBE_MAX_RING_DESC 8192
Ack for this part of the change. Just ran a quick test using ixgbe and
things stopped working pretty quickly with an Rx ring size of 32.
With the doc update above dropped:
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2] net/ixgbe: fix min Rx/Tx descriptors
2025-02-21 8:23 [PATCH] net/ixgbe: fix min Rx/Tx descriptors Mingjin Ye
2025-02-21 14:33 ` Bruce Richardson
2025-02-21 14:36 ` Bruce Richardson
@ 2025-02-25 9:12 ` Mingjin Ye
2025-02-25 15:53 ` Bruce Richardson
2 siblings, 1 reply; 5+ messages in thread
From: Mingjin Ye @ 2025-02-25 9:12 UTC (permalink / raw)
To: dev; +Cc: Mingjin Ye, stable, Anatoly Burakov, Vladimir Medvedkin
The minimum free packet threshold (tx_free_thresh) and the minimum RS bit
threshold (tx_rs_thresh) both have a default value of 32. Therefore, the
default minimum number of ring descriptors value is 64.
For reference, see "Configuration of Transmit Queues" in
doc/guides/prog_guide/ethdev/ethdev.rst
Fixes: dee5f1fd5fc7 ("ixgbe: get queue info and descriptor limits")
Cc: stable@dpdk.org
Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
v2: Change doc.
---
doc/guides/nics/ixgbe.rst | 5 +++--
drivers/net/intel/ixgbe/ixgbe_rxtx.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
index c5c6a6c34b..aaed324e15 100644
--- a/doc/guides/nics/ixgbe.rst
+++ b/doc/guides/nics/ixgbe.rst
@@ -68,11 +68,12 @@ Ensure that the following pre-conditions are satisfied:
* (rxq->nb_rx_desc % rxq->rx_free_thresh) == 0
-* rxq->nb_rx_desc < (IXGBE_MAX_RING_DESC - RTE_PMD_IXGBE_RX_MAX_BURST)
+* rxq->nb_rx_desc >= IXGBE_MIN_RING_DESC
+
+* rxq->nb_rx_desc <= IXGBE_MAX_RING_DESC
These conditions are checked in the code.
-Scattered packets are not supported in this mode.
If an incoming packet is greater than the maximum acceptable length of one "mbuf" data size (by default, the size is 2 KB),
vPMD for RX would be disabled.
diff --git a/drivers/net/intel/ixgbe/ixgbe_rxtx.h b/drivers/net/intel/ixgbe/ixgbe_rxtx.h
index 278f665108..54569c7ade 100644
--- a/drivers/net/intel/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/intel/ixgbe/ixgbe_rxtx.h
@@ -26,7 +26,7 @@
* descriptors should meet the following condition:
* (num_ring_desc * sizeof(rx/tx descriptor)) % 128 == 0
*/
-#define IXGBE_MIN_RING_DESC 32
+#define IXGBE_MIN_RING_DESC 64
#define IXGBE_MAX_RING_DESC 8192
#define RTE_PMD_IXGBE_TX_MAX_BURST 32
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] net/ixgbe: fix min Rx/Tx descriptors
2025-02-25 9:12 ` [PATCH v2] " Mingjin Ye
@ 2025-02-25 15:53 ` Bruce Richardson
0 siblings, 0 replies; 5+ messages in thread
From: Bruce Richardson @ 2025-02-25 15:53 UTC (permalink / raw)
To: Mingjin Ye; +Cc: dev, stable, Anatoly Burakov, Vladimir Medvedkin
On Tue, Feb 25, 2025 at 09:12:36AM +0000, Mingjin Ye wrote:
> The minimum free packet threshold (tx_free_thresh) and the minimum RS bit
> threshold (tx_rs_thresh) both have a default value of 32. Therefore, the
> default minimum number of ring descriptors value is 64.
>
> For reference, see "Configuration of Transmit Queues" in
> doc/guides/prog_guide/ethdev/ethdev.rst
>
> Fixes: dee5f1fd5fc7 ("ixgbe: get queue info and descriptor limits")
> Cc: stable@dpdk.org
>
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
> v2: Change doc.
> ---
> doc/guides/nics/ixgbe.rst | 5 +++--
> drivers/net/intel/ixgbe/ixgbe_rxtx.h | 2 +-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
> index c5c6a6c34b..aaed324e15 100644
> --- a/doc/guides/nics/ixgbe.rst
> +++ b/doc/guides/nics/ixgbe.rst
> @@ -68,11 +68,12 @@ Ensure that the following pre-conditions are satisfied:
>
> * (rxq->nb_rx_desc % rxq->rx_free_thresh) == 0
>
> -* rxq->nb_rx_desc < (IXGBE_MAX_RING_DESC - RTE_PMD_IXGBE_RX_MAX_BURST)
> +* rxq->nb_rx_desc >= IXGBE_MIN_RING_DESC
> +
> +* rxq->nb_rx_desc <= IXGBE_MAX_RING_DESC
>
> These conditions are checked in the code.
>
> -Scattered packets are not supported in this mode.
> If an incoming packet is greater than the maximum acceptable length of one "mbuf" data size (by default, the size is 2 KB),
> vPMD for RX would be disabled.
>
These two lines in the doc should be removed as well, since they also imply
the unavailability of scattered Rx.
Fixing this by dropping them on apply.
> diff --git a/drivers/net/intel/ixgbe/ixgbe_rxtx.h b/drivers/net/intel/ixgbe/ixgbe_rxtx.h
> index 278f665108..54569c7ade 100644
> --- a/drivers/net/intel/ixgbe/ixgbe_rxtx.h
> +++ b/drivers/net/intel/ixgbe/ixgbe_rxtx.h
> @@ -26,7 +26,7 @@
> * descriptors should meet the following condition:
> * (num_ring_desc * sizeof(rx/tx descriptor)) % 128 == 0
> */
> -#define IXGBE_MIN_RING_DESC 32
> +#define IXGBE_MIN_RING_DESC 64
> #define IXGBE_MAX_RING_DESC 8192
>
> #define RTE_PMD_IXGBE_TX_MAX_BURST 32
> --
> 2.25.1
>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Applied to dpdk-next-net-intel.
Thanks,
/Bruce
^ permalink raw reply [flat|nested] 5+ messages in thread