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>
Subject: Re: [dpdk-dev] [PATCH 3/3] rte_flow: add new action for traffic metering and policing
Date: Tue, 19 Sep 2017 19:00:51 +0200 [thread overview]
Message-ID: <20170919170051.GP2481@6wind.com> (raw)
In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D891267BABC13D@IRSMSX108.ger.corp.intel.com>
Hi Cristian,
On Tue, Sep 19, 2017 at 04:36:50PM +0000, Dumitrescu, Cristian wrote:
<snip>
> > > /**
> > > + * 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().
> > */
> > > +};
> > > +
> >
> > Default mask definition is missing.
> >
>
> I do not understand this comment. This is a flow action, not a flow item (that might be a packet field with an associated mask); this mtr_id is similar to queue ID/index/VF ID from other flow actions, none having any mask attached. Adrien, can you please clarify?
Yes, I actually misread it as a pattern item definition for some
reason. Nothing to see here, move along!
>
> > > +/**
> > > * Definition of a single action.
> > > *
> > > * A list of actions is terminated by a END action.
> > > --
> > > 2.7.4
> > >
> >
> > Even if MTR is a separate API, please add to this commit:
> >
> > - Documentation update: guides/prog_guide/rte_flow.rst
> > - Testpmd update: app/test-pmd/cmdline_flow.c
> > - Testpmd documentation update:
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >
> > You can find examples in previous commits related to rte_flow.
> >
>
> All of these items are a must and will get done, but do they have to be done in the same patch set?
That'd be much better as far as I'm involved in this review (rte_flow
changes). You should put them in the same patch as the above.
> My plan was to introduce test-pmd updates through separate patch sets after the API is accepted. I know you had these items done in the same patch set for rte_flow, but there are other APIs such as eventdev and ethdev traffic management which introduced sample app one release later.
In that case, could you split these changes in two parts?
This patch could bring the basic MTR action support in testpmd, by this I
mean the ability to type a flow command with such an action and not get
rejected with a "bad arguments" error since it is actually part of the API,
even if nothing is connected to that action at this point. The rule should
however get rejected by its lack of support in the underlying PMD.
Same idea for rte_flow and testpmd documentation update, this patch only
needs the minimum amount describing what this action is without a link to
the actual MTR documentation, which is not present at this point.
Subsequent commits shall update these as they complete the MTR API.
--
Adrien Mazarguil
6WIND
next prev parent reply other threads:[~2017-09-19 17:01 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 [this message]
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
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=20170919170051.GP2481@6wind.com \
--to=adrien.mazarguil@6wind.com \
--cc=cristian.dumitrescu@intel.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
--cc=jerin.jacob@caviumnetworks.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).