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_*'...
btw, while checking for the 'struct rte_flow_item_eth', pahole shows it is using
20 bytes, and I suspect this is not the intention with the reserved field:
struct rte_flow_item_eth {
struct rte_ether_addr dst; /* 0 6 */
struct rte_ether_addr src; /* 6 6 */
uint16_t type; /* 12 2 */
/* Bitfield combined with previous fields */
uint32_t has_vlan:1; /* 12:15 4 */
/* XXX 31 bits hole, try to pack */
uint32_t reserved:31; /* 16: 1 4 */
/* size: 20, cachelines: 1, members: 5 */
/* bit holes: 1, sum bit holes: 31 bits */
/* bit_padding: 1 bits */
/* last cacheline: 20 bytes */
};
'has_vlan' seems combined with previous field to make together 32 bits. So the
'reserved' field is occupying a new 32 bits all by itself.
What about changing the struct as following, while we can change the ABI:
struct rte_flow_item_eth {
struct rte_ether_addr dst; /* 0 6 */
struct rte_ether_addr src; /* 6 6 */
uint16_t type; /* 12 2 */
uint16_t has_vlan:1; /* 14:15 2 */
uint16_t reserved:15; /* 14: 0 2 */
/* size: 16, cachelines: 1, members: 5 */
/* last cacheline: 16 bytes */
};
