Re: [PATCH 1/3] lib: dpdk spec to skip red for ingress policer

[Jerin Jacob <jerinjacobk@gmail.com>]

On Thu, Feb 2, 2023 at 1:01 PM Ori Kam <orika@nvidia.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Wednesday, 1 February 2023 20:37
> >
> > On Wed, Feb 1, 2023 at 11:19 PM Ori Kam <orika@nvidia.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > Sent: Friday, 27 January 2023 8:23
> > > > To: Ori Kam <orika@nvidia.com>
> > > > Cc: Rakesh Kudurumalla <rkudurumalla@marvell.com>; Stephen
> > Hemminger
> > > > <stephen@networkplumber.org>; NBU-Contact-Thomas Monjalon
> > > > (EXTERNAL) <thomas@monjalon.net>; Ferruh Yigit
> > <ferruh.yigit@amd.com>;
> > > > Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org;
> > > > NBU-Contact-Adrien Mazarguil (EXTERNAL)
> > <adrien.mazarguil@6wind.com>
> > > > Subject: Re: [PATCH 1/3] lib: dpdk spec to skip red for ingress policer
> > > >
> > > > On Thu, Jan 26, 2023 at 8:43 PM Ori Kam <orika@nvidia.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> > > > > > Sent: Wednesday, 18 January 2023 10:10
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Rakesh Kudurumalla
> > > > > > > Sent: Tuesday, January 10, 2023 12:12 PM
> > > > > > > To: Ori Kam <orika@nvidia.com>; Jerin Jacob
> > <jerinjacobk@gmail.com>;
> > > > > > > Stephen Hemminger <stephen@networkplumber.org>
> > > > > > > Cc: NBU-Contact-Thomas Monjalon (EXTERNAL)
> > > > <thomas@monjalon.net>;
> > > > > > > Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
> > > > > > > <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; NBU-Contact-
> > > > Adrien
> > > > > > > Mazarguil (EXTERNAL) <adrien.mazarguil@6wind.com>
> > > > > > > Subject: RE: [PATCH 1/3] lib: dpdk spec to skip red for ingress policer
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Ori Kam <orika@nvidia.com>
> > > > > > > > Sent: Monday, December 26, 2022 10:30 PM
> > > > > > > > To: Jerin Jacob <jerinjacobk@gmail.com>; Stephen Hemminger
> > > > > > > > <stephen@networkplumber.org>
> > > > > > > > Cc: Rakesh Kudurumalla <rkudurumalla@marvell.com>; NBU-
> > Contact-
> > > > > > > Thomas
> > > > > > > > Monjalon (EXTERNAL) <thomas@monjalon.net>; Ferruh Yigit
> > > > > > > > <ferruh.yigit@amd.com>; Andrew Rybchenko
> > > > > > > > <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; NBU-
> > Contact-
> > > > > > Adrien
> > > > > > > > Mazarguil (EXTERNAL) <adrien.mazarguil@6wind.com>
> > > > > > > > Subject: [EXT] RE: [PATCH 1/3] lib: dpdk spec to skip red for ingress
> > > > > > > > policer
> > > > > > > >
> > > > > > > > External Email
> > > > > > > >
> > > > > > > > ----------------------------------------------------------------------
> > > > > > > > Hi All,
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > > Sent: Thursday, 22 December 2022 7:27
> > > > > > > > >
> > > > > > > > > On Thu, Dec 22, 2022 at 8:32 AM Stephen Hemminger
> > > > > > > > > <stephen@networkplumber.org> wrote:
> > > > > > > > > >
> > > > > > > > > > On Thu, 22 Dec 2022 07:09:02 +0530 Rakesh Kudurumalla
> > > > > > > > > > <rkudurumalla@marvell.com> wrote:
> > > > > > > > > >
> > > > > > > > > > > Dropping of packets based on RED can be skipped with
> > meter
> > > > > > > > > > > action, when RED is configured using
> > > > > > > > > > > rte_eth_cman_config_set()
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Rakesh Kudurumalla
> > > > <rkudurumalla@marvell.com>
> > > > > > > > > >
> > > > > > > > > > Should this be more general and apply to all congestion
> > > > management
> > > > > > > > > > options. Assuming the hardware can do something better than
> > > > RED.
> > > > > > > > >
> > > > > > > > > Yes. We can use "enum rte_cman_mode mode" in the
> > descriptor to
> > > > > > > > > future- proof.
> > > > > > > >
> > > > > > > > I'm missing the idea of this new action, I understand that is related
> > > > > > > > to Jerin congestion patches.
> > > > > > > > But I fail to see why we need it? Is it to mark some metadata that
> > > > > > > > will have some effect on the congestion result? (I assume the
> > system
> > > > > > > > is implemented in the HW)
> > > > > > >
> > > > > > > Yes. It is implemented in HW. Congestion management is applied on
> > > > > > ethdev
> > > > > > > Rx queue using rte_eth_cman_config() API. Once it is configured, it
> > > > applies
> > > > > > to
> > > > > > > all the packets that steer towards that particular ethdev Rx queue.
> > This
> > > > > > > feature help to skip the congestion management processing based
> > on
> > > > the
> > > > > > > packet color identified by the rte_flow meter object. For example,
> > If
> > > > one
> > > > > > Rx
> > > > > > > queue configured as RED congestion  and application wants to
> > bypass
> > > > the
> > > > > > > RED congestion  processing for all GREEN color packet can be
> > expressed
> > > > > > > though this API proposal.
> > > > > >
> > > > > > Hi Ori Kam,
> > > > > >
> > > > > > Let me know if above information  would give clear idea on skip RED
> > > > action
> > > > >
> > > > > I think so, to put it in my own words,  when setting this the selected
> > packet
> > > > is treated as
> > > > > green packet?
> > > > >
> > > > > If so, can we use the meter_color field? If you want the packet to be
> > green
> > > > just set the
> > > > > field to green?
> > > >
> > > > It is already there in one form. See following in existing header file.
> > > >
> > > > /**
> > > >  * Meter policy
> > > >  */
> > > > struct rte_mtr_meter_policy_params {
> > > >         /**
> > > >          * Policy action list per color.
> > > >          * actions[i] potentially represents a chain of rte_flow actions
> > > >          * terminated by the END action, exactly as specified by the rte_flow
> > > >          * API for the flow definition, and not just a single action.
> > > >          */
> > > >         const struct rte_flow_action *actions[RTE_COLORS];
> > > > };
> > > >
> > > >
> > > Sorry I'm not sure I understand,
> > > I know we can have colors in the meter, but this feature is about
> > > somehow telling the rxq to skip the red packet and treat it as green right?
> >
> > Yes. When rte_mtr_meter_policy_params::actions[RTE_COLOR_GREEN] set
> > as
> > RTE_FLOW_ACTION_TYPE_SKIP_CMAN for the given meter object,
> > it is indicating to SKIP the CMAN configuration applied to the rxq on
> > the downstream path if meter assigns a GREEN color.
> > (RQ section is as usual as existing path, either via ethdev RSS or
> > rte_flow RSS action or rte_fow Queue action).
> >
>
> Sorry for all my questions just trying to find the best solution,
> Some follow up questions,
> 1. except the skip_cman what other actions is going to be in the action list? Queue? Jump?

No specific restriction from speciation POV. Queue, Jump, RSS most
useful candidates.

> 2. why can't we use the meter color as input if the cman should be skipped or not?

Because, cman skip is happening in ethdev Rx and in our case meter
object action is the one trigger for this.

>
> Thanks,
> Ori
>
> > >
> > > > >
> > > > > Best,
> > > > > Ori
> > > > >