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,
