From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"thomas@monjalon.net" <thomas@monjalon.net>,
"jerin.jacob@caviumnetworks.com" <jerin.jacob@caviumnetworks.com>,
"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
"Doherty, Declan" <declan.doherty@intel.com>,
"Wiles, Keith" <keith.wiles@intel.com>
Subject: Re: [dpdk-dev] [RFC 3/3] rte_flow: add new action for traffic metering and policing
Date: Mon, 10 Jul 2017 17:21:00 +0200 [thread overview]
Message-ID: <20170710152100.GW19852@6wind.com> (raw)
In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D891267BA6560F@IRSMSX108.ger.corp.intel.com>
Hi Cristian,
Took me a while to reply and I didn't see any update in the meantime, is
this RFC still relevant?
More comments below.
On Tue, Jun 06, 2017 at 06:37:57PM +0000, Dumitrescu, Cristian wrote:
> Hi Adrien,
>
> Thanks for reviewing this proposal.
>
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Thursday, June 1, 2017 4:14 PM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > Cc: dev@dpdk.org; thomas@monjalon.net;
> > jerin.jacob@caviumnetworks.com; hemant.agrawal@nxp.com; Doherty,
> > Declan <declan.doherty@intel.com>; Wiles, Keith <keith.wiles@intel.com>
> > Subject: Re: [RFC 3/3] rte_flow: add new action for traffic metering and
> > policing
> >
> > Hi Cristian,
> >
> > On Tue, May 30, 2017 at 05:44:13PM +0100, Cristian Dumitrescu wrote:
> > > Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > > ---
> > > lib/librte_ether/rte_flow.h | 22 ++++++++++++++++++++++
> > > 1 file changed, 22 insertions(+)
> > >
> > > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> > > index c47edbc..2942ca7 100644
> > > --- a/lib/librte_ether/rte_flow.h
> > > +++ b/lib/librte_ether/rte_flow.h
> > > @@ -881,6 +881,14 @@ enum rte_flow_action_type {
> > > * See struct rte_flow_action_vf.
> > > */
> > > RTE_FLOW_ACTION_TYPE_VF,
> > > +
> > > + /**
> > > + * Traffic metering and policing (MTR).
> > > + *
> > > + * See struct rte_flow_action_meter.
> > > + * See file rte_mtr.h for MTR object configuration.
> > > + */
> > > + RTE_FLOW_ACTION_TYPE_METER,
> > > };
> > >
> > > /**
> > > @@ -974,6 +982,20 @@ struct rte_flow_action_vf {
> > > };
> > >
> > > /**
> > > + * RTE_FLOW_ACTION_TYPE_METER
> > > + *
> > > + * Traffic metering and policing (MTR).
> > > + *
> > > + * Packets matched by items of this type can be either dropped or passed
> > to the
> > > + * next item with their color set by the MTR object.
> > > + *
> > > + * Non-terminating by default.
> > > + */
> > > +struct rte_flow_action_meter {
> > > + uint32_t mtr_id; /**< MTR object ID created with rte_mtr_create().
> > */
> > > +};
> > > +
> > > +/**
> > > * Definition of a single action.
> > > *
> > > * A list of actions is terminated by a END action.
> >
> > Assuming this action is provided to the underlying PMD, can you describe
> > what happens next; what is a PMD supposed to do when creating the flow
> > rule
> > and the impact on its data path?
> >
>
> Metering is just another flow action that needs to be supported by rte_flow API.
>
> Typically, NICs supporting this action have an array of metering & policing contexts on their data path, which are abstracted as MTR objects in our API.
> - rte_mtr_create() configures an MTR object, with no association to any of the known flows yet.
> - On NIC side, the driver configures one of the available metering & policing contexts.
> - rte_flow_create() defines the flow (match rule) and its set of actions, with metering & policing as one of the actions.
> - On NIC side, the driver configures a flow/filter for traffic classification/distribution/bifurcation, with the metering & policing context enabled for this flow.
>
> At run-time, any packet matching this flow will execute this action, which involves metering (packet is assigned a color) and policing (packet may be recolored or dropped, as configured), with stats being updated as well.
Thanks, this description should be part of the documentation in the final
patch. The relationship between rte_mtr and rte_flow objects must be
described as well, for instance making clear that one cannot remove a mtr
object if a flow rule depends on it.
> > It looks like mtr_id is arbitrarily set by the user calling
> > rte_mtr_create(), which means the PMD has to look up the associated MTR
> > context somehow.
> >
> > How about making the rte_mtr_create() API return an opaque rte_mtr
> > object
> > pointer provided back to all API functions as well as through this action
> > instead, and not leave it up to the user?
> >
>
> Of course, it can be done this way as well, but IMHO probably not the best idea from the application perspective. We had a similar discussion when we defined the ethdev traffic management API [1].
>
> Object handles can be integers, void pointers or pointers to opaque structures, and each of these approaches are allowed and used by DPDK APIs. Here is an example why I think using integers for MTR object handle makes the life of the application easier:
> - Let's assume we have several actions for a flow (a1, a2, a3, ...).
> - When handles are pointers to opaque structures, app typically needs to save all of them in a per flow data structure: struct a1 *p1, struct a2 *p2, struct a3 *p3.
> -This results in increased complexity and size for the app tables, which can be avoided.
> - When handles are integers generated by the app as opposed of driver, the app can simply use a single index - let's cal it flow_id - and register it as the handle to each of these flow actions.
> - No more fake tables.
> - No more worries about the pointer being valid in one address space and not valid in another.
>
> There is some handle lookup to be done by the driver, but this is a trivial task, and checking the validity of the handle (input parameter) is the first thing done by any API function, regardless of which handle style is used.
All this sounds reasonable from the control plane standpoint. My comment was
more generally besides the above rte_flow action, are we sure mtr_id will
never be used to perform actions from the data plane?
Otherwise driver lookup is perhaps a trivial task but is nonetheless
expensive. This step can be avoided if mtr_id contains enough information to
locate the related object as fast as possible inside the PMD without the
need for an intermediate lookup table.
In which case, making mtr_id a pointer-sized opaque value (e.g. uintptr_t)
provided by the PMD when calling rte_mtr_create() gives more flexibility to
the PMD. For some, a basic index value could be enough while for others, an
intermediate structure could be necessary.
Admittedly this is exactly the same as a pointer type to an opaque object,
address space-related issues would have to be managed by the PMD either
way. Applications are not supposed to dereference such objects.
> [1] http://www.dpdk.org/ml/archives/dev/2017-February/057368.html
--
Adrien Mazarguil
6WIND
next prev parent reply other threads:[~2017-07-10 15:21 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-30 16:44 [dpdk-dev] [RFC 0/3] rte_mtr: generic ethdev API for " Cristian Dumitrescu
2017-05-30 16:44 ` [dpdk-dev] [RFC 1/3] ethdev: add traffic metering and policing ops get API Cristian Dumitrescu
2017-05-30 16:44 ` [dpdk-dev] [RFC 2/3] ethdev: add new rte_mtr API for traffic metering and policing Cristian Dumitrescu
2017-08-26 0:06 ` [dpdk-dev] [PATCH 0/3] rte_mtr: generic ethdev API for " Cristian Dumitrescu
2017-08-26 0:06 ` [dpdk-dev] [PATCH 1/3] ethdev: add new eth_dev_ops function for mtr ops get Cristian Dumitrescu
2017-09-13 5:50 ` Jerin Jacob
2017-09-19 15:52 ` Dumitrescu, Cristian
2017-10-05 13:09 ` [dpdk-dev] [PATCH V2 0/5] rte_mtr: generic ethdev api for metering and policing Cristian Dumitrescu
2017-10-05 13:09 ` [dpdk-dev] [PATCH V2 1/5] ethdev: add new flow action " Cristian Dumitrescu
2017-10-06 13:57 ` Adrien Mazarguil
2017-10-06 14:50 ` Dumitrescu, Cristian
2017-10-06 14:45 ` [dpdk-dev] [PATCH V3 0/5] rte_mtr: generic ethdev api " Cristian Dumitrescu
2017-10-06 14:45 ` [dpdk-dev] [PATCH V3 1/5] ethdev: add new flow action " Cristian Dumitrescu
2017-10-06 14:55 ` Adrien Mazarguil
2017-10-13 12:22 ` [dpdk-dev] [PATCH V4 0/5] rte_mtr: generic ethdev api " Cristian Dumitrescu
2017-10-13 12:22 ` [dpdk-dev] [PATCH V4 1/5] ethdev: add new flow action " Cristian Dumitrescu
2017-10-18 2:55 ` Jerin Jacob
2017-10-13 12:22 ` [dpdk-dev] [PATCH V4 2/5] ethdev: add new eth_dev_ops function for mtr ops get Cristian Dumitrescu
2017-10-17 12:40 ` Hemant Agrawal
2017-10-13 12:22 ` [dpdk-dev] [PATCH V4 3/5] ethdev: add new api for traffic metering and policing Cristian Dumitrescu
2017-10-17 12:39 ` Hemant Agrawal
2017-10-18 2:58 ` Jerin Jacob
2017-10-13 12:22 ` [dpdk-dev] [PATCH V4 4/5] doc: ethdev traffic metering and policing api Cristian Dumitrescu
2017-10-13 15:43 ` Mcnamara, John
2017-10-13 12:22 ` [dpdk-dev] [PATCH V4 5/5] app/testpmd: cli for traffic metering and policing Cristian Dumitrescu
2017-10-16 9:49 ` Wu, Jingjing
2017-10-16 10:10 ` Wu, Jingjing
2017-10-20 12:15 ` [dpdk-dev] [PATCH V4 0/5] rte_mtr: generic ethdev api for " Dumitrescu, Cristian
2017-10-06 14:45 ` [dpdk-dev] [PATCH V3 2/5] ethdev: add new eth_dev_ops function for mtr ops get Cristian Dumitrescu
2017-10-12 10:58 ` Hemant Agrawal
2017-10-06 14:45 ` [dpdk-dev] [PATCH V3 3/5] ethdev: add new api for traffic metering and policing Cristian Dumitrescu
2017-10-12 10:48 ` Hemant Agrawal
2017-10-12 10:54 ` Hemant Agrawal
2017-10-13 12:29 ` Dumitrescu, Cristian
2017-10-06 14:45 ` [dpdk-dev] [PATCH V3 4/5] doc: ethdev traffic metering and policing api Cristian Dumitrescu
2017-10-12 14:59 ` Mcnamara, John
2017-10-13 12:26 ` Dumitrescu, Cristian
2017-10-12 15:01 ` Mcnamara, John
2017-10-06 14:45 ` [dpdk-dev] [PATCH V3 5/5] app/testpmd: cli for traffic metering and policing Cristian Dumitrescu
2017-10-13 6:32 ` Wu, Jingjing
2017-10-13 12:30 ` Dumitrescu, Cristian
2017-10-05 13:09 ` [dpdk-dev] [PATCH V2 2/5] ethdev: add new eth_dev_ops function for mtr ops get Cristian Dumitrescu
2017-10-05 13:09 ` [dpdk-dev] [PATCH V2 3/5] ethdev: add new api for traffic metering and policing Cristian Dumitrescu
2017-10-05 13:09 ` [dpdk-dev] [PATCH V2 4/5] doc: ethdev traffic metering and policing api Cristian Dumitrescu
2017-10-05 13:09 ` [dpdk-dev] [PATCH V2 5/5] app/testpmd: cli for traffic metering and policing Cristian Dumitrescu
2017-10-06 13:58 ` Adrien Mazarguil
2017-08-26 0:06 ` [dpdk-dev] [PATCH 2/3] ethdev: add new rte_mtr API " Cristian Dumitrescu
2017-09-01 8:09 ` Hemant Agrawal
2017-09-04 14:32 ` Dumitrescu, Cristian
2017-09-06 9:15 ` Hemant Agrawal
2017-09-19 16:14 ` Dumitrescu, Cristian
2017-09-21 13:20 ` Thomas Monjalon
2017-10-06 10:03 ` Dumitrescu, Cristian
2017-08-26 0:06 ` [dpdk-dev] [PATCH 3/3] rte_flow: add new action " Cristian Dumitrescu
2017-09-06 16:23 ` Adrien Mazarguil
2017-09-19 16:36 ` Dumitrescu, Cristian
2017-09-19 17:00 ` Adrien Mazarguil
2017-10-06 10:02 ` Dumitrescu, Cristian
2017-09-21 13:28 ` [dpdk-dev] [PATCH 0/3] rte_mtr: generic ethdev API for " Thomas Monjalon
2017-05-30 16:44 ` [dpdk-dev] [RFC 3/3] rte_flow: add new action for traffic " Cristian Dumitrescu
2017-06-01 15:13 ` Adrien Mazarguil
2017-06-06 18:37 ` Dumitrescu, Cristian
2017-07-10 15:21 ` Adrien Mazarguil [this message]
2017-07-12 18:06 ` Dumitrescu, Cristian
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=20170710152100.GW19852@6wind.com \
--to=adrien.mazarguil@6wind.com \
--cc=cristian.dumitrescu@intel.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
--cc=jerin.jacob@caviumnetworks.com \
--cc=keith.wiles@intel.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).