[dpdk-dev] [PATCH v1 11/16] ethdev: refine TPID handling in flow API

Nélio Laranjeiro nelio.laranjeiro at 6wind.com
Thu Apr 5 11:55:10 CEST 2018


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.mazarguil at 6wind.com>
> Cc: Ferruh Yigit <ferruh.yigit at intel.com>
> Cc: Thomas Monjalon <thomas at monjalon.net>
> Cc: Wenzhuo Lu <wenzhuo.lu at intel.com>
> Cc: Jingjing Wu <jingjing.wu at intel.com>
> Cc: Ajit Khaparde <ajit.khaparde at broadcom.com>
> Cc: Somnath Kotur <somnath.kotur at broadcom.com>
> Cc: John Daley <johndale at cisco.com>
> Cc: Hyong Youb Kim <hyonkim at cisco.com>
> Cc: Beilei Xing <beilei.xing at intel.com>
> Cc: Qi Zhang <qi.z.zhang at intel.com>
> Cc: Konstantin Ananyev <konstantin.ananyev at intel.com>
> Cc: Nelio Laranjeiro <nelio.laranjeiro at 6wind.com>
> Cc: Yongseok Koh <yskoh at mellanox.com>
> Cc: Jacek Siuda <jck at semihalf.com>
> Cc: Tomasz Duszynski <tdu at semihalf.com>
> Cc: Dmitri Epshtein <dima at marvell.com>
> Cc: Natalie Samsonov <nsamsono at marvell.com>
> Cc: Jianbo Liu <jianbo.liu at arm.com>
> Cc: Andrew Rybchenko <arybchenko at solarflare.com>
> Cc: Pascal Mazon <pascal.mazon at 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


More information about the dev mailing list