Re: [dpdk-dev] [PATCH] ethdev: add support for WRED thresholds in bytes

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----Original Message-----
> From: Rao, Nikhil
> Sent: Tuesday, February 20, 2018 1:31 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; Shelton, Benjamin H <benjamin.h.shelton@intel.com>;
> Vangati, Narender <narender.vangati@intel.com>; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>; Rao, Nikhil <nikhil.rao@intel.com>
> Subject: [PATCH] ethdev: add support for WRED thresholds in bytes
> 
> WRED thresholds can be specified in bytes if the TM leaf
> node supports it. Also extend WRED thresholds to 32 bits from 16.
> 
> TM capability (port/level/queue) fields
> cman_wred_packet_mode_supported and
> cman_wred_byte_mode_supported, when non-zero, indicate support for
> WRED
> thresholds in packets and bytes respectively.
> 
> The packet_mode member of struct rte_tm_wred_params, when non-zero,
> indicates that the min and max thresholds are specified in
> packets and when zero, indicates that the min and max thresholds
> are specified in bytes.
> 
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> ---
> 
> Thanks to Cristian for his help with developing this patch.
> 
>  lib/librte_ether/rte_tm.h                | 53
> ++++++++++++++++++++++++++++++--
>  drivers/net/softnic/rte_eth_softnic_tm.c | 22 +++++++++++++
>  2 files changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_tm.h b/lib/librte_ether/rte_tm.h
> index 2b25a87..23098ab 100644
> --- a/lib/librte_ether/rte_tm.h
> +++ b/lib/librte_ether/rte_tm.h
> @@ -377,6 +377,22 @@ struct rte_tm_capabilities {
>  	 */
>  	uint32_t sched_wfq_weight_max;
> 
> +	/** WRED packet mode support. When non-zero, this parameter
> indicates
> +	 * that there is atleast one leaf node that supports the WRED packet
> +	 * mode, which might not be true for all the leaf nodes. In packet
> +	 * mode, the WRED thresholds specify the queue length in packets,
> as
> +	 * opposed to bytes.
> +	 */
> +	int cman_wred_packet_mode_supported;
> +
> +	/** WRED byte mode support. When non-zero, this parameter
> indicates that
> +	 * there is atleast one leaf node that supports the WRED byte mode,
> +	 * which might not be true for all the leaf nodes. In byte mode, the
> +	 * WRED thresholds specify the queue length in bytes, as opposed to
> +	 * packets.
> +	 */
> +	int cman_wred_byte_mode_supported;
> +
>  	/** Head drop algorithm support. When non-zero, this parameter
>  	 * indicates that there is at least one leaf node that supports the
> head
>  	 * drop algorithm, which might not be true for all the leaf nodes.
> @@ -628,6 +644,24 @@ struct rte_tm_level_capabilities {
>  			 */
>  			uint32_t shaper_shared_n_max;
> 
> +			/** WRED packet mode support. When non-zero,
> this
> +			 * parameter indicates that there is atleast one leaf
> +			 * node on this level that supports the WRED packet
> +			 * mode, which might not be true for all the leaf
> +			 * nodes. In packet mode, the WRED thresholds
> specify
> +			 * the queue length in packets, as opposed to bytes.
> +			 */
> +			int cman_wred_packet_mode_supported;
> +
> +			/** WRED byte mode support. When non-zero, this
> +			 * parameter indicates that there is atleast one leaf
> +			 * node on this level that supports the WRED byte
> mode,
> +			 * which might not be true for all the leaf nodes. In
> +			 * byte mode, the WRED thresholds specify the
> queue
> +			 * length in bytes, as opposed to packets.
> +			 */
> +			int cman_wred_byte_mode_supported;
> +
>  			/** Head drop algorithm support. When non-zero,
> this
>  			 * parameter indicates that there is at least one leaf
>  			 * node on this level that supports the head drop
> @@ -743,6 +777,12 @@ struct rte_tm_node_capabilities {
> 
>  		/** Items valid only for leaf nodes. */
>  		struct {
> +			/** WRED packet mode support for current node. */
> +			int cman_wred_packet_mode_supported;
> +
> +			/** WRED byte mode support for current node. */
> +			int cman_wred_byte_mode_supported;
> +
>  			/** Head drop algorithm support for current node.
> */
>  			int cman_head_drop_supported;
> 
> @@ -791,10 +831,10 @@ enum rte_tm_cman_mode {
>   */
>  struct rte_tm_red_params {
>  	/** Minimum queue threshold */
> -	uint16_t min_th;
> +	uint32_t min_th;
> 
>  	/** Maximum queue threshold */
> -	uint16_t max_th;
> +	uint32_t max_th;
> 
>  	/** Inverse of packet marking probability maximum value (maxp),
> i.e.
>  	 * maxp_inv = 1 / maxp
> @@ -815,10 +855,19 @@ struct rte_tm_red_params {
>   * WRED context is used to perform congestion management for a single
> leaf
>   * node, while a shared WRED context is used to perform congestion
> management
>   * for a group of leaf nodes.
> + *
> + * @see struct rte_tm_capabilities::cman_wred_packet_mode_supported
> + * @see struct rte_tm_capabilities::cman_wred_byte_mode_supported
>   */
>  struct rte_tm_wred_params {
>  	/** One set of RED parameters per packet color */
>  	struct rte_tm_red_params red_params[RTE_TM_COLORS];
> +
> +	/** When non-zero, the *min_th* and *max_th* thresholds are
> specified
> +	 * in packets (WRED packet mode). When zero, the *min_th* and
> *max_th*
> +	 * thresholds are specified in bytes (WRED byte mode)
> +	 */
> +	int packet_mode;
>  };
> 
>  /**
> diff --git a/drivers/net/softnic/rte_eth_softnic_tm.c
> b/drivers/net/softnic/rte_eth_softnic_tm.c
> index 79f1c6a..e915c36 100644
> --- a/drivers/net/softnic/rte_eth_softnic_tm.c
> +++ b/drivers/net/softnic/rte_eth_softnic_tm.c
> @@ -479,6 +479,8 @@
>  	.sched_wfq_n_groups_max = 1,
>  	.sched_wfq_weight_max = UINT32_MAX,
> 
> +	.cman_wred_packet_mode_supported = WRED_SUPPORTED,
> +	.cman_wred_byte_mode_supported = 0,
>  	.cman_head_drop_supported = 0,
>  	.cman_wred_context_n_max = 0,
>  	.cman_wred_context_private_n_max = 0,
> @@ -667,6 +669,8 @@
>  			.shaper_shared_n_max = 0,
> 
>  			.cman_head_drop_supported = 0,
> +			.cman_wred_packet_mode_supported =
> WRED_SUPPORTED,
> +			.cman_wred_byte_mode_supported = 0,
>  			.cman_wred_context_private_supported =
> WRED_SUPPORTED,
>  			.cman_wred_context_shared_n_max = 0,
> 
> @@ -828,6 +832,8 @@
> 
>  		{.leaf = {
>  			.cman_head_drop_supported = 0,
> +			.cman_wred_packet_mode_supported =
> WRED_SUPPORTED,
> +			.cman_wred_byte_mode_supported = 0,
>  			.cman_wred_context_private_supported =
> WRED_SUPPORTED,
>  			.cman_wred_context_shared_n_max = 0,
>  		} },
> @@ -1226,6 +1232,14 @@
>  			NULL,
>  			rte_strerror(EINVAL));
> 
> +	/* WRED profile should be in packet mode */
> +	if (profile->packet_mode == 0)
> +		return -rte_tm_error_set(error,
> +			ENOTSUP,
> +			RTE_TM_ERROR_TYPE_WRED_PROFILE,
> +			NULL,
> +			rte_strerror(ENOTSUP));
> +
>  	/* WRED profile must not exist. */
>  	wp = tm_wred_profile_search(dev, wred_profile_id);
>  	if (wp)
> @@ -1248,6 +1262,14 @@
>  		uint16_t min_th = profile->red_params[color].min_th;
>  		uint16_t max_th = profile->red_params[color].max_th;
> 
> +		if (profile->red_params[color].min_th >= (1 << 16) ||
> +				profile->red_params[color].max_th >= (1 <<
> 16))
> +			return -rte_tm_error_set(error,
> +						EINVAL,
> +
> 	RTE_TM_ERROR_TYPE_WRED_PROFILE,
> +						NULL,
> +						rte_strerror(EINVAL));
> +
>  		if (min_th > max_th || max_th == 0)
>  			return -rte_tm_error_set(error,
>  				EINVAL,
> --
> 1.8.3.1

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Applied to next-qos tree with some minor adjustments in file rte_eth_softnic_tm.c:
- moved check on profile->packet_mode after the check that profile is not NULL
- consolidated checks on min_th and max_th under a single block
- replaced 1 << 16 with UINT16_MAX

Thanks!