DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jerin Jacob <jerinjacobk@gmail.com>
To: Matan Azrad <matan@nvidia.com>
Cc: Li Zhang <lizh@nvidia.com>, Dekel Peled <dekelp@nvidia.com>,
	Ori Kam <orika@nvidia.com>,
	 Slava Ovsiienko <viacheslavo@nvidia.com>,
	Shahaf Shuler <shahafs@nvidia.com>,
	 Liron Himi <lironh@marvell.com>,
	Jasvinder Singh <jasvinder.singh@intel.com>,
	 NBU-Contact-Thomas Monjalon <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	 Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	 Cristian Dumitrescu <cristian.dumitrescu@intel.com>,
	dpdk-dev <dev@dpdk.org>,  Raslan Darawsheh <rasland@nvidia.com>,
	Roni Bar Yanai <roniba@nvidia.com>
Subject: Re: [dpdk-dev] [PATCH 1/2] [RFC]: ethdev: add pre-defined meter policy API
Date: Wed, 31 Mar 2021 16:20:06 +0530	[thread overview]
Message-ID: <CALBAE1PaLfmq_Gf25JP1TwH17-rqADOFzYqp9gqY+KpUBnomBA@mail.gmail.com> (raw)
In-Reply-To: <BL0PR12MB24832A777E83E7D447DCA1F4DF7E9@BL0PR12MB2483.namprd12.prod.outlook.com>

On Tue, Mar 30, 2021 at 2:01 AM Matan Azrad <matan@nvidia.com> wrote:
>
> Hi Jerin
>
> Thanks for the review.
> PSB
>
> From: Jerin Jacob
> > On Thu, Mar 18, 2021 at 2:28 PM Li Zhang <lizh@nvidia.com> wrote:
> > >
> > > Currently, the flow meter policy does not support multiple actions per
> > > color; also the allowed action types per color are very limited.
> > > In addition, the policy cannot be pre-defined.
> > >
> > > Due to the growing in flow actions offload abilities there is a
> > > potential for the user to use variety of actions per color differently.
> > > This new meter policy API comes to allow this potential in the most
> > > ethdev common way using rte_flow action definition.
> > > A list of rte_flow actions will be provided by the user per color in
> > > order to create a meter policy.
> > > In addition, the API forces to pre-define the policy before the meters
> > > creation in order to allow sharing of single policy with multiple
> > > meters efficiently.
> > >
> > > meter_policy_id is added into struct rte_mtr_params.
> > > So that it can get the policy during the meters creation.
> > >
> > > Policy id 0 is default policy. Action per color as below:
> > > green - no action, yellow - no action, red - drop
> > >
> > > Allow coloring the packet using a new rte_flow_action_color as could
> > > be done by the old policy API,
> > >
> > > The next API function were added:
> > > - rte_mtr_meter_policy_add
> > > - rte_mtr_meter_policy_delete
> > > - rte_mtr_meter_policy_update
> > > - rte_mtr_meter_policy_validate
> > > The next struct was changed:
> > > - rte_mtr_params
> > > - rte_mtr_capabilities
> > > The next API was deleted:
> > > - rte_mtr_policer_actions_update
> > >
> > > Signed-off-by: Li Zhang <lizh@nvidia.com>
> > > ---
> > >  lib/librte_ethdev/rte_flow.h       |  18 ++++
> > >  lib/librte_ethdev/rte_mtr.c        |  55 ++++++++--
> > >  lib/librte_ethdev/rte_mtr.h        | 166 ++++++++++++++++++++---------
> > >  lib/librte_ethdev/rte_mtr_driver.h |  45 ++++++--
> > >  4 files changed, 210 insertions(+), 74 deletions(-)
> > >
> > > diff --git a/lib/librte_ethdev/rte_flow.h
> > > b/lib/librte_ethdev/rte_flow.h index 669e677e91..5f38aa7fa4 100644
> > > --- a/lib/librte_ethdev/rte_flow.h
> > > +++ b/lib/librte_ethdev/rte_flow.h
> > > @@ -31,6 +31,7 @@
> > >  #include <rte_ecpri.h>
> > >  #include <rte_mbuf.h>
> > >  #include <rte_mbuf_dyn.h>
> > > +#include <rte_meter.h>
> > >
> > >  #ifdef __cplusplus
> > >  extern "C" {
> > > @@ -2236,6 +2237,13 @@ enum rte_flow_action_type {
> > >          * See struct rte_flow_action_modify_field.
> > >          */
> > >         RTE_FLOW_ACTION_TYPE_MODIFY_FIELD,
> > > +
> > > +       /**
> > > +        * Color the packet to reflect the meter color result.
> > > +        *
> > > +        * See struct rte_flow_action_color.
> > > +        */
> > > +       RTE_FLOW_ACTION_TYPE_COlOR,
> >
> > Based on my understanding of this API,
> > 1) Application creates the policy
> > 2) Attachs the policy ID to meter object in params If so, Why we need this new
> > action?
>
> Yes,
> In the new policy API the meter actions will be defined by rte_flow actions.
> The old policy API used rte_mtr actions: color green\color yellow\color red\drop.
>
> This new rte_flow COLOR action comes to replace the old policy API "color" actions which set the color field in mbuf.

OK. Looks good to me. I think, it is better to change to
RTE_FLOW_ACTION_TYPE_METER_COLOR to
denote it as meter-specific action.

>
>
> > >  };
> > >
> > >  /**
> > > @@ -2828,6 +2836,16 @@ struct rte_flow_action_set_dscp {
> > >   */
> > >  struct rte_flow_shared_action;
> > >
> > > +/**
> > > + * Meter policy add
> > > + *
> > > + * Create a new meter policy. The new policy
> > > + * is used to create single or multiple MTR objects.
> > > + *
> > > + * @param[in] port_id
> > > + *   The port identifier of the Ethernet device.
> > > + * @param[in] policy_id
> > > + *   Policy identifier for the new meter policy.
> > > + * @param[in] actions
> > > + *   Associated actions per color.
> > > + *   list NULL is legal and means no special action.
> > > + *   (list terminated by the END action).
> > > + * @param[out] error
> > > + *   Error details. Filled in only on error, when not NULL.
> > > + * @return
> > > + *   0 on success, non-zero error code otherwise.
> > > + */
> > > +__rte_experimental
> > > +int
> > > +rte_mtr_meter_policy_add(uint16_t port_id,
> >
> >
> > _create() may be better here instead of _add() as you have used _delete()
>
> Yes!

OK

>
> > > +       uint32_t policy_id,
> > > +       const struct rte_flow_action *actions[RTE_COLORS],
> >
> >
> > 1) Does this mean that MLX HW can support any rte_flow actions like, if packet
> > color is green do RTE_FLOW_ACTION_TYPE_SECURITY etc.
>
> Theoretically yes, we can support most of the actions.
> For the first stage we are going to support next: queue\RSS\port id\mark\tag\jump.
>
> For example, user can select different queue\port id per color.


OK.

>
> > 2) Is there any real-world use case other than using normal action like pass or
> > drop as it is used in conjunction with meter object?
>
> Yes, I wrote above.
>
> > 3) Marvell HW has the following policy actions
> > a) PASS
> > b) DROP
> > c) RED (Random early discard)
> >
> > Both (a) and (c) are not in enumated as rte_flow_actions.
>
> (a) is in, just don't specify any action.
>
> Can you explain what is "Random early discard"?

https://en.wikipedia.org/wiki/Random_early_detection

> How did you specify it in the old\current meter policy API?

It can not. We are planning to add support for the meter in the new cnxk driver.
Planning change the meter API based on the driver patch.

>
>
> Note, that after the policy actions the device should continue to do the rest of the actions in the flow (after meter) if no termination action in the policy color.
>
> >
> > Should we take rte_flow_action or create meter-specific policy actions?
>
> This patch removes the meter-specific policy actions.
> You need to use rte_flow action.
>
> By the way, can you help to adjust Marvell driver to the change?

The current driver(octeontx2) is not using meter.



>

  reply	other threads:[~2021-03-31 10:50 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-18  8:58 Li Zhang
2021-03-18  8:58 ` [dpdk-dev] [PATCH 2/2] [RFC]: ethdev: manage meter API object handles by the drivers Li Zhang
2021-03-23 21:33   ` Dumitrescu, Cristian
2021-03-25  8:21     ` Matan Azrad
2021-03-25 23:16       ` Ajit Khaparde
2021-03-29 19:56         ` Matan Azrad
2021-03-27 13:15       ` Jerin Jacob
2021-03-29 20:10         ` Matan Azrad
2021-03-31 10:22           ` Jerin Jacob
2021-03-23 21:02 ` [dpdk-dev] [PATCH 1/2] [RFC]: ethdev: add pre-defined meter policy API Dumitrescu, Cristian
2021-03-25  6:56   ` Matan Azrad
2021-03-29  9:23     ` Ori Kam
2021-03-29 16:24       ` Dumitrescu, Cristian
2021-04-01 13:13         ` Ori Kam
2021-04-01 13:35           ` Dumitrescu, Cristian
2021-04-01 14:22             ` Ori Kam
2021-03-29 16:08     ` Dumitrescu, Cristian
2021-03-29 20:43       ` Matan Azrad
2021-03-31 15:46         ` Dumitrescu, Cristian
2021-04-04 13:48           ` Matan Azrad
2021-03-29 10:38 ` Jerin Jacob
2021-03-29 20:31   ` Matan Azrad
2021-03-31 10:50     ` Jerin Jacob [this message]
2021-04-13  0:14 ` [dpdk-dev] [PATCH v3 0/2] Support " Li Zhang
2021-04-13  0:14   ` [dpdk-dev] [PATCH v3 1/2] ethdev: add pre-defined " Li Zhang
2021-04-13 14:19     ` Dumitrescu, Cristian
2021-04-14  3:23       ` Li Zhang
2021-04-13 14:59     ` Dumitrescu, Cristian
2021-04-14  4:55       ` Li Zhang
2021-04-14  8:02         ` Thomas Monjalon
2021-04-14  8:31           ` Matan Azrad
2021-04-14  8:47           ` Asaf Penso
2021-04-14  8:59             ` Li Zhang
2021-04-14  9:04             ` Thomas Monjalon
2021-04-14 14:00             ` Dumitrescu, Cristian
2021-04-14 16:21               ` Li Zhang
2021-04-13 16:25     ` Kinsella, Ray
2021-04-13  0:14   ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-14  3:12 ` [dpdk-dev] [PATCH v4 0/2] Support meter policy API Li Zhang
2021-04-14  3:12   ` [dpdk-dev] [PATCH v4 1/2] ethdev: add pre-defined " Li Zhang
2021-04-14  3:12   ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-14  6:32 ` [dpdk-dev] [PATCH v5 0/2] Support meter policy API Li Zhang
2021-04-14  6:32   ` [dpdk-dev] [PATCH v5 1/2] ethdev: add pre-defined " Li Zhang
2021-04-14  6:32   ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-14  8:57 ` [dpdk-dev] [PATCH v6 0/2] Support meter policy API Li Zhang
2021-04-14  8:57   ` [dpdk-dev] [PATCH v6 1/2] ethdev: add pre-defined " Li Zhang
2021-04-14 16:16     ` Dumitrescu, Cristian
2021-04-15  1:59       ` Li Zhang
2021-04-14 22:21     ` Singh, Jasvinder
2021-04-15  2:00       ` Li Zhang
2021-04-14  8:58   ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-15  4:54 ` [dpdk-dev] [PATCH v7 0/2] Support meter policy API Li Zhang
2021-04-15  4:54   ` [dpdk-dev] [PATCH v7 1/2] ethdev: add pre-defined " Li Zhang
2021-04-15  4:54   ` [dpdk-dev] [PATCH v7 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-15  9:20 ` [dpdk-dev] [PATCH v8 0/2] Support meter policy API Li Zhang
2021-04-15  9:20   ` [dpdk-dev] [PATCH v8 1/2] ethdev: add pre-defined " Li Zhang
2021-04-15 15:13     ` Ori Kam
2021-04-19 12:34     ` Singh, Jasvinder
2021-04-19 16:13       ` Jiawei(Jonny) Wang
2021-04-15  9:20   ` [dpdk-dev] [PATCH v8 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-19 16:08   ` [dpdk-dev] [PATCH v9 0/2] Support meter policy API Jiawei Wang
2021-04-19 16:08     ` [dpdk-dev] [PATCH v9 1/2] ethdev: add pre-defined " Jiawei Wang
2021-04-20 11:18       ` Dumitrescu, Cristian
2021-04-20 12:55         ` Asaf Penso
2021-04-20 21:01           ` Dumitrescu, Cristian
2021-04-19 16:08     ` [dpdk-dev] [PATCH v9 2/2] app/testpmd: support policy actions per color Jiawei Wang
2021-04-20 11:36     ` [dpdk-dev] [PATCH v9 0/2] Support meter policy API Ferruh Yigit
2021-04-20 14:08       ` Jiawei(Jonny) Wang
2021-04-20 14:04     ` [dpdk-dev] [PATCH v10 " Jiawei Wang
2021-04-20 14:04       ` [dpdk-dev] [PATCH v10 1/2] ethdev: add pre-defined " Jiawei Wang
2021-04-20 17:12         ` Ajit Khaparde
2021-04-21 19:43         ` Thomas Monjalon
2021-04-22  1:29           ` Li Zhang
2021-04-20 14:04       ` [dpdk-dev] [PATCH v10 2/2] app/testpmd: support policy actions per color Jiawei Wang
2021-04-20 17:14         ` Ajit Khaparde
2021-04-21 10:23       ` [dpdk-dev] [PATCH v10 0/2] Support meter policy API Ferruh Yigit
2021-04-20 17:56 ` [dpdk-dev] [PATCH 1/2] [RFC]: ethdev: add pre-defined " Stephen Hemminger
2021-04-21  2:49   ` Li Zhang

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=CALBAE1PaLfmq_Gf25JP1TwH17-rqADOFzYqp9gqY+KpUBnomBA@mail.gmail.com \
    --to=jerinjacobk@gmail.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=cristian.dumitrescu@intel.com \
    --cc=dekelp@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jasvinder.singh@intel.com \
    --cc=lironh@marvell.com \
    --cc=lizh@nvidia.com \
    --cc=matan@nvidia.com \
    --cc=orika@nvidia.com \
    --cc=rasland@nvidia.com \
    --cc=roniba@nvidia.com \
    --cc=shahafs@nvidia.com \
    --cc=thomas@monjalon.net \
    --cc=viacheslavo@nvidia.com \
    /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).