On 11/22/2020 2:15 PM, Thomas Monjalon wrote:
22/11/2020 14:28, Jack Min:
From: Ferruh Yigit <ferruh.yi...@intel.com>
On 11/16/2020 7:55 AM, Xiaoyu Min wrote:
From: Xiaoyu Min <jack...@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 <jack...@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.

Reply via email to