DPDK patches and discussions
 help / color / mirror / Atom feed
From: Nithin Dabilpuram <ndabilpuram@marvell.com>
To: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
Cc: Nithin Dabilpuram <nithind1988@gmail.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"jerinj@marvell.com" <jerinj@marvell.com>,
	"kkanas@marvell.com" <kkanas@marvell.com>
Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add tm support for shaper config in pkt mode
Date: Tue, 21 Apr 2020 15:28:13 +0530	[thread overview]
Message-ID: <20200421095813.GA2740@outlook.office365.com> (raw)
In-Reply-To: <BYAPR11MB2935A494665EB195165F8BE3EBD50@BYAPR11MB2935.namprd11.prod.outlook.com>

Hi Cristian,

On Tue, Apr 21, 2020 at 09:30:55AM +0000, Dumitrescu, Cristian wrote:
> Hi Nithin,
> 
> > -----Original Message-----
> > From: Nithin Dabilpuram <nithind1988@gmail.com>
> > Sent: Saturday, April 11, 2020 12:44 PM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> > Rybchenko <arybchenko@solarflare.com>
> > Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> > Dabilpuram <ndabilpuram@marvell.com>
> > Subject: [PATCH v2 1/4] ethdev: add tm support for shaper config in pkt
> > mode
> > 
> > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > 
> > Some NIC hardware support shaper to work in packet mode i.e
> > shaping or ratelimiting traffic is in packets per second (PPS) as
> > opposed to default bytes per second (BPS). Hence this patch
> > adds support to configure shared or private shaper in packet mode,
> > provide rate in PPS and add related tm capabilities in port/level/node
> > capability structures.
> > 
> > This patch also updates tm port/level/node capability structures with
> > exiting features of scheduler wfq packet mode, scheduler wfq byte mode
> > and private/shared shaper byte mode.
> > 
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > ---
> > 
> > v1..v2:
> > - Add seperate capability for shaper and scheduler pktmode and bytemode.
> > - Add packet_mode field in struct rte_tm_shaper_params to indicate
> > packet mode shaper profile.
> > 
> >  lib/librte_ethdev/rte_tm.h | 156
> > ++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 154 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/librte_ethdev/rte_tm.h b/lib/librte_ethdev/rte_tm.h
> > index f9c0cf3..38fff4c 100644
> > --- a/lib/librte_ethdev/rte_tm.h
> > +++ b/lib/librte_ethdev/rte_tm.h
> > @@ -250,6 +250,23 @@ struct rte_tm_capabilities {
> >  	 */
> >  	uint64_t shaper_private_rate_max;
> > 
> > +	/** Shaper private packet mode supported. When non-zero, this
> > parameter
> > +	 * indicates that there is atleast one node that can be configured
> 
> Typo (please fix all occurrences): atleast -> at least
Ack.
> 
> > +	 * with packet mode in it's private shaper. When shaper is configured
> 
> Typo (please fix all occurrences): it's -> its
Ack.
> 
> > +	 * in packet mode, committed/peak rate provided is interpreted
> > +	 * in packets per second.
> > +	 */
> > +	int shaper_private_packet_mode_supported;
> > +
> > +	/** Shaper private byte mode supported. When non-zero, this
> > parameter
> > +	 * indicates that there is atleast one node that can be configured
> > +	 * with byte mode in it's private shaper. When shaper is configured
> > +	 * in byte mode, committed/peak rate provided is interpreted in
> > +	 * bytes per second.
> > +	 */
> > +	int shaper_private_byte_mode_supported;
> > +
> > +
> >  	/** Maximum number of shared shapers. The value of zero indicates
> > that
> >  	 * shared shapers are not supported.
> >  	 */
> > @@ -284,6 +301,21 @@ struct rte_tm_capabilities {
> >  	 */
> >  	uint64_t shaper_shared_rate_max;
> > 
> > +	/** Shaper shared packet mode supported. When non-zero, this
> > parameter
> > +	 * indicates a shared shaper can be configured with packet mode.
> > +	 * When shared shaper is configured in packet mode,
> > committed/peak rate
> > +	 * provided is interpreted in packets per second.
> > +	 */
> > +	int shaper_shared_packet_mode_supported;
> > +
> > +	/** Shaper shared byte mode supported. When non-zero, this
> > parameter
> > +	 * indicates that a shared shaper can be configured with byte mode.
> > +	 * When shared shaper is configured in byte mode, committed/peak
> > rate
> > +	 * provided is interpreted in bytes per second.
> > +	 */
> > +	int shaper_shared_byte_mode_supported;
> > +
> > +
> >  	/** Minimum value allowed for packet length adjustment for any
> > private
> >  	 * or shared shaper.
> >  	 */
> > @@ -339,6 +371,22 @@ struct rte_tm_capabilities {
> >  	 */
> >  	uint32_t sched_wfq_weight_max;
> > 
> > +	/** WFQ packet mode supported. When non-zero, this parameter
> > indicates
> > +	 * that there is at least one non-leaf node that supports packet mode
> > +	 * for WFQ among its children. WFQ weights will be applied against
> > +	 * packet count for scheduling children when a non-leaf node
> > +	 * is configured appropriately.
> > +	 */
> > +	int sched_wfq_packet_mode_supported;
> > +
> > +	/** WFQ byte mode supported. When non-zero, this parameter
> > indicates
> > +	 * that there is at least one non-leaf node that supports byte mode
> > +	 * for WFQ among its children. WFQ weights will be applied against
> > +	 * bytes for scheduling children when a non-leaf node is configured
> > +	 * appropriately.
> > +	 */
> > +	int sched_wfq_byte_mode_supported;
> > +
> >  	/** 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
> > @@ -485,6 +533,24 @@ struct rte_tm_level_capabilities {
> >  			 */
> >  			uint64_t shaper_private_rate_max;
> > 
> > +			/** Shaper private packet mode supported. When
> > non-zero,
> > +			 * this parameter indicates there is atleast one
> > +			 * non-leaf node at this level that can be configured
> > +			 * with packet mode in its private shaper. When
> > private
> > +			 * shaper is configured in packet mode,
> > committed/peak
> > +			 * rate provided is interpreted in packets per second.
> > +			 */
> > +			int shaper_private_packet_mode_supported;
> > +
> > +			/** Shaper private byte mode supported. When
> > non-zero,
> > +			 * this parameter indicates there is atleast one
> > +			 * non-leaf node at this level that can be configured
> > +			 * with byte mode in its private shaper. When private
> > +			 * shaper is configured in byte mode,
> > committed/peak
> > +			 * rate provided is interpreted in bytes per second.
> > +			 */
> > +			int shaper_private_byte_mode_supported;
> > +
> >  			/** Maximum number of shared shapers that any
> > non-leaf
> >  			 * node on this level can be part of. The value of zero
> >  			 * indicates that shared shapers are not supported by
> > @@ -554,6 +620,25 @@ struct rte_tm_level_capabilities {
> >  			 */
> >  			uint32_t sched_wfq_weight_max;
> > 
> > +			/** WFQ packet mode supported. When non-zero,
> > this
> > +			 * parameter indicates that there is at least one
> > +			 * non-leaf node at this level that supports packet
> > +			 * mode for WFQ among its children. WFQ weights
> > will
> > +			 * be applied against packet count for scheduling
> > +			 * children when a non-leaf node is configured
> > +			 * appropriately.
> > +			 */
> > +			int sched_wfq_packet_mode_supported;
> > +
> > +			/** WFQ byte mode supported. When non-zero, this
> > +			 * parameter indicates that there is at least one
> > +			 * non-leaf node at this level that supports byte
> > +			 * mode for WFQ among its children. WFQ weights
> > will
> > +			 * be applied against bytes for scheduling children
> > +			 * when a non-leaf node is configured appropriately.
> > +			 */
> > +			int sched_wfq_byte_mode_supported;
> > +
> >  			/** 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
> > @@ -596,6 +681,24 @@ struct rte_tm_level_capabilities {
> >  			 */
> >  			uint64_t shaper_private_rate_max;
> > 
> > +			/** Shaper private packet mode supported. When
> > non-zero,
> > +			 * this parameter indicates there is atleast one leaf
> > +			 * node at this level that can be configured with
> > +			 * packet mode in its private shaper. When private
> > +			 * shaper is configured in packet mode,
> > committed/peak
> > +			 * rate provided is interpreted in packets per second.
> > +			 */
> > +			int shaper_private_packet_mode_supported;
> > +
> > +			/** Shaper private byte mode supported. When
> > non-zero,
> > +			 * this parameter indicates there is atleast one leaf
> > +			 * node at this level that can be configured with
> > +			 * byte mode in its private shaper. When private
> > shaper
> > +			 * is configured in byte mode, committed/peak rate
> > +			 * provided is interpreted in bytes per second.
> > +			 */
> > +			int shaper_private_byte_mode_supported;
> > +
> >  			/** Maximum number of shared shapers that any
> > leaf node
> >  			 * on this level can be part of. The value of zero
> >  			 * indicates that shared shapers are not supported by
> 
> You are missing the shaper_shared_(packet, byte)_mode supported for non-leaf and leaf nodes in struct rte_tm_level_capabilities.
> 
> The description of this nodes should be aligned with the description of e.g. shaper_shared_n_max field: basically, we want to say that, when true, the flag signifies there is at least on non-leaf/leaf node on this level that can be part of a shared shaper that works in packet/byte mode. Makes sense?

I intentionally didn't add shaper_shared_(packet, byte)_mode in node and level
capabilities and added it in only global cap assuming existing semantics are 
enforcing that. 

Currently, except for 'shaper_shared_n_max', all the other existing shared shaper capabilities like 
shaper_shared_dual_rate_n_max, shaper_shared_rate_min, etc are only provided in global cap.

I felt the semantics are as such because, shared shaper doesn't really belong to any node
or level and any node from any level can attach to a particular shared shaper. Isn't it so
?



> 
> > @@ -686,6 +789,20 @@ struct rte_tm_node_capabilities {
> >  	 */
> >  	uint64_t shaper_private_rate_max;
> > 
> > +	/** Shaper private packet mode supported. When non-zero, this
> > parameter
> > +	 * indicates private shaper of current node can be configured with
> > +	 * packet mode. When configured in packet mode, committed/peak
> > rate
> > +	 * provided is interpreted in packets per second.
> > +	 */
> > +	int shaper_private_packet_mode_supported;
> > +
> > +	/** Shaper private byte mode supported. When non-zero, this
> > parameter
> > +	 * indicates private shaper of current node can be configured with
> > +	 * byte mode. When configured in byte mode, committed/peak rate
> > +	 * provided is interpreted in bytes per second.
> > +	 */
> > +	int shaper_private_byte_mode_supported;
> > +
> >  	/** Maximum number of shared shapers the current node can be
> > part of.
> >  	 * The value of zero indicates that shared shapers are not supported
> > by
> >  	 * the current node.
> 
> You are missing the shaper_shared_(packet, byte)_mode supported (applicable for both non-leaf and leaf nodes, so it should occur only once in the common part of the struct) in struct rte_tm_node_capabilities. See the above comment on the applicable description style.

Please see above.
>
> > @@ -735,6 +852,23 @@ struct rte_tm_node_capabilities {
> >  			 * WFQ weight, so WFQ is reduced to FQ.
> >  			 */
> >  			uint32_t sched_wfq_weight_max;
> > +
> > +			/** WFQ packet mode supported. When non-zero,
> > this
> > +			 * parameter indicates that current node supports
> > packet
> > +			 * mode for WFQ among its children. WFQ weights
> > will be
> > +			 * applied against packet count for scheduling
> > children
> > +			 * when configured appropriately.
> > +			 */
> > +			int sched_wfq_packet_mode_supported;
> > +
> > +			/** WFQ byte mode supported. When non-zero, this
> > +			 * parameter indicates that current node supports
> > byte
> > +			 * mode for WFQ among its children. WFQ weights
> > will be
> > +			 * applied against  bytes for scheduling children when
> > +			 * configured appropriately.
> > +			 */
> > +			int sched_wfq_byte_mode_supported;
> > +
> >  		} nonleaf;
> > 
> >  		/** Items valid only for leaf nodes. */
> > @@ -836,10 +970,10 @@ struct rte_tm_wred_params {
> >   * Token bucket
> >   */
> >  struct rte_tm_token_bucket {
> > -	/** Token bucket rate (bytes per second) */
> > +	/** Token bucket rate (bytes per second or packets per second) */
> >  	uint64_t rate;
> > 
> > -	/** Token bucket size (bytes), a.k.a. max burst size */
> > +	/** Token bucket size (bytes or packets), a.k.a. max burst size */
> >  	uint64_t size;
> >  };
> > 
> > @@ -860,6 +994,11 @@ struct rte_tm_token_bucket {
> >   * Dual rate shapers use both the committed and the peak token buckets.
> > The
> >   * rate of the peak bucket has to be bigger than zero, as well as greater than
> >   * or equal to the rate of the committed bucket.
> > + *
> > + * @see struct
> > rte_tm_capabilities::shaper_private_packet_mode_supported
> > + * @see struct rte_tm_capabilities::shaper_private_byte_mode_supported
> > + * @see struct
> > rte_tm_capabilities::shaper_shared_packet_mode_supported
> > + * @see struct rte_tm_capabilities::shaper_shared_byte_mode_supported
> >   */
> >  struct rte_tm_shaper_params {
> >  	/** Committed token bucket */
> > @@ -874,6 +1013,17 @@ struct rte_tm_shaper_params {
> >  	 * RTE_TM_ETH_FRAMING_OVERHEAD_FCS).
> >  	 */
> >  	int32_t pkt_length_adjust;
> > +
> > +	/** When zero, the private or shared shaper that is associated to this
> > +	 * profile works in byte mode and hence *rate* and *size* fields in
> > +	 * both token bucket configurations are specified in bytes per second
> > +	 * and bytes respectively.
> > +	 * When non-zero, that private or shared shaper works in packet
> > mode and
> > +	 * hence *rate* and *size* fields in both token bucket configurations
> > +	 * are specified in packets per second and packets respectively. In
> > +	 * packet mode, *pkt_length_adjust* is ignored.
> > +	 */
> 
> Please move the last statement ("In packet mode, *pkt_length_adjust* is ignored.") to the description of the pkt_length_adjust field.
Ack.
> 
> > +	int packet_mode;
> >  };
> > 
> >  /**
> > @@ -925,6 +1075,8 @@ struct rte_tm_node_params {
> >  			 * 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.
> > +			 * @see struct
> > rte_tm_node_capabilities::sched_wfq_packet_mode_supported
> > +			 * @see struct
> > rte_tm_node_capabilities::sched_wfq_byte_mode_supported
> >  			 */
> >  			int *wfq_weight_mode;
> > 
> > --
> > 2.8.4
> 
> Regards,
> Cristian

  reply	other threads:[~2020-04-21  9:58 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-30 16:00 [dpdk-dev] [PATCH 1/2] ethdev: add tm cap for private shaper packet mode Nithin Dabilpuram
2020-03-30 16:00 ` [dpdk-dev] [PATCH 2/2] app/testpmd: add tm non leaf node pktmode command Nithin Dabilpuram
2020-04-07  7:30 ` [dpdk-dev] [PATCH 1/2] ethdev: add tm cap for private shaper packet mode Nithin Dabilpuram
2020-04-07 16:31 ` Dumitrescu, Cristian
2020-04-07 17:21   ` [dpdk-dev] [EXT] " Nithin Dabilpuram
2020-04-10 11:45     ` Dumitrescu, Cristian
2020-04-10 11:56       ` Nithin Dabilpuram
2020-04-11 11:44 ` [dpdk-dev] [PATCH v2 1/4] ethdev: add tm support for shaper config in pkt mode Nithin Dabilpuram
2020-04-11 11:44   ` [dpdk-dev] [PATCH v2 2/4] drivers/net: update tm capability for existing pmds Nithin Dabilpuram
2020-04-11 11:44   ` [dpdk-dev] [PATCH v2 3/4] app/testpmd: add tm cmd for non leaf and shaper pktmode Nithin Dabilpuram
2020-04-11 11:44   ` [dpdk-dev] [PATCH v2 4/4] net/octeontx2: support tm length adjust and pkt mode Nithin Dabilpuram
2020-04-16 13:48   ` [dpdk-dev] [PATCH v2 1/4] ethdev: add tm support for shaper config in " Ferruh Yigit
2020-04-21  5:11     ` [dpdk-dev] [EXT] " Nithin Dabilpuram
2020-04-21  9:30   ` [dpdk-dev] " Dumitrescu, Cristian
2020-04-21  9:58     ` Nithin Dabilpuram [this message]
2020-04-21 10:23       ` Dumitrescu, Cristian
2020-04-21 11:55         ` [dpdk-dev] [EXT] " Nithin Dabilpuram
2020-04-22  7:59 ` [dpdk-dev] [PATCH v3] " Nithin Dabilpuram
2020-04-22  7:59   ` [dpdk-dev] [PATCH v3 2/4] drivers/net: update tm capability for existing pmds Nithin Dabilpuram
2020-04-22  7:59   ` [dpdk-dev] [PATCH v3 3/4] app/testpmd: add tm cmd for non leaf and shaper pktmode Nithin Dabilpuram
2020-04-22  7:59   ` [dpdk-dev] [PATCH v3 4/4] net/octeontx2: support tm length adjust and pkt mode Nithin Dabilpuram
2020-04-22  8:09   ` [dpdk-dev] [PATCH v3] ethdev: add tm support for shaper config in " Nithin Dabilpuram
2020-04-22 12:18     ` Singh, Jasvinder
2020-04-22 17:21       ` [dpdk-dev] [EXT] " Nithin Dabilpuram
2020-04-22 10:10   ` [dpdk-dev] " Dumitrescu, Cristian
2020-04-22 11:31     ` [dpdk-dev] [EXT] " Nithin Dabilpuram
2020-04-22 11:49       ` Nithin Dabilpuram
2020-04-22 11:59         ` Dumitrescu, Cristian
2020-04-22 12:01       ` Dumitrescu, Cristian
2020-04-22  8:05 ` [dpdk-dev] [PATCH v3 1/4] " Nithin Dabilpuram
2020-04-22 17:21 ` [dpdk-dev] [PATCH v4 " Nithin Dabilpuram
2020-04-22 17:21   ` [dpdk-dev] [PATCH v4 2/4] drivers/net: update tm capability for existing pmds Nithin Dabilpuram
2020-04-22 17:21   ` [dpdk-dev] [PATCH v4 3/4] app/testpmd: add tm cmd for non leaf and shaper pktmode Nithin Dabilpuram
2020-04-22 17:21   ` [dpdk-dev] [PATCH v4 4/4] net/octeontx2: support tm length adjust and pkt mode Nithin Dabilpuram
2020-04-24 10:28   ` [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper config in " Dumitrescu, Cristian
2020-04-25 20:09     ` Ferruh Yigit
2020-04-27  9:19       ` Dumitrescu, Cristian
2020-04-27 16:12         ` Ferruh Yigit
2020-04-27 16:28           ` Dumitrescu, Cristian
2020-04-28 15:30             ` Thomas Monjalon
2020-04-28 17:35               ` Dumitrescu, Cristian
2020-04-27 16:29           ` Jerin Jacob
2020-04-27 16:49             ` Ferruh Yigit
2020-04-27 16:59               ` Jerin Jacob
2020-04-28 11:51                 ` [dpdk-dev] [EXT] " Nithin Dabilpuram
2020-04-28 13:56                   ` Ferruh Yigit
2020-04-28 14:06                 ` [dpdk-dev] " Ferruh Yigit
2020-04-28 14:45                   ` Bruce Richardson
2020-04-28 15:04                     ` Luca Boccassi
2020-04-28 15:54                       ` Thomas Monjalon
2020-04-29  8:45                         ` Dumitrescu, Cristian
2020-04-29  9:03                           ` Bruce Richardson
2020-05-01 10:27                             ` Ferruh Yigit
2020-05-01 13:16                               ` [dpdk-dev] [EXT] " Nithin Dabilpuram
2020-08-25 16:59                                 ` Ferruh Yigit
2020-09-07 11:12                                   ` Nithin Dabilpuram
2020-09-14 13:01                                     ` Ferruh Yigit
2020-05-01 13:18                         ` [dpdk-dev] " Jerin Jacob
2020-05-05  8:01                           ` Ray Kinsella
2020-04-28 15:42                     ` Ray Kinsella

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200421095813.GA2740@outlook.office365.com \
    --to=ndabilpuram@marvell.com \
    --cc=arybchenko@solarflare.com \
    --cc=cristian.dumitrescu@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jerinj@marvell.com \
    --cc=kkanas@marvell.com \
    --cc=nithind1988@gmail.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).