From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7E4ECA052A; Fri, 25 Dec 2020 06:21:58 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2A299CA1A; Fri, 25 Dec 2020 06:21:56 +0100 (CET) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id A20C9CA0A for ; Fri, 25 Dec 2020 06:21:54 +0100 (CET) IronPort-SDR: l18wUvraKprWBd3VAc0M/+6y7cPYGdXcSQJay3jFej+BS3hdn4dj2JEL1WT51K0d/GoSZRRgkv DX5cO61/C/lg== X-IronPort-AV: E=McAfee;i="6000,8403,9845"; a="173624592" X-IronPort-AV: E=Sophos;i="5.78,447,1599548400"; d="scan'208";a="173624592" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Dec 2020 21:21:52 -0800 IronPort-SDR: EWUmHEkuvMSYZ8eqsy4+7tVOLtD2hz9FLNtziMDg4nTjyx9Q6ueBob7mJMVj/JzeFWGJQ+h8gj xfQlD6wxoAZQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.78,447,1599548400"; d="scan'208";a="340067615" Received: from fmsmsx602.amr.corp.intel.com ([10.18.126.82]) by fmsmga007.fm.intel.com with ESMTP; 24 Dec 2020 21:21:52 -0800 Received: from shsmsx604.ccr.corp.intel.com (10.109.6.214) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 24 Dec 2020 21:21:51 -0800 Received: from shsmsx605.ccr.corp.intel.com (10.109.6.215) by SHSMSX604.ccr.corp.intel.com (10.109.6.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 25 Dec 2020 13:21:49 +0800 Received: from shsmsx605.ccr.corp.intel.com ([10.109.6.215]) by SHSMSX605.ccr.corp.intel.com ([10.109.6.215]) with mapi id 15.01.1713.004; Fri, 25 Dec 2020 13:21:49 +0800 From: "Cao, Yahui" To: "Yan, Zhirun" , "dev@dpdk.org" , "Zhang, Qi Z" , "Wang, Xiao W" , "Guo, Junfeng" CC: "Su, Simei" , "Xu, Ting" , "Zhang, Yuying" Thread-Topic: [PATCH v1 2/5] net/ice: refactor flow pattern parser Thread-Index: AQHW12ZfGNdOsQsH+UOlS2gZ4F//P6oHSWvw Date: Fri, 25 Dec 2020 05:21:49 +0000 Message-ID: <2e7933eca3084b798cffe4a992be1246@intel.com> References: <20201221065150.1600719-1-zhirun.yan@intel.com> <20201221065150.1600719-3-zhirun.yan@intel.com> In-Reply-To: <20201221065150.1600719-3-zhirun.yan@intel.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 x-originating-ip: [10.239.127.36] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v1 2/5] net/ice: refactor flow pattern parser 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Yan, Zhirun > Sent: Monday, December 21, 2020 2:52 PM > To: dev@dpdk.org; Zhang, Qi Z ; Cao, Yahui ; Wang, Xiao W ; > Guo, Junfeng > Cc: Su, Simei ; Xu, Ting ; Zhang, = Yuying ; Yan, Zhirun > > Subject: [PATCH v1 2/5] net/ice: refactor flow pattern parser >=20 > Distinguish inner/outer input set. And avoid too many nested > conditionals in each type's parser. input_set_f is used for > outer fields, input_set_l is used for inner or non-tunnel > fields. >=20 > Signed-off-by: Zhirun Yan > --- > drivers/net/ice/ice_fdir_filter.c | 467 +++++++++++++++--------------- > 1 file changed, 229 insertions(+), 238 deletions(-) >=20 > diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir= _filter.c > index 92b8a7e6ad..1f2576a444 100644 > --- a/drivers/net/ice/ice_fdir_filter.c > +++ b/drivers/net/ice/ice_fdir_filter.c > @@ -1646,7 +1646,9 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adap= ter *ad, > const struct rte_flow_item_vxlan *vxlan_spec, *vxlan_mask; > const struct rte_flow_item_gtp *gtp_spec, *gtp_mask; > const struct rte_flow_item_gtp_psc *gtp_psc_spec, *gtp_psc_mask; > - uint64_t input_set =3D ICE_INSET_NONE; > + uint64_t input_set_l =3D ICE_INSET_NONE; > + uint64_t input_set_f =3D ICE_INSET_NONE; > + uint64_t *input_set; > uint8_t flow_type =3D ICE_FLTR_PTYPE_NONF_NONE; > uint8_t ipv6_addr_mask[16] =3D { > 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, > @@ -1655,289 +1657,276 @@ ice_fdir_parse_pattern(__rte_unused struct ice_= adapter *ad, > uint32_t vtc_flow_cpu; > uint16_t ether_type; > enum rte_flow_item_type next_type; > + bool is_outer =3D true; > + struct ice_fdir_extra *p_ext_data; > + struct ice_fdir_v4 *p_v4; > + struct ice_fdir_v6 *p_v6; >=20 > + for (item =3D pattern; item->type !=3D RTE_FLOW_ITEM_TYPE_END; item++) = { > + if (item->type =3D=3D RTE_FLOW_ITEM_TYPE_VXLAN) > + tunnel_type =3D ICE_FDIR_TUNNEL_TYPE_VXLAN; > + if (item->type =3D=3D RTE_FLOW_ITEM_TYPE_GTPU) > + tunnel_type =3D ICE_FDIR_TUNNEL_TYPE_GTPU; > + if (item->type =3D=3D RTE_FLOW_ITEM_TYPE_GTP_PSC) > + tunnel_type =3D ICE_FDIR_TUNNEL_TYPE_GTPU_EH; > + } > + > + /* This loop parse flow pattern and distinguish Non-tunnel and tunnel > + * flow. input_set_l is used for non-tunnel and tunnel inner part. > + */ ... >=20 > + input_set =3D (tunnel_type && is_outer) ? > + &input_set_f : &input_set_l; [Cao, Yahui] input_set_l used for inner filed or non-tunnel filed looks con= fusing, can we just use input_set_l as non-tunnel filed or tunnel outer fie= ld ? > + > + if (l3 =3D=3D RTE_FLOW_ITEM_TYPE_IPV4) > + p_v4 =3D (tunnel_type && is_outer) ? > + &filter->input.ip_outer.v4 : > + &filter->input.ip.v4; > + if (l3 =3D=3D RTE_FLOW_ITEM_TYPE_IPV6) > + p_v6 =3D (tunnel_type && is_outer) ? > + &filter->input.ip_outer.v6 : > + &filter->input.ip.v6; > + [Cao, Yahui] p_v4 and p_v6 pointer assignment looks redundant since it will= be assigned in IPV4 and IPV6 switch case statement. > switch (item_type) { ... > + > + p_v4 =3D (tunnel_type && is_outer) ? > + &filter->input.ip_outer.v4 : > + &filter->input.ip.v4; [Cao, Yahui] it is assigned here again > + p_v4->dst_ip =3D ipv4_spec->hdr.dst_addr; > + p_v4->src_ip =3D ipv4_spec->hdr.src_addr; > + p_v4->tos =3D ipv4_spec->hdr.type_of_service; > break; ...... > + > + p_v6 =3D (tunnel_type && is_outer) ? > + &filter->input.ip_outer.v6 : > + &filter->input.ip.v6; [Cao, Yahui] it is assigned here again > + rte_memcpy(&p_v6->dst_ip, ipv6_spec->hdr.dst_addr, 16); > + rte_memcpy(&p_v6->src_ip, ipv6_spec->hdr.src_addr, 16); > + vtc_flow_cpu =3D rte_be_to_cpu_32(ipv6_spec->hdr.vtc_flow); > + p_v6->tc =3D (uint8_t)(vtc_flow_cpu >> ICE_FDIR_IPV6_TC_OFFSET); > + p_v6->proto =3D ipv6_spec->hdr.proto; > + p_v6->hlim =3D ipv6_spec->hdr.hop_limits; > break; ...... > filter->tunnel_type =3D tunnel_type; > filter->input.flow_type =3D flow_type; > - filter->input_set =3D input_set; > + filter->input_set =3D input_set_l; > + filter->outer_input_set =3D input_set_f; [Cao, Yahui] Correspondingly here, input_set and outer_input_set naming is = also confusing. >=20 > return 0; > } > -- > 2.25.1