[dpdk-dev] [PATCH 4/5] net/iavf: fix protocol size for virtchnl copy
Ferruh Yigit
ferruh.yigit at intel.com
Mon Nov 23 11:02:51 CET 2020
On 11/22/2020 2:15 PM, Thomas Monjalon wrote:
> 22/11/2020 14:28, Jack Min:
>> From: Ferruh Yigit <ferruh.yigit at intel.com>
>>> On 11/16/2020 7:55 AM, Xiaoyu Min wrote:
>>>> From: Xiaoyu Min <jackmin at nvidia.com>
>>>>
>>>> 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 <jackmin at nvidia.com>
>>>> ---
>>>> 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.
More information about the dev
mailing list