On Wed, Apr 04, 2018 at 05:56:49PM +0200, Adrien Mazarguil wrote:
> 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.
>
> Summary of changes for PMDs that implement ETH, VLAN or E_TAG pattern
> items:
>
> - bnxt: EtherType matching is supported, and vlan->inner_type overrides
> eth->type if the latter has standard TPID value 0x8100, otherwise an
> error is triggered.
>
> - 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 a configurable TPID value for the FDIR filter,
> with existing limitations on allowed EtherType values. The remaining
> filter types (VXLAN, NVGRE, QINQ) do not support EtherType matching.
>
> - ixgbe: same as e1000.
>
> - mlx4: EtherType/TPID matching is not supported, no impact.
>
> - mlx5: same as bnxt.
>
> - mrvl: EtherType matching is supported but eth->type cannot be specified
> when a VLAN item is present. However vlan->inner_type is used if
> specified.
>
> - sfc: same as bnxt with QinQ TPID value 0x88a8 additionally supported.
>
> - tap: same as bnxt.
>
> Signed-off-by: Adrien Mazarguil <adrien.mazarg...@6wind.com>
> Cc: Ferruh Yigit <ferruh.yi...@intel.com>
> Cc: Thomas Monjalon <tho...@monjalon.net>
> Cc: Wenzhuo Lu <wenzhuo...@intel.com>
> Cc: Jingjing Wu <jingjing...@intel.com>
> Cc: Ajit Khaparde <ajit.khapa...@broadcom.com>
> Cc: Somnath Kotur <somnath.ko...@broadcom.com>
> Cc: John Daley <johnd...@cisco.com>
> Cc: Hyong Youb Kim <hyon...@cisco.com>
> Cc: Beilei Xing <beilei.x...@intel.com>
> Cc: Qi Zhang <qi.z.zh...@intel.com>
> Cc: Konstantin Ananyev <konstantin.anan...@intel.com>
> Cc: Nelio Laranjeiro <nelio.laranje...@6wind.com>
> Cc: Yongseok Koh <ys...@mellanox.com>
> Cc: Jacek Siuda <j...@semihalf.com>
> Cc: Tomasz Duszynski <t...@semihalf.com>
> Cc: Dmitri Epshtein <d...@marvell.com>
> Cc: Natalie Samsonov <nsams...@marvell.com>
> Cc: Jianbo Liu <jianbo....@arm.com>
> Cc: Andrew Rybchenko <arybche...@solarflare.com>
> Cc: Pascal Mazon <pascal.ma...@6wind.com>
>
> ---
>
> Hi PMD maintainers, while I'm pretty confident in these changes, I could
> not validate them with all devices.
>
> It would be great if you could apply this patch, run testpmd, create VLAN
> flow rules with/without inner EtherType as described and send matching
> traffic while making sure nothing was broken in the process.
>
> Thanks!
> ---
> app/test-pmd/cmdline_flow.c | 17 +++---
> doc/guides/nics/tap.rst | 2 +-
> doc/guides/prog_guide/rte_flow.rst | 21 +++++--
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 +-
> drivers/net/bnxt/bnxt_filter.c | 38 ++++++++++--
> drivers/net/enic/enic_flow.c | 21 ++++---
> drivers/net/i40e/i40e_flow.c | 74 ++++++++++++++++++++----
> drivers/net/mlx5/mlx5_flow.c | 14 ++++-
> drivers/net/mvpp2/mrvl_flow.c | 27 +++++++--
> drivers/net/sfc/sfc_flow.c | 27 +++++++++
> drivers/net/tap/tap_flow.c | 16 +++--
> lib/librte_ether/rte_flow.h | 24 +++++---
> 12 files changed, 227 insertions(+), 58 deletions(-)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 2fbd3d8ef..3a486032d 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -99,11 +99,11 @@ enum index {
> ITEM_ETH_SRC,
> ITEM_ETH_TYPE,
> ITEM_VLAN,
> - ITEM_VLAN_TPID,
> ITEM_VLAN_TCI,
> ITEM_VLAN_PCP,
> ITEM_VLAN_DEI,
> ITEM_VLAN_VID,
> + ITEM_VLAN_INNER_TYPE,
> ITEM_IPV4,
> ITEM_IPV4_TOS,
> ITEM_IPV4_TTL,
> @@ -505,11 +505,11 @@ static const enum index item_eth[] = {
> };
>
> static const enum index item_vlan[] = {
> - ITEM_VLAN_TPID,
> ITEM_VLAN_TCI,
> ITEM_VLAN_PCP,
> ITEM_VLAN_DEI,
> ITEM_VLAN_VID,
> + ITEM_VLAN_INNER_TYPE,
> ITEM_NEXT,
> ZERO,
> };
> @@ -1142,12 +1142,6 @@ static const struct token token_list[] = {
> .next = NEXT(item_vlan),
> .call = parse_vc,
> },
> - [ITEM_VLAN_TPID] = {
> - .name = "tpid",
> - .help = "tag protocol identifier",
> - .next = NEXT(item_vlan, NEXT_ENTRY(UNSIGNED), item_param),
> - .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vlan, tpid)),
> - },
> [ITEM_VLAN_TCI] = {
> .name = "tci",
> .help = "tag control information",
> @@ -1175,6 +1169,13 @@ static const struct token token_list[] = {
> .args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_vlan,
> tci, "\x0f\xff")),
> },
> + [ITEM_VLAN_INNER_TYPE] = {
> + .name = "inner_type",
> + .help = "inner EtherType",
> + .next = NEXT(item_vlan, NEXT_ENTRY(UNSIGNED), item_param),
> + .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vlan,
> + inner_type)),
> + },
> [ITEM_IPV4] = {
> .name = "ipv4",
> .help = "match IPv4 header",
> diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
> index 76eb0bde4..bcf3efe9e 100644
> --- a/doc/guides/nics/tap.rst
> +++ b/doc/guides/nics/tap.rst
> @@ -97,7 +97,7 @@ The kernel support can be checked with this command::
> Supported items:
>
> - eth: src and dst (with variable masks), and eth_type (0xffff mask).
> -- vlan: vid, pcp, tpid, but not eid. (requires kernel 4.9)
> +- vlan: vid, pcp, but not eid. (requires kernel 4.9)
> - ipv4/6: src and dst (with variable masks), and ip_proto (0xffff mask).
> - udp/tcp: src and dst port (0xffff) mask.
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index c893d737a..f1b9d8d76 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -784,9 +784,15 @@ Item: ``ETH``
>
> Matches an Ethernet header.
>
> +The ``type`` field either stands for "EtherType" or "TPID" when followed by
> +so-called layer 2.5 pattern items such as ``RTE_FLOW_ITEM_TYPE_VLAN``. In
> +the latter case, ``type`` refers to that of the outer header, with the inner
> +EtherType/TPID provided by the subsequent pattern item. This is the same
> +order as on the wire.
> +
> - ``dst``: destination MAC.
> - ``src``: source MAC.
> -- ``type``: EtherType.
> +- ``type``: EtherType or TPID.
> - Default ``mask`` matches destination and source addresses only.
>
> Item: ``VLAN``
> @@ -794,9 +800,13 @@ Item: ``VLAN``
>
> Matches an 802.1Q/ad VLAN tag.
>
> -- ``tpid``: tag protocol identifier.
> +The corresponding standard outer EtherType (TPID) values are ``0x8100`` or
> +``0x88a8`` (in case of outer QinQ). It can be overridden by the preceding
> +pattern item.
> +
> - ``tci``: tag control information.
> -- Default ``mask`` matches TCI only.
> +- ``inner_type``: inner EtherType or TPID.
> +- Default ``mask`` matches the VID part of TCI only (lower 12 bits).
>
> Item: ``IPV4``
> ^^^^^^^^^^^^^^
> @@ -866,12 +876,15 @@ Item: ``E_TAG``
>
> Matches an IEEE 802.1BR E-Tag header.
>
> -- ``tpid``: tag protocol identifier (0x893F)
> +The corresponding standard outer EtherType (TPID) value is 0x893f.
> +It can be overridden by the preceding pattern item.
> +
> - ``epcp_edei_in_ecid_b``: E-Tag control information (E-TCI), E-PCP (3b),
> E-DEI (1b), ingress E-CID base (12b).
> - ``rsvd_grp_ecid_b``: reserved (2b), GRP (2b), E-CID base (12b).
> - ``in_ecid_e``: ingress E-CID ext.
> - ``ecid_e``: E-CID ext.
> +- ``inner_type``: inner EtherType or TPID.
> - Default ``mask`` simultaneously matches GRP and E-CID base.
>
> Item: ``NVGRE``
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 738461f44..25fac8430 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3223,15 +3223,15 @@ This section lists supported pattern items and their
> attributes, if any.
>
> - ``dst {MAC-48}``: destination MAC.
> - ``src {MAC-48}``: source MAC.
> - - ``type {unsigned}``: EtherType.
> + - ``type {unsigned}``: EtherType or TPID.
>
> - ``vlan``: match 802.1Q/ad VLAN tag.
>
> - - ``tpid {unsigned}``: tag protocol identifier.
> - ``tci {unsigned}``: tag control information.
> - ``pcp {unsigned}``: priority code point.
> - ``dei {unsigned}``: drop eligible indicator.
> - ``vid {unsigned}``: VLAN identifier.
> + - ``inner_type {unsigned}``: inner EtherType or TPID.
>
> - ``ipv4``: match IPv4 header.
>
> diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
> index 0d735edf6..a97a8dd46 100644
> --- a/drivers/net/bnxt/bnxt_filter.c
> +++ b/drivers/net/bnxt/bnxt_filter.c
> @@ -327,6 +327,7 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
> uint32_t vf = 0;
> int use_ntuple;
> uint32_t en = 0;
> + uint32_t en_ethertype;
> int dflt_vnic;
>
> use_ntuple = bnxt_filter_type_check(pattern, error);
> @@ -336,6 +337,9 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
>
> filter->filter_type = use_ntuple ?
> HWRM_CFA_NTUPLE_FILTER : HWRM_CFA_EM_FILTER;
> + en_ethertype = use_ntuple ?
> + NTUPLE_FLTR_ALLOC_INPUT_EN_ETHERTYPE :
> + EM_FLOW_ALLOC_INPUT_EN_ETHERTYPE;
>
> while (item->type != RTE_FLOW_ITEM_TYPE_END) {
> if (item->last) {
> @@ -405,30 +409,52 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
> if (eth_mask->type) {
> filter->ethertype =
> rte_be_to_cpu_16(eth_spec->type);
> - en |= use_ntuple ?
> - NTUPLE_FLTR_ALLOC_INPUT_EN_ETHERTYPE :
> - EM_FLOW_ALLOC_INPUT_EN_ETHERTYPE;
> + en |= en_ethertype;
> }
>
> break;
> case RTE_FLOW_ITEM_TYPE_VLAN:
> vlan_spec = item->spec;
> vlan_mask = item->mask;
> + if (en & en_ethertype &&
> + filter->ethertype != RTE_BE16(0x8100)) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "unsupported outer TPID");
> + return -rte_errno;
> + }
> if (vlan_mask->tci &&
> - vlan_mask->tci == RTE_BE16(0x0fff) &&
> - !vlan_mask->tpid) {
> + vlan_mask->tci == RTE_BE16(0x0fff)) {
> /* Only the VLAN ID can be matched. */
> filter->l2_ovlan =
> rte_be_to_cpu_16(vlan_spec->tci &
> RTE_BE16(0x0fff));
> en |= EM_FLOW_ALLOC_INPUT_EN_OVLAN_VID;
> - } else if (vlan_mask->tci || vlan_mask->tpid) {
> + } else if (vlan_mask->tci) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ITEM,
> item,
> "VLAN mask is invalid");
> return -rte_errno;
> }
> + if (vlan_mask->inner_type &&
> + vlan_mask->inner_type != RTE_BE16(0xffff)) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "inner ethertype mask not"
> + " valid");
> + return -rte_errno;
> + }
> + if (vlan_mask->inner_type) {
> + filter->ethertype =
> + rte_be_to_cpu_16(vlan_spec->inner_type);
> + en |= en_ethertype;
> + } else {
> + filter->ethertype = RTE_BE16(0x0000);
> + en &= ~en_ethertype;
> + }
>
> break;
> case RTE_FLOW_ITEM_TYPE_IPV4:
> diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
> index c5c98b870..a413740ab 100644
> --- a/drivers/net/enic/enic_flow.c
> +++ b/drivers/net/enic/enic_flow.c
> @@ -4,6 +4,7 @@
>
> #include <errno.h>
> #include <stdint.h>
> +#include <rte_byteorder.h>
> #include <rte_log.h>
> #include <rte_ethdev_driver.h>
> #include <rte_flow_driver.h>
> @@ -545,16 +546,22 @@ enic_copy_item_vlan_v2(const struct rte_flow_item *item,
> if (!spec)
> return 0;
>
> - /* Don't support filtering in tpid */
> - if (mask) {
> - if (mask->tpid != 0)
> - return ENOTSUP;
> - } else {
> + if (!mask)
> mask = &rte_flow_item_vlan_mask;
> - RTE_ASSERT(mask->tpid == 0);
> - }
>
> if (*inner_ofst == 0) {
> + struct ether_hdr *eth_mask =
> + (void *)gp->layer[FILTER_GENERIC_1_L2].mask;
> + struct ether_hdr *eth_val =
> + (void *)gp->layer[FILTER_GENERIC_1_L2].val;
> +
> + /* Exactly one TPID value is allowed if specified */
> + if ((eth_val->ether_type & eth_mask->ether_type) !=
> + (RTE_BE16(0x8100) & eth_mask->ether_type))
> + return ENOTSUP;
> + eth_mask->ether_type = mask->inner_type;
> + eth_val->ether_type = spec->inner_type;
> +
> /* Outer header. Use the vlan mask/val fields */
> gp->mask_vlan = mask->tci;
> gp->val_vlan = spec->tci;
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
> index 1b336df74..c6dd889ad 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -2490,24 +2490,36 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
> "Invalid MAC_addr mask.");
> return -rte_errno;
> }
> + }
> + if (eth_spec && eth_mask && eth_mask->type) {
> + enum rte_flow_item_type next = (item + 1)->type;
>
> - if ((eth_mask->type & UINT16_MAX) ==
> - UINT16_MAX) {
> - input_set |= I40E_INSET_LAST_ETHER_TYPE;
> - filter->input.flow.l2_flow.ether_type =
> - eth_spec->type;
> + if (eth_mask->type != RTE_BE16(0xffff)) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Invalid type mask.");
> + return -rte_errno;
> }
>
> ether_type = rte_be_to_cpu_16(eth_spec->type);
> - if (ether_type == ETHER_TYPE_IPv4 ||
> - ether_type == ETHER_TYPE_IPv6 ||
> - ether_type == ETHER_TYPE_ARP ||
> - ether_type == outer_tpid) {
> +
> + if ((next == RTE_FLOW_ITEM_TYPE_VLAN &&
> + ether_type != outer_tpid) ||
> + (next != RTE_FLOW_ITEM_TYPE_VLAN &&
> + (ether_type == ETHER_TYPE_IPv4 ||
> + ether_type == ETHER_TYPE_IPv6 ||
> + ether_type == ETHER_TYPE_ARP ||
> + ether_type == outer_tpid))) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ITEM,
> item,
> "Unsupported ether_type.");
> return -rte_errno;
> + } else if (next != RTE_FLOW_ITEM_TYPE_VLAN) {
> + input_set |= I40E_INSET_LAST_ETHER_TYPE;
> + filter->input.flow.l2_flow.ether_type =
> + eth_spec->type;
> }
> }
>
> @@ -2518,6 +2530,14 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
> case RTE_FLOW_ITEM_TYPE_VLAN:
> vlan_spec = item->spec;
> vlan_mask = item->mask;
> +
> + if (input_set & I40E_INSET_LAST_ETHER_TYPE) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Unsupported outer TPID.");
> + return -rte_errno;
> + }
> if (vlan_spec && vlan_mask) {
> if (vlan_mask->tci ==
> rte_cpu_to_be_16(I40E_TCI_MASK)) {
> @@ -2526,6 +2546,33 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
> vlan_spec->tci;
> }
> }
> + if (vlan_spec && vlan_mask && vlan_mask->inner_type) {
> + if (vlan_mask->inner_type != RTE_BE16(0xffff)) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Invalid inner_type"
> + " mask.");
> + return -rte_errno;
> + }
> +
> + ether_type =
> + rte_be_to_cpu_16(vlan_spec->inner_type);
> +
> + if (ether_type == ETHER_TYPE_IPv4 ||
> + ether_type == ETHER_TYPE_IPv6 ||
> + ether_type == ETHER_TYPE_ARP ||
> + ether_type == outer_tpid) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Unsupported inner_type.");
> + return -rte_errno;
> + }
> + input_set |= I40E_INSET_LAST_ETHER_TYPE;
> + filter->input.flow.l2_flow.ether_type =
> + vlan_spec->inner_type;
> + }
>
> pctype = I40E_FILTER_PCTYPE_L2_PAYLOAD;
> layer_idx = I40E_FLXPLD_L2_IDX;
> @@ -3284,7 +3331,8 @@ i40e_flow_parse_vxlan_pattern(__rte_unused struct
> rte_eth_dev *dev,
> case RTE_FLOW_ITEM_TYPE_VLAN:
> vlan_spec = item->spec;
> vlan_mask = item->mask;
> - if (!(vlan_spec && vlan_mask)) {
> + if (!(vlan_spec && vlan_mask) ||
> + vlan_mask->inner_type) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ITEM,
> item,
> @@ -3514,7 +3562,8 @@ i40e_flow_parse_nvgre_pattern(__rte_unused struct
> rte_eth_dev *dev,
> case RTE_FLOW_ITEM_TYPE_VLAN:
> vlan_spec = item->spec;
> vlan_mask = item->mask;
> - if (!(vlan_spec && vlan_mask)) {
> + if (!(vlan_spec && vlan_mask) ||
> + vlan_mask->inner_type) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ITEM,
> item,
> @@ -4022,7 +4071,8 @@ i40e_flow_parse_qinq_pattern(__rte_unused struct
> rte_eth_dev *dev,
> vlan_spec = item->spec;
> vlan_mask = item->mask;
>
> - if (!(vlan_spec && vlan_mask)) {
> + if (!(vlan_spec && vlan_mask) ||
> + vlan_mask->inner_type) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ITEM,
> item,
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index bc1176819..f03413d32 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -17,6 +17,7 @@
> #pragma GCC diagnostic error "-Wpedantic"
> #endif
>
> +#include <rte_byteorder.h>
> #include <rte_common.h>
> #include <rte_eth_ctrl.h>
> #include <rte_ethdev_driver.h>
> @@ -306,6 +307,7 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {
> .actions = valid_actions,
> .mask = &(const struct rte_flow_item_vlan){
> .tci = -1,
> + .inner_type = -1,
> },
> .default_mask = &rte_flow_item_vlan_mask,
> .mask_sz = sizeof(struct rte_flow_item_vlan),
> @@ -1285,6 +1287,7 @@ mlx5_flow_create_vlan(const struct rte_flow_item *item,
> struct mlx5_flow_parse *parser = data->parser;
> struct ibv_flow_spec_eth *eth;
> const unsigned int eth_size = sizeof(struct ibv_flow_spec_eth);
> + const char *msg = "VLAN cannot be empty";
>
> if (spec) {
> unsigned int i;
> @@ -1306,12 +1309,21 @@ mlx5_flow_create_vlan(const struct rte_flow_item
> *item,
> */
> if (!eth->mask.vlan_tag)
> goto error;
> + /* Exactly one TPID value is allowed if specified. */
> + if ((eth->val.ether_type & eth->mask.ether_type) !=
> + (RTE_BE16(0x8100) & eth->mask.ether_type)) {
You can use ETHER_TYPE_VLAN present in rte_ether.h instead of hard coded
values.
> + msg = "unsupported outer TPID";
> + goto error;
> + }
> + eth->val.ether_type = spec->inner_type;
> + eth->mask.ether_type = mask->inner_type;
> + eth->val.ether_type &= eth->mask.ether_type;
> }
> return 0;
> }
> error:
> return rte_flow_error_set(data->error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM,
> - item, "VLAN cannot be empty");
> + item, msg);
> }
>
> /**
> diff --git a/drivers/net/mvpp2/mrvl_flow.c b/drivers/net/mvpp2/mrvl_flow.c
> index 8fd4dbfb1..6604a411f 100644
> --- a/drivers/net/mvpp2/mrvl_flow.c
> +++ b/drivers/net/mvpp2/mrvl_flow.c
> @@ -1091,12 +1091,6 @@ mrvl_parse_vlan(const struct rte_flow_item *item,
> if (ret)
> return ret;
>
> - if (mask->tpid) {
> - rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM,
> - NULL, "Not supported by classifier\n");
> - return -rte_errno;
> - }
> -
> m = rte_be_to_cpu_16(mask->tci);
> if (m & MRVL_VLAN_ID_MASK) {
> RTE_LOG(WARNING, PMD, "vlan id mask is ignored\n");
> @@ -1112,6 +1106,27 @@ mrvl_parse_vlan(const struct rte_flow_item *item,
> goto out;
> }
>
> + if (flow->pattern & F_TYPE) {
> + rte_flow_error_set(error, ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_ITEM, item,
> + "outer TPID cannot be explicitly matched"
> + " when VLAN item is also specified\n");
> + return -rte_errno;
> + }
> + if (mask->inner_type) {
> + struct rte_flow_item_eth spec_eth = {
> + .type = spec->inner_type,
> + };
> + struct rte_flow_item_eth mask_eth = {
> + .type = mask->inner_type,
> + };
> +
> + RTE_LOG(WARNING, PMD, "inner eth type mask is ignored\n");
> + ret = mrvl_parse_type(spec_eth, mask_eth, flow);
> + if (ret)
> + goto out;
> + }
> +
> return 0;
> out:
> rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
> index bf9609735..515a61325 100644
> --- a/drivers/net/sfc/sfc_flow.c
> +++ b/drivers/net/sfc/sfc_flow.c
> @@ -352,6 +352,7 @@ sfc_flow_parse_vlan(const struct rte_flow_item *item,
> const struct rte_flow_item_vlan *mask = NULL;
> const struct rte_flow_item_vlan supp_mask = {
> .tci = rte_cpu_to_be_16(ETH_VLAN_ID_MAX),
> + .inner_type = RTE_BE16(0xffff),
> };
>
> rc = sfc_flow_parse_init(item,
> @@ -394,6 +395,32 @@ sfc_flow_parse_vlan(const struct rte_flow_item *item,
> return -rte_errno;
> }
>
> + /*
> + * If an EtherType was already specified, make sure it is a valid
> + * TPID for the current VLAN layer before overwriting it with the
> + * specified inner type.
> + */
> + if (efx_spec->efs_match_flags & EFX_FILTER_MATCH_ETHER_TYPE &&
> + efx_spec->efs_ether_type != RTE_BE16(0x8100) &&
> + efx_spec->efs_ether_type != RTE_BE16(0x88a8)) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM, item,
> + "Unsupported outer TPID");
> + return -rte_errno;
> + }
> + if (!mask->inner_type) {
> + efx_spec->efs_match_flags &= ~EFX_FILTER_MATCH_ETHER_TYPE;
> + efx_spec->efs_ether_type = RTE_BE16(0x0000);
> + } else if (mask->inner_type == supp_mask.inner_type) {
> + efx_spec->efs_match_flags |= EFX_FILTER_MATCH_ETHER_TYPE;
> + efx_spec->efs_ether_type = rte_be_to_cpu_16(spec->inner_type);
> + } else {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM, item,
> + "Bad mask for VLAN inner_type");
> + return -rte_errno;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
> index e5eb50fc5..e53eff6ce 100644
> --- a/drivers/net/tap/tap_flow.c
> +++ b/drivers/net/tap/tap_flow.c
> @@ -270,13 +270,13 @@ static const struct tap_flow_items tap_flow_items[] = {
> .items = ITEMS(RTE_FLOW_ITEM_TYPE_IPV4,
> RTE_FLOW_ITEM_TYPE_IPV6),
> .mask = &(const struct rte_flow_item_vlan){
> - .tpid = -1,
> /* DEI matching is not supported */
> #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> .tci = 0xffef,
> #else
> .tci = 0xefff,
> #endif
> + .inner_type = -1,
> },
> .mask_sz = sizeof(struct rte_flow_item_vlan),
> .default_mask = &rte_flow_item_vlan_mask,
> @@ -578,13 +578,21 @@ tap_flow_create_vlan(const struct rte_flow_item *item,
> void *data)
> /* use default mask if none provided */
> if (!mask)
> mask = tap_flow_items[RTE_FLOW_ITEM_TYPE_VLAN].default_mask;
> - /* TC does not support tpid masking. Only accept if exact match. */
> - if (mask->tpid && mask->tpid != 0xffff)
> + /* check that previous eth type is compatible with VLAN */
> + if (info->eth_type && info->eth_type != RTE_BE16(ETH_P_8021Q))
> return -1;
> /* Double-tagging not supported. */
> - if (spec && mask->tpid && spec->tpid != htons(ETH_P_8021Q))
> + if (info->vlan)
> return -1;
> info->vlan = 1;
> + if (mask->inner_type) {
> + /* TC does not support partial eth_type masking */
> + if (mask->inner_type != RTE_BE16(0xffff))
> + return -1;
> + info->eth_type = spec->inner_type;
> + } else {
> + info->eth_type = 0;
> + }
> if (!flow)
> return 0;
> msg = &flow->msg;
> diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> index 1b222ba60..15e383f95 100644
> --- a/lib/librte_ether/rte_flow.h
> +++ b/lib/librte_ether/rte_flow.h
> @@ -454,11 +454,17 @@ static const struct rte_flow_item_raw
> rte_flow_item_raw_mask = {
> * RTE_FLOW_ITEM_TYPE_ETH
> *
> * Matches an Ethernet header.
> + *
> + * The @p type field either stands for "EtherType" or "TPID" when followed
> + * by so-called layer 2.5 pattern items such as RTE_FLOW_ITEM_TYPE_VLAN. In
> + * the latter case, @p type refers to that of the outer header, with the
> + * inner EtherType/TPID provided by the subsequent pattern item. This is the
> + * same order as on the wire.
> */
> struct rte_flow_item_eth {
> struct ether_addr dst; /**< Destination MAC. */
> struct ether_addr src; /**< Source MAC. */
> - rte_be16_t type; /**< EtherType. */
> + rte_be16_t type; /**< EtherType or TPID. */
> };
>
> /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
> @@ -475,19 +481,20 @@ static const struct rte_flow_item_eth
> rte_flow_item_eth_mask = {
> *
> * Matches an 802.1Q/ad VLAN tag.
> *
> - * This type normally follows either RTE_FLOW_ITEM_TYPE_ETH or
> - * RTE_FLOW_ITEM_TYPE_VLAN.
> + * The corresponding standard outer EtherType (TPID) values are 0x8100 or
> + * 0x88a8 (in case of outer QinQ). It can be overridden by the preceding
> + * pattern item.
> */
> struct rte_flow_item_vlan {
> - rte_be16_t tpid; /**< Tag protocol identifier. */
> rte_be16_t tci; /**< Tag control information. */
> + rte_be16_t inner_type; /**< Inner EtherType or TPID. */
> };
>
> /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
> #ifndef __cplusplus
> static const struct rte_flow_item_vlan rte_flow_item_vlan_mask = {
> - .tpid = RTE_BE16(0x0000),
> - .tci = RTE_BE16(0xffff),
> + .tci = RTE_BE16(0x0fff),
> + .inner_type = RTE_BE16(0x0000),
> };
> #endif
>
> @@ -636,9 +643,11 @@ static const struct rte_flow_item_vxlan
> rte_flow_item_vxlan_mask = {
> * RTE_FLOW_ITEM_TYPE_E_TAG.
> *
> * Matches a E-tag header.
> + *
> + * The corresponding standard outer EtherType (TPID) value is 0x893f.
> + * It can be overridden by the preceding pattern item.
> */
> struct rte_flow_item_e_tag {
> - rte_be16_t tpid; /**< Tag protocol identifier (0x893F). */
> /**
> * E-Tag control information (E-TCI).
> * E-PCP (3b), E-DEI (1b), ingress E-CID base (12b).
> @@ -648,6 +657,7 @@ struct rte_flow_item_e_tag {
> rte_be16_t rsvd_grp_ecid_b;
> uint8_t in_ecid_e; /**< Ingress E-CID ext. */
> uint8_t ecid_e; /**< E-CID ext. */
> + rte_be16_t inner_type; /**< Inner EtherType or TPID. */
> };
>
> /** Default mask for RTE_FLOW_ITEM_TYPE_E_TAG. */
> --
> 2.11.0
--
Nélio Laranjeiro
6WIND
