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 D25FEA04B1; Mon, 23 Nov 2020 11:03:03 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 79683375B; Mon, 23 Nov 2020 11:03:01 +0100 (CET) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id B063AA3 for ; Mon, 23 Nov 2020 11:02:59 +0100 (CET) IronPort-SDR: XC9SA4t0vFOkVKyHhHdBDETyl6vQnfakguwnrOYByT8huWyNRrL3Hz0y49k5+zxt1OITTA36/E zSbRoxa6Yx+A== X-IronPort-AV: E=McAfee;i="6000,8403,9813"; a="189849647" X-IronPort-AV: E=Sophos;i="5.78,363,1599548400"; d="scan'208";a="189849647" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Nov 2020 02:02:57 -0800 IronPort-SDR: H30ScV+ap+3UisvF8SsYpTTsqiIjtFiXzPjG4DR50pqhTJqR0XwipdbM1YHbrDRoD1eda+uY9q IerNYdNzxKqg== X-IronPort-AV: E=Sophos;i="5.78,363,1599548400"; d="scan'208";a="327144329" Received: from tcherepa-mobl.ccr.corp.intel.com (HELO [10.252.20.15]) ([10.252.20.15]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Nov 2020 02:02:55 -0800 To: Thomas Monjalon , Xiaoyu Min , Jingjing Wu , Beilei Xing Cc: dev@dpdk.org, Andrew Rybchenko , Ori Kam , Dekel Peled References: <8990f612-b3b1-bdbc-8da3-9a0e45d5c3bc@intel.com> <2457523.IGjbyyMuxV@thomas> From: Ferruh Yigit Message-ID: Date: Mon, 23 Nov 2020 10:02:51 +0000 MIME-Version: 1.0 In-Reply-To: <2457523.IGjbyyMuxV@thomas> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 4/5] net/iavf: fix protocol size for virtchnl copy 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" On 11/22/2020 2:15 PM, Thomas Monjalon wrote: > 22/11/2020 14:28, Jack Min: >> From: Ferruh Yigit >>> On 11/16/2020 7:55 AM, Xiaoyu Min wrote: >>>> From: Xiaoyu Min >>>> >>>> The rte_flow_item_vlan items are refined. >>>> The structs do not exactly represent the packet bits captured on the >>>> wire anymore so should only copy real header instead of the whole struct. >>>> >>>> Replace the rte_flow_item_* with the existing corresponding rte_*_hdr. >>>> >>>> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN >>> items") >>>> >>>> Signed-off-by: Xiaoyu Min >>>> --- >>>> drivers/net/iavf/iavf_fdir.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/iavf/iavf_fdir.c b/drivers/net/iavf/iavf_fdir.c >>>> index d683a468c1..7054bde0b9 100644 >>>> --- a/drivers/net/iavf/iavf_fdir.c >>>> +++ b/drivers/net/iavf/iavf_fdir.c >>>> @@ -541,7 +541,7 @@ iavf_fdir_parse_pattern(__rte_unused struct >>> iavf_adapter *ad, >>>> VIRTCHNL_ADD_PROTO_HDR_FIELD_BIT(hdr, >>> ETH, ETHERTYPE); >>>> >>>> rte_memcpy(hdr->buffer, >>>> - eth_spec, sizeof(*eth_spec)); >>>> + eth_spec, sizeof(struct rte_ether_hdr)); >>> >>> This requires 'struct rte_flow_item_eth' should have 'struct rte_ether_hdr' as >>> first element, and I suspect this usage exists in a few more locations, but I >>> wonder if this assumption is real and documented in somewhere? >>> I am not just talking about 'struct rte_flow_item_eth', but for all >>> 'rte_flow_item_*'... >>> >> I think this is not documented and this assumption is not real. >> I've created one ticket on Bugzilla (https://bugs.dpdk.org/show_bug.cgi?id=581) to track. > > So it probably means this patch is not enough for iavf. > It would require iavf maintainers (Cc'ed) to follow up. > I will apply the rest of the series. > I think the iavf patch is OK. My comment was general, on how new fields added to the "struct rte_flow_item_eth", which is causing 4 bytes of waste unintentionally. And if we don't fix it in this release, we won't able to fix it until next release. I think first thing is to get the iavf fix. Later we can discuss to get the struct change in this release or not.