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 2F29CA04E6; Tue, 17 Nov 2020 16:31:49 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BDF3637B1; Tue, 17 Nov 2020 16:31:47 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 3235F2AB for ; Tue, 17 Nov 2020 16:31:46 +0100 (CET) IronPort-SDR: k9hcu0CTcYzgui83oKFrdwXvelotKTz/N1qS/24xKn/p//LkphRXiQJN5tFMsr0EU2BfsjXkqm 3dMdmt5shI3A== X-IronPort-AV: E=McAfee;i="6000,8403,9808"; a="158718829" X-IronPort-AV: E=Sophos;i="5.77,485,1596524400"; d="scan'208";a="158718829" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Nov 2020 07:31:43 -0800 IronPort-SDR: jUEZNOt3Vxjc7086rNWUBl+jfkoxgaeec75543Ee6GrUmvU2V9GnANvgRu6EVIU/AoUXp3YDik iHHt8l0pgJgA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,485,1596524400"; d="scan'208";a="340931987" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by orsmga002.jf.intel.com with ESMTP; 17 Nov 2020 07:31:43 -0800 Received: from shsmsx601.ccr.corp.intel.com (10.109.6.141) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 17 Nov 2020 07:31:42 -0800 Received: from shsmsx605.ccr.corp.intel.com (10.109.6.215) by SHSMSX601.ccr.corp.intel.com (10.109.6.141) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 17 Nov 2020 23:31:40 +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; Tue, 17 Nov 2020 23:31:40 +0800 From: "Cao, Yahui" To: "Yan, Zhirun" , "dev@dpdk.org" , "Zhang, Qi Z" , "Wang, Xiao W" , "Su, Simei" Thread-Topic: [PATCH v1] net/ice: refactor flow pattern parser Thread-Index: AQHWvL7y6e0lGtQm6ESFbbHnGjUYX6nMbyPg Date: Tue, 17 Nov 2020 15:31:40 +0000 Message-ID: <567972babaca49939efeebe98859eb50@intel.com> References: <20201117084524.3610038-1-zhirun.yan@intel.com> In-Reply-To: <20201117084524.3610038-1-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.108.32.68] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v1] 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" Hi Zhirun, I think it is great to refactor ice fdir to differentiate inner an= d outer field vector in a more clear way. Thanks for your commit. It seems there still needs some effort to complete this patch. > -----Original Message----- > From: Yan, Zhirun > Sent: Tuesday, November 17, 2020 4:45 PM > To: dev@dpdk.org; Zhang, Qi Z ; Cao, Yahui ; Wang, Xiao W ; > Su, Simei > Cc: Yan, Zhirun > Subject: [PATCH v1] net/ice: refactor flow pattern parser >=20 > Distinguish inner/outer fields. And avoid too many nested conditionals > in each type's parser. [Cao, Yahui]=20 Since this is quite a huge refactor, could you give a more detailed descrip= tion in the commit message? Thanks. It would be better if you can make them a patchset. >=20 > Signed-off-by: Zhirun Yan > --- > drivers/net/ice/ice_fdir_filter.c | 504 ++++++++++++++++-------------- > 1 file changed, 269 insertions(+), 235 deletions(-) >=20 > diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir= _filter.c > index 175abcdd5c..b53ed30b1c 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 inner_input_set =3D ICE_INSET_NONE; > + uint64_t outer_input_set =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,315 @@ 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; > + break; > + } [Cao, Yahui]=20 You should take both of VXLAN and GTP cases into consideration > + } > + > + /* This loop parse flow pattern and distinguish Non-tunnel and tunnel > + * flow. For tunnel flow, reuse non-tunnel structure to track inner > + * part. > + * > + * is_outer tunnel_type p_input_set input_set_bit data_struct > + * Non-Tun Y N inner outer origin > + * Tun-out Y Y outer outer outer > + * Tun-in N Y inner inner origin > + */ > for (item =3D pattern; item->type !=3D RTE_FLOW_ITEM_TYPE_END; item++) = { > if (item->last) { > rte_flow_error_set(error, EINVAL, > - RTE_FLOW_ERROR_TYPE_ITEM, > - item, > - "Not support range"); > + RTE_FLOW_ERROR_TYPE_ITEM, > + item, > + "Not support range"); > return -rte_errno; > } > item_type =3D item->type; >=20 > + input_set =3D (tunnel_type && is_outer) ? > + &outer_input_set : > + &inner_input_set; > + > + 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]=20 Why do you put p_v4 value assignment out of switch case RTE_FLOW_ITEM_TYPE_= IPV4? Why do you put p_v6 value assignment out of switch case RTE_FLOW_ITEM_TYPE_= IPV6? > switch (item_type) { > case RTE_FLOW_ITEM_TYPE_ETH: > + flow_type =3D ICE_FLTR_PTYPE_NON_IP_L2; > eth_spec =3D item->spec; > eth_mask =3D item->mask; > - next_type =3D (item + 1)->type; >=20 > - if (eth_spec && eth_mask) { > - if (!rte_is_zero_ether_addr(ð_mask->dst)) { > - input_set |=3D ICE_INSET_DMAC; > - rte_memcpy(&filter->input.ext_data.dst_mac, > - ð_spec->dst, > - RTE_ETHER_ADDR_LEN); > - } > + if (!(eth_spec && eth_mask)) > + break; >=20 > - if (!rte_is_zero_ether_addr(ð_mask->src)) { > - input_set |=3D ICE_INSET_SMAC; > - rte_memcpy(&filter->input.ext_data.src_mac, > - ð_spec->src, > - RTE_ETHER_ADDR_LEN); > - } > + *input_set |=3D is_outer ? ICE_PROT_MAC_OUTER : ICE_PROT_MAC_INNER; [Cao, Yahui]=20 ICE_PROT_XXX is internally used. You should use ICE_INSET_XXX version. The same comment applies for similar case elsewhere. > + if (!rte_is_zero_ether_addr(ð_mask->dst)) > + *input_set |=3D ICE_DMAC; [Cao, Yahui]=20 You should not use ICE_DMAC here. You should use ICE_INSET_XXX version. The same comment applies for similar case elsewhere. > + if (!rte_is_zero_ether_addr(ð_mask->src)) > + *input_set |=3D ICE_SMAC; >=20 > - /* Ignore this field except for ICE_FLTR_PTYPE_NON_IP_L2 */ > - if (eth_mask->type =3D=3D RTE_BE16(0xffff) && > - next_type =3D=3D RTE_FLOW_ITEM_TYPE_END) { > - input_set |=3D ICE_INSET_ETHERTYPE; > - ether_type =3D rte_be_to_cpu_16(eth_spec->type); > - > - if (ether_type =3D=3D RTE_ETHER_TYPE_IPV4 || > - ether_type =3D=3D RTE_ETHER_TYPE_IPV6) { > - rte_flow_error_set(error, EINVAL, > - RTE_FLOW_ERROR_TYPE_ITEM, > - item, > - "Unsupported ether_type."); > - return -rte_errno; > - } > - > - rte_memcpy(&filter->input.ext_data.ether_type, .... > } > -- > 2.25.1