From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: "Zhang, Qi Z" <qi.z.zhang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Doherty, Declan" <declan.doherty@intel.com>,
"Chandran, Sugesh" <sugesh.chandran@intel.com>,
"Glynn, Michael J" <michael.j.glynn@intel.com>,
"Liu, Yu Y" <yu.y.liu@intel.com>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"Richardson, Bruce" <bruce.richardson@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2 4/4] ether: add packet modification aciton in flow API
Date: Mon, 16 Apr 2018 15:30:59 +0200 [thread overview]
Message-ID: <20180416133059.GY4957@6wind.com> (raw)
In-Reply-To: <039ED4275CED7440929022BC67E7061153191023@SHSMSX103.ccr.corp.intel.com>
On Fri, Apr 13, 2018 at 01:47:15PM +0000, Zhang, Qi Z wrote:
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Thursday, April 12, 2018 6:23 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; Chandran,
> > Sugesh <sugesh.chandran@intel.com>; Glynn, Michael J
> > <michael.j.glynn@intel.com>; Liu, Yu Y <yu.y.liu@intel.com>; Ananyev,
> > Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce
> > <bruce.richardson@intel.com>
> > Subject: Re: [PATCH v2 4/4] ether: add packet modification aciton in flow API
> >
> > On Thu, Apr 12, 2018 at 08:50:14AM +0000, Zhang, Qi Z wrote:
> > > Hi Adrien
> > >
> > > > -----Original Message-----
> > > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > > > Sent: Thursday, April 12, 2018 3:04 PM
> > > > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > > > Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>;
> > > > Chandran, Sugesh <sugesh.chandran@intel.com>; Glynn, Michael J
> > > > <michael.j.glynn@intel.com>; Liu, Yu Y <yu.y.liu@intel.com>;
> > > > Ananyev, Konstantin <konstantin.ananyev@intel.com>; Richardson,
> > > > Bruce <bruce.richardson@intel.com>
> > > > Subject: Re: [PATCH v2 4/4] ether: add packet modification aciton in
> > > > flow API
> > > >
> > > > On Sun, Apr 01, 2018 at 05:19:22PM -0400, Qi Zhang wrote:
> > > > > Add new actions that be used to modify packet content with generic
> > > > > semantic:
> > > > >
> > > > > RTE_FLOW_ACTION_TYPE_FIELD_UPDATE:
> > > > > - update specific field of packet
> > > > > RTE_FLWO_ACTION_TYPE_FIELD_INCREMENT:
> > > > > - increament specific field of packet
> > > > > RTE_FLWO_ACTION_TYPE_FIELD_DECREMENT:
> > > > > - decreament specific field of packet
> > > > > RTE_FLWO_ACTION_TYPE_FIELD_COPY:
> > > > > - copy data from one field to another in packet.
> > > > >
> > > > > All action use struct rte_flow_item parameter to match the pattern
> > > > > that going to be modified, if no pattern match, the action just be
> > > > > skipped.
<snip>
> > > > What happens when this action is attempted on non-matching traffic
> > > > must be documented here as well. Refer to discussion re "ethdev: Add
> > > > tunnel encap/decap actions" [3]. To be on the safe side, it must be
> > > > documented as resulting in undefined behavior.
> > >
> > > so what is "undefined behavior" you means?
> > > The rule is:
> > > If a packet matched pattern in action, it will be modified, otherwise
> > > the action just take no effect is this idea acceptable?
> >
> > Not really, what happens will depend on the underlying device. It's better to
> > document it as undefined because you can't predict the result. Some devices
> > will cause packets to be lost, others will let them through unchanged, others
> > will crash the system after formatting the hard drive, no one knows.
>
> OK, basically I think "undefined behavior" is not friendly to application, we should avoid.
> But you are right, we need to consider device with different behavior for " modification on non-matched pattern"
> I'm thinking why driver can't avoid non-matched pattern modification if the device does not support?
> For example, driver can reject a flow ETH/IPV4 with TCP action, but may accept ETH/IPV4/TCP with TCP action base on
> its capability.
Drivers are free to accept an action or not depending on what is guaranteed
to be matched on the pattern side. It's fine as long as the resulting flow
rule works exactly as documented. Consistency is much more important to
applications than offloads proper.
Depending on device capabilities and the importance given to offload
specific use cases by vendors, PMD support may range from a basic 1:1
translation attempt between rte_flow and device format, to an all out
processing effort resulting in multiple device flow rules and whatnot to
satisfy the request by any means necessary (see mlx5 RSS support on empty
patterns in case you're curious).
Whichever approach you choose (basic or complex), my recommendation is
simply to make sure the PMD reports an error whenever a flow rule is
ambiguous and could result in unexpected behavior if applied as is to the
device.
The error message should also be helpful. A message such as "unable to apply
flow rule" is pretty useless, while "this action is not supported when X
pattern item is not present" actually gives useful information.
"Undefined behavior" is for application writers. It means that if a PMD
happens to accept the rule in question, what happens isn't covered by
documentation. Ideally a PMD shouldn't accept it in the first place
though.
--
Adrien Mazarguil
6WIND
next prev parent reply other threads:[~2018-04-16 13:31 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-28 23:29 [dpdk-dev] [PATCH 0/4] rte_flow extension for vSwitch acceleration Qi Zhang
2018-03-28 23:29 ` [dpdk-dev] [PATCH 1/4] ether: add flow action to redirect packet in a switch domain Qi Zhang
2018-03-29 10:48 ` Pattan, Reshma
2018-03-29 11:20 ` Pattan, Reshma
2018-03-28 23:29 ` [dpdk-dev] [PATCH 2/4] ether: add flow last hit query support Qi Zhang
2018-03-28 23:29 ` [dpdk-dev] [PATCH 3/4] ether: add more protocol support in flow API Qi Zhang
2018-03-29 14:32 ` Pattan, Reshma
2018-03-28 23:29 ` [dpdk-dev] [PATCH 4/4] ether: add packet modification aciton " Qi Zhang
2018-03-29 15:23 ` Pattan, Reshma
2018-04-02 3:35 ` Zhang, Qi Z
2018-04-01 21:19 ` [dpdk-dev] [PATCH v2 0/4] rte_flow extension for vSwitch acceleration Qi Zhang
2018-04-01 21:19 ` [dpdk-dev] [PATCH v2 1/4] ether: add flow action to redirect packet to a port Qi Zhang
2018-04-11 16:30 ` Adrien Mazarguil
2018-04-01 21:19 ` [dpdk-dev] [PATCH v2 2/4] ether: add flow last hit query support Qi Zhang
2018-04-11 16:31 ` Adrien Mazarguil
2018-04-01 21:19 ` [dpdk-dev] [PATCH v2 3/4] ether: add more protocol support in flow API Qi Zhang
2018-04-11 16:32 ` Adrien Mazarguil
2018-04-12 5:12 ` Zhang, Qi Z
2018-04-12 9:19 ` Adrien Mazarguil
2018-04-12 10:00 ` Zhang, Qi Z
2018-04-01 21:19 ` [dpdk-dev] [PATCH v2 4/4] ether: add packet modification aciton " Qi Zhang
2018-04-12 7:03 ` Adrien Mazarguil
2018-04-12 8:50 ` Zhang, Qi Z
2018-04-12 10:22 ` Adrien Mazarguil
2018-04-13 13:47 ` Zhang, Qi Z
2018-04-16 13:30 ` Adrien Mazarguil [this message]
2018-04-16 15:03 ` Zhang, Qi Z
2018-04-17 9:55 ` Adrien Mazarguil
2018-04-17 10:32 ` Zhang, Qi Z
2018-04-10 14:12 ` [dpdk-dev] [PATCH v2 0/4] rte_flow extension for vSwitch acceleration Thomas Monjalon
2018-04-16 5:16 ` [dpdk-dev] [PATCH v3 " Qi Zhang
2018-04-16 5:16 ` [dpdk-dev] [PATCH v3 1/4] ethdev: add more protocol support in flow API Qi Zhang
2018-04-16 5:16 ` [dpdk-dev] [PATCH v3 2/4] ethdev: add packet field set aciton " Qi Zhang
2018-04-16 5:16 ` [dpdk-dev] [PATCH v3 3/4] ethdev: add TTL change actions " Qi Zhang
2018-04-16 5:48 ` Shahaf Shuler
2018-04-16 8:12 ` Adrien Mazarguil
2018-04-16 8:56 ` Shahaf Shuler
2018-04-16 9:59 ` Adrien Mazarguil
2018-04-16 5:16 ` [dpdk-dev] [PATCH v3 4/4] ethdev: add VLAN and MPLS pop push action " Qi Zhang
2018-04-16 6:10 ` [dpdk-dev] [PATCH v3 0/4] rte_flow extension for vSwitch acceleration Qi Zhang
2018-04-16 6:10 ` [dpdk-dev] [PATCH v3 1/4] ethdev: add more protocol support in flow API Qi Zhang
2018-04-19 14:48 ` Adrien Mazarguil
2018-04-16 6:10 ` [dpdk-dev] [PATCH v3 2/4] ethdev: add packet field set aciton " Qi Zhang
2018-04-19 14:48 ` Adrien Mazarguil
2018-04-20 2:24 ` Zhang, Qi Z
2018-04-20 8:54 ` Adrien Mazarguil
2018-04-16 6:10 ` [dpdk-dev] [PATCH v3 3/4] ethdev: add TTL change actions " Qi Zhang
2018-04-19 14:48 ` Adrien Mazarguil
2018-04-16 6:10 ` [dpdk-dev] [PATCH v3 4/4] ethdev: add VLAN and MPLS pop push action " Qi Zhang
2018-04-17 7:34 ` Shahaf Shuler
2018-04-19 14:49 ` Adrien Mazarguil
2018-04-23 6:36 ` [dpdk-dev] [PATCH v4 0/3] rte_flow extension for vSwitch acceleration Qi Zhang
2018-04-23 6:36 ` [dpdk-dev] [PATCH v4 1/3] ethdev: add more protocol support in flow API Qi Zhang
2018-04-23 6:36 ` [dpdk-dev] [PATCH v4 2/3] ethdev: add TTL change actions " Qi Zhang
2018-04-23 6:36 ` [dpdk-dev] [PATCH v4 3/3] ethdev: add VLAN and MPLS " Qi Zhang
2018-04-24 15:58 ` [dpdk-dev] [PATCH v5 0/3] rte_flow extension for vSwitch acceleration Adrien Mazarguil
2018-04-24 15:58 ` [dpdk-dev] [PATCH v5 1/3] ethdev: add neighbor discovery support to flow API Adrien Mazarguil
2018-04-24 15:59 ` [dpdk-dev] [PATCH v5 2/3] ethdev: add TTL change actions " Adrien Mazarguil
2018-04-24 15:59 ` [dpdk-dev] [PATCH v5 3/3] ethdev: add VLAN and MPLS " Adrien Mazarguil
2018-04-25 13:54 ` [dpdk-dev] [PATCH v5 0/3] rte_flow extension for vSwitch acceleration Zhang, Qi Z
2018-04-25 22:44 ` Ferruh Yigit
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=20180416133059.GY4957@6wind.com \
--to=adrien.mazarguil@6wind.com \
--cc=bruce.richardson@intel.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@intel.com \
--cc=michael.j.glynn@intel.com \
--cc=qi.z.zhang@intel.com \
--cc=sugesh.chandran@intel.com \
--cc=yu.y.liu@intel.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).