From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id D61141B6CA for ; Wed, 10 Oct 2018 20:42:13 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Oct 2018 11:42:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,365,1534834800"; d="scan'208";a="77025705" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga007.fm.intel.com with ESMTP; 10 Oct 2018 11:36:29 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 10 Oct 2018 11:36:29 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.111]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.183]) with mapi id 14.03.0319.002; Thu, 11 Oct 2018 02:36:27 +0800 From: "Zhang, Qi Z" To: "Zhao1, Wei" , mocan CC: "dev@dpdk.org" , "Lu, Wenzhuo" Thread-Topic: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in front to jump over ntuple filter case Thread-Index: AQHUVXKWfeVpWDyczk6TegBSJB81TaUCZkPwgBJAPACABDUgwA== Date: Wed, 10 Oct 2018 18:36:27 +0000 Message-ID: <039ED4275CED7440929022BC67E70611532BF9CE@SHSMSX103.ccr.corp.intel.com> References: <1537249732-7530-1-git-send-email-faicker.mo@ucloud.cn> <039ED4275CED7440929022BC67E70611532A1A27@SHSMSX103.ccr.corp.intel.com> <27452153.f1c7.16614f29e5f.Coremail.faicker.mo@ucloud.cn> <039ED4275CED7440929022BC67E70611532A6812@SHSMSX103.ccr.corp.intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTcxN2M4M2ItYTlkZS00NjAzLThiYzctY2E2NTM2YTU5MDk1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiQldvRXB0OVZpcnprdVptMFVLYzY1RFBRRUFja2xHNnQ4ZHMxXC8xYVlqWW5jTEJsb3ozejM3ZTBHM2RGR1Z1N2MifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 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] net/ixgbe: put 5tuple check in front to jump over ntuple filter case 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, 10 Oct 2018 18:42:14 -0000 > -----Original Message----- > From: Zhao1, Wei > Sent: Monday, October 8, 2018 2:46 AM > To: Zhang, Qi Z ; mocan > Cc: dev@dpdk.org; Lu, Wenzhuo > Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in fro= nt > to jump over ntuple filter case >=20 > Hi, >=20 >=20 > > -----Original Message----- > > From: Zhang, Qi Z > > Sent: Wednesday, September 26, 2018 7:14 PM > > To: mocan ; Zhao1, Wei > > Cc: dev@dpdk.org > > Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in > > front to jump over ntuple filter case > > > > OK, got your point. We should not reject a possible valid fdir flow at > > n-tuple flow check stage. > > > > Review-by: Qi Zhang >=20 >=20 > I agree with the point of " We should not reject a possible valid fdir fl= ow at > n-tuple flow check stage". > But, I think the fix patch should be more generic for all types filter of= this > problem. > Maybe, we should delete all " goto out" in function ixgbe_flow_create(). > Then, it will go to ntuple filter and ethertype filter, syn filter and f= dir > filter ,l2_tn_filter one by one. > And aslo, we should code as >=20 > { >=20 > Ntuple: > .......... > if(ret) > Goto ethertype > .......... >=20 > Ethertype: >=20 > .......... > if(ret) > Goto fdir filter > ......... >=20 > fdir filter: >=20 > if(ret) > Goto l2_tn_filter >=20 > l2_tn_filter: >=20 > ............. >=20 > } >=20 > This fix patch only solve the problem of ntuple and fdir. > Qi, What do you think of this? I'm not the author of this part of code, so my understanding of current imp= lementation is: It assume a flow will not be ambiguous which means if it match to some filt= er parser (ixgbe_parse_xxx_filter), it is not necessary to match on a diffe= rent filter. But I'm not sure if the assumption is correct or not, (this depends on the = knowledge of the device capability),=20 So do you mean the assumption is not correct? If you think a generic fix is= necessary, I have below comments 1. it is better be done by Intel people with enough validation=20 2. two options for patch submit. Submit a v2 with the generic fix, and it will be captured in this release. If it is not urgent, we can just accept current one first, then have a sep= arate patch in next release. Thanks Qi =20 >=20 > > > > Thanks > > Qi > > > > From: mocan [mailto:faicker.mo@ucloud.cn] > > Sent: Wednesday, September 26, 2018 4:16 PM > > To: Zhang, Qi Z > > Cc: dev@dpdk.org > > Subject: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in > > front to jump over ntuple filter case > > > > Hi Qi, > > In ixgbe_flow_create function, ntuple filter is parsed first. If the > > flow is considered to be ntuple filter, it will not go on to judge > > ethertype filter, syn filter and fdir filter. > > In the function ntuple_filter_to_5tuple, 5 tuple info is checked, but > > it's too late to jump over the ntuple filter if it's a fdir filter. > > > > > > > > > > > > > > At 2018-09-21 23:48:37, "Zhang, Qi Z" wrote: > > >Hi Faicker: > > > > > >> -----Original Message----- > > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of faicker.mo > > >> Sent: Tuesday, September 18, 2018 1:49 PM > > >> To: dev@dpdk.org > > >> Cc: faicker.mo > > >> Subject: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in front to > > >> jump > > over > > >> ntuple filter case > > >> > > >> From: "faicker.mo" > > >> > > >> Check in func ntuple_filter_to_5tuple is too late for fdir filter > > >> rule, add > > check > > >> in func cons_parse_ntuple_filter. > > > > > >Would you explain more about the intention for this patch? > > >Though it can be more fast to reject an invalid flow, but why it is > > >too late in > > your case? > > > > > >Thanks > > >Qi > > > > > > > > >> > > >> Signed-off-by: faicker.mo > > >> --- > > >> drivers/net/ixgbe/ixgbe_flow.c | 29 > > +++++++++++++++++++++++++++++ > > >> 1 file changed, 29 insertions(+) > > >> > > >> diff --git a/drivers/net/ixgbe/ixgbe_flow.c > > b/drivers/net/ixgbe/ixgbe_flow.c > > >> index 1adf1b8..f0fafeb 100644 > > >> --- a/drivers/net/ixgbe/ixgbe_flow.c > > >> +++ b/drivers/net/ixgbe/ixgbe_flow.c > > >> @@ -363,6 +363,17 @@ const struct rte_flow_action > > *next_no_void_action( > > >> item, "Not supported by ntuple filter"); > > >> return -rte_errno; > > >> } > > >> + if ((ipv4_mask->hdr.src_addr !=3D 0 && > > >> + ipv4_mask->hdr.src_addr !=3D UINT32_MAX) || > > >> + (ipv4_mask->hdr.dst_addr !=3D 0 && > > >> + ipv4_mask->hdr.dst_addr !=3D UINT32_MAX) || > > >> + (ipv4_mask->hdr.next_proto_id !=3D UINT8_MAX && > > >> + ipv4_mask->hdr.next_proto_id !=3D 0)) { > > >> + rte_flow_error_set(error, > > >> + EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, > > >> + item, "Not supported by ntuple filter"); > > >> + return -rte_errno; > > >> + } > > >> > > >> filter->dst_ip_mask =3D ipv4_mask->hdr.dst_addr; > > >> filter->src_ip_mask =3D ipv4_mask->hdr.src_addr; @@ -432,6 > > +443,15 > > >> @@ const struct rte_flow_action *next_no_void_action( > > >> item, "Not supported by ntuple filter"); > > >> return -rte_errno; > > >> } > > >> + if ((tcp_mask->hdr.src_port !=3D 0 && > > >> + tcp_mask->hdr.src_port !=3D UINT16_MAX) || > > >> + (tcp_mask->hdr.dst_port !=3D 0 && > > >> + tcp_mask->hdr.dst_port !=3D UINT16_MAX)) { > > >> + rte_flow_error_set(error, > > >> + EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, > > >> + item, "Not supported by ntuple filter"); > > >> + return -rte_errno; > > >> + } > > >> > > >> filter->dst_port_mask =3D tcp_mask->hdr.dst_port; > > >> filter->src_port_mask =3D tcp_mask->hdr.src_port; @@ -467,6 > > >> +487,15 @@ const struct rte_flow_action *next_no_void_action( > > >> item, "Not supported by ntuple filter"); > > >> return -rte_errno; > > >> } > > >> + if ((udp_mask->hdr.src_port !=3D 0 && > > >> + udp_mask->hdr.src_port !=3D UINT16_MAX) || > > >> + (udp_mask->hdr.dst_port !=3D 0 && > > >> + udp_mask->hdr.dst_port !=3D UINT16_MAX)) { > > >> + rte_flow_error_set(error, > > >> + EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, > > >> + item, "Not supported by ntuple filter"); > > >> + return -rte_errno; > > >> + } > > >> > > >> filter->dst_port_mask =3D udp_mask->hdr.dst_port; > > >> filter->src_port_mask =3D udp_mask->hdr.src_port; > > >> -- > > >> 1.8.3.1 > > >> > > >