* [PATCH 0/1] net/mlx5: show rx/tx descriptor ring limitations in rte_eth_dev_info @ 2024-06-16 17:38 Igor Gutorov 2024-06-16 17:38 ` [PATCH 1/1] " Igor Gutorov 0 siblings, 1 reply; 6+ messages in thread From: Igor Gutorov @ 2024-06-16 17:38 UTC (permalink / raw) To: dsosnowski, viacheslavo, orika, suanmingm, matan; +Cc: dev, Igor Gutorov Hello, This patch fixes an issue with my ConnectX5 NIC (15b3:1019) showing 65535 as the maximum number of RX descriptors. So, a disclaimer: I don't know whether other NICs (ConnectX6?) allow larger values. The value of 8192 was obtained experimentally with ethtool: ethtool -G *nic* rx 8192, as I do not have the NIC programmer's manual. Using anything larger than 8192 resulted in an error. Similarly, anything in the [8193, 32768) range with the DPDK PMD results in an rx queue allocation failure. Igor Gutorov (1): net/mlx5: show rx/tx descriptor ring limitations in rte_eth_dev_info drivers/net/mlx5/mlx5_defs.h | 3 +++ drivers/net/mlx5/mlx5_ethdev.c | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) -- 2.45.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] net/mlx5: show rx/tx descriptor ring limitations in rte_eth_dev_info 2024-06-16 17:38 [PATCH 0/1] net/mlx5: show rx/tx descriptor ring limitations in rte_eth_dev_info Igor Gutorov @ 2024-06-16 17:38 ` Igor Gutorov 2024-06-17 7:18 ` Slava Ovsiienko 0 siblings, 1 reply; 6+ messages in thread From: Igor Gutorov @ 2024-06-16 17:38 UTC (permalink / raw) To: dsosnowski, viacheslavo, orika, suanmingm, matan; +Cc: dev, Igor Gutorov Currently, rte_eth_dev_info.rx_desc_lim.nb_max shows 65535 as a 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) ``` This patch fixes these issues. Signed-off-by: Igor Gutorov <igootorov@gmail.com> --- drivers/net/mlx5/mlx5_defs.h | 3 +++ drivers/net/mlx5/mlx5_ethdev.c | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h index dc5216cb24..df608f0921 100644 --- a/drivers/net/mlx5/mlx5_defs.h +++ b/drivers/net/mlx5/mlx5_defs.h @@ -84,6 +84,9 @@ #define MLX5_RX_DEFAULT_BURST 64U #define MLX5_TX_DEFAULT_BURST 64U +/* Maximum number of descriptors in an RX/TX ring */ +#define MLX5_MAX_RING_DESC 8192 + /* Number of packets vectorized Rx can simultaneously process in a loop. */ #define MLX5_VPMD_DESCS_PER_LOOP 4 diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c index aea799341c..d5be1ff1aa 100644 --- a/drivers/net/mlx5/mlx5_ethdev.c +++ b/drivers/net/mlx5/mlx5_ethdev.c @@ -22,6 +22,7 @@ #include <mlx5_malloc.h> +#include "mlx5_defs.h" #include "mlx5_rxtx.h" #include "mlx5_rx.h" #include "mlx5_tx.h" @@ -345,6 +346,8 @@ 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 = MLX5_MAX_RING_DESC; + info->tx_desc_lim.nb_max = MLX5_MAX_RING_DESC; 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; @@ -774,7 +777,7 @@ mlx5_hairpin_cap_get(struct rte_eth_dev *dev, struct rte_eth_hairpin_cap *cap) cap->max_nb_queues = UINT16_MAX; cap->max_rx_2_tx = 1; cap->max_tx_2_rx = 1; - cap->max_nb_desc = 8192; + cap->max_nb_desc = MLX5_MAX_RING_DESC; hca_attr = &priv->sh->cdev->config.hca_attr; cap->rx_cap.locked_device_memory = hca_attr->hairpin_data_buffer_locked; cap->rx_cap.rte_memory = 0; -- 2.45.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 1/1] net/mlx5: show rx/tx descriptor ring limitations in rte_eth_dev_info 2024-06-16 17:38 ` [PATCH 1/1] " Igor Gutorov @ 2024-06-17 7:18 ` Slava Ovsiienko 2024-06-18 22:56 ` [PATCH v2 0/1] net/mlx5: fix incorrect rx/tx descriptor " Igor Gutorov 0 siblings, 1 reply; 6+ messages in thread From: Slava Ovsiienko @ 2024-06-17 7:18 UTC (permalink / raw) To: Igor Gutorov, Dariusz Sosnowski, Ori Kam, Suanming Mou, Matan Azrad; +Cc: dev Hi, Igor Thank you for the patch. 1. The absolute max descriptor number supported by ConnectX hardware is 32768. 2. The actual max descriptor number supported by the port (and its related representors) reported in log_max_wq_sz in HCA.caps. This value should be queried and save in mlx5_devx_cmd_query_hca_attr() routine. 3. mlx5_rx_queue_pre_setup() should check requested descriptor number and reject if it exceeds log_max_wq_sz 4. Please, format your patch according to the "fix" template. With best regards, Slava > -----Original Message----- > From: Igor Gutorov <igootorov@gmail.com> > Sent: Sunday, June 16, 2024 8:38 PM > To: Dariusz Sosnowski <dsosnowski@nvidia.com>; Slava Ovsiienko > <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; Suanming Mou > <suanmingm@nvidia.com>; Matan Azrad <matan@nvidia.com> > Cc: dev@dpdk.org; Igor Gutorov <igootorov@gmail.com> > Subject: [PATCH 1/1] net/mlx5: show rx/tx descriptor ring limitations in > rte_eth_dev_info > > Currently, rte_eth_dev_info.rx_desc_lim.nb_max shows 65535 as a 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) ``` > > This patch fixes these issues. > > Signed-off-by: Igor Gutorov <igootorov@gmail.com> > --- > drivers/net/mlx5/mlx5_defs.h | 3 +++ > drivers/net/mlx5/mlx5_ethdev.c | 5 ++++- > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h index > dc5216cb24..df608f0921 100644 > --- a/drivers/net/mlx5/mlx5_defs.h > +++ b/drivers/net/mlx5/mlx5_defs.h > @@ -84,6 +84,9 @@ > #define MLX5_RX_DEFAULT_BURST 64U > #define MLX5_TX_DEFAULT_BURST 64U > > +/* Maximum number of descriptors in an RX/TX ring */ #define > +MLX5_MAX_RING_DESC 8192 > + > /* Number of packets vectorized Rx can simultaneously process in a loop. */ > #define MLX5_VPMD_DESCS_PER_LOOP 4 > > diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c > index aea799341c..d5be1ff1aa 100644 > --- a/drivers/net/mlx5/mlx5_ethdev.c > +++ b/drivers/net/mlx5/mlx5_ethdev.c > @@ -22,6 +22,7 @@ > > #include <mlx5_malloc.h> > > +#include "mlx5_defs.h" > #include "mlx5_rxtx.h" > #include "mlx5_rx.h" > #include "mlx5_tx.h" > @@ -345,6 +346,8 @@ 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 = MLX5_MAX_RING_DESC; > + info->tx_desc_lim.nb_max = MLX5_MAX_RING_DESC; > 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; @@ - > 774,7 +777,7 @@ mlx5_hairpin_cap_get(struct rte_eth_dev *dev, struct > rte_eth_hairpin_cap *cap) > cap->max_nb_queues = UINT16_MAX; > cap->max_rx_2_tx = 1; > cap->max_tx_2_rx = 1; > - cap->max_nb_desc = 8192; > + cap->max_nb_desc = MLX5_MAX_RING_DESC; > hca_attr = &priv->sh->cdev->config.hca_attr; > cap->rx_cap.locked_device_memory = hca_attr- > >hairpin_data_buffer_locked; > cap->rx_cap.rte_memory = 0; > -- > 2.45.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 0/1] net/mlx5: fix incorrect rx/tx descriptor limitations in rte_eth_dev_info 2024-06-17 7:18 ` Slava Ovsiienko @ 2024-06-18 22:56 ` Igor Gutorov 2024-06-18 22:56 ` [PATCH v2] " Igor Gutorov 2024-06-23 11:34 ` [PATCH v2 0/1] " Slava Ovsiienko 0 siblings, 2 replies; 6+ messages in thread From: Igor Gutorov @ 2024-06-18 22:56 UTC (permalink / raw) To: viacheslavo; +Cc: dev, dsosnowski, igootorov, matan, orika, suanmingm Hi, Slava > 1. The absolute max descriptor number supported by ConnectX hardware is 32768. > 2. The actual max descriptor number supported by the port (and its related representors) > reported in log_max_wq_sz in HCA.caps. This value should be queried and save in mlx5_devx_cmd_query_hca_attr() routine. > 3. mlx5_rx_queue_pre_setup() should check requested descriptor number and reject if it exceeds log_max_wq_sz Thank you for the guidelines! I've also added the same check to mlx5_tx_queue_pre_setup(), I'm assuming log_max_wq_sz can be used for both RX and TX. Is an `int` appropriate for `log_max_wq_sz`? Seems like a `uint8_t` is sufficient, but I've left it an `int` for consistency with the other `log_max_*` values. Also, I've noticed a similar issue with MTU, it is also reported as 65535 in `rte_eth_dev_info.max_mtu`. I'd like to send a separate patch to fix that too. What's the procedure for reading max supported MTU? > 4. Please, format your patch according to the "fix" template. I've reworded the commit message a little bit. But I don't see these issues on Bugzilla, I've stumbled upon them independently. If you'd like the bug reports to be created, let me know. Sincerely, Igor Igor Gutorov (1): net/mlx5: fix incorrect rx/tx descriptor limitations in rte_eth_dev_info 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(+) -- 2.45.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] net/mlx5: fix incorrect rx/tx descriptor limitations in rte_eth_dev_info 2024-06-18 22:56 ` [PATCH v2 0/1] net/mlx5: fix incorrect rx/tx descriptor " Igor Gutorov @ 2024-06-18 22:56 ` Igor Gutorov 2024-06-23 11:34 ` [PATCH v2 0/1] " Slava Ovsiienko 1 sibling, 0 replies; 6+ messages in thread From: Igor Gutorov @ 2024-06-18 22:56 UTC (permalink / raw) To: viacheslavo; +Cc: dev, dsosnowski, igootorov, matan, orika, suanmingm 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", + 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" -- 2.45.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2 0/1] net/mlx5: fix incorrect rx/tx descriptor limitations in rte_eth_dev_info 2024-06-18 22:56 ` [PATCH v2 0/1] net/mlx5: fix incorrect rx/tx descriptor " Igor Gutorov 2024-06-18 22:56 ` [PATCH v2] " Igor Gutorov @ 2024-06-23 11:34 ` Slava Ovsiienko 1 sibling, 0 replies; 6+ messages in thread From: Slava Ovsiienko @ 2024-06-23 11:34 UTC (permalink / raw) To: Igor Gutorov; +Cc: dev, Dariusz Sosnowski, Matan Azrad, Ori Kam, Suanming Mou Hi, Igor Thank you for the v2. The patch looks good to me, please see my further comments below. > > 1. The absolute max descriptor number supported by ConnectX hardware is > 32768. > > 2. The actual max descriptor number supported by the port (and its related > representors) > > reported in log_max_wq_sz in HCA.caps. This value should be queried and > save in mlx5_devx_cmd_query_hca_attr() routine. > > 3. mlx5_rx_queue_pre_setup() should check requested descriptor number > > and reject if it exceeds log_max_wq_sz > > Thank you for the guidelines! I've also added the same check to > mlx5_tx_queue_pre_setup(), I'm assuming log_max_wq_sz can be used for > both RX and TX. > > Is an `int` appropriate for `log_max_wq_sz`? Seems like a `uint8_t` is sufficient, > but I've left it an `int` for consistency with the other `log_max_*` values. Right, uint8_t looks to be enough. No objection to optimize others to uint8_t. > Also, I've noticed a similar issue with MTU, it is also reported as 65535 in > `rte_eth_dev_info.max_mtu`. I'd like to send a separate patch to fix that too. > What's the procedure for reading max supported MTU? MTU is not reported directly by HCA. It is per port settings and can be read from PMTU - Port MTU Register. ACCESS_REGISTER command should be used. Please, see: https://network.nvidia.com/files/doc-2020/ethernet-adapters-programming-manual.pdf And thorough testing of accessing this register is needed - over physical port, over the representors, over the VFs and SFs. Rollback to 0xFFFF should be implemented, if register can't be accessed. Also, this reported max MTU might be not supported for SFs/VFs, where MTU is defined by hypervisor settings. > > > 4. Please, format your patch according to the "fix" template. > > I've reworded the commit message a little bit. But I don't see these issues on > Bugzilla, I've stumbled upon them independently. If you'd like the bug reports to > be created, let me know. I meant this: https://doc.dpdk.org/guides/contributing/patches.html Please see chapter "8.7. Commit Messages: Body" about "Fixes" and "Cc: stable@dpdk.org". Also, please run checking script: /devtools/check-git-log.sh' -1 to verify commit message compliance. With best regards, Slava ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-06-23 11:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-06-16 17:38 [PATCH 0/1] net/mlx5: show rx/tx descriptor ring limitations in rte_eth_dev_info Igor Gutorov 2024-06-16 17:38 ` [PATCH 1/1] " Igor Gutorov 2024-06-17 7:18 ` Slava Ovsiienko 2024-06-18 22:56 ` [PATCH v2 0/1] net/mlx5: fix incorrect rx/tx descriptor " Igor Gutorov 2024-06-18 22:56 ` [PATCH v2] " Igor Gutorov 2024-06-23 11:34 ` [PATCH v2 0/1] " Slava Ovsiienko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).