Re: [PATCH V2 1/5] ethdev: add new flow action for metering and policing

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Friday, October 6, 2017 2:58 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; thomas@monjalon.net; Wu, Jingjing
> <jingjing.wu@intel.com>; hemant.agrawal@nxp.com;
> jerin.jacob@caviumnetworks.com; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Subject: Re: [PATCH V2 1/5] ethdev: add new flow action for metering and
> policing
> 
> On Thu, Oct 05, 2017 at 02:09:30PM +0100, Cristian Dumitrescu wrote:
> > Metering and policing action typically sits on top of flow classification,
> > which is why MTR objects are enabled through a newly introduced flow
> > action.
> >
> > The configuration of MTR objects is done in their own namespace (rte_mtr)
> > within the librte_ether library. The MTR object is hooked into ethdev RX
> > processing path using the "meter" flow action.
> >
> > Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> 
> Looks good, one minor comment about the METER action semantics before
> acking
> this patch, see below.
> 
> > ---
> >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> >  lib/librte_ether/rte_flow.h        | 22 ++++++++++++++++++++++
> >  2 files changed, 46 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> > index 662a912..6b9cdc2 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -1354,6 +1354,30 @@ rule or if packets are not addressed to a VF in
> the first place.
> >     | ``vf``       | VF ID to redirect packets to   |
> >     +--------------+--------------------------------+
> >
> > +Action: ``METER``
> > +^^^^^^^^^^^^^^^^^
> > +
> > +Applies a stage of metering and policing.
> > +
> > +The metering and policing (MTR) object has to be first created using the
> > +rte_mtr_create() API function. The ID of the MTR object is specified as
> > +action parameter. One or several meter actions can be added to the same
> > +flow. More than one flow can use the same MTR object through the
> meter
> > +action. The MTR object can be further updated or queried using the
> > +rte_mtr* API.
> 
> rte_flow is currently documented [1] as ignoring several actions of the same
> kind in a given rule, in which case only the last one is taken into
> account. It's a deliberate design choice to simplify implementation, which
> I now think wasn't such a good idea after all.
> 
> I'm therefore not opposed to remove this restriction globally, however in
> that case rte_flow documentation must be modified and PMDs reworked to
> implement the new behavior (failing by default when encountering multiple
> actions of the same time instead of ignoring them). It's probably a little
> late to do this now though.
> 
> So my suggestion for the time being is to not describe what happens when
> several MTR actions are provided. We'll keep this for the next overhaul of
> rte_flow (some other changes are already planned).


Hi Adrian,

Done in V3 just sent.

Thanks,
Cristian

> 
> [1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#actions
> 
> --
> Adrien Mazarguil
> 6WIND