Bug 958 - Most drivers are silently ignoring 'has_vlan' rte_flow match criteria
Summary: Most drivers are silently ignoring 'has_vlan' rte_flow match criteria
Status: CONFIRMED
Alias: None
Product: DPDK
Classification: Unclassified
Component: ethdev (show other bugs)
Version: 21.11
Hardware: All All
: Normal major
Target Milestone: ---
Assignee: Thomas Monjalon
URL:
Depends on:
Blocks:
 
Reported: 2022-03-15 21:01 CET by Ilya Maximets
Modified: 2023-11-01 17:09 CET (History)
4 users (show)



Attachments

Description Ilya Maximets 2022-03-15 21:01:57 CET
VLAN presence attribute 'has_vlan' in the 'struct rte_flow_item_eth' was
introduced by commit:
  09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")

However, AFAICT, to the date, only 2 drivers actually supports that attribute.
These are mlx5 and sfc.

All other drivers (i40e, bnxt, ...) are not checking 'has_vlan' at all.
Since the attribute is not checked by the drivers, it's not possible to
identify if the particular driver supports it or not.  Driver will just
accept the structure in most cases and validation will pass.

Documentation at https://doc.dpdk.org/guides/nics/overview.html says that
most of the drivers supports 'eth' rte_flow item, but that is clearly
wrong, since they doesn't support 'has_vlan' flag.  All of them, except
for mlx5 and sfc should be marked as partial support (P).

This problem makes the 'has_vlan' attribute practically unusable by any
application, e.g. OVS, which is not designed to work with a particular
driver, but needs to support a large variety of different hardware.

From the OVS perspective, we seem to need the 'has_vlan' functionality in
order to fix the vlan matching bug, but we can not use it, because that
will introduce a lot of issues with drivers that silently ignore this flow
attribute:
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=284866

On a side note, rte_flow_item_eth.{src, dst}  was supposed to be removed in
21.11 release (according to the deprecation notice), but these fields are
still in the structure and most drivers are still using them.
Comment 1 Thomas Monjalon 2022-03-15 23:57:57 CET
Please could you send a patch to mark drivers with partial support?
It would be a good first step before aligning them with has_vlan support.

About the deprecation, it is a separate topic which deserves a separate bug or mail. I agree some cleanup should be done in rte_flow in general.

Let's use this bug ID to track the support of has_vlan in drivers.
Comment 2 Ilya Maximets 2022-03-16 13:14:38 CET
(In reply to Thomas Monjalon from comment #1)
> Please could you send a patch to mark drivers with partial support?
> It would be a good first step before aligning them with has_vlan support.

OK.  Posted:
  https://patches.dpdk.org/project/dpdk/patch/20220316120157.390311-1-i.maximets@ovn.org/

While making the patch I found that cnxk currently supports the flag,
and I also found that vlan item has to be marked the same way because
of the 'has_more_vlan' flag.
Comment 3 Thomas Monjalon 2022-03-16 14:48:45 CET
Thanks. I would like to merge this patch after 22.03 release,
so we can start discussing how to align all drivers.
Comment 4 Ilya Maximets 2022-07-20 17:07:35 CEST
Any progress on this topic?
Comment 5 dengkaiwen 2023-11-01 03:59:18 CET
Hi All,

No reply for a long time, I'm going to close this ticket for now, so please contact me if you still have questions.

Thanks
Kaiwen Deng
Comment 6 Ilya Maximets 2023-11-01 16:53:06 CET
The RESOLVED FIXED is definitely a wrong status for this one as it is not fixed.
And it is still an issue, so IMHO the bug should stay open.
Comment 7 Ilya Maximets 2023-11-01 17:09:32 CET
It also looks like the issue regressed and now NFP driver claims to support
both ETH and VLAN items, while I can't find actual use of has_vlan or
has_more_vlan in the driver code outside of the check that is designed to
accept items with these fields set.  So, it explicitly accepts them, but
fully ignores...

Note You need to log in before you can comment on or make changes to this bug.