Re: [dpdk-dev] [RFC v4 1/4] ethdev: add meter PPS profile

[Ajit Khaparde <ajit.khaparde@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 7410 bytes --]

On Mon, Mar 1, 2021 at 5:20 AM Dumitrescu, Cristian
<cristian.dumitrescu@intel.com> wrote:
>
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Li Zhang
> > Sent: Monday, March 1, 2021 10:35 AM
> > To: dekelp@nvidia.com; orika@nvidia.com; viacheslavo@nvidia.com;
> > matan@nvidia.com
> > Cc: dev@dpdk.org; thomas@monjalon.net; rasland@nvidia.com;
> > mb@smartsharesystems.com; ajit.khaparde@broadcom.com
> > Subject: [dpdk-dev] [RFC v4 1/4] ethdev: add meter PPS profile
> >
> > Currently meter algorithms only supports rate is bytes per second(BPS).
> > Add this new meter srTCMp algorithm to support rate is packet per second.
> > So that it can meter traffic by packet per second.
> > The below structure will be extended:
> > rte_mtr_algorithm
> > rte_mtr_meter_profile
> > Signed-off-by: Li Zhang <lizh@nvidia.com>
> > ---
> >  .../traffic_metering_and_policing.rst         |  3 +-
> >  doc/guides/rel_notes/release_20_11.rst        |  5 +++
> >  lib/librte_ethdev/rte_mtr.h                   | 32 +++++++++++++++++++
> >  3 files changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/guides/prog_guide/traffic_metering_and_policing.rst
> > b/doc/guides/prog_guide/traffic_metering_and_policing.rst
> > index 90c781eb1d..4d2405d44a 100644
> > --- a/doc/guides/prog_guide/traffic_metering_and_policing.rst
> > +++ b/doc/guides/prog_guide/traffic_metering_and_policing.rst
> > @@ -17,7 +17,8 @@ The main features are:
> >  * Part of DPDK rte_ethdev API
> >  * Capability query API
> >  * Metering algorithms: RFC 2697 Single Rate Three Color Marker (srTCM),
> > RFC 2698
> > -  and RFC 4115 Two Rate Three Color Marker (trTCM)
> > +  and RFC 4115 Two Rate Three Color Marker (trTCM),
> > +  Single Rate Three Color Marker, Packet based (srTCMp).
> >  * Policer actions (per meter output color): recolor, drop
> >  * Statistics (per policer output color)
> >
> > diff --git a/doc/guides/rel_notes/release_20_11.rst
> > b/doc/guides/rel_notes/release_20_11.rst
> > index 7405a9864f..de04886cc9 100644
> > --- a/doc/guides/rel_notes/release_20_11.rst
> > +++ b/doc/guides/rel_notes/release_20_11.rst
> > @@ -429,6 +429,11 @@ New Features
> >    can leverage IOAT DMA channels with vhost asynchronous APIs.
> >    See the :doc:`../sample_app_ug/vhost` for more details.
> >
> > +* **Added support for meter PPS profile.**
> > +
> > +  Currently meter algorithms only supports bytes per second(BPS).
> > +  Add this new meter algorithm to support packet per second (PPS) mode.
> > +  So that it can meter traffic by packet per second.
> >
> >  Removed Items
> >  -------------
> > diff --git a/lib/librte_ethdev/rte_mtr.h b/lib/librte_ethdev/rte_mtr.h
> > index 916a09c5c3..f27a4b5354 100644
> > --- a/lib/librte_ethdev/rte_mtr.h
> > +++ b/lib/librte_ethdev/rte_mtr.h
> > @@ -119,6 +119,11 @@ enum rte_mtr_algorithm {
> >
> >       /** Two Rate Three Color Marker (trTCM) - IETF RFC 4115. */
> >       RTE_MTR_TRTCM_RFC4115,
> > +
> > +     /** Single Rate Three Color Marker, Packet based (srTCMp).
> > +      * - - similar to IETF RFC 2697 but rate is packet per second.
> > +      */
> > +     RTE_MTR_SRTCMP,
> >  };
> >
> >  /**
> > @@ -171,6 +176,20 @@ struct rte_mtr_meter_profile {
> >                       /** Excess Burst Size (EBS) (bytes). */
> >                       uint64_t ebs;
> >               } trtcm_rfc4115;
> > +
> > +             /** 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;
> >       };
> >  };
> >
> > @@ -317,6 +336,13 @@ struct rte_mtr_capabilities {
> >        */
> >       uint32_t meter_trtcm_rfc4115_n_max;
> >
> > +     /** Maximum number of MTR objects that can have their meter
> > configured
> > +      * to run the srTCMp algorithm. The value of 0
> > +      * indicates this metering algorithm is not supported.
> > +      * The maximum value is *n_max*.
> > +      */
> > +     uint32_t meter_srtcmp_n_max;
> > +
> >       /** Maximum traffic rate that can be metered by a single MTR
> > object. For
> >        * srTCM RFC 2697, this is the maximum CIR rate. For trTCM RFC 2698,
> >        * this is the maximum PIR rate. For trTCM RFC 4115, this is the
> > maximum
> > @@ -342,6 +368,12 @@ struct rte_mtr_capabilities {
> >        */
> >       int color_aware_trtcm_rfc4115_supported;
> >
> > +     /**
> > +      * When non-zero, it indicates that color aware mode is supported
> > for
> > +      * the srTCMp metering algorithm.
> > +      */
> > +     int color_aware_srtcmp_supported;
> > +
> >       /** When non-zero, it indicates that the policer packet recolor
> > actions
> >        * are supported.
> >        * @see enum rte_mtr_policer_action
> > --
> > 2.21.0
>
> Hi Li,
>
> As specified in the MAINTEINERS file of DPDK, I am the maintainer of this API, so please make sure you add my email in the To: list of future revisions of this patch set.
>
> Isn't this a duplicate of this other patchset that you authored as well: http://patchwork.dpdk.org/project/dpdk/patch/20210301094000.183002-2-lizh@nvidia.com/ ? Which one do you want to keep? I am pasting below my reply to this other patchset.
>
> We had this same problem earlier for the rte_tm.h API, where people asked to add support for WRED and shaper rates specified in packets to the existing byte rate support. I am more than happy to support adding the same here, but please let's adopt the same solution here rather than invent a different approach.
>
> Please refer to struct rte_tm_wred_params and struct rte_tm_shaper_params from rte_tm.h: the packets vs. bytes mode is explicitly specified through the use of a flag called packet_mode that is added to the WRED and shaper profile. When packet_mode is 0, the profile rates and bucket sizes are specified in bytes per second and bytes, respectively; when packet_mode is not 0, the profile rates and bucket sizes are specified in packets and packets per second, respectively. The same profile parameters are used, no need to invent additional algorithms (such as srTCM - packet mode) or profile data structures. Can we do the same here, please?
>
> This is a quick summary of the required API changes to add support for the packet mode, they are minimal:
> a) Introduce the packet_mode flag in the profile parameters data structure.
> b) Change the description (comment) of the rate and bucket size parameters in the meter profile parameters data structures to reflect that their values represents either bytes or packets, depending on the value of the new flag packet_mode from the same structure.
> c) Add the relevant capabilities: just search for "packet" in the rte_tm.h capabilities data structures and apply the same to the rte_mtr.h capabilities, when applicable.
Overall I think this is a better approach. And default packet_mode
will be bytes.


>
> Regards,
> Cristian