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 86833A04E7; Wed, 4 Nov 2020 10:18:59 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5E9C1C864; Wed, 4 Nov 2020 10:18:57 +0100 (CET) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 117CBC858; Wed, 4 Nov 2020 10:18:53 +0100 (CET) IronPort-SDR: 8b4tWMEWCfHSJRfiBaVF2rADLiAXm9sucBhHsFVqcn1dW7+LV5SpAock9dx3Ehc8MlOntnPiaD 5nSgdizqFGEg== X-IronPort-AV: E=McAfee;i="6000,8403,9794"; a="166599403" X-IronPort-AV: E=Sophos;i="5.77,450,1596524400"; d="scan'208";a="166599403" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Nov 2020 01:18:52 -0800 IronPort-SDR: SJsaMSTRA9wsE8JyuhZvKk8UXXq8GiFBI7OEaVgGPhhBV6QbjVWYeXVEv/x9lwA6kdkhl+nsBy q1Hohs+mvUzw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,450,1596524400"; d="scan'208";a="306349634" Received: from fmsmsx604.amr.corp.intel.com ([10.18.126.84]) by fmsmga007.fm.intel.com with ESMTP; 04 Nov 2020 01:18:51 -0800 Received: from shsmsx603.ccr.corp.intel.com (10.109.6.143) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 4 Nov 2020 01:18:51 -0800 Received: from shsmsx601.ccr.corp.intel.com (10.109.6.141) by SHSMSX603.ccr.corp.intel.com (10.109.6.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 4 Nov 2020 17:18:49 +0800 Received: from shsmsx601.ccr.corp.intel.com ([10.109.6.141]) by SHSMSX601.ccr.corp.intel.com ([10.109.6.141]) with mapi id 15.01.1713.004; Wed, 4 Nov 2020 17:18:49 +0800 From: "Guo, Jia" To: "Di, ChenxuX" , "dev@dpdk.org" CC: "Xing, Beilei" , "Wang, Haiyue" , "Di, ChenxuX" , "stable@dpdk.org" Thread-Topic: [PATCH 2/2] net/i40e: fix incorrect FDIR flex pit Thread-Index: AQHWsoa/Hth79k8IikqvXRfAnkBksqm3r8xQ Date: Wed, 4 Nov 2020 09:18:49 +0000 Message-ID: <36fa7db4c27244ddb043fb7fd3e0f11a@intel.com> References: <20201104082959.63800-1-chenxux.di@intel.com> <20201104082959.63800-3-chenxux.di@intel.com> In-Reply-To: <20201104082959.63800-3-chenxux.di@intel.com> Accept-Language: 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 2/2] net/i40e: fix incorrect FDIR flex pit 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, chenxu > -----Original Message----- > From: Chenxu Di > Sent: Wednesday, November 4, 2020 4:30 PM > To: dev@dpdk.org > Cc: Xing, Beilei ; Guo, Jia ; W= ang, > Haiyue ; Di, ChenxuX ; > stable@dpdk.org > Subject: [PATCH 2/2] net/i40e: fix incorrect FDIR flex pit >=20 > The register of FDIR flex pit should not be set during flow validate. > It should be set when flow create. >=20 > Fixes: 6ced3dd72f5f ("net/i40e: support flexible payload parsing for FDIR= ") > Cc: stable@dpdk.org >=20 > Signed-off-by: Chenxu Di > --- > drivers/net/i40e/i40e_ethdev.h | 21 ++++--- > drivers/net/i40e/i40e_fdir.c | 100 > +++++++++++++++++++++++++++++++++ > drivers/net/i40e/i40e_flow.c | 98 +++----------------------------- > 3 files changed, 119 insertions(+), 100 deletions(-) >=20 > diff --git a/drivers/net/i40e/i40e_ethdev.h > b/drivers/net/i40e/i40e_ethdev.h index 6be929f65..e00133c88 100644 > --- a/drivers/net/i40e/i40e_ethdev.h > +++ b/drivers/net/i40e/i40e_ethdev.h > @@ -599,12 +599,22 @@ enum i40e_fdir_ip_type { > I40E_FDIR_IPTYPE_IPV6, > }; >=20 > +/* > + * Structure to store flex pit for flow diretor. > + */ > +struct i40e_fdir_flex_pit { > + uint8_t src_offset; /* offset in words from the beginning of payload > */ > + uint8_t size; /* size in words */ > + uint8_t dst_offset; /* offset in words of flexible payload */ }; > + > /* A structure used to contain extend input of flow */ struct > i40e_fdir_flow_ext { > uint16_t vlan_tci; > uint8_t flexbytes[RTE_ETH_FDIR_MAX_FLEXLEN]; > /* It is filled by the flexible payload to match. */ > uint8_t flex_mask[I40E_FDIR_MAX_FLEX_LEN]; > + uint8_t raw_id; > uint8_t is_vf; /* 1 for VF, 0 for port dev */ > uint16_t dst_id; /* VF ID, available when is_vf is 1*/ > bool inner_ip; /* If there is inner ip */ > @@ -613,6 +623,8 @@ struct i40e_fdir_flow_ext { > bool customized_pctype; /* If customized pctype is used */ > bool pkt_template; /* If raw packet template is used */ > bool is_udp; /* ipv4|ipv6 udp flow */ > + enum i40e_flxpld_layer_idx layer_idx; > + struct i40e_fdir_flex_pit flex_pit[I40E_MAX_FLXPLD_LAYER * > +I40E_MAX_FLXPLD_FIED]; > }; >=20 > /* A structure used to define the input for a flow director filter entry= */ @@ > -664,15 +676,6 @@ struct i40e_fdir_filter_conf { > struct i40e_fdir_action action; /* Action taken when match */ }; >=20 > -/* > - * Structure to store flex pit for flow diretor. > - */ > -struct i40e_fdir_flex_pit { > - uint8_t src_offset; /* offset in words from the beginning of payload > */ > - uint8_t size; /* size in words */ > - uint8_t dst_offset; /* offset in words of flexible payload */ > -}; > - > struct i40e_fdir_flex_mask { > uint8_t word_mask; /**< Bit i enables word i of flexible payload */ > uint8_t nb_bitmask; > diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c = index > e31464eb7..e64cb2fd0 100644 > --- a/drivers/net/i40e/i40e_fdir.c > +++ b/drivers/net/i40e/i40e_fdir.c > @@ -1765,6 +1765,81 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev, > return ret; > } >=20 > +static int > +i40e_flow_store_flex_pit(struct i40e_pf *pf, > + struct i40e_fdir_flex_pit *flex_pit, > + enum i40e_flxpld_layer_idx layer_idx, > + uint8_t raw_id) > +{ > + uint8_t field_idx; > + > + field_idx =3D layer_idx * I40E_MAX_FLXPLD_FIED + raw_id; > + /* Check if the configuration is conflicted */ > + if (pf->fdir.flex_pit_flag[layer_idx] && > + (pf->fdir.flex_set[field_idx].src_offset !=3D flex_pit->src_offset = || > + pf->fdir.flex_set[field_idx].size !=3D flex_pit->size || > + pf->fdir.flex_set[field_idx].dst_offset !=3D flex_pit->dst_offset)= ) > + return -1; > + > + /* Check if the configuration exists. */ > + if (pf->fdir.flex_pit_flag[layer_idx] && > + (pf->fdir.flex_set[field_idx].src_offset =3D=3D flex_pit->src_offse= t && > + pf->fdir.flex_set[field_idx].size =3D=3D flex_pit->size && > + pf->fdir.flex_set[field_idx].dst_offset =3D=3D flex_pit->dst_offse= t)) > + return 1; > + > + pf->fdir.flex_set[field_idx].src_offset =3D > + flex_pit->src_offset; > + pf->fdir.flex_set[field_idx].size =3D > + flex_pit->size; > + pf->fdir.flex_set[field_idx].dst_offset =3D > + flex_pit->dst_offset; > + > + return 0; > +} > + > +static void > +i40e_flow_set_fdir_flex_pit(struct i40e_pf *pf, > + enum i40e_flxpld_layer_idx layer_idx, > + uint8_t raw_id) > +{ > + struct i40e_hw *hw =3D I40E_PF_TO_HW(pf); > + uint32_t flx_pit, flx_ort; > + uint8_t field_idx; > + uint16_t min_next_off =3D 0; /* in words */ > + uint8_t i; > + > + if (raw_id) { > + flx_ort =3D (1 << I40E_GLQF_ORT_FLX_PAYLOAD_SHIFT) | > + (raw_id << I40E_GLQF_ORT_FIELD_CNT_SHIFT) | > + (layer_idx * I40E_MAX_FLXPLD_FIED); > + I40E_WRITE_GLB_REG(hw, I40E_GLQF_ORT(33 + layer_idx), > flx_ort); > + } > + > + /* Set flex pit */ > + for (i =3D 0; i < raw_id; i++) { > + field_idx =3D layer_idx * I40E_MAX_FLXPLD_FIED + i; > + flx_pit =3D MK_FLX_PIT(pf->fdir.flex_set[field_idx].src_offset, > + pf->fdir.flex_set[field_idx].size, > + pf->fdir.flex_set[field_idx].dst_offset); > + > + I40E_WRITE_REG(hw, I40E_PRTQF_FLX_PIT(field_idx), > flx_pit); > + min_next_off =3D pf->fdir.flex_set[field_idx].src_offset + > + pf->fdir.flex_set[field_idx].size; > + } > + > + for (; i < I40E_MAX_FLXPLD_FIED; i++) { > + /* set the non-used register obeying register's constrain */ > + field_idx =3D layer_idx * I40E_MAX_FLXPLD_FIED + i; > + flx_pit =3D MK_FLX_PIT(min_next_off, > NONUSE_FLX_PIT_FSIZE, > + NONUSE_FLX_PIT_DEST_OFF); > + I40E_WRITE_REG(hw, I40E_PRTQF_FLX_PIT(field_idx), > flx_pit); > + min_next_off++; > + } > + > + pf->fdir.flex_pit_flag[layer_idx] =3D 1; } > + > static int > i40e_flow_store_flex_mask(struct i40e_pf *pf, > enum i40e_filter_pctype pctype, > @@ -1889,13 +1964,17 @@ i40e_flow_add_del_fdir_filter(struct > rte_eth_dev *dev, { > struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > struct i40e_pf *pf =3D I40E_DEV_PRIVATE_TO_PF(dev->data- > >dev_private); > + enum i40e_flxpld_layer_idx layer_idx =3D I40E_FLXPLD_L2_IDX; > unsigned char *pkt =3D NULL; > enum i40e_filter_pctype pctype; > struct i40e_fdir_info *fdir_info =3D &pf->fdir; > uint8_t flex_mask[I40E_FDIR_MAX_FLEX_LEN]; > struct i40e_fdir_filter *node; > struct i40e_fdir_filter check_filter; /* Check if the filter exists */ > + struct i40e_fdir_flex_pit flex_pit; > + bool cfg_flex_pit =3D true; > bool wait_status =3D true; > + uint8_t field_idx; > int ret =3D 0; > int i; >=20 > @@ -1931,6 +2010,27 @@ i40e_flow_add_del_fdir_filter(struct rte_eth_dev > *dev, >=20 > if (add) { > if (!filter->input.flow_ext.customized_pctype) { > + for (i =3D 0; i < filter->input.flow_ext.raw_id; i++) { > + layer_idx =3D filter->input.flow_ext.layer_idx; > + field_idx =3D layer_idx * > I40E_MAX_FLXPLD_FIED + i; > + flex_pit =3D filter- > >input.flow_ext.flex_pit[field_idx]; > + > + /* Store flex pit to SW */ > + ret =3D i40e_flow_store_flex_pit(pf, &flex_pit, > + layer_idx, i); > + if (ret < 0) { > + PMD_DRV_LOG(ERR, "Conflict with > the" > + " first flexible rule."); > + return -EINVAL; > + } else if (ret > 0) { > + cfg_flex_pit =3D false; > + } > + } > + > + if (cfg_flex_pit) > + i40e_flow_set_fdir_flex_pit(pf, layer_idx, > + filter- > >input.flow_ext.raw_id); > + > /* Store flex mask to SW */ > for (i =3D 0; i < I40E_FDIR_MAX_FLEX_LEN; i++) > flex_mask[i] =3D > diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c > index 4f916494e..098ae13ab 100644 > --- a/drivers/net/i40e/i40e_flow.c > +++ b/drivers/net/i40e/i40e_flow.c > @@ -2240,81 +2240,6 @@ i40e_flow_check_raw_item(const struct > rte_flow_item *item, > return 0; > } >=20 > -static int > -i40e_flow_store_flex_pit(struct i40e_pf *pf, > - struct i40e_fdir_flex_pit *flex_pit, > - enum i40e_flxpld_layer_idx layer_idx, > - uint8_t raw_id) > -{ > - uint8_t field_idx; > - > - field_idx =3D layer_idx * I40E_MAX_FLXPLD_FIED + raw_id; > - /* Check if the configuration is conflicted */ > - if (pf->fdir.flex_pit_flag[layer_idx] && > - (pf->fdir.flex_set[field_idx].src_offset !=3D flex_pit->src_offset = || > - pf->fdir.flex_set[field_idx].size !=3D flex_pit->size || > - pf->fdir.flex_set[field_idx].dst_offset !=3D flex_pit->dst_offset)= ) > - return -1; > - > - /* Check if the configuration exists. */ > - if (pf->fdir.flex_pit_flag[layer_idx] && > - (pf->fdir.flex_set[field_idx].src_offset =3D=3D flex_pit->src_offse= t && > - pf->fdir.flex_set[field_idx].size =3D=3D flex_pit->size && > - pf->fdir.flex_set[field_idx].dst_offset =3D=3D flex_pit->dst_offse= t)) > - return 1; > - > - pf->fdir.flex_set[field_idx].src_offset =3D > - flex_pit->src_offset; > - pf->fdir.flex_set[field_idx].size =3D > - flex_pit->size; > - pf->fdir.flex_set[field_idx].dst_offset =3D > - flex_pit->dst_offset; > - > - return 0; > -} > - > -static void > -i40e_flow_set_fdir_flex_pit(struct i40e_pf *pf, > - enum i40e_flxpld_layer_idx layer_idx, > - uint8_t raw_id) > -{ > - struct i40e_hw *hw =3D I40E_PF_TO_HW(pf); > - uint32_t flx_pit, flx_ort; > - uint8_t field_idx; > - uint16_t min_next_off =3D 0; /* in words */ > - uint8_t i; > - > - if (raw_id) { > - flx_ort =3D (1 << I40E_GLQF_ORT_FLX_PAYLOAD_SHIFT) | > - (raw_id << I40E_GLQF_ORT_FIELD_CNT_SHIFT) | > - (layer_idx * I40E_MAX_FLXPLD_FIED); > - I40E_WRITE_GLB_REG(hw, I40E_GLQF_ORT(33 + layer_idx), > flx_ort); > - } > - > - /* Set flex pit */ > - for (i =3D 0; i < raw_id; i++) { > - field_idx =3D layer_idx * I40E_MAX_FLXPLD_FIED + i; > - flx_pit =3D MK_FLX_PIT(pf->fdir.flex_set[field_idx].src_offset, > - pf->fdir.flex_set[field_idx].size, > - pf->fdir.flex_set[field_idx].dst_offset); > - > - I40E_WRITE_REG(hw, I40E_PRTQF_FLX_PIT(field_idx), > flx_pit); > - min_next_off =3D pf->fdir.flex_set[field_idx].src_offset + > - pf->fdir.flex_set[field_idx].size; > - } > - > - for (; i < I40E_MAX_FLXPLD_FIED; i++) { > - /* set the non-used register obeying register's constrain */ > - field_idx =3D layer_idx * I40E_MAX_FLXPLD_FIED + i; > - flx_pit =3D MK_FLX_PIT(min_next_off, > NONUSE_FLX_PIT_FSIZE, > - NONUSE_FLX_PIT_DEST_OFF); > - I40E_WRITE_REG(hw, I40E_PRTQF_FLX_PIT(field_idx), > flx_pit); > - min_next_off++; > - } > - > - pf->fdir.flex_pit_flag[layer_idx] =3D 1; > -} > - > static int > i40e_flow_set_fdir_inset(struct i40e_pf *pf, > enum i40e_filter_pctype pctype, > @@ -2534,10 +2459,10 @@ i40e_flow_parse_fdir_pattern(struct > rte_eth_dev *dev, > struct i40e_fdir_flex_pit flex_pit; > uint8_t next_dst_off =3D 0; > uint16_t flex_size; > - bool cfg_flex_pit =3D true; > uint16_t ether_type; > uint32_t vtc_flow_cpu; > bool outer_ip =3D true; > + uint8_t field_idx; > int ret; >=20 > memset(off_arr, 0, sizeof(off_arr)); > @@ -3089,6 +3014,7 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev > *dev, >=20 > flex_size =3D 0; > memset(&flex_pit, 0, sizeof(struct > i40e_fdir_flex_pit)); > + field_idx =3D layer_idx * I40E_MAX_FLXPLD_FIED + > raw_id; > flex_pit.size =3D > raw_spec->length / sizeof(uint16_t); > flex_pit.dst_offset =3D > @@ -3115,18 +3041,6 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev > *dev, > return -rte_errno; > } >=20 > - /* Store flex pit to SW */ > - ret =3D i40e_flow_store_flex_pit(pf, &flex_pit, > - layer_idx, raw_id); > - if (ret < 0) { > - rte_flow_error_set(error, EINVAL, > - RTE_FLOW_ERROR_TYPE_ITEM, > - item, > - "Conflict with the first flexible rule."); > - return -rte_errno; > - } else if (ret > 0) > - cfg_flex_pit =3D false; > - > for (i =3D 0; i < raw_spec->length; i++) { > j =3D i + next_dst_off; > filter->input.flow_ext.flexbytes[j] =3D @@ - > 3137,6 +3051,11 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev, >=20 > next_dst_off +=3D raw_spec->length; > raw_id++; > + > + memcpy(&filter->input.flow_ext.flex_pit[field_idx], > + &flex_pit, sizeof(struct i40e_fdir_flex_pit)); > + filter->input.flow_ext.layer_idx =3D layer_idx; > + filter->input.flow_ext.raw_id =3D raw_id; > break; > case RTE_FLOW_ITEM_TYPE_VF: > vf_spec =3D item->spec; > @@ -3222,9 +3141,6 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev > *dev, > "Invalid pattern mask."); > return -rte_errno; > } > - > - if (cfg_flex_pit) > - i40e_flow_set_fdir_flex_pit(pf, layer_idx, raw_id); Seems that multiple switch case set the layer_idx not only the place you de= lete, please double check if these case all need to handle. > } >=20 > filter->input.pctype =3D pctype; > -- > 2.17.1