From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id AADE58D93 for ; Mon, 16 Apr 2018 17:03:42 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Apr 2018 08:03:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,459,1517904000"; d="scan'208";a="220808786" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga005.fm.intel.com with ESMTP; 16 Apr 2018 08:03:41 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 16 Apr 2018 08:03:40 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.151]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.197]) with mapi id 14.03.0319.002; Mon, 16 Apr 2018 23:03:39 +0800 From: "Zhang, Qi Z" To: Adrien Mazarguil CC: "dev@dpdk.org" , "Doherty, Declan" , "Chandran, Sugesh" , "Glynn, Michael J" , "Liu, Yu Y" , "Ananyev, Konstantin" , "Richardson, Bruce" Thread-Topic: [PATCH v2 4/4] ether: add packet modification aciton in flow API Thread-Index: AQHT1Yc999EdaimTG0+PJsbVJ3brcKQDa0EQ Date: Mon, 16 Apr 2018 15:03:39 +0000 Message-ID: <039ED4275CED7440929022BC67E70611531924ED@SHSMSX103.ccr.corp.intel.com> References: <1522279780-34842-1-git-send-email-qi.z.zhang@intel.com> <1522617562-25940-1-git-send-email-qi.z.zhang@intel.com> <1522617562-25940-5-git-send-email-qi.z.zhang@intel.com> <20180412070343.GO4957@6wind.com> <039ED4275CED7440929022BC67E70611531903F5@SHSMSX103.ccr.corp.intel.com> <20180412102255.GQ4957@6wind.com> <039ED4275CED7440929022BC67E7061153191023@SHSMSX103.ccr.corp.intel.com> <20180416133059.GY4957@6wind.com> In-Reply-To: <20180416133059.GY4957@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzA2OTE2YzAtYTRiNi00YzA1LTk0Y2QtN2YyZWM5ZTI3NjRkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJnK2c0ZGhaTVA2dXVOc0lydmMxcWlIV1hSVkdMaHo2aXZtZEFKcjk1ZkJONWYrdmF4d1NQdERqcmpJYmZiZHQ4In0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 4/4] ether: add packet modification aciton in flow API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Apr 2018 15:03:45 -0000 > -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > Sent: Monday, April 16, 2018 9:31 PM > To: Zhang, Qi Z > Cc: dev@dpdk.org; Doherty, Declan ; Chandran, > Sugesh ; Glynn, Michael J > ; Liu, Yu Y ; Ananyev, > Konstantin ; Richardson, Bruce > > Subject: Re: [PATCH v2 4/4] ether: add packet modification aciton in flow= API >=20 > 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 > > > Cc: dev@dpdk.org; Doherty, Declan ; > > > Chandran, Sugesh ; Glynn, Michael J > > > ; Liu, Yu Y ; > > > Ananyev, Konstantin ; Richardson, > > > Bruce > > > 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 > > > > > Cc: dev@dpdk.org; Doherty, Declan ; > > > > > Chandran, Sugesh ; Glynn, Michael J > > > > > ; Liu, Yu Y ; > > > > > Ananyev, Konstantin ; Richardson, > > > > > Bruce > > > > > 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. > > > > > > 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 applicati= on, we > should avoid. > > But you are right, we need to consider device with different behavior f= or " > 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. >=20 > Drivers are free to accept an action or not depending on what is guarante= ed > to be matched on the pattern side. It's fine as long as the resulting flo= w rule > works exactly as documented. Consistency is much more important to > applications than offloads proper. >=20 > Depending on device capabilities and the importance given to offload spec= ific > 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 effo= rt > resulting in multiple device flow rules and whatnot to satisfy the reques= t by > any means necessary (see mlx5 RSS support on empty patterns in case you'r= e > curious). >=20 > 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. >=20 > 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 w= hen X > pattern item is not present" actually gives useful information. >=20 > "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 thoug= h. OK, I'm trying to understand why "Undefined behavior" is necessary for this= action. For example, we have a device that can offload TCP layer modification, for = packet that contain TCP, packet be modified, for no-tcp packet, the packet = will be dropped Also the device does not support TCP filter, so we are not able to create a= flow to filter TCP packet only.=20 Then without "Undefined behavior", the driver has to reject any flow with T= CP packet modification action, since it can't guarantee no-tcp packet not b= e impacted,=20 while with " Undefined behavior", a flow with tcp action is actually is a "= rule in question" and it can "happens to" be accepted by the driver? =20 Regards Qi >=20 > -- > Adrien Mazarguil > 6WIND