From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Li Zhang
Sent: Monday, March 1, 2021 4:16 AM

Hi Morten,

Thank you for your comments.
It is packet count instead of frames count.
So 4K TCP packet should be handle as one instead of 3 Ethernet Frames.

We will update the struct as below:
enum rte_mtr_algorithm {
......
	/** Single Rate Three Color Marker, Packet based (srTCMp).
	 * - - similar to IETF RFC 2697 but rate is packet per second.
	 */
	RTE_MTR_SRTCMP,
}

struct rte_mtr_meter_profile {
......
		/** Items only valid when *alg* is set to srTCMp. */
		struct {
			/** Committed Information Rate (CIR)
			 * (packets/second).
			 */
			uint64_t cir;

			/** Committed Burst Size (CBS) (packets). */
			uint64_t cbs;

			/** Excess Burst Size (EBS) (packets). */
			uint64_t ebs;
		} srtcmp;
}

Regards,
Li Zhang

Looks good.

Acked-By: Morten Brørup <redacted>

quoted

-----Original Message-----
From: dev <redacted> On Behalf Of Morten Br?rup
Sent: Tuesday, February 23, 2021 4:25 PM
To: Li Zhang <redacted>; Ferruh Yigit

[off-list ref]; Dekel

quoted

Peled [off-list ref]; Ori Kam [off-list ref]; Slava

Ovsiienko

quoted

[off-list ref]; Matan Azrad [off-list ref]; Dumitrescu,
Cristian [off-list ref]
Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon <redacted>;
Raslan Darawsheh [off-list ref]
Subject: Re: [dpdk-dev] [PATCH] [RFC, v2]: adds support PPS(packet

per second)

quoted

on meter

quoted

From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Li Zhang
Sent: Tuesday, February 23, 2021 3:07 AM

Thanks for your comments.
We changed the struct as below:
struct rte_mtr_meter_profile {
 ......
		/** Items only valid when alg is set to sprTCM. */
		struct {
			/** Committed Information Packet Rate (CIPR). */
			uint64_t cipr;

			/** Committed Packet Burst Size (CPBS). */
			uint64_t cpbs;

			/** Excess Packet Burst Size (EPBS). */
			uint64_t epbs;
		} sprtcm;
}

That is certainly not an improvement! Please stick with the broadly

accepted

quoted

industry standard names (CIR, CBS, EBS, etc.). Newly invented names

like CIPR

quoted

will only cause confusion. Please note that CIR, CBS, EBS don't say

anything

quoted

about their units; it could be bytes, packets, cells, frames,

messages or

quoted

anything else. So it is perfectly natural using the same names for

packets

quoted

instead of bytes.

I propose the following modifications instead.

For the algorithm, keep the "Single Rate Three Color Marker" name

mentioned

quoted

in the RFCs, but indicate that the variant is packet based:

	/** Single Rate Three Color Marker, Packet based (srTCMp). */
	RTE_MTR_SRTCMP,

Or if you prefer the packet based indicator at the front instead:

	/** Packet based Single Rate Three Color Marker (psrTCM). */
	RTE_MTR_PSRTCM,

And for the meter profile, keep the industry standard names, but

update the

quoted

descriptions:

 		/** Items only valid when *alg* is set to srTCMp. */
 		struct {
 			/** Committed Information Rate (CIR)
(packets/second). */
 			uint64_t cir;

 			/** Committed Burst Size (CBS) (packets). */
 			uint64_t cbs;

 			/** Excess Burst Size (EBS) (packets). */
 			uint64_t ebs;
 		} srtcmp;


Come to think of it: Is the unit packets, or is it actually Ethernet

frames? In

quoted

other words: Does a 4 KB TCP packet that the NIC's offload function

chops up

quoted

into three Ethernet frames count as one or three in this algorithm?


Med venlig hilsen / kind regards
- Morten Brørup

quoted

Regards,
Li Zhang

quoted

-----Original Message-----
From: dev <redacted> On Behalf Of Morten Br?rup
Sent: Friday, February 12, 2021 3:41 PM
To: Ferruh Yigit <redacted>; Li Zhang

[off-list ref]; Dekel

quoted

Peled [off-list ref]; Ori Kam [off-list ref]; Slava

Ovsiienko

quoted

[off-list ref]; Matan Azrad [off-list ref];
Dumitrescu, Cristian [off-list ref]
Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon

[off-list ref];

quoted

quoted

Raslan Darawsheh [off-list ref]
Subject: Re: [dpdk-dev] [PATCH] [RFC, v2]: adds support

PPS(packet

quoted

quoted

per second)

quoted

on meter

quoted

From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh

Yigit

quoted

quoted

quoted

quoted

Sent: Thursday, January 28, 2021 7:28 PM

On 1/25/2021 1:20 AM, Li Zhang wrote:

quoted

Currently the flow Meter algorithms in rte_flow only supports

bytes

quoted

quoted

quoted

per second(BPS).
Such as Single Rate Three Color Marker (srTCM rfc2697) This

RFC

quoted

quoted

adds

quoted

quoted

quoted

the packet per second definition in Meter algorithms

structure,

quoted

quoted

to

quoted

quoted

quoted

support the rte_mtr APIs with type srTCM pps mode.
The below structure will be extended:
rte_mtr_algorithm
rte_mtr_meter_profile
Signed-off-by: Li Zhang <redacted>

cc'ed Cristian for review/comment.

quoted

---
  lib/librte_ethdev/rte_mtr.h | 28

++++++++++++++++++++++++++++

quoted

quoted

quoted

quoted

quoted

  1 file changed, 28 insertions(+)

diff --git a/lib/librte_ethdev/rte_mtr.h

b/lib/librte_ethdev/rte_mtr.h

quoted

index 916a09c5c3..3e88904faf 100644

--- a/lib/librte_ethdev/rte_mtr.h
+++ b/lib/librte_ethdev/rte_mtr.h

@@ -119,6 +119,9 @@ enum rte_mtr_algorithm {

  	/** Two Rate Three Color Marker (trTCM) - IETF RFC

4115. */

quoted

quoted

quoted

quoted

quoted

  	RTE_MTR_TRTCM_RFC4115,
+
+	/** Single Rate Three Color Marker (srTCM) in Packet per

second

quoted

quoted

mode */

quoted

+	RTE_MTR_SRTCM_PPS,
  };

  /**

@@ -171,6 +174,18 @@ struct rte_mtr_meter_profile {
  			/** Excess Burst Size (EBS) (bytes). */
  			uint64_t ebs;
  		} trtcm_rfc4115;
+
+		/** Items only valid when *alg* is set to srTCM -

PPS. */

quoted

quoted

quoted

+		struct {
+			/** Committed Information Rate

(CIR)(packets/second).

quoted

*/

quoted

+			uint64_t cir;
+
+			/** Committed Burst Size (CBS) (bytes). */
+			uint64_t cbs;
+
+			/** Excess Burst Size (EBS) (bytes). */
+			uint64_t ebs;
+		} srtcm_pps;

In PPS mode, the burst sizes (ebs, ebs) must be packets, not

bytes.

quoted

quoted

quoted

quoted

quoted

  	};
  };

@@ -317,6 +332,13 @@ struct rte_mtr_capabilities {
  	 */
  	uint32_t meter_trtcm_rfc4115_n_max;

+	/** Maximum number of MTR objects that can have their

meter

quoted

quoted

quoted

configured

quoted

+	 * to run the srTCM packet per second algorithm. The value

of 0

quoted

quoted

quoted

+	 * indicates this metering algorithm is not supported.
+	 * The maximum value is *n_max*.
+	 */
+	uint32_t meter_srtcm_pps_n_max;
+
  	/** Maximum traffic rate that can be metered by a

single

quoted

quoted

MTR

quoted

quoted

object. For

quoted

  	 * srTCM RFC 2697, this is the maximum CIR rate. For

trTCM

quoted

quoted

RFC

quoted

quoted

2698,

quoted

  	 * this is the maximum PIR rate. For trTCM RFC 4115,

this

quoted

quoted

is the

quoted

quoted

maximum

quoted

@@ -342,6 +364,12 @@ struct rte_mtr_capabilities {
  	 */
  	int color_aware_trtcm_rfc4115_supported;

+	/**
+	 * When non-zero, it indicates that color aware mode is

supported

quoted

quoted

for

quoted

+	 * the srTCM packet per second  metering algorithm.

No need for two spaces between the words second and metering.

quoted

quoted

+	 */
+	int color_aware_srtcm_pps_supported;
+
  	/** When non-zero, it indicates that the policer

packet

quoted

quoted

recolor

quoted

quoted

actions

quoted

  	 * are supported.
  	 * @see enum rte_mtr_policer_action