RE: [PATCH 3/3] net/mlx5: fix missing adjustment MPRQ stride devargs

[Raslan Darawsheh <rasland@nvidia.com>]

Hi,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Tuesday, November 23, 2021 8:38 PM
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Michael
> Baum <michaelba@nvidia.com>; stable@dpdk.org
> Subject: [PATCH 3/3] net/mlx5: fix missing adjustment MPRQ stride devargs
> 
> From: Michael Baum <michaelba@nvidia.com>
> 
> In Multy-Packet RQ creation, the user can choose the number of strides and
> their size in bytes. The user updates it using specific devargs for both of
> these parameters.
> The above two parameters determine the size of the WQE which is actually
> their product of multiplication.
> 
> If the user selects values that are not in the supported range, the PMD
> changes them to default values. However, apart from the range limitations
> for each parameter individually there is also a minimum value on their
> multiplication. When the user selects values that their multiplication are
> lower than minimum value, no adjustment is made and the creation of the
> WQE fails.
> 
> This patch adds an adjustment in these cases as well. When the user selects
> values whose multiplication is lower than the minimum, they are replaced
> with the default values.
> 
> Fixes: ecb160456aed ("net/mlx5: add device parameter for MPRQ stride
> size")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
>  drivers/net/mlx5/linux/mlx5_os.c |  56 +++------
>  drivers/net/mlx5/mlx5.h          |   4 +
>  drivers/net/mlx5/mlx5_rxq.c      | 209 +++++++++++++++++++++----------
>  3 files changed, 159 insertions(+), 110 deletions(-)
> 
> diff --git a/drivers/net/mlx5/linux/mlx5_os.c
> b/drivers/net/mlx5/linux/mlx5_os.c
> index 70472efc29..3e496d68ea 100644
> --- a/drivers/net/mlx5/linux/mlx5_os.c
> +++ b/drivers/net/mlx5/linux/mlx5_os.c
> @@ -881,10 +881,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>  	unsigned int mpls_en = 0;
>  	unsigned int swp = 0;
>  	unsigned int mprq = 0;
> -	unsigned int mprq_min_stride_size_n = 0;
> -	unsigned int mprq_max_stride_size_n = 0;
> -	unsigned int mprq_min_stride_num_n = 0;
> -	unsigned int mprq_max_stride_num_n = 0;
>  	struct rte_ether_addr mac;
>  	char name[RTE_ETH_NAME_MAX_LEN];
>  	int own_domain_id = 0;
> @@ -1039,15 +1035,17 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>  			mprq_caps.max_single_wqe_log_num_of_strides);
>  		DRV_LOG(DEBUG, "\tsupported_qpts: %d",
>  			mprq_caps.supported_qpts);
> +		DRV_LOG(DEBUG, "\tmin_stride_wqe_log_size: %d",
> +			config->mprq.log_min_stride_wqe_size);
>  		DRV_LOG(DEBUG, "device supports Multi-Packet RQ");
>  		mprq = 1;
> -		mprq_min_stride_size_n =
> +		config->mprq.log_min_stride_size =
>  			mprq_caps.min_single_stride_log_num_of_bytes;
> -		mprq_max_stride_size_n =
> +		config->mprq.log_max_stride_size =
>  			mprq_caps.max_single_stride_log_num_of_bytes;
> -		mprq_min_stride_num_n =
> +		config->mprq.log_min_stride_num =
>  			mprq_caps.min_single_wqe_log_num_of_strides;
> -		mprq_max_stride_num_n =
> +		config->mprq.log_max_stride_num =
>  			mprq_caps.max_single_wqe_log_num_of_strides;
>  	}
>  #endif
> @@ -1548,36 +1546,7 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>  		config->hw_fcs_strip = 0;
>  	DRV_LOG(DEBUG, "FCS stripping configuration is %ssupported",
>  		(config->hw_fcs_strip ? "" : "not "));
> -	if (config->mprq.enabled && mprq) {
> -		if (config->mprq.log_stride_num &&
> -		    (config->mprq.log_stride_num >
> mprq_max_stride_num_n ||
> -		     config->mprq.log_stride_num <
> mprq_min_stride_num_n)) {
> -			config->mprq.log_stride_num =
> -
> RTE_MIN(RTE_MAX(MLX5_MPRQ_DEFAULT_LOG_STRIDE_NUM,
> -					       mprq_min_stride_num_n),
> -				       mprq_max_stride_num_n);
> -			DRV_LOG(WARNING,
> -				"the number of strides"
> -				" for Multi-Packet RQ is out of range,"
> -				" setting default value (%u)",
> -				1 << config->mprq.log_stride_num);
> -		}
> -		if (config->mprq.log_stride_size &&
> -		    (config->mprq.log_stride_size > mprq_max_stride_size_n
> ||
> -		     config->mprq.log_stride_size < mprq_min_stride_size_n))
> {
> -			config->mprq.log_stride_size =
> -
> RTE_MIN(RTE_MAX(MLX5_MPRQ_DEFAULT_LOG_STRIDE_SIZE,
> -					      mprq_min_stride_size_n),
> -				      mprq_max_stride_size_n);
> -			DRV_LOG(WARNING,
> -				"the size of a stride"
> -				" for Multi-Packet RQ is out of range,"
> -				" setting default value (%u)",
> -				1 << config->mprq.log_stride_size);
> -		}
> -		config->mprq.log_min_stride_size =
> mprq_min_stride_size_n;
> -		config->mprq.log_max_stride_size =
> mprq_max_stride_size_n;
> -	} else if (config->mprq.enabled && !mprq) {
> +	if (config->mprq.enabled && !mprq) {
>  		DRV_LOG(WARNING, "Multi-Packet RQ isn't supported");
>  		config->mprq.enabled = 0;
>  	}
> @@ -2068,7 +2037,8 @@ mlx5_device_bond_pci_match(const char
> *ibdev_name,  }
> 
>  static void
> -mlx5_os_config_default(struct mlx5_dev_config *config)
> +mlx5_os_config_default(struct mlx5_dev_config *config,
> +		       struct mlx5_common_dev_config *cconf)
>  {
>  	memset(config, 0, sizeof(*config));
>  	config->mps = MLX5_ARG_UNSET;
> @@ -2080,6 +2050,10 @@ mlx5_os_config_default(struct mlx5_dev_config
> *config)
>  	config->vf_nl_en = 1;
>  	config->mprq.max_memcpy_len =
> MLX5_MPRQ_MEMCPY_DEFAULT_LEN;
>  	config->mprq.min_rxqs_num = MLX5_MPRQ_MIN_RXQS;
> +	config->mprq.log_min_stride_wqe_size = cconf->devx ?
> +					cconf-
> >hca_attr.log_min_stride_wqe_sz :
> +
> 	MLX5_MPRQ_LOG_MIN_STRIDE_WQE_SIZE;
> +	config->mprq.log_stride_num =
> MLX5_MPRQ_DEFAULT_LOG_STRIDE_NUM;
>  	config->dv_esw_en = 1;
>  	config->dv_flow_en = 1;
>  	config->decap_en = 1;
> @@ -2496,7 +2470,7 @@ mlx5_os_pci_probe_pf(struct
> mlx5_common_device *cdev,
>  		uint32_t restore;
> 
>  		/* Default configuration. */
> -		mlx5_os_config_default(&dev_config);
> +		mlx5_os_config_default(&dev_config, &cdev->config);
>  		dev_config.vf = dev_config_vf;
>  		list[i].eth_dev = mlx5_dev_spawn(cdev->dev, &list[i],
>  						 &dev_config, &eth_da);
> @@ -2666,7 +2640,7 @@ mlx5_os_auxiliary_probe(struct
> mlx5_common_device *cdev)
>  	if (ret != 0)
>  		return ret;
>  	/* Set default config data. */
> -	mlx5_os_config_default(&config);
> +	mlx5_os_config_default(&config, &cdev->config);
>  	config.sf = 1;
>  	/* Init spawn data. */
>  	spawn.max_port = 1;
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> 4ba90db816..c01fb9566e 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -279,6 +279,10 @@ struct mlx5_dev_config {
>  		unsigned int log_stride_size; /* Log size of a stride. */
>  		unsigned int log_min_stride_size; /* Log min size of a
> stride.*/
>  		unsigned int log_max_stride_size; /* Log max size of a
> stride.*/
> +		unsigned int log_min_stride_num; /* Log min num of strides.
> */
> +		unsigned int log_max_stride_num; /* Log max num of
> strides. */
> +		unsigned int log_min_stride_wqe_size;
> +		/* Log min WQE size, (size of single stride)*(num of
> strides).*/
>  		unsigned int max_memcpy_len;
>  		/* Maximum packet size to memcpy Rx packets. */
>  		unsigned int min_rxqs_num;
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index e76bfaa000..891ac3d874 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -1528,6 +1528,126 @@ mlx5_max_lro_msg_size_adjust(struct
> rte_eth_dev *dev, uint16_t idx,
>  		priv->max_lro_msg_size * MLX5_LRO_SEG_CHUNK_SIZE);  }
> 
> +/**
> + * Prepare both size and number of stride for Multi-Packet RQ.
> + *
> + * @param dev
> + *   Pointer to Ethernet device.
> + * @param idx
> + *   RX queue index.
> + * @param desc
> + *   Number of descriptors to configure in queue.
> + * @param rx_seg_en
> + *   Indicator if Rx segment enables, if so Multi-Packet RQ doesn't enable.
> + * @param min_mbuf_size
> + *   Non scatter min mbuf size, max_rx_pktlen plus overhead.
> + * @param actual_log_stride_num
> + *   Log number of strides to configure for this queue.
> + * @param actual_log_stride_size
> + *   Log stride size to configure for this queue.
> + *
> + * @return
> + *   0 if Multi-Packet RQ is supported, otherwise -1.
> + */
> +static int
> +mlx5_mprq_prepare(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
> +		  bool rx_seg_en, uint32_t min_mbuf_size,
> +		  uint32_t *actual_log_stride_num,
> +		  uint32_t *actual_log_stride_size)
> +{
> +	struct mlx5_priv *priv = dev->data->dev_private;
> +	struct mlx5_dev_config *config = &priv->config;
> +	uint32_t log_min_stride_num = config->mprq.log_min_stride_num;
> +	uint32_t log_max_stride_num = config->mprq.log_max_stride_num;
> +	uint32_t log_def_stride_num =
> +
> 	RTE_MIN(RTE_MAX(MLX5_MPRQ_DEFAULT_LOG_STRIDE_NUM,
> +					log_min_stride_num),
> +				log_max_stride_num);
> +	uint32_t log_min_stride_size = config->mprq.log_min_stride_size;
> +	uint32_t log_max_stride_size = config->mprq.log_max_stride_size;
> +	uint32_t log_def_stride_size =
> +
> 	RTE_MIN(RTE_MAX(MLX5_MPRQ_DEFAULT_LOG_STRIDE_SIZE,
> +					log_min_stride_size),
> +				log_max_stride_size);
> +	uint32_t log_stride_wqe_size;
> +
> +	if (mlx5_check_mprq_support(dev) != 1 || rx_seg_en)
> +		goto unsupport;
typo in this label name, will fix during integration.
[..]
Kindest regards
Raslan Darawsheh