From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 6498211D4 for ; Wed, 30 Aug 2017 17:12:09 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga105.fm.intel.com with ESMTP; 30 Aug 2017 08:12:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,448,1498546800"; d="scan'208";a="895563753" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by FMSMGA003.fm.intel.com with ESMTP; 30 Aug 2017 08:12:07 -0700 Received: from irsmsx112.ger.corp.intel.com (10.108.20.5) by irsmsx110.ger.corp.intel.com (163.33.3.25) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 30 Aug 2017 16:12:06 +0100 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.167]) by irsmsx112.ger.corp.intel.com ([169.254.1.110]) with mapi id 14.03.0319.002; Wed, 30 Aug 2017 16:12:06 +0100 From: "Iremonger, Bernard" To: Adrien Mazarguil CC: "dev@dpdk.org" , "Yigit, Ferruh" , "Ananyev, Konstantin" , "Dumitrescu, Cristian" Thread-Topic: [PATCH v2 3/6] librte_ether: initialise IPv4 protocol mask for rte_flow Thread-Index: AQHTHby61ZLcrb4WZEWT6CbEBfEvG6KczbkAgAAbp3CAAAXWgIAAGVEA Date: Wed, 30 Aug 2017 15:12:06 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C24E042CC1@IRSMSX108.ger.corp.intel.com> References: <1503496275-27492-1-git-send-email-bernard.iremonger@intel.com> <1503677438-27591-4-git-send-email-bernard.iremonger@intel.com> <20170830123912.GJ4301@6wind.com> <8CEF83825BEC744B83065625E567D7C24E042C4F@IRSMSX108.ger.corp.intel.com> <20170830143903.GK4301@6wind.com> In-Reply-To: <20170830143903.GK4301@6wind.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGQ2YTcxOGEtMjFhYy00ZDBkLTliYjYtY2IyMThiYmEyMmY0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IlhmUGhTdzlLQmlEYTlTWjYxbXU4d1BJcjRLQkI0QlwvbFJWQkhzTVQ2QU5rPSJ9 x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 3/6] librte_ether: initialise IPv4 protocol mask for rte_flow 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: Wed, 30 Aug 2017 15:12:09 -0000 Hi Adrien, > -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > Sent: Wednesday, August 30, 2017 3:39 PM > To: Iremonger, Bernard > Cc: dev@dpdk.org; Yigit, Ferruh ; Ananyev, > Konstantin ; Dumitrescu, Cristian > > Subject: Re: [PATCH v2 3/6] librte_ether: initialise IPv4 protocol mask f= or > rte_flow >=20 > On Wed, Aug 30, 2017 at 01:28:04PM +0000, Iremonger, Bernard wrote: > > Hi Adrien, > > > > > -----Original Message----- > > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > > > Sent: Wednesday, August 30, 2017 1:39 PM > > > To: Iremonger, Bernard > > > Cc: dev@dpdk.org; Yigit, Ferruh ; Ananyev, > > > Konstantin ; Dumitrescu, Cristian > > > > > > Subject: Re: [PATCH v2 3/6] librte_ether: initialise IPv4 protocol > > > mask for rte_flow > > > > > > Hi Bernard, > > > > > > On Fri, Aug 25, 2017 at 05:10:35PM +0100, Bernard Iremonger wrote: > > > > Initialise the next_proto_id mask in the default mask for > > > > rte_flow_item_type_ipv4. > > > > > > > > Signed-off-by: Bernard Iremonger > > > > --- > > > > lib/librte_ether/rte_flow.h | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/lib/librte_ether/rte_flow.h > > > > b/lib/librte_ether/rte_flow.h index bba6169..59c42fa 100644 > > > > --- a/lib/librte_ether/rte_flow.h > > > > +++ b/lib/librte_ether/rte_flow.h > > > > @@ -489,6 +489,7 @@ struct rte_flow_item_ipv4 { #ifndef > > > > __cplusplus static const struct rte_flow_item_ipv4 > rte_flow_item_ipv4_mask =3D { > > > > .hdr =3D { > > > > + .next_proto_id =3D 0xff, > > > > > > Please don't change the default mask to cover this field as it means > > > all rte_flow-based applications that do not provide a specific mask > > > (.mask =3D=3D NULL) have to always set this field to some valid value= . > > > This is not a convenient default behavior. > > > > > > > .src_addr =3D RTE_BE32(0xffffffff), > > > > .dst_addr =3D RTE_BE32(0xffffffff), > > > > }, > > > > -- > > > > 1.9.1 > > > > > > > > > > I'll have to NACK this change. The example application should define > > > its own mask if next_proto_id must be always set. > > > > Surely for IPv4 the next_proto_id will always be set to TCP(6) , UDP(17= ) or > SCTP (132). > > If the mask is 0 for next_proto_id then it is not possible to match on = the > protocol. >=20 > Applications normally match the next protocol implicitly by providing it = as the > subsequent item (e.g. in testpmd by writing "eth / ip / tcp" instead of "= eth / > ip next_proto_id spec 6"). >=20 > This change forces users to write "eth / ip next_proto_id spec 6 / tcp" o= r face > an error due to an uninitialized next_proto_id (which might be garbage du= e > to uninitialized memory, not just 0). >=20 > > I can define an ipv4_mask in the application if you insist. >=20 > Yes please, a better suggestion would be to rely on the subsequent item > type and not on the value of this field. >=20 > These default masks must only cover basic fields like source/destination > addresses and ports for most protocols. >=20 > -- > Adrien Mazarguil > 6WIND I will drop this patch and send a v3 patch set. I will define an ipv4_mask in the application. Regards, Bernard.