[dpdk-dev] [PATCH 1/2] ethdev: add tm cap for private shaper packet mode

Dumitrescu, Cristian cristian.dumitrescu at intel.com
Tue Apr 7 18:31:47 CEST 2020


Hi Nithin,

> -----Original Message-----
> From: Nithin Dabilpuram <ndabilpuram at marvell.com>
> Sent: Monday, March 30, 2020 5:00 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu at intel.com>; Thomas Monjalon
> <thomas at monjalon.net>; Yigit, Ferruh <ferruh.yigit at intel.com>; Andrew
> Rybchenko <arybchenko at solarflare.com>
> Cc: dev at dpdk.org; jerinj at marvell.com; kkanas at marvell.com; Nithin
> Dabilpuram <ndabilpuram at marvell.com>
> Subject: [PATCH 1/2] ethdev: add tm cap for private shaper packet mode
> 
> Some NIC hardware have private shaper attached to
> every node and has a limitation where packet mode is applied
> both to the scheduling of a node's children using WFQ and
> shaping of traffic out of the private shaper.
> This cannot be expressed using existing capabilities or configurations.
> 
> So this patch adds a tm capability that if set by a PMD implies that
> packet mode when configured is even applied to private shaper
> connected to that node. This also implies the limitation
> that all the SP children of that node should have same mode
> at any point of time i.e either packet mode or byte mode and
> same applies to private shaper in that NIC PMD.
> 
> This patch also adds missing capability that tells whether PMD
> supports wfq weight mode or not.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram at marvell.com>
> ---
>  lib/librte_ethdev/rte_tm.h | 62
> +++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 59 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_tm.h b/lib/librte_ethdev/rte_tm.h
> index f9c0cf3..50bcea6 100644
> --- a/lib/librte_ethdev/rte_tm.h
> +++ b/lib/librte_ethdev/rte_tm.h
> @@ -339,6 +339,20 @@ struct rte_tm_capabilities {
>  	 */
>  	uint32_t sched_wfq_weight_max;
> 
> +	/** WFQ weight mode supported. Non-zero value indicates wfq
> weight mode
> +	 * is supported and a SP child (even a wfq group) can be configured
> to
> +	 * use packet-mode or byte-mode for weight calculations.
> +	 */
> +	int sched_wfq_weight_mode_supported;
> +

This is incorrect, as the WFQ support, including the weight aspect of WFQ, is already part of the existing set of capabilities: see sched_wfq_weight_max and the other sched_wfq_* capability fields.

> +	/** Private shaper and scheduler weight mode.
> +	 * When non-zero value indicates that all SP children should have
> +	 * same weight mode and the same mode applies to private
> +	 * shaper as well. This is only valid if
> +	 * *sched_wfq_weight_mode_supported* is set.
> +	 */
> +	int sched_shaper_private_weight_mode;
> +

If I understand your intention correctly, you are trying to introduce packet mode (in addition to the existing byte mode) for (1) scheduler WFQ weights and for (2) shaper rates. Basically, the ability to express WFQ weights in bytes as well as packets, and the ability to express shaper rates in bytes and well as packets. Is this correct?

Assuming yes, we probably need to do it in a slightly different way:
1/ Similar to the WRED packet mode that was introduced by Nikhil Rao's patches a while ago (in addition to WRED's byte mode), see WRED capability and configuration.
2/ Decouple between scheduler and shaper. So we should add sched_* fields and shaper_* fields, but never sched_shaper_*, as it creates a functional dependency that does not exist.

In line with methodology already used for WRED, I suggest:
a) Scheduler WFQ capabilities (TM/level/node): sched_wfq_packet_mode_supported, sched_wfq_byte_mode_supported
b) Shaper capabilities (TM/level/node): shaper_rate_packet_mode_supported, shaper_rate_byte_mode_supported.
c) Shaper profile (struct rte_tm_shaper_params): add an integer packet_mode flag with 0 = byte-mode (default) and 1 = packet mode for the values in struct rte_tm_token_bucket.

It is important to note that the API must allow a combination of packet mode and byte mode (for different nodes, not for the same), but an implementation can support either a single mode or both (should be enforced by the driver).

>  	/** WRED packet mode support. When non-zero, this parameter
> indicates
>  	 * that there is at least one leaf node that supports the WRED packet
>  	 * mode, which might not be true for all the leaf nodes. In packet
> @@ -554,6 +568,21 @@ struct rte_tm_level_capabilities {
>  			 */
>  			uint32_t sched_wfq_weight_max;
> 
> +			/** WFQ weight mode supported. Non-zero value
> indicates
> +			 * wfq weight mode is supported and a SP child
> +			 * (even a wfq group) can be configured to use
> +			 * packet-mode or byte-mode for weight
> calculations.
> +			 */
> +			int sched_wfq_weight_mode_supported;
> +
> +			/** Private shaper and scheduler weight mode.
> +			 * When non-zero value indicates that all SP children
> +			 * should have same weight mode and the same
> mode
> +			 * applies to private shaper as well. This is only
> +			 * valid if *sched_wfq_weight_mode_supported* is
> set.
> +			 */
> +			int sched_shaper_private_weight_mode;
> +
>  			/** Mask of statistics counter types supported by the
>  			 * non-leaf nodes on this level. Every supported
>  			 * statistics counter type is supported by at least one

See above the comments on TM capabilities.

> @@ -735,6 +764,21 @@ struct rte_tm_node_capabilities {
>  			 * WFQ weight, so WFQ is reduced to FQ.
>  			 */
>  			uint32_t sched_wfq_weight_max;
> +
> +			/** WFQ weight mode supported. Non-zero value
> indicates
> +			 * wfq weight mode is supported and a SP child
> +			 * (even a wfq group) can be configured to use
> +			 * packet-mode or byte-mode for weight
> calculations.
> +			 */
> +			int sched_wfq_weight_mode_supported;
> +
> +			/** Private shaper and scheduler weight mode.
> +			 * When non-zero value indicates that all SP children
> +			 * should have same weight mode and the same
> mode
> +			 * applies to private shaper as well. This is only
> +			 * valid if *sched_wfq_weight_mode_supported* is
> set.
> +			 */
> +			int sched_shaper_private_weight_mode;
>  		} nonleaf;
> 
>  		/** Items valid only for leaf nodes. */

See above the comments on the TM capabilities.

> @@ -836,10 +880,19 @@ struct rte_tm_wred_params {
>   * Token bucket
>   */
>  struct rte_tm_token_bucket {
> -	/** Token bucket rate (bytes per second) */
> +	/** Token bucket rate. This is in "bytes per second" by default.
> +	 * For private shaper attached to node that is set in packet mode
> +	 * and tm capability *sched_shaper_private_weight_mode* is set,
> +	 * this is interpreted as "packets per second".
> +	 */
>  	uint64_t rate;
> 
> -	/** Token bucket size (bytes), a.k.a. max burst size */
> +	/** Token bucket size, a.k.a. max burst size.
> +	 * This is in "bytes" by default.
> +	 * For private shaper attached to node that is set in packet mode
> +	 * and tm capability *sched_shaper_private_weight_mode* is set,
> +	 * this is interpreted as "packets".
> +	 */
>  	uint64_t size;
>  };
> 

Comments are not correct, as API should allow a combination of both the packet mode and the byte mode (for different nodes, not for the same node), so both capabilities shaper_rate_packet_mode and shaper_rate_byte_mode can be set. Hence, the comments should not specify a capability, but the fact that these values can specify either byte or packets, depending on a flag elsewhere.

> @@ -924,7 +977,10 @@ struct rte_tm_node_params {
>  			 * indicates that WFQ is to be used for all priorities.
>  			 * When non-NULL, it points to a pre-allocated array
> of
>  			 * *n_sp_priorities* values, with non-zero value for
> -			 * byte-mode and zero for packet-mode.
> +			 * byte-mode and zero for packet-mode. The same
> mode is
> +			 * used for private shaper connected to this node if
> +			 * tm capability
> *sched_shaper_private_weight_mode* is
> +			 * true.
>  			 */

This comment is incorrect, as sched should not be combined with shaper. The user should select between packet mode and byte mode for the WFQ weight independently of the mode for the shaper rate, although an implementation (driver) should enforce the correct values.

>  			int *wfq_weight_mode;
> 
> --
> 2.8.4


Regards,
Cristian


More information about the dev mailing list