From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0E29B45AD9; Mon, 7 Oct 2024 22:37:50 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D4D5B40285; Mon, 7 Oct 2024 22:37:49 +0200 (CEST) Received: from mail-pg1-f181.google.com (mail-pg1-f181.google.com [209.85.215.181]) by mails.dpdk.org (Postfix) with ESMTP id 3211C4026C for ; Mon, 7 Oct 2024 22:37:49 +0200 (CEST) Received: by mail-pg1-f181.google.com with SMTP id 41be03b00d2f7-7db637d1e4eso3544183a12.2 for ; Mon, 07 Oct 2024 13:37:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1728333468; x=1728938268; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=9yI+H0LDlS1kfQmHCGFVmUCIW0RrUwr42b9Mf3DBnZg=; b=hobNSX+Q7xdh+sOgtbm3QCPrv/eIBtdTWpdO7dLCPm5N60ZuAH3PCBAWbO3Cr2tfGW LXUJV26GlyZaHUdskI4JQz1LZjsb7PSnwR69rr3p6+YXqHPDilKjaSYOHVjFiGeYK4nz qCIkJ3VeNheT7ypRYYDy1AgCcjBbkwZU9HaoNS9cU3Dy3TxlSt0wf9nINxgrQvn1gOEI xksUBmsuz8d/GXVyQRvjphCxPZ8x9C57UsSwWBFWc7pyaTfIi+upqfXrhTXLRmvREsWX Q+cN7CQQtDqGf3MY0khn5Z8s1MpAYqPGn7w9PApEoOClIgUln7bFDLF4+TWUolSMZaBB 9TwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728333468; x=1728938268; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=9yI+H0LDlS1kfQmHCGFVmUCIW0RrUwr42b9Mf3DBnZg=; b=dnK9Z0mFNn+tV9w0BFL/K+rv0jMh8oQhnd5z4d8G5lL7bFf1eEcs6+PaeIDRdTqIsW 6HL8dlikA2krc+AoH6j1+C4pFMq6Aj9o8UxrrE2dXpAnO8hFYbHAd3kwWfY+y/nU1kvJ OTMKV7LP7nmBanrdqs5JJehPHQBUKH45RB6dxrOk6XYS/6R3yRNFadsQwQJJgtgijFOi tr1o7uwEduAe0ioJ6uUb4a3YdiFx7LOnKhRTD8xtu1+eQVuZ3TMFmmebQJ0/z0ostNuv N1Gcy5xTZvpRIZdKt9aW4JFsFP1ekg2P/52aRMDMRRSfnt9CFU0xm7Lu2+2zWT8UAnVM x5EA== X-Forwarded-Encrypted: i=1; AJvYcCUWIcz5dzPUjZTrIVNrIpxY8xd63QfHsVmq9HOQ8cY0vXXtaqJHjL6inl6EyErgl+0+xf0=@dpdk.org X-Gm-Message-State: AOJu0Yz9fK7rQPBMu2w/jgEOChPZJ8FRCT/NuaX7UDPk/Ja+f6x0Tmzc hRuaCaein3wKKqeyqwdtRrIoJcUBo3+3mdwc5FWQPdKsCaInx0nxMwbAXu+NhUo= X-Google-Smtp-Source: AGHT+IFdogDzYyS14N7qPPRy5Vg6z4UlkxbRptCpwkr9eb3/5Zt/s2nkG8C9YN5fbzlr0VqggcVYcA== X-Received: by 2002:a05:6a20:d495:b0:1d5:1354:5256 with SMTP id adf61e73a8af0-1d6dfabad57mr20087967637.39.1728333468323; Mon, 07 Oct 2024 13:37:48 -0700 (PDT) Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7e9f682d056sm5414590a12.40.2024.10.07.13.37.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Oct 2024 13:37:48 -0700 (PDT) Date: Mon, 7 Oct 2024 13:37:46 -0700 From: Stephen Hemminger To: Igor Gutorov Cc: viacheslavo@nvidia.com, dev@dpdk.org, dsosnowski@nvidia.com, matan@nvidia.com, orika@nvidia.com, suanmingm@nvidia.com Subject: Re: [PATCH v2] net/mlx5: fix incorrect rx/tx descriptor limitations in rte_eth_dev_info Message-ID: <20241007133746.3ece1bd6@hermes.local> In-Reply-To: <20240618225642.1036624-2-igootorov@gmail.com> References: <20240618225642.1036624-1-igootorov@gmail.com> <20240618225642.1036624-2-igootorov@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, 19 Jun 2024 01:56:42 +0300 Igor Gutorov 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 > --- > 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).