Re: [dpdk-dev] [PATCH 1/4] net/mlx5: add a devarg to specify MPRQ stride size

[Slava Ovsiienko <viacheslavo@mellanox.com>]

> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@mellanox.com>
> Sent: Wednesday, April 1, 2020 0:53
> To: dev@dpdk.org
> Cc: Raslan Darawsheh <rasland@mellanox.com>; Matan Azrad
> <matan@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> ferruh.yigit@intel.com; Thomas Monjalon <thomas@monjalon.net>
> Subject: [PATCH 1/4] net/mlx5: add a devarg to specify MPRQ stride size
> 
> Define a device parameter to configure log 2 of a stride size for MPRQ
> - mprq_log_stride_size. User is able to specify a stride size in a range allowed
> by an underlying hardware. The default stride size is defined as
> 2048 bytes to encompass most commonly used packet sizes in the Internet
> (MTU 1518 and less) and will be used in case a maximum configured packet
> size cannot fit into the largest possible stride size. Otherwise a stride size is
> set to a large enough value to encompass a whole packet.
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

> ---
>  drivers/net/mlx5/mlx5.c      | 34 +++++++++++++++++++++++++++-------
>  drivers/net/mlx5/mlx5.h      |  1 +
>  drivers/net/mlx5/mlx5_defs.h |  3 +++
>  drivers/net/mlx5/mlx5_rxq.c  | 22 +++++++++++++---------
>  4 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> 6a11b14..a2ba6d3 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -63,6 +63,9 @@
>  /* Device parameter to configure log 2 of the number of strides for MPRQ. */
> #define MLX5_RX_MPRQ_LOG_STRIDE_NUM "mprq_log_stride_num"
> 
> +/* Device parameter to configure log 2 of the stride size for MPRQ. */
> +#define MLX5_RX_MPRQ_LOG_STRIDE_SIZE "mprq_log_stride_size"
> +
>  /* Device parameter to limit the size of memcpy'd packet for MPRQ. */
> #define MLX5_RX_MPRQ_MAX_MEMCPY_LEN "mprq_max_memcpy_len"
> 
> @@ -1531,6 +1534,8 @@ struct mlx5_flow_id_pool *
>  		config->mprq.enabled = !!tmp;
>  	} else if (strcmp(MLX5_RX_MPRQ_LOG_STRIDE_NUM, key) == 0) {
>  		config->mprq.stride_num_n = tmp;
> +	} else if (strcmp(MLX5_RX_MPRQ_LOG_STRIDE_SIZE, key) == 0) {
> +		config->mprq.stride_size_n = tmp;
>  	} else if (strcmp(MLX5_RX_MPRQ_MAX_MEMCPY_LEN, key) == 0) {
>  		config->mprq.max_memcpy_len = tmp;
>  	} else if (strcmp(MLX5_RXQS_MIN_MPRQ, key) == 0) { @@ -1627,6
> +1632,7 @@ struct mlx5_flow_id_pool *
>  		MLX5_RXQ_PKT_PAD_EN,
>  		MLX5_RX_MPRQ_EN,
>  		MLX5_RX_MPRQ_LOG_STRIDE_NUM,
> +		MLX5_RX_MPRQ_LOG_STRIDE_SIZE,
>  		MLX5_RX_MPRQ_MAX_MEMCPY_LEN,
>  		MLX5_RXQS_MIN_MPRQ,
>  		MLX5_TXQ_INLINE,
> @@ -2302,8 +2308,6 @@ struct mlx5_flow_id_pool *
>  			mprq_caps.min_single_wqe_log_num_of_strides;
>  		mprq_max_stride_num_n =
>  			mprq_caps.max_single_wqe_log_num_of_strides;
> -		config.mprq.stride_num_n =
> RTE_MAX(MLX5_MPRQ_STRIDE_NUM_N,
> -						   mprq_min_stride_num_n);
>  	}
>  #endif
>  	if (RTE_CACHE_LINE_SIZE == 128 &&
> @@ -2617,17 +2621,32 @@ struct mlx5_flow_id_pool *  #endif
>  	}
>  	if (config.mprq.enabled && mprq) {
> -		if (config.mprq.stride_num_n > mprq_max_stride_num_n ||
> -		    config.mprq.stride_num_n < mprq_min_stride_num_n) {
> +		if (config.mprq.stride_num_n &&
> +		    (config.mprq.stride_num_n > mprq_max_stride_num_n ||
> +		     config.mprq.stride_num_n < mprq_min_stride_num_n)) {
>  			config.mprq.stride_num_n =
> -				RTE_MAX(MLX5_MPRQ_STRIDE_NUM_N,
> -					mprq_min_stride_num_n);
> +
> 	RTE_MIN(RTE_MAX(MLX5_MPRQ_STRIDE_NUM_N,
> +						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.stride_num_n);
>  		}
> +		if (config.mprq.stride_size_n &&
> +		    (config.mprq.stride_size_n > mprq_max_stride_size_n ||
> +		     config.mprq.stride_size_n < mprq_min_stride_size_n)) {
> +			config.mprq.stride_size_n =
> +
> 	RTE_MIN(RTE_MAX(MLX5_MPRQ_STRIDE_SIZE_N,
> +						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.stride_size_n);
> +		}
>  		config.mprq.min_stride_size_n = mprq_min_stride_size_n;
>  		config.mprq.max_stride_size_n = mprq_max_stride_size_n;
>  	} else if (config.mprq.enabled && !mprq) { @@ -3361,7 +3380,8 @@
> struct mlx5_flow_id_pool *
>  		.mr_ext_memseg_en = 1,
>  		.mprq = {
>  			.enabled = 0, /* Disabled by default. */
> -			.stride_num_n = MLX5_MPRQ_STRIDE_NUM_N,
> +			.stride_num_n = 0,
> +			.stride_size_n = 0,
>  			.max_memcpy_len =
> MLX5_MPRQ_MEMCPY_DEFAULT_LEN,
>  			.min_rxqs_num = MLX5_MPRQ_MIN_RXQS,
>  		},
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> 62b0810..c8e2454 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -176,6 +176,7 @@ struct mlx5_dev_config {
>  	struct {
>  		unsigned int enabled:1; /* Whether MPRQ is enabled. */
>  		unsigned int stride_num_n; /* Number of strides. */
> +		unsigned int stride_size_n; /* Size of a stride. */
>  		unsigned int min_stride_size_n; /* Min size of a stride. */
>  		unsigned int max_stride_size_n; /* Max size of a stride. */
>  		unsigned int max_memcpy_len;
> diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
> index 19e8253..260f584 100644
> --- a/drivers/net/mlx5/mlx5_defs.h
> +++ b/drivers/net/mlx5/mlx5_defs.h
> @@ -143,6 +143,9 @@
>  /* Log 2 of the default number of strides per WQE for Multi-Packet RQ. */
> #define MLX5_MPRQ_STRIDE_NUM_N 6U
> 
> +/* Log 2 of the default size of a stride per WQE for Multi-Packet RQ.
> +*/ #define MLX5_MPRQ_STRIDE_SIZE_N 11U
> +
>  /* Two-byte shift is disabled for Multi-Packet RQ. */  #define
> MLX5_MPRQ_TWO_BYTE_SHIFT 0
> 
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c index
> 0a95e3c..85fcfe6 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -1856,25 +1856,26 @@ struct mlx5_rxq_ctrl *
>  		strd_headroom_en = 1;
>  		mprq_stride_size = non_scatter_min_mbuf_size;
>  	}
> +	if (!config->mprq.stride_num_n)
> +		config->mprq.stride_num_n = MLX5_MPRQ_STRIDE_NUM_N;
> +	if (!config->mprq.stride_size_n)
> +		config->mprq.stride_size_n = (mprq_stride_size <=
> +				(1U << config->mprq.max_stride_size_n)) ?
> +			log2above(mprq_stride_size) :
> MLX5_MPRQ_STRIDE_SIZE_N;
>  	/*
>  	 * This Rx queue can be configured as a Multi-Packet RQ if all of the
>  	 * following conditions are met:
>  	 *  - MPRQ is enabled.
>  	 *  - The number of descs is more than the number of strides.
> -	 *  - max_rx_pkt_len plus overhead is less than the max size of a
> -	 *    stride.
>  	 *  Otherwise, enable Rx scatter if necessary.
>  	 */
> -	if (mprq_en &&
> -	    desc > (1U << config->mprq.stride_num_n) &&
> -	    mprq_stride_size <= (1U << config->mprq.max_stride_size_n)) {
> +	if (mprq_en && desc > (1U << config->mprq.stride_num_n)) {
>  		/* TODO: Rx scatter isn't supported yet. */
>  		tmpl->rxq.sges_n = 0;
>  		/* Trim the number of descs needed. */
>  		desc >>= config->mprq.stride_num_n;
>  		tmpl->rxq.strd_num_n = config->mprq.stride_num_n;
> -		tmpl->rxq.strd_sz_n = RTE_MAX(log2above(mprq_stride_size),
> -					      config->mprq.min_stride_size_n);
> +		tmpl->rxq.strd_sz_n = config->mprq.stride_size_n;
>  		tmpl->rxq.strd_shift_en = MLX5_MPRQ_TWO_BYTE_SHIFT;
>  		tmpl->rxq.strd_headroom_en = strd_headroom_en;
>  		tmpl->rxq.mprq_max_memcpy_len =
> RTE_MIN(first_mb_free_size, @@ -1924,9 +1925,12 @@ struct mlx5_rxq_ctrl *
>  		DRV_LOG(WARNING,
>  			"port %u MPRQ is requested but cannot be enabled"
>  			" (requested: desc = %u, stride_sz = %u,"
> -			" supported: min_stride_num = %u, max_stride_sz =
> %u).",
> -			dev->data->port_id, desc, mprq_stride_size,
> +			" supported: min_stride_num = %u, min_stride_sz =
> %u,"
> +			"max_stride_sz = %u).",
> +			dev->data->port_id, desc,
> +			(1 << config->mprq.stride_size_n),
>  			(1 << config->mprq.stride_num_n),
> +			(1 << config->mprq.min_stride_size_n),
>  			(1 << config->mprq.max_stride_size_n));
>  	DRV_LOG(DEBUG, "port %u maximum number of segments per
> packet: %u",
>  		dev->data->port_id, 1 << tmpl->rxq.sges_n);
> --
> 1.8.3.1