[v2] net/mlx5: fix incorrect rx/tx descriptor limitations in rte_eth_dev_info

Message ID 20240618225642.1036624-2-igootorov@gmail.com (mailing list archive)
State Changes Requested, archived
Delegated to: Raslan Darawsheh
Headers
Series [v2] net/mlx5: fix incorrect rx/tx descriptor limitations in rte_eth_dev_info |

Checks

Context Check Description
ci/loongarch-compilation success Compilation OK
ci/checkpatch success coding style OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Igor Gutorov June 18, 2024, 10:56 p.m. UTC
Currently, `rte_eth_dev_info.rx_desc_lim.nb_max` as well as
`rte_eth_dev_info.tx_desc_lim.nb_max` shows 65535 as the limit,
which results in a few problems:

* It is an incorrect value
* Allocating an RX queue and passing `rx_desc_lim.nb_max` results in an
  integer overflow and 0 ring size:

```
rte_eth_rx_queue_setup(0, 0, rx_desc_lim.nb_max, 0, NULL, mb_pool);
```

Which overflows ring size and generates the following log:
```
mlx5_net: port 0 increased number of descriptors in Rx queue 0 to the
next power of two (0)
```
The same holds for allocating a TX queue.

This patch fixes these issues.

Signed-off-by: Igor Gutorov <igootorov@gmail.com>
---
 drivers/common/mlx5/mlx5_devx_cmds.c | 1 +
 drivers/common/mlx5/mlx5_devx_cmds.h | 1 +
 drivers/net/mlx5/mlx5_ethdev.c       | 4 ++++
 drivers/net/mlx5/mlx5_rxq.c          | 8 ++++++++
 drivers/net/mlx5/mlx5_txq.c          | 8 ++++++++
 5 files changed, 22 insertions(+)
  

Comments

Stephen Hemminger Oct. 7, 2024, 8:37 p.m. UTC | #1
On Wed, 19 Jun 2024 01:56:42 +0300
Igor Gutorov <igootorov@gmail.com> wrote:

> Currently, `rte_eth_dev_info.rx_desc_lim.nb_max` as well as
> `rte_eth_dev_info.tx_desc_lim.nb_max` shows 65535 as the limit,
> which results in a few problems:
> 
> * It is an incorrect value
> * Allocating an RX queue and passing `rx_desc_lim.nb_max` results in an
>   integer overflow and 0 ring size:
> 
> ```
> rte_eth_rx_queue_setup(0, 0, rx_desc_lim.nb_max, 0, NULL, mb_pool);
> ```
> 
> Which overflows ring size and generates the following log:
> ```
> mlx5_net: port 0 increased number of descriptors in Rx queue 0 to the
> next power of two (0)
> ```
> The same holds for allocating a TX queue.
> 
> This patch fixes these issues.
> 
> Signed-off-by: Igor Gutorov <igootorov@gmail.com>
> ---
>  drivers/common/mlx5/mlx5_devx_cmds.c | 1 +
>  drivers/common/mlx5/mlx5_devx_cmds.h | 1 +
>  drivers/net/mlx5/mlx5_ethdev.c       | 4 ++++
>  drivers/net/mlx5/mlx5_rxq.c          | 8 ++++++++
>  drivers/net/mlx5/mlx5_txq.c          | 8 ++++++++
>  5 files changed, 22 insertions(+)
> 
> diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c b/drivers/common/mlx5/mlx5_devx_cmds.c
> index 9b7ababae7..fe7a53835a 100644
> --- a/drivers/common/mlx5/mlx5_devx_cmds.c
> +++ b/drivers/common/mlx5/mlx5_devx_cmds.c
> @@ -1027,6 +1027,7 @@ mlx5_devx_cmd_query_hca_attr(void *ctx,
>  	attr->log_max_qp = MLX5_GET(cmd_hca_cap, hcattr, log_max_qp);
>  	attr->log_max_cq_sz = MLX5_GET(cmd_hca_cap, hcattr, log_max_cq_sz);
>  	attr->log_max_qp_sz = MLX5_GET(cmd_hca_cap, hcattr, log_max_qp_sz);
> +	attr->log_max_wq_sz = MLX5_GET(cmd_hca_cap, hcattr, log_max_wq_sz);
>  	attr->log_max_mrw_sz = MLX5_GET(cmd_hca_cap, hcattr, log_max_mrw_sz);
>  	attr->log_max_pd = MLX5_GET(cmd_hca_cap, hcattr, log_max_pd);
>  	attr->log_max_srq = MLX5_GET(cmd_hca_cap, hcattr, log_max_srq);
> diff --git a/drivers/common/mlx5/mlx5_devx_cmds.h b/drivers/common/mlx5/mlx5_devx_cmds.h
> index c79f8dc48d..abd12ea6e1 100644
> --- a/drivers/common/mlx5/mlx5_devx_cmds.h
> +++ b/drivers/common/mlx5/mlx5_devx_cmds.h
> @@ -267,6 +267,7 @@ struct mlx5_hca_attr {
>  	struct mlx5_hca_flow_attr flow;
>  	struct mlx5_hca_flex_attr flex;
>  	struct mlx5_hca_crypto_mmo_attr crypto_mmo;
> +	int log_max_wq_sz;
>  	int log_max_qp_sz;
>  	int log_max_cq_sz;
>  	int log_max_qp;
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index aea799341c..4ef47b3cc7 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -345,6 +345,10 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
>  	info->flow_type_rss_offloads = ~MLX5_RSS_HF_MASK;
>  	mlx5_set_default_params(dev, info);
>  	mlx5_set_txlimit_params(dev, info);
> +	info->rx_desc_lim.nb_max =
> +		1 << priv->sh->cdev->config.hca_attr.log_max_wq_sz;
> +	info->tx_desc_lim.nb_max =
> +		1 << priv->sh->cdev->config.hca_attr.log_max_wq_sz;
>  	if (priv->sh->cdev->config.hca_attr.mem_rq_rmp &&
>  	    priv->obj_ops.rxq_obj_new == devx_obj_ops.rxq_obj_new)
>  		info->dev_capa |= RTE_ETH_DEV_CAPA_RXQ_SHARE;
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index f67aaa6178..f66a283983 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -655,6 +655,14 @@ mlx5_rx_queue_pre_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t *desc,
>  	struct mlx5_rxq_priv *rxq;
>  	bool empty;
>  
> +	if (*desc > 1 << priv->sh->cdev->config.hca_attr.log_max_wq_sz) {
> +		DRV_LOG(ERR,
> +			"port %u number of descriptors requested for Rx queue"
> +			" %u is more than supported",
> +			dev->data->port_id, idx);
> +		rte_errno = EINVAL;
> +		return -EINVAL;
> +	}
>  	if (!rte_is_power_of_2(*desc)) {
>  		*desc = 1 << log2above(*desc);
>  		DRV_LOG(WARNING,
> diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
> index da4236f99a..07128c37e7 100644
> --- a/drivers/net/mlx5/mlx5_txq.c
> +++ b/drivers/net/mlx5/mlx5_txq.c
> @@ -332,6 +332,14 @@ mlx5_tx_queue_pre_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t *desc)
>  {
>  	struct mlx5_priv *priv = dev->data->dev_private;
>  
> +	if (*desc > 1 << priv->sh->cdev->config.hca_attr.log_max_wq_sz) {
> +		DRV_LOG(ERR,
> +			"port %u number of descriptors requested for Tx queue"
> +			" %u is more than supported",

Would help to print out value, something like:
			"Port %u Tx descriptors %u exceeds limit %u"

> +			dev->data->port_id, idx);
> +		rte_errno = EINVAL;
> +		return -EINVAL;
> +	}
>  	if (*desc <= MLX5_TX_COMP_THRESH) {
>  		DRV_LOG(WARNING,
>  			"port %u number of descriptors requested for Tx queue"

It is preferable not to break error messages across source lines.
Often, long messages are not that helpful anyway.

The values for log_max_XXX should really be unsigned and ideally a type that corresponds
to a width (like uint32).
  

Patch

diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c b/drivers/common/mlx5/mlx5_devx_cmds.c
index 9b7ababae7..fe7a53835a 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.c
+++ b/drivers/common/mlx5/mlx5_devx_cmds.c
@@ -1027,6 +1027,7 @@  mlx5_devx_cmd_query_hca_attr(void *ctx,
 	attr->log_max_qp = MLX5_GET(cmd_hca_cap, hcattr, log_max_qp);
 	attr->log_max_cq_sz = MLX5_GET(cmd_hca_cap, hcattr, log_max_cq_sz);
 	attr->log_max_qp_sz = MLX5_GET(cmd_hca_cap, hcattr, log_max_qp_sz);
+	attr->log_max_wq_sz = MLX5_GET(cmd_hca_cap, hcattr, log_max_wq_sz);
 	attr->log_max_mrw_sz = MLX5_GET(cmd_hca_cap, hcattr, log_max_mrw_sz);
 	attr->log_max_pd = MLX5_GET(cmd_hca_cap, hcattr, log_max_pd);
 	attr->log_max_srq = MLX5_GET(cmd_hca_cap, hcattr, log_max_srq);
diff --git a/drivers/common/mlx5/mlx5_devx_cmds.h b/drivers/common/mlx5/mlx5_devx_cmds.h
index c79f8dc48d..abd12ea6e1 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.h
+++ b/drivers/common/mlx5/mlx5_devx_cmds.h
@@ -267,6 +267,7 @@  struct mlx5_hca_attr {
 	struct mlx5_hca_flow_attr flow;
 	struct mlx5_hca_flex_attr flex;
 	struct mlx5_hca_crypto_mmo_attr crypto_mmo;
+	int log_max_wq_sz;
 	int log_max_qp_sz;
 	int log_max_cq_sz;
 	int log_max_qp;
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index aea799341c..4ef47b3cc7 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -345,6 +345,10 @@  mlx5_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
 	info->flow_type_rss_offloads = ~MLX5_RSS_HF_MASK;
 	mlx5_set_default_params(dev, info);
 	mlx5_set_txlimit_params(dev, info);
+	info->rx_desc_lim.nb_max =
+		1 << priv->sh->cdev->config.hca_attr.log_max_wq_sz;
+	info->tx_desc_lim.nb_max =
+		1 << priv->sh->cdev->config.hca_attr.log_max_wq_sz;
 	if (priv->sh->cdev->config.hca_attr.mem_rq_rmp &&
 	    priv->obj_ops.rxq_obj_new == devx_obj_ops.rxq_obj_new)
 		info->dev_capa |= RTE_ETH_DEV_CAPA_RXQ_SHARE;
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index f67aaa6178..f66a283983 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -655,6 +655,14 @@  mlx5_rx_queue_pre_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t *desc,
 	struct mlx5_rxq_priv *rxq;
 	bool empty;
 
+	if (*desc > 1 << priv->sh->cdev->config.hca_attr.log_max_wq_sz) {
+		DRV_LOG(ERR,
+			"port %u number of descriptors requested for Rx queue"
+			" %u is more than supported",
+			dev->data->port_id, idx);
+		rte_errno = EINVAL;
+		return -EINVAL;
+	}
 	if (!rte_is_power_of_2(*desc)) {
 		*desc = 1 << log2above(*desc);
 		DRV_LOG(WARNING,
diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
index da4236f99a..07128c37e7 100644
--- a/drivers/net/mlx5/mlx5_txq.c
+++ b/drivers/net/mlx5/mlx5_txq.c
@@ -332,6 +332,14 @@  mlx5_tx_queue_pre_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t *desc)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 
+	if (*desc > 1 << priv->sh->cdev->config.hca_attr.log_max_wq_sz) {
+		DRV_LOG(ERR,
+			"port %u number of descriptors requested for Tx queue"
+			" %u is more than supported",
+			dev->data->port_id, idx);
+		rte_errno = EINVAL;
+		return -EINVAL;
+	}
 	if (*desc <= MLX5_TX_COMP_THRESH) {
 		DRV_LOG(WARNING,
 			"port %u number of descriptors requested for Tx queue"