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

Zhang, Qi Z qi.z.zhang at intel.com
Thu Apr 5 16:20:34 CEST 2018


Hi Adrien:

> 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!
> ---
> 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;
> +			}

Please correct me I'm wrong, now I40E_INSET_LAST_ETHER_TYPE will only be set when next != RTE_FLOW_ITEM_TYPE_VLAN
while, RTE_FLOW_ITEM_TYPE_VLAN will only be the next to RTE_FLOW_ITEM_TYPE_ETH for fdir, so above check seems unnecessary ?

Thanks
Qi

>  			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,


More information about the dev mailing list