ethdev: fix TPID handling in flow API
authorAdrien Mazarguil <adrien.mazarguil@6wind.com>
Wed, 25 Apr 2018 15:27:56 +0000 (17:27 +0200)
committerFerruh Yigit <ferruh.yigit@intel.com>
Fri, 27 Apr 2018 17:00:54 +0000 (18:00 +0100)
commite58638c32411b7ae60ed4dea131728faee962327
treeecd6551f2fb79454f578ae6de587ec8cc8165e0e
parent18aee2861a1f8c5f7511fed32ecc59295c26f79a
ethdev: fix TPID handling in flow API

TPID handling in rte_flow VLAN and E_TAG pattern item definitions is not
consistent with the normal stacking order of pattern items, which is
confusing to applications.

Problem is that when followed by one of these layers, the EtherType field
of the preceding layer keeps its "inner" definition, and the "outer" TPID
is provided by the subsequent layer, the reverse of how a packet looks like
on the wire:

 Wire:     [ ETH TPID = A | VLAN EtherType = B | B DATA ]
 rte_flow: [ ETH EtherType = B | VLAN TPID = A | B DATA ]

Worse, when QinQ is involved, the stacking order of VLAN layers is
unspecified. It is unclear whether it should be reversed (innermost to
outermost) as well given TPID applies to the previous layer:

 Wire:       [ ETH TPID = A | VLAN TPID = B | VLAN EtherType = C | C DATA ]
 rte_flow 1: [ ETH EtherType = C | VLAN TPID = B | VLAN TPID = A | C DATA ]
 rte_flow 2: [ ETH EtherType = C | VLAN TPID = A | VLAN TPID = B | C DATA ]

While specifying EtherType/TPID is hopefully rarely necessary, the stacking
order in case of QinQ and the lack of documentation remain an issue.

This patch replaces TPID in the VLAN pattern item with an inner
EtherType/TPID as is usually done everywhere else (e.g. struct vlan_hdr),
clarifies documentation and updates all relevant code.

It breaks ABI compatibility for the following public functions:

- rte_flow_copy()
- rte_flow_create()
- rte_flow_query()
- rte_flow_validate()

Summary of changes for PMDs that implement ETH, VLAN or E_TAG pattern
items:

- bnxt: EtherType matching is supported with and without VLAN, but TPID
  matching is not and triggers an error.

- e1000: EtherType matching is only supported with the ETHERTYPE filter,
  which does not support VLAN matching, therefore no impact.

- enic: same as bnxt.

- i40e: same as bnxt with existing FDIR limitations on allowed EtherType
  values. The remaining filter types (VXLAN, NVGRE, QINQ) do not support
  EtherType matching.

- ixgbe: same as e1000, with additional minor change to rely on the new
  E-Tag macro definition.

- mlx4: EtherType/TPID matching is not supported, no impact.

- mlx5: same as bnxt.

- mvpp2: same as bnxt.

- sfc: same as bnxt.

- tap: same as bnxt.

Fixes: b1a4b4cbc0a8 ("ethdev: introduce generic flow API")
Fixes: 99e7003831c3 ("net/ixgbe: parse L2 tunnel filter")

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
15 files changed:
app/test-pmd/cmdline_flow.c
doc/guides/nics/tap.rst
doc/guides/prog_guide/rte_flow.rst
doc/guides/rel_notes/release_18_05.rst
doc/guides/testpmd_app_ug/testpmd_funcs.rst
drivers/net/bnxt/bnxt_filter.c
drivers/net/enic/enic_flow.c
drivers/net/i40e/i40e_flow.c
drivers/net/ixgbe/ixgbe_ethdev.c
drivers/net/mlx5/mlx5_flow.c
drivers/net/mvpp2/mrvl_flow.c
drivers/net/sfc/sfc_flow.c
drivers/net/tap/tap_flow.c
lib/librte_ether/rte_flow.h
lib/librte_net/rte_ether.h