From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH V2 1/5] ethdev: add new flow action for metering and policing Date: Fri, 6 Oct 2017 15:57:58 +0200 Message-ID: <20171006135758.GD3871@6wind.com> References: <1503705973-80742-2-git-send-email-cristian.dumitrescu@intel.com> <1507208974-180500-1-git-send-email-cristian.dumitrescu@intel.com> <1507208974-180500-2-git-send-email-cristian.dumitrescu@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, thomas@monjalon.net, jingjing.wu@intel.com, hemant.agrawal@nxp.com, jerin.jacob@caviumnetworks.com, jasvinder.singh@intel.com To: Cristian Dumitrescu Return-path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id A63C71B19A for ; Fri, 6 Oct 2017 15:58:10 +0200 (CEST) Received: by mail-wm0-f49.google.com with SMTP id k4so8119090wmc.1 for ; Fri, 06 Oct 2017 06:58:10 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1507208974-180500-2-git-send-email-cristian.dumitrescu@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 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). [1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#actions -- Adrien Mazarguil 6WIND