[dpdk-dev] [PATCH] ethdev: add VLAN attributes to ETH and VLAN items

[dpdk-dev] [PATCH] ethdev: add VLAN attributes to ETH and VLAN items

[Dekel Peled]

From: Dekel Peled <dekelp@mellanox.com>

This patch implements the change proposes in RFC [1], adding dedicated
fields to ETH and VLAN items structs, to clearly define the required
characteristic of a packet, and enable precise match criteria.

[1] https://mails.dpdk.org/archives/dev/2020-August/177536.html

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
 doc/guides/rel_notes/release_20_11.rst |  7 +++++++
 lib/librte_ethdev/rte_flow.h           | 16 +++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index 7f9d0dd..199c60b 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -173,6 +173,13 @@ API Changes
   * ``_rte_eth_dev_callback_process()`` -> ``rte_eth_dev_callback_process()``
   * ``_rte_eth_dev_reset`` -> ``rte_eth_dev_internal_reset()``
 
+* ethdev: Added new field ``vlan_exist`` to structure ``rte_flow_item_eth``,
+  indicating that at least one VLAN exists in the packet header.
+  
+* ethdev: Added new field ``more_vlans_exist`` to structure
+  ``rte_flow_item_vlan``, indicating that at least one more VLAN exists in
+  packet header, following this VLAN.
+
 * rawdev: Added a structure size parameter to the functions
   ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
   ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``,
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index da8bfa5..39d04ef 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -723,14 +723,18 @@ struct rte_flow_item_raw {
  * If the @p type field contains a TPID value, then only tagged packets with the
  * specified TPID will match the pattern.
  * Otherwise, only untagged packets will match the pattern.
- * If the @p ETH item is the only item in the pattern, and the @p type field
- * is not specified, then both tagged and untagged packets will match the
- * pattern.
+ * The field @p vlan_exist can be used to match specific packet types, instead
+ * of using the @p type field.
+ * This can be used to match any type of tagged packets.
+ * If the @p type and @p vlan_exist fields are not specified, then both tagged
+ * and untagged packets will match the pattern.
  */
 struct rte_flow_item_eth {
 	struct rte_ether_addr dst; /**< Destination MAC. */
 	struct rte_ether_addr src; /**< Source MAC. */
 	rte_be16_t type; /**< EtherType or TPID. */
+	uint32_t vlan_exist:1; /**< At least one VLAN exist in header. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
@@ -752,10 +756,16 @@ struct rte_flow_item_eth {
  * the preceding pattern item.
  * If a @p VLAN item is present in the pattern, then only tagged packets will
  * match the pattern.
+ * The field @p more_vlans_exist can be used to match specific packet types,
+ * instead of using the @p inner_type field.
+ * This can be used to match any type of tagged packets.
  */
 struct rte_flow_item_vlan {
 	rte_be16_t tci; /**< Tag control information. */
 	rte_be16_t inner_type; /**< Inner EtherType or TPID. */
+	uint32_t more_vlans_exist:1;
+	/**< At least one more VLAN exist in header, following this VLAN. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
-- 
1.8.3.1

[dpdk-dev] [PATCH v2] ethdev: add VLAN attributes to ETH and VLAN items

[Dekel Peled]

From: Dekel Peled <dekelp@mellanox.com>

This patch implements the change proposes in RFC [1], adding dedicated
fields to ETH and VLAN items structs, to clearly define the required
characteristic of a packet, and enable precise match criteria.

[1] https://mails.dpdk.org/archives/dev/2020-August/177536.html

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
 doc/guides/rel_notes/release_20_11.rst |  7 +++++++
 lib/librte_ethdev/rte_flow.h           | 16 +++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index 7f9d0dd..801a744 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -173,6 +173,13 @@ API Changes
   * ``_rte_eth_dev_callback_process()`` -> ``rte_eth_dev_callback_process()``
   * ``_rte_eth_dev_reset`` -> ``rte_eth_dev_internal_reset()``
 
+* ethdev: Added new field ``vlan_exist`` to structure ``rte_flow_item_eth``,
+  indicating that at least one VLAN exists in the packet header.
+
+* ethdev: Added new field ``more_vlans_exist`` to structure
+  ``rte_flow_item_vlan``, indicating that at least one more VLAN exists in
+  packet header, following this VLAN.
+
 * rawdev: Added a structure size parameter to the functions
   ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
   ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``,
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index da8bfa5..39d04ef 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -723,14 +723,18 @@ struct rte_flow_item_raw {
  * If the @p type field contains a TPID value, then only tagged packets with the
  * specified TPID will match the pattern.
  * Otherwise, only untagged packets will match the pattern.
- * If the @p ETH item is the only item in the pattern, and the @p type field
- * is not specified, then both tagged and untagged packets will match the
- * pattern.
+ * The field @p vlan_exist can be used to match specific packet types, instead
+ * of using the @p type field.
+ * This can be used to match any type of tagged packets.
+ * If the @p type and @p vlan_exist fields are not specified, then both tagged
+ * and untagged packets will match the pattern.
  */
 struct rte_flow_item_eth {
 	struct rte_ether_addr dst; /**< Destination MAC. */
 	struct rte_ether_addr src; /**< Source MAC. */
 	rte_be16_t type; /**< EtherType or TPID. */
+	uint32_t vlan_exist:1; /**< At least one VLAN exist in header. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
@@ -752,10 +756,16 @@ struct rte_flow_item_eth {
  * the preceding pattern item.
  * If a @p VLAN item is present in the pattern, then only tagged packets will
  * match the pattern.
+ * The field @p more_vlans_exist can be used to match specific packet types,
+ * instead of using the @p inner_type field.
+ * This can be used to match any type of tagged packets.
  */
 struct rte_flow_item_vlan {
 	rte_be16_t tci; /**< Tag control information. */
 	rte_be16_t inner_type; /**< Inner EtherType or TPID. */
+	uint32_t more_vlans_exist:1;
+	/**< At least one more VLAN exist in header, following this VLAN. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
-- 
1.8.3.1

[dpdk-dev] [PATCH v3] ethdev: add VLAN attributes to ETH and VLAN items

[Dekel Peled]

This patch implements the change proposes in RFC [1], adding dedicated
fields to ETH and VLAN items structs, to clearly define the required
characteristic of a packet, and enable precise match criteria.
Documentation is updated accordingly.

[1] https://mails.dpdk.org/archives/dev/2020-August/177536.html

---
v2: fix checkpatch comment.
v3: updates description in comments and documentation.
---

Signed-off-by: Dekel Peled <dekelp@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst     | 14 ++++++++++++--
 doc/guides/rel_notes/release_20_11.rst |  7 +++++++
 lib/librte_ethdev/rte_flow.h           | 18 +++++++++++++++---
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 119b128..9ce3d04 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -908,12 +908,16 @@ order as on the wire.
 If the ``type`` field contains a TPID value, then only tagged packets with the
 specified TPID will match the pattern.
 Otherwise, only untagged packets will match the pattern.
-If the ``ETH`` item is the only item in the pattern, and the ``type`` field is
-not specified, then both tagged and untagged packets will match the pattern.
+The field ``vlan_exist`` can be used to match specific packet types, instead
+of using the ``type`` field.
+This can be used to match any type of tagged packets.
+If the ``type`` and ``vlan_exist`` fields are not specified, then both tagged
+and untagged packets will match the pattern.
 
 - ``dst``: destination MAC.
 - ``src``: source MAC.
 - ``type``: EtherType or TPID.
+- ``vlan_exist``: At least one VLAN exist in packet header.
 - Default ``mask`` matches destination and source addresses only.
 
 Item: ``VLAN``
@@ -926,9 +930,15 @@ The corresponding standard outer EtherType (TPID) values are
 preceding pattern item.
 If a ``VLAN`` item is present in the pattern, then only tagged packets will
 match the pattern.
+The field ``more_vlans_exist`` can be used to match specific packet types,
+instead of using the ``inner_type field``.
+This can be used to match any type of tagged packets.
+If the ``inner_type`` and ``more_vlans_exist`` fields are not specified,
+then any tagged packets will match the pattern.
 
 - ``tci``: tag control information.
 - ``inner_type``: inner EtherType or TPID.
+- ``more_vlans_exist``: at least one more VLAN exist in packet header, after this VLAN.
 - Default ``mask`` matches the VID part of TCI only (lower 12 bits).
 
 Item: ``IPV4``
diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index 0b2a370..aceac07 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -198,6 +198,13 @@ API Changes
 
 * vhost: Moved vDPA APIs from experimental to stable.
 
+* ethdev: Added new field ``vlan_exist`` to structure ``rte_flow_item_eth``,
+  indicating that at least one VLAN exists in the packet header.
+
+* ethdev: Added new field ``more_vlans_exist`` to structure
+  ``rte_flow_item_vlan``, indicating that at least one more VLAN exists in
+  packet header, following this VLAN.
+
 * rawdev: Added a structure size parameter to the functions
   ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
   ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``,
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index da8bfa5..c3ac1e3 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -723,14 +723,18 @@ struct rte_flow_item_raw {
  * If the @p type field contains a TPID value, then only tagged packets with the
  * specified TPID will match the pattern.
  * Otherwise, only untagged packets will match the pattern.
- * If the @p ETH item is the only item in the pattern, and the @p type field
- * is not specified, then both tagged and untagged packets will match the
- * pattern.
+ * The field @p vlan_exist can be used to match specific packet types, instead
+ * of using the @p type field.
+ * This can be used to match any type of tagged packets.
+ * If the @p type and @p vlan_exist fields are not specified, then both tagged
+ * and untagged packets will match the pattern.
  */
 struct rte_flow_item_eth {
 	struct rte_ether_addr dst; /**< Destination MAC. */
 	struct rte_ether_addr src; /**< Source MAC. */
 	rte_be16_t type; /**< EtherType or TPID. */
+	uint32_t vlan_exist:1; /**< At least one VLAN exist in packet header. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
@@ -752,10 +756,18 @@ struct rte_flow_item_eth {
  * the preceding pattern item.
  * If a @p VLAN item is present in the pattern, then only tagged packets will
  * match the pattern.
+ * The field @p more_vlans_exist can be used to match specific packet types,
+ * instead of using the @p inner_type field.
+ * This can be used to match any type of tagged packets.
+ * If the @p inner_type and @p more_vlans_exist fields are not specified,
+ * then any tagged packets will match the pattern.
  */
 struct rte_flow_item_vlan {
 	rte_be16_t tci; /**< Tag control information. */
 	rte_be16_t inner_type; /**< Inner EtherType or TPID. */
+	uint32_t more_vlans_exist:1;
+	/**< At least one more VLAN exist in packet header, after this VLAN. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] ethdev: add VLAN attributes to ETH and VLAN items

[Maxime Leroy]

Hi Dekel,

On Thu, Oct 1, 2020 at 8:49 PM Dekel Peled <dekelp@nvidia.com> wrote:
>
> From: Dekel Peled <dekelp@mellanox.com>
>
> This patch implements the change proposes in RFC [1], adding dedicated
> fields to ETH and VLAN items structs, to clearly define the required
> characteristic of a packet, and enable precise match criteria.
>
> [1] https://mails.dpdk.org/archives/dev/2020-August/177536.html
>
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  doc/guides/rel_notes/release_20_11.rst |  7 +++++++
>  lib/librte_ethdev/rte_flow.h           | 16 +++++++++++++---
>  2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
> index 7f9d0dd..199c60b 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -173,6 +173,13 @@ API Changes
>    * ``_rte_eth_dev_callback_process()`` -> ``rte_eth_dev_callback_process()``
>    * ``_rte_eth_dev_reset`` -> ``rte_eth_dev_internal_reset()``
>
> +* ethdev: Added new field ``vlan_exist`` to structure ``rte_flow_item_eth``,
> +  indicating that at least one VLAN exists in the packet header.
> +
> +* ethdev: Added new field ``more_vlans_exist`` to structure
> +  ``rte_flow_item_vlan``, indicating that at least one more VLAN exists in
> +  packet header, following this VLAN.
> +
>  * rawdev: Added a structure size parameter to the functions
>    ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
>    ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``,
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index da8bfa5..39d04ef 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -723,14 +723,18 @@ struct rte_flow_item_raw {
>   * If the @p type field contains a TPID value, then only tagged packets with the
>   * specified TPID will match the pattern.
>   * Otherwise, only untagged packets will match the pattern.
> - * If the @p ETH item is the only item in the pattern, and the @p type field
> - * is not specified, then both tagged and untagged packets will match the
> - * pattern.
> + * The field @p vlan_exist can be used to match specific packet types, instead
> + * of using the @p type field.
> + * This can be used to match any type of tagged packets.
> + * If the @p type and @p vlan_exist fields are not specified, then both tagged
> + * and untagged packets will match the pattern.
>   */
>  struct rte_flow_item_eth {
>         struct rte_ether_addr dst; /**< Destination MAC. */
>         struct rte_ether_addr src; /**< Source MAC. */
>         rte_be16_t type; /**< EtherType or TPID. */
> +       uint32_t vlan_exist:1; /**< At least one VLAN exist in header. */
> +       uint32_t reserved:31; /**< Reserved, must be zero. */
>  };

To resume:
- type and vlan_exists fields not specified:  tag and untagged matched
- with vlan_exists, match only tag or untagged
- with type matching specific ethernet type
- vlan_exists and type should not setted at the same time ?

With this new specification, I think you address all the use cases.
That's great !

>
>  /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
> @@ -752,10 +756,16 @@ struct rte_flow_item_eth {
>   * the preceding pattern item.
>   * If a @p VLAN item is present in the pattern, then only tagged packets will
>   * match the pattern.
> + * The field @p more_vlans_exist can be used to match specific packet types,
> + * instead of using the @p inner_type field.
> + * This can be used to match any type of tagged packets.
>   */

Could you please specify what the expected behavior when inner_type
and more_vlans_exist are not specified .
What is the default behavior ?

>  struct rte_flow_item_vlan {
>         rte_be16_t tci; /**< Tag control information. */
>         rte_be16_t inner_type; /**< Inner EtherType or TPID. */
> +       uint32_t more_vlans_exist:1;
> +       /**< At least one more VLAN exist in header, following this VLAN. */
> +       uint32_t reserved:31; /**< Reserved, must be zero. */
>  };
>
>  /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
> --
> 1.8.3.1
>

I am still wondering, why not using a new item 'NOT' for example to
match only eth packet not tagged ?
example: eth / not vlan. It's a more generic solution.

Here in this commit, we add a reference on VLAN fields on ethernet header.
But tomorrow, we could do the same for mpls by adding mpls_exists in
the eth item and so on.

In fact, we  have the same needs for IPv6 options. To match for
example, ipv6 packet with no fragment option.
With a NOT field, it can be easily done: > eth / ipv6 / no ipv6_frag.

Adding new fields 'item'_exists into eth and ipv6 do the jobs, but
having a NOT attribute is a more generic solution.

It could address many other use cases like matching any udp packets
that are not vxlan ( eth / ipv4 / vxlan / not udp),

Let me know what you think about that.

Regards,

Maxime

Re: [dpdk-dev] [PATCH] ethdev: add VLAN attributes to ETH and VLAN items

[Thomas Monjalon]

02/10/2020 14:39, Maxime Leroy:
> Hi Dekel,
> 
> On Thu, Oct 1, 2020 at 8:49 PM Dekel Peled <dekelp@nvidia.com> wrote:
> >
> > From: Dekel Peled <dekelp@mellanox.com>
> >
> > This patch implements the change proposes in RFC [1], adding dedicated
> > fields to ETH and VLAN items structs, to clearly define the required
> > characteristic of a packet, and enable precise match criteria.

Please add more explanations, without relying on what is in RFC.
We need to clearly understand the motivation and why
this implementation is chosen.

If you correctly thread your patch with previous ones,
it should not be needed to mention RFC.

> >
> > [1] https://mails.dpdk.org/archives/dev/2020-August/177536.html
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> 
> I am still wondering, why not using a new item 'NOT' for example to
> match only eth packet not tagged ?
> example: eth / not vlan. It's a more generic solution.
> 
> Here in this commit, we add a reference on VLAN fields on ethernet header.
> But tomorrow, we could do the same for mpls by adding mpls_exists in
> the eth item and so on.
> 
> In fact, we  have the same needs for IPv6 options. To match for
> example, ipv6 packet with no fragment option.
> With a NOT field, it can be easily done: > eth / ipv6 / no ipv6_frag.
> 
> Adding new fields 'item'_exists into eth and ipv6 do the jobs, but
> having a NOT attribute is a more generic solution.
> 
> It could address many other use cases like matching any udp packets
> that are not vxlan ( eth / ipv4 / vxlan / not udp),
> 
> Let me know what you think about that.

You're right Maxime, a NOT operator looks better for the user,
but it is expected to be very hard to implement and offload.

Re: [dpdk-dev] [PATCH] ethdev: add VLAN attributes to ETH and VLAN items

[Dekel Peled]

Thanks, PSB.

> -----Original Message-----
> From: Maxime Leroy <maxime.leroy@6wind.com>
> Sent: Friday, October 2, 2020 3:39 PM
> To: Dekel Peled <dekelp@nvidia.com>
> Cc: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; ferruh.yigit@intel.com;
> arybchenko@solarflare.com; dev@dpdk.org; Dekel Peled
> <dekelp@mellanox.com>
> Subject: Re: [PATCH] ethdev: add VLAN attributes to ETH and VLAN items
> 
> Hi Dekel,
> 
> On Thu, Oct 1, 2020 at 8:49 PM Dekel Peled <dekelp@nvidia.com> wrote:
> >
> > From: Dekel Peled <dekelp@mellanox.com>
> >
> > This patch implements the change proposes in RFC [1], adding dedicated
> > fields to ETH and VLAN items structs, to clearly define the required
> > characteristic of a packet, and enable precise match criteria.
> >
> > [1]
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail
> > s.dpdk.org%2Farchives%2Fdev%2F2020-
> August%2F177536.html&amp;data=02%7C
> >
> 01%7Cdekelp%40nvidia.com%7Cc12bfd3f662747f7b7c408d866d0376f%7C430
> 83d15
> >
> 727340c1b7db39efd9ccc17a%7C0%7C0%7C637372391779092411&amp;sdata=
> yeOKvc
> > 4r0dL09UZ65%2Bt4qWJqJmcp21VyPSK%2FhbablKI%3D&amp;reserved=0
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > ---
> >  doc/guides/rel_notes/release_20_11.rst |  7 +++++++
> >  lib/librte_ethdev/rte_flow.h           | 16 +++++++++++++---
> >  2 files changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/doc/guides/rel_notes/release_20_11.rst
> > b/doc/guides/rel_notes/release_20_11.rst
> > index 7f9d0dd..199c60b 100644
> > --- a/doc/guides/rel_notes/release_20_11.rst
> > +++ b/doc/guides/rel_notes/release_20_11.rst
> > @@ -173,6 +173,13 @@ API Changes
> >    * ``_rte_eth_dev_callback_process()`` ->
> ``rte_eth_dev_callback_process()``
> >    * ``_rte_eth_dev_reset`` -> ``rte_eth_dev_internal_reset()``
> >
> > +* ethdev: Added new field ``vlan_exist`` to structure
> > +``rte_flow_item_eth``,
> > +  indicating that at least one VLAN exists in the packet header.
> > +
> > +* ethdev: Added new field ``more_vlans_exist`` to structure
> > +  ``rte_flow_item_vlan``, indicating that at least one more VLAN
> > +exists in
> > +  packet header, following this VLAN.
> > +
> >  * rawdev: Added a structure size parameter to the functions
> >    ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
> >    ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``, diff
> > --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index da8bfa5..39d04ef 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -723,14 +723,18 @@ struct rte_flow_item_raw {
> >   * If the @p type field contains a TPID value, then only tagged packets with
> the
> >   * specified TPID will match the pattern.
> >   * Otherwise, only untagged packets will match the pattern.
> > - * If the @p ETH item is the only item in the pattern, and the @p
> > type field
> > - * is not specified, then both tagged and untagged packets will match
> > the
> > - * pattern.
> > + * The field @p vlan_exist can be used to match specific packet
> > + types, instead
> > + * of using the @p type field.
> > + * This can be used to match any type of tagged packets.
> > + * If the @p type and @p vlan_exist fields are not specified, then
> > + both tagged
> > + * and untagged packets will match the pattern.
> >   */
> >  struct rte_flow_item_eth {
> >         struct rte_ether_addr dst; /**< Destination MAC. */
> >         struct rte_ether_addr src; /**< Source MAC. */
> >         rte_be16_t type; /**< EtherType or TPID. */
> > +       uint32_t vlan_exist:1; /**< At least one VLAN exist in header. */
> > +       uint32_t reserved:31; /**< Reserved, must be zero. */
> >  };
> 
> To resume:
> - type and vlan_exists fields not specified:  tag and untagged matched
> - with vlan_exists, match only tag or untagged
> - with type matching specific ethernet type
> - vlan_exists and type should not setted at the same time ?

PMD should validate they don't conflict.

> 
> With this new specification, I think you address all the use cases.
> That's great !
> 

Glad to see we agree on this.

> >
> >  /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */ @@ -752,10 +756,16
> @@
> > struct rte_flow_item_eth {
> >   * the preceding pattern item.
> >   * If a @p VLAN item is present in the pattern, then only tagged packets
> will
> >   * match the pattern.
> > + * The field @p more_vlans_exist can be used to match specific packet
> > + types,
> > + * instead of using the @p inner_type field.
> > + * This can be used to match any type of tagged packets.
> >   */
> 
> Could you please specify what the expected behavior when inner_type and
> more_vlans_exist are not specified .
> What is the default behavior ?
> 

You wrote above for the eth item, if the user didn't specify it means don't-care.

> >  struct rte_flow_item_vlan {
> >         rte_be16_t tci; /**< Tag control information. */
> >         rte_be16_t inner_type; /**< Inner EtherType or TPID. */
> > +       uint32_t more_vlans_exist:1;
> > +       /**< At least one more VLAN exist in header, following this VLAN. */
> > +       uint32_t reserved:31; /**< Reserved, must be zero. */
> >  };
> >
> >  /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
> > --
> > 1.8.3.1
> >
> 
> I am still wondering, why not using a new item 'NOT' for example to match
> only eth packet not tagged ?
> example: eth / not vlan. It's a more generic solution.
> 
> Here in this commit, we add a reference on VLAN fields on ethernet header.
> But tomorrow, we could do the same for mpls by adding mpls_exists in the
> eth item and so on.
> 
> In fact, we  have the same needs for IPv6 options. To match for example,
> ipv6 packet with no fragment option.
> With a NOT field, it can be easily done: > eth / ipv6 / no ipv6_frag.
> 
> Adding new fields 'item'_exists into eth and ipv6 do the jobs, but having a
> NOT attribute is a more generic solution.
> 
> It could address many other use cases like matching any udp packets that are
> not vxlan ( eth / ipv4 / vxlan / not udp),
> 
> Let me know what you think about that.

I agree with Thomas Monjalon response on this.

> 
> Regards,
> 
> Maxime

Re: [dpdk-dev] [PATCH] ethdev: add VLAN attributes to ETH and VLAN items

[Maxime Leroy]

On Mon, Oct 5, 2020 at 11:37 AM Dekel Peled <dekelp@nvidia.com> wrote:
>
> Thanks, PSB.
>
> > -----Original Message-----
> > From: Maxime Leroy <maxime.leroy@6wind.com>
> > Sent: Friday, October 2, 2020 3:39 PM
> > To: Dekel Peled <dekelp@nvidia.com>
> > Cc: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> > <thomas@monjalon.net>; ferruh.yigit@intel.com;
> > arybchenko@solarflare.com; dev@dpdk.org; Dekel Peled
> > <dekelp@mellanox.com>
> > Subject: Re: [PATCH] ethdev: add VLAN attributes to ETH and VLAN items
> >
> > Hi Dekel,
> >
> > On Thu, Oct 1, 2020 at 8:49 PM Dekel Peled <dekelp@nvidia.com> wrote:
> > >
> > > From: Dekel Peled <dekelp@mellanox.com>
> > >
> > > This patch implements the change proposes in RFC [1], adding dedicated
> > > fields to ETH and VLAN items structs, to clearly define the required
> > > characteristic of a packet, and enable precise match criteria.
> > >
> > > [1]
> > >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail
> > > s.dpdk.org%2Farchives%2Fdev%2F2020-
> > August%2F177536.html&amp;data=02%7C
> > >
> > 01%7Cdekelp%40nvidia.com%7Cc12bfd3f662747f7b7c408d866d0376f%7C430
> > 83d15
> > >
> > 727340c1b7db39efd9ccc17a%7C0%7C0%7C637372391779092411&amp;sdata=
> > yeOKvc
> > > 4r0dL09UZ65%2Bt4qWJqJmcp21VyPSK%2FhbablKI%3D&amp;reserved=0
> > >
> > > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > > ---
> > >  doc/guides/rel_notes/release_20_11.rst |  7 +++++++
> > >  lib/librte_ethdev/rte_flow.h           | 16 +++++++++++++---
> > >  2 files changed, 20 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/doc/guides/rel_notes/release_20_11.rst
> > > b/doc/guides/rel_notes/release_20_11.rst
> > > index 7f9d0dd..199c60b 100644
> > > --- a/doc/guides/rel_notes/release_20_11.rst
> > > +++ b/doc/guides/rel_notes/release_20_11.rst
> > > @@ -173,6 +173,13 @@ API Changes
> > >    * ``_rte_eth_dev_callback_process()`` ->
> > ``rte_eth_dev_callback_process()``
> > >    * ``_rte_eth_dev_reset`` -> ``rte_eth_dev_internal_reset()``
> > >
> > > +* ethdev: Added new field ``vlan_exist`` to structure
> > > +``rte_flow_item_eth``,
> > > +  indicating that at least one VLAN exists in the packet header.
> > > +
> > > +* ethdev: Added new field ``more_vlans_exist`` to structure
> > > +  ``rte_flow_item_vlan``, indicating that at least one more VLAN
> > > +exists in
> > > +  packet header, following this VLAN.
> > > +
> > >  * rawdev: Added a structure size parameter to the functions
> > >    ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
> > >    ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``, diff
> > > --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > > index da8bfa5..39d04ef 100644
> > > --- a/lib/librte_ethdev/rte_flow.h
> > > +++ b/lib/librte_ethdev/rte_flow.h
> > > @@ -723,14 +723,18 @@ struct rte_flow_item_raw {
> > >   * If the @p type field contains a TPID value, then only tagged packets with
> > the
> > >   * specified TPID will match the pattern.
> > >   * Otherwise, only untagged packets will match the pattern.
> > > - * If the @p ETH item is the only item in the pattern, and the @p
> > > type field
> > > - * is not specified, then both tagged and untagged packets will match
> > > the
> > > - * pattern.
> > > + * The field @p vlan_exist can be used to match specific packet
> > > + types, instead
> > > + * of using the @p type field.
> > > + * This can be used to match any type of tagged packets.
> > > + * If the @p type and @p vlan_exist fields are not specified, then
> > > + both tagged
> > > + * and untagged packets will match the pattern.
> > >   */
> > >  struct rte_flow_item_eth {
> > >         struct rte_ether_addr dst; /**< Destination MAC. */
> > >         struct rte_ether_addr src; /**< Source MAC. */
> > >         rte_be16_t type; /**< EtherType or TPID. */
> > > +       uint32_t vlan_exist:1; /**< At least one VLAN exist in header. */
> > > +       uint32_t reserved:31; /**< Reserved, must be zero. */
> > >  };
> >
> > To resume:
> > - type and vlan_exists fields not specified:  tag and untagged matched
> > - with vlan_exists, match only tag or untagged
> > - with type matching specific ethernet type
> > - vlan_exists and type should not setted at the same time ?
>
> PMD should validate they don't conflict.
>
> >
> > With this new specification, I think you address all the use cases.
> > That's great !
> >
>
> Glad to see we agree on this.
>
> > >
> > >  /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */ @@ -752,10 +756,16
> > @@
> > > struct rte_flow_item_eth {
> > >   * the preceding pattern item.
> > >   * If a @p VLAN item is present in the pattern, then only tagged packets
> > will
> > >   * match the pattern.
> > > + * The field @p more_vlans_exist can be used to match specific packet
> > > + types,
> > > + * instead of using the @p inner_type field.
> > > + * This can be used to match any type of tagged packets.
> > >   */
> >
> > Could you please specify what the expected behavior when inner_type and
> > more_vlans_exist are not specified .
> > What is the default behavior ?
> >
>
> You wrote above for the eth item, if the user didn't specify it means don't-care.
Could you please add the same comment for the vlan pattern ?

>
> > >  struct rte_flow_item_vlan {
> > >         rte_be16_t tci; /**< Tag control information. */
> > >         rte_be16_t inner_type; /**< Inner EtherType or TPID. */
> > > +       uint32_t more_vlans_exist:1;
> > > +       /**< At least one more VLAN exist in header, following this VLAN. */
> > > +       uint32_t reserved:31; /**< Reserved, must be zero. */
> > >  };
> > >
> > >  /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
> > > --
> > > 1.8.3.1
> > >
> >
> > I am still wondering, why not using a new item 'NOT' for example to match
> > only eth packet not tagged ?
> > example: eth / not vlan. It's a more generic solution.
> >
> > Here in this commit, we add a reference on VLAN fields on ethernet header.
> > But tomorrow, we could do the same for mpls by adding mpls_exists in the
> > eth item and so on.
> >
> > In fact, we  have the same needs for IPv6 options. To match for example,
> > ipv6 packet with no fragment option.
> > With a NOT field, it can be easily done: > eth / ipv6 / no ipv6_frag.
> >
> > Adding new fields 'item'_exists into eth and ipv6 do the jobs, but having a
> > NOT attribute is a more generic solution.
> >
> > It could address many other use cases like matching any udp packets that are
> > not vxlan ( eth / ipv4 / vxlan / not udp),
> >
> > Let me know what you think about that.
>
> I agree with Thomas Monjalon response on this.

RTE_FLOW pattern is here to have a generic way to describe a flow.

Of course, hardware nics don't need to support any type of pattern.
It's why we have rte_flow_validate functions to be sure that the
hardware can match this type of pattern.

For example, the not attribute could be only supported for vlan item
with mlx5 nics. (i.e. eth / not vlan).

When a user adds a flow with a pattern including a not attribute, if
the pmd doesn't support it, it should return -ENOTSUP.

Later, if we add support of not attribute with mpls (i.e. eth / not
mpls) in mlx5 pmd, modification can be done on the pmd side, without
any API changes.

You are already adding new '_exits' fields in IPv6 item. It's why I
think having a generic solution like a NOT attribute, it's a better
solution.

If we continue to add '_exists' fields in each item (like you are
doing with IPv6 item), I think we will need to do an API rework to
have a generic solution.

Regards,

Maxime

Re: [dpdk-dev] [PATCH] ethdev: add VLAN attributes to ETH and VLAN items

[Ori Kam]

Sorry for jumping late,


> -----Original Message-----
> From: Maxime Leroy <maxime.leroy@6wind.com>
> Sent: Wednesday, October 7, 2020 2:52 PM
> Subject: Re: [PATCH] ethdev: add VLAN attributes to ETH and VLAN items
> 
> On Mon, Oct 5, 2020 at 11:37 AM Dekel Peled <dekelp@nvidia.com> wrote:
> >
> > Thanks, PSB.
> >
> > > -----Original Message-----
> > > From: Maxime Leroy <maxime.leroy@6wind.com>
> > > Sent: Friday, October 2, 2020 3:39 PM
> > > To: Dekel Peled <dekelp@nvidia.com>
> > > Cc: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> > > <thomas@monjalon.net>; ferruh.yigit@intel.com;
> > > arybchenko@solarflare.com; dev@dpdk.org; Dekel Peled
> > > <dekelp@mellanox.com>
> > > Subject: Re: [PATCH] ethdev: add VLAN attributes to ETH and VLAN items
> > >
> > > Hi Dekel,
> > >
> > > On Thu, Oct 1, 2020 at 8:49 PM Dekel Peled <dekelp@nvidia.com> wrote:
> > > >
> > > > From: Dekel Peled <dekelp@mellanox.com>
> > > >
> > > > This patch implements the change proposes in RFC [1], adding dedicated
> > > > fields to ETH and VLAN items structs, to clearly define the required
> > > > characteristic of a packet, and enable precise match criteria.
> > > >
> > > > [1]
> > > >
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail
> > > > s.dpdk.org%2Farchives%2Fdev%2F2020-
> > > August%2F177536.html&amp;data=02%7C
> > > >
> > >
> 01%7Cdekelp%40nvidia.com%7Cc12bfd3f662747f7b7c408d866d0376f%7C430
> > > 83d15
> > > >
> > >
> 727340c1b7db39efd9ccc17a%7C0%7C0%7C637372391779092411&amp;sdata=
> > > yeOKvc
> > > > 4r0dL09UZ65%2Bt4qWJqJmcp21VyPSK%2FhbablKI%3D&amp;reserved=0
> > > >
> > > > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > > > ---
> > > >  doc/guides/rel_notes/release_20_11.rst |  7 +++++++
> > > >  lib/librte_ethdev/rte_flow.h           | 16 +++++++++++++---
> > > >  2 files changed, 20 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/doc/guides/rel_notes/release_20_11.rst
> > > > b/doc/guides/rel_notes/release_20_11.rst
> > > > index 7f9d0dd..199c60b 100644
> > > > --- a/doc/guides/rel_notes/release_20_11.rst
> > > > +++ b/doc/guides/rel_notes/release_20_11.rst
> > > > @@ -173,6 +173,13 @@ API Changes
> > > >    * ``_rte_eth_dev_callback_process()`` ->
> > > ``rte_eth_dev_callback_process()``
> > > >    * ``_rte_eth_dev_reset`` -> ``rte_eth_dev_internal_reset()``
> > > >
> > > > +* ethdev: Added new field ``vlan_exist`` to structure
> > > > +``rte_flow_item_eth``,
> > > > +  indicating that at least one VLAN exists in the packet header.
> > > > +
> > > > +* ethdev: Added new field ``more_vlans_exist`` to structure
> > > > +  ``rte_flow_item_vlan``, indicating that at least one more VLAN
> > > > +exists in
> > > > +  packet header, following this VLAN.
> > > > +
> > > >  * rawdev: Added a structure size parameter to the functions
> > > >    ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
> > > >    ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``, diff
> > > > --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > > > index da8bfa5..39d04ef 100644
> > > > --- a/lib/librte_ethdev/rte_flow.h
> > > > +++ b/lib/librte_ethdev/rte_flow.h
> > > > @@ -723,14 +723,18 @@ struct rte_flow_item_raw {
> > > >   * If the @p type field contains a TPID value, then only tagged packets
> with
> > > the
> > > >   * specified TPID will match the pattern.
> > > >   * Otherwise, only untagged packets will match the pattern.
> > > > - * If the @p ETH item is the only item in the pattern, and the @p
> > > > type field
> > > > - * is not specified, then both tagged and untagged packets will match
> > > > the
> > > > - * pattern.
> > > > + * The field @p vlan_exist can be used to match specific packet
> > > > + types, instead
> > > > + * of using the @p type field.
> > > > + * This can be used to match any type of tagged packets.
> > > > + * If the @p type and @p vlan_exist fields are not specified, then
> > > > + both tagged
> > > > + * and untagged packets will match the pattern.
> > > >   */
> > > >  struct rte_flow_item_eth {
> > > >         struct rte_ether_addr dst; /**< Destination MAC. */
> > > >         struct rte_ether_addr src; /**< Source MAC. */
> > > >         rte_be16_t type; /**< EtherType or TPID. */
> > > > +       uint32_t vlan_exist:1; /**< At least one VLAN exist in header. */
> > > > +       uint32_t reserved:31; /**< Reserved, must be zero. */
> > > >  };
> > >
> > > To resume:
> > > - type and vlan_exists fields not specified:  tag and untagged matched
> > > - with vlan_exists, match only tag or untagged
> > > - with type matching specific ethernet type
> > > - vlan_exists and type should not setted at the same time ?
> >
> > PMD should validate they don't conflict.
> >
> > >
> > > With this new specification, I think you address all the use cases.
> > > That's great !
> > >
> >
> > Glad to see we agree on this.
> >
> > > >
> > > >  /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */ @@ -752,10
> +756,16
> > > @@
> > > > struct rte_flow_item_eth {
> > > >   * the preceding pattern item.
> > > >   * If a @p VLAN item is present in the pattern, then only tagged packets
> > > will
> > > >   * match the pattern.
> > > > + * The field @p more_vlans_exist can be used to match specific packet
> > > > + types,
> > > > + * instead of using the @p inner_type field.
> > > > + * This can be used to match any type of tagged packets.
> > > >   */
> > >
> > > Could you please specify what the expected behavior when inner_type and
> > > more_vlans_exist are not specified .
> > > What is the default behavior ?
> > >
> >
> > You wrote above for the eth item, if the user didn't specify it means don't-
> care.
> Could you please add the same comment for the vlan pattern ?
> 
> >
> > > >  struct rte_flow_item_vlan {
> > > >         rte_be16_t tci; /**< Tag control information. */
> > > >         rte_be16_t inner_type; /**< Inner EtherType or TPID. */
> > > > +       uint32_t more_vlans_exist:1;
> > > > +       /**< At least one more VLAN exist in header, following this VLAN. */
> > > > +       uint32_t reserved:31; /**< Reserved, must be zero. */
> > > >  };
> > > >
> > > >  /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
> > > > --
> > > > 1.8.3.1
> > > >
> > >
> > > I am still wondering, why not using a new item 'NOT' for example to match
> > > only eth packet not tagged ?
> > > example: eth / not vlan. It's a more generic solution.
> > >
> > > Here in this commit, we add a reference on VLAN fields on ethernet
> header.
> > > But tomorrow, we could do the same for mpls by adding mpls_exists in the
> > > eth item and so on.
> > >
> > > In fact, we  have the same needs for IPv6 options. To match for example,
> > > ipv6 packet with no fragment option.
> > > With a NOT field, it can be easily done: > eth / ipv6 / no ipv6_frag.
> > >
> > > Adding new fields 'item'_exists into eth and ipv6 do the jobs, but having a
> > > NOT attribute is a more generic solution.
> > >
> > > It could address many other use cases like matching any udp packets that
> are
> > > not vxlan ( eth / ipv4 / vxlan / not udp),
> > >
> > > Let me know what you think about that.
> >
> > I agree with Thomas Monjalon response on this.
> 
> RTE_FLOW pattern is here to have a generic way to describe a flow.
> 
> Of course, hardware nics don't need to support any type of pattern.
> It's why we have rte_flow_validate functions to be sure that the
> hardware can match this type of pattern.
> 
> For example, the not attribute could be only supported for vlan item
> with mlx5 nics. (i.e. eth / not vlan).
> 
> When a user adds a flow with a pattern including a not attribute, if
> the pmd doesn't support it, it should return -ENOTSUP.
> 
> Later, if we add support of not attribute with mpls (i.e. eth / not
> mpls) in mlx5 pmd, modification can be done on the pmd side, without
> any API changes.
> 
> You are already adding new '_exits' fields in IPv6 item. It's why I
> think having a generic solution like a NOT attribute, it's a better
> solution.
> 
> If we continue to add '_exists' fields in each item (like you are
> doing with IPv6 item), I think we will need to do an API rework to
> have a generic solution.
> 
> Regards,
> 
> Maxime

First I'm all in favor of adding a not item, but it is very tricky and should be designed very carefully.
Also using a not will get the rules to be very complicated.
For example think about the following case:
Application want only packets without any extensions, using the suggested API it is very simple
just set exits = 0 with mask = 1.
While if we use the not I'm not sure how it should look, since we need number of not,
it is not just enough to say next proto is not XXX since we need to cover all possible
extensions, also there might be ordering issue which the not can't support.

So my thinking is that we should go with the suggested approach and regardless see
how can we add the not.

In any case the exit should not be the goto solution but again in case of extension it make
sense since order is not guaranteed.

What do you think?

Best,
Ori

Re: [dpdk-dev] [PATCH] ethdev: add VLAN attributes to ETH and VLAN items

[Dekel Peled]

Thanks, PSB.

> -----Original Message-----
> From: Ori Kam <orika@nvidia.com>
> Sent: Wednesday, October 7, 2020 3:14 PM
> To: Maxime Leroy <maxime.leroy@6wind.com>; Dekel Peled
> <dekelp@nvidia.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>
> Cc: ferruh.yigit@intel.com; arybchenko@solarflare.com; dev@dpdk.org
> Subject: RE: [PATCH] ethdev: add VLAN attributes to ETH and VLAN items
> 
> Sorry for jumping late,

Please note v2 of this patch is in ML since Oct. 1st.
We should continue the discussion on the latest thread.
More comments below.

> 
> 
> > -----Original Message-----
> > From: Maxime Leroy <maxime.leroy@6wind.com>
> > Sent: Wednesday, October 7, 2020 2:52 PM
> > Subject: Re: [PATCH] ethdev: add VLAN attributes to ETH and VLAN items
> >
> > On Mon, Oct 5, 2020 at 11:37 AM Dekel Peled <dekelp@nvidia.com> wrote:
> > >
> > > Thanks, PSB.
> > >
> > > > -----Original Message-----
> > > > From: Maxime Leroy <maxime.leroy@6wind.com>
> > > > Sent: Friday, October 2, 2020 3:39 PM
> > > > To: Dekel Peled <dekelp@nvidia.com>
> > > > Cc: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> > > > <thomas@monjalon.net>; ferruh.yigit@intel.com;
> > > > arybchenko@solarflare.com; dev@dpdk.org; Dekel Peled
> > > > <dekelp@mellanox.com>
> > > > Subject: Re: [PATCH] ethdev: add VLAN attributes to ETH and VLAN
> > > > items
> > > >
> > > > Hi Dekel,
> > > >
> > > > On Thu, Oct 1, 2020 at 8:49 PM Dekel Peled <dekelp@nvidia.com>
> wrote:
> > > > >
> > > > > From: Dekel Peled <dekelp@mellanox.com>
> > > > >
> > > > > This patch implements the change proposes in RFC [1], adding
> > > > > dedicated fields to ETH and VLAN items structs, to clearly
> > > > > define the required characteristic of a packet, and enable precise
> match criteria.
> > > > >
> > > > > [1]
> > > > >
> > > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > mail
> > > > > s.dpdk.org%2Farchives%2Fdev%2F2020-
> > > > August%2F177536.html&amp;data=02%7C
> > > > >
> > > >
> >
> 01%7Cdekelp%40nvidia.com%7Cc12bfd3f662747f7b7c408d866d0376f%7C430
> > > > 83d15
> > > > >
> > > >
> >
> 727340c1b7db39efd9ccc17a%7C0%7C0%7C637372391779092411&amp;sdata=
> > > > yeOKvc
> > > > >
> 4r0dL09UZ65%2Bt4qWJqJmcp21VyPSK%2FhbablKI%3D&amp;reserved=0
> > > > >
> > > > > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > > > > ---
> > > > >  doc/guides/rel_notes/release_20_11.rst |  7 +++++++
> > > > >  lib/librte_ethdev/rte_flow.h           | 16 +++++++++++++---
> > > > >  2 files changed, 20 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/doc/guides/rel_notes/release_20_11.rst
> > > > > b/doc/guides/rel_notes/release_20_11.rst
> > > > > index 7f9d0dd..199c60b 100644
> > > > > --- a/doc/guides/rel_notes/release_20_11.rst
> > > > > +++ b/doc/guides/rel_notes/release_20_11.rst
> > > > > @@ -173,6 +173,13 @@ API Changes
> > > > >    * ``_rte_eth_dev_callback_process()`` ->
> > > > ``rte_eth_dev_callback_process()``
> > > > >    * ``_rte_eth_dev_reset`` -> ``rte_eth_dev_internal_reset()``
> > > > >
> > > > > +* ethdev: Added new field ``vlan_exist`` to structure
> > > > > +``rte_flow_item_eth``,
> > > > > +  indicating that at least one VLAN exists in the packet header.
> > > > > +
> > > > > +* ethdev: Added new field ``more_vlans_exist`` to structure
> > > > > +  ``rte_flow_item_vlan``, indicating that at least one more
> > > > > +VLAN exists in
> > > > > +  packet header, following this VLAN.
> > > > > +
> > > > >  * rawdev: Added a structure size parameter to the functions
> > > > >    ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
> > > > >    ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``,
> > > > > diff --git a/lib/librte_ethdev/rte_flow.h
> > > > > b/lib/librte_ethdev/rte_flow.h index da8bfa5..39d04ef 100644
> > > > > --- a/lib/librte_ethdev/rte_flow.h
> > > > > +++ b/lib/librte_ethdev/rte_flow.h
> > > > > @@ -723,14 +723,18 @@ struct rte_flow_item_raw {
> > > > >   * If the @p type field contains a TPID value, then only tagged
> > > > > packets
> > with
> > > > the
> > > > >   * specified TPID will match the pattern.
> > > > >   * Otherwise, only untagged packets will match the pattern.
> > > > > - * If the @p ETH item is the only item in the pattern, and the
> > > > > @p type field
> > > > > - * is not specified, then both tagged and untagged packets will
> > > > > match the
> > > > > - * pattern.
> > > > > + * The field @p vlan_exist can be used to match specific packet
> > > > > + types, instead
> > > > > + * of using the @p type field.
> > > > > + * This can be used to match any type of tagged packets.
> > > > > + * If the @p type and @p vlan_exist fields are not specified,
> > > > > + then both tagged
> > > > > + * and untagged packets will match the pattern.
> > > > >   */
> > > > >  struct rte_flow_item_eth {
> > > > >         struct rte_ether_addr dst; /**< Destination MAC. */
> > > > >         struct rte_ether_addr src; /**< Source MAC. */
> > > > >         rte_be16_t type; /**< EtherType or TPID. */
> > > > > +       uint32_t vlan_exist:1; /**< At least one VLAN exist in header. */
> > > > > +       uint32_t reserved:31; /**< Reserved, must be zero. */
> > > > >  };
> > > >
> > > > To resume:
> > > > - type and vlan_exists fields not specified:  tag and untagged
> > > > matched
> > > > - with vlan_exists, match only tag or untagged
> > > > - with type matching specific ethernet type
> > > > - vlan_exists and type should not setted at the same time ?
> > >
> > > PMD should validate they don't conflict.
> > >
> > > >
> > > > With this new specification, I think you address all the use cases.
> > > > That's great !
> > > >
> > >
> > > Glad to see we agree on this.
> > >
> > > > >
> > > > >  /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */ @@ -752,10
> > +756,16
> > > > @@
> > > > > struct rte_flow_item_eth {
> > > > >   * the preceding pattern item.
> > > > >   * If a @p VLAN item is present in the pattern, then only
> > > > > tagged packets
> > > > will
> > > > >   * match the pattern.
> > > > > + * The field @p more_vlans_exist can be used to match specific
> > > > > + packet types,
> > > > > + * instead of using the @p inner_type field.
> > > > > + * This can be used to match any type of tagged packets.
> > > > >   */
> > > >
> > > > Could you please specify what the expected behavior when
> > > > inner_type and more_vlans_exist are not specified .
> > > > What is the default behavior ?
> > > >
> > >
> > > You wrote above for the eth item, if the user didn't specify it
> > > means don't-
> > care.
> > Could you please add the same comment for the vlan pattern ?

I will send v3 with added description.

> >
> > >
> > > > >  struct rte_flow_item_vlan {
> > > > >         rte_be16_t tci; /**< Tag control information. */
> > > > >         rte_be16_t inner_type; /**< Inner EtherType or TPID. */
> > > > > +       uint32_t more_vlans_exist:1;
> > > > > +       /**< At least one more VLAN exist in header, following this
> VLAN. */
> > > > > +       uint32_t reserved:31; /**< Reserved, must be zero. */
> > > > >  };
> > > > >
> > > > >  /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
> > > > > --
> > > > > 1.8.3.1
> > > > >
> > > >
> > > > I am still wondering, why not using a new item 'NOT' for example
> > > > to match only eth packet not tagged ?
> > > > example: eth / not vlan. It's a more generic solution.
> > > >
> > > > Here in this commit, we add a reference on VLAN fields on ethernet
> > header.
> > > > But tomorrow, we could do the same for mpls by adding mpls_exists
> > > > in the eth item and so on.
> > > >
> > > > In fact, we  have the same needs for IPv6 options. To match for
> > > > example,
> > > > ipv6 packet with no fragment option.
> > > > With a NOT field, it can be easily done: > eth / ipv6 / no ipv6_frag.
> > > >
> > > > Adding new fields 'item'_exists into eth and ipv6 do the jobs, but
> > > > having a NOT attribute is a more generic solution.
> > > >
> > > > It could address many other use cases like matching any udp
> > > > packets that
> > are
> > > > not vxlan ( eth / ipv4 / vxlan / not udp),
> > > >
> > > > Let me know what you think about that.
> > >
> > > I agree with Thomas Monjalon response on this.
> >
> > RTE_FLOW pattern is here to have a generic way to describe a flow.
> >
> > Of course, hardware nics don't need to support any type of pattern.
> > It's why we have rte_flow_validate functions to be sure that the
> > hardware can match this type of pattern.
> >
> > For example, the not attribute could be only supported for vlan item
> > with mlx5 nics. (i.e. eth / not vlan).
> >
> > When a user adds a flow with a pattern including a not attribute, if
> > the pmd doesn't support it, it should return -ENOTSUP.
> >
> > Later, if we add support of not attribute with mpls (i.e. eth / not
> > mpls) in mlx5 pmd, modification can be done on the pmd side, without
> > any API changes.
> >
> > You are already adding new '_exits' fields in IPv6 item. It's why I
> > think having a generic solution like a NOT attribute, it's a better
> > solution.
> >
> > If we continue to add '_exists' fields in each item (like you are
> > doing with IPv6 item), I think we will need to do an API rework to
> > have a generic solution.
> >
> > Regards,
> >
> > Maxime
> 
> First I'm all in favor of adding a not item, but it is very tricky and should be
> designed very carefully.
> Also using a not will get the rules to be very complicated.
> For example think about the following case:
> Application want only packets without any extensions, using the suggested
> API it is very simple just set exits = 0 with mask = 1.
> While if we use the not I'm not sure how it should look, since we need
> number of not, it is not just enough to say next proto is not XXX since we
> need to cover all possible extensions, also there might be ordering issue
> which the not can't support.
> 
> So my thinking is that we should go with the suggested approach and
> regardless see how can we add the not.
> 
> In any case the exit should not be the goto solution but again in case of
> extension it make sense since order is not guaranteed.
> 
> What do you think?
> 
> Best,
> Ori
> 

[dpdk-dev] [PATCH v4 0/2] support VLAN attributes in ETH and VLAN items

[Dekel Peled]

This series implements the change proposes in RFC [1], adding dedicated
fields to ETH and VLAN items structs, to clearly define the required
characteristic of a packet, and enable precise match criteria.
testpmd CLI is updated accordingly.
Documentation is updated accordingly.

[1] https://mails.dpdk.org/archives/dev/2020-August/177536.html

---
v2: fix checkpatch comment.
v3: updates description in comments and documentation.
v4: add testpmd patch with application support.
---

Dekel Peled (2):
  ethdev: add VLAN attributes to ETH and VLAN items
  app/testpmd: support VLAN attributes in ETH and VLAN items

 app/test-pmd/cmdline_flow.c            | 18 ++++++++++++++++++
 doc/guides/prog_guide/rte_flow.rst     | 14 ++++++++++++--
 doc/guides/rel_notes/deprecation.rst   |  5 -----
 doc/guides/rel_notes/release_20_11.rst |  7 +++++++
 lib/librte_ethdev/rte_flow.h           | 18 +++++++++++++++---
 5 files changed, 52 insertions(+), 10 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v4 1/2] ethdev: add VLAN attributes to ETH and VLAN items

[Dekel Peled]

This patch implements the change proposes in RFC [1], adding dedicated
fields to ETH and VLAN items structs, to clearly define the required
characteristic of a packet, and enable precise match criteria.
Documentation is updated accordingly.

[1] https://mails.dpdk.org/archives/dev/2020-August/177536.html

Signed-off-by: Dekel Peled <dekelp@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst     | 14 ++++++++++++--
 doc/guides/rel_notes/deprecation.rst   |  5 -----
 doc/guides/rel_notes/release_20_11.rst |  7 +++++++
 lib/librte_ethdev/rte_flow.h           | 18 +++++++++++++++---
 4 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index f26a6c2..40230d3 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -908,12 +908,16 @@ order as on the wire.
 If the ``type`` field contains a TPID value, then only tagged packets with the
 specified TPID will match the pattern.
 Otherwise, only untagged packets will match the pattern.
-If the ``ETH`` item is the only item in the pattern, and the ``type`` field is
-not specified, then both tagged and untagged packets will match the pattern.
+The field ``has_vlan`` can be used to match specific packet types, instead
+of using the ``type`` field.
+This can be used to match any type of tagged packets.
+If the ``type`` and ``has_vlan`` fields are not specified, then both tagged
+and untagged packets will match the pattern.
 
 - ``dst``: destination MAC.
 - ``src``: source MAC.
 - ``type``: EtherType or TPID.
+- ``has_vlan``: packet header contains at least one VLAN.
 - Default ``mask`` matches destination and source addresses only.
 
 Item: ``VLAN``
@@ -926,9 +930,15 @@ The corresponding standard outer EtherType (TPID) values are
 preceding pattern item.
 If a ``VLAN`` item is present in the pattern, then only tagged packets will
 match the pattern.
+The field ``has_more_vlan`` can be used to match specific packet types,
+instead of using the ``inner_type field``.
+This can be used to match any type of tagged packets.
+If the ``inner_type`` and ``has_more_vlan`` fields are not specified,
+then any tagged packets will match the pattern.
 
 - ``tci``: tag control information.
 - ``inner_type``: inner EtherType or TPID.
+- ``has_more_vlan``: packet header contains at least one more VLAN, after this VLAN.
 - Default ``mask`` matches the VID part of TCI only (lower 12 bits).
 
 Item: ``IPV4``
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 584e720..72011b0 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -154,11 +154,6 @@ Deprecation Notices
   as deprecated in DPDK 20.11, along with the associated macros ``ETH_MIRROR_*``.
   This API will be fully removed in DPDK 21.11.
 
-* ethdev: The ``struct rte_flow_item_eth`` and ``struct rte_flow_item_vlan``
-  structs will be modified, to include an additional value, indicating existence
-  or absence of a VLAN header following the current header, as proposed in RFC
-  https://mails.dpdk.org/archives/dev/2020-August/177536.html.
-
 * ethdev: The ``struct rte_flow_item_ipv6`` struct will be modified to include
   additional values, indicating existence or absence of IPv6 extension headers
   following the IPv6 header, as proposed in RFC
diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index 30db8f2..4932d82 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -292,6 +292,13 @@ API Changes
 
 * vhost: Moved vDPA APIs from experimental to stable.
 
+* ethdev: Added new field ``has_vlan`` to structure ``rte_flow_item_eth``,
+  indicating that packet header contains at least one VLAN.
+
+* ethdev: Added new field ``has_more_vlan`` to structure
+  ``rte_flow_item_vlan``, indicating that packet header contains at least one
+  more VLAN, after this VLAN.
+
 * rawdev: Added a structure size parameter to the functions
   ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
   ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``,
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 3d5fb09..cb3bb5c 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -723,14 +723,18 @@ struct rte_flow_item_raw {
  * If the @p type field contains a TPID value, then only tagged packets with the
  * specified TPID will match the pattern.
  * Otherwise, only untagged packets will match the pattern.
- * If the @p ETH item is the only item in the pattern, and the @p type field
- * is not specified, then both tagged and untagged packets will match the
- * pattern.
+ * The field @p has_vlan can be used to match specific packet types, instead
+ * of using the @p type field.
+ * This can be used to match any type of tagged packets.
+ * If the @p type and @p has_vlan fields are not specified, then both tagged
+ * and untagged packets will match the pattern.
  */
 struct rte_flow_item_eth {
 	struct rte_ether_addr dst; /**< Destination MAC. */
 	struct rte_ether_addr src; /**< Source MAC. */
 	rte_be16_t type; /**< EtherType or TPID. */
+	uint32_t has_vlan:1; /**< Packet header contains at least one VLAN. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
@@ -752,10 +756,18 @@ struct rte_flow_item_eth {
  * the preceding pattern item.
  * If a @p VLAN item is present in the pattern, then only tagged packets will
  * match the pattern.
+ * The field @p has_more_vlan can be used to match specific packet types,
+ * instead of using the @p inner_type field.
+ * This can be used to match any type of tagged packets.
+ * If the @p inner_type and @p has_more_vlan fields are not specified,
+ * then any tagged packets will match the pattern.
  */
 struct rte_flow_item_vlan {
 	rte_be16_t tci; /**< Tag control information. */
 	rte_be16_t inner_type; /**< Inner EtherType or TPID. */
+	uint32_t has_more_vlan:1;
+	/**< Packet header contains at least one more VLAN, after this VLAN. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add VLAN attributes to ETH and VLAN items

[Thomas Monjalon]

14/10/2020 20:53, Dekel Peled:
> @@ -292,6 +292,13 @@ API Changes
>  
>  * vhost: Moved vDPA APIs from experimental to stable.
>  
> +* ethdev: Added new field ``has_vlan`` to structure ``rte_flow_item_eth``,
> +  indicating that packet header contains at least one VLAN.
> +
> +* ethdev: Added new field ``has_more_vlan`` to structure
> +  ``rte_flow_item_vlan``, indicating that packet header contains at least one
> +  more VLAN, after this VLAN.
> +

The ethdev changes should be grouped with others above vhost one.

[...]
> - * If the @p ETH item is the only item in the pattern, and the @p type field
> - * is not specified, then both tagged and untagged packets will match the
> - * pattern.
> + * The field @p has_vlan can be used to match specific packet types, instead
> + * of using the @p type field.
> + * This can be used to match any type of tagged packets.

If I understand well, these two sentences could be merged in one:
The field @p has_vlan can be used to match tagged packets,
instead of using the @p type field.

> + * If the @p type and @p has_vlan fields are not specified, then both tagged
> + * and untagged packets will match the pattern.

OK

[...]
> + * The field @p has_more_vlan can be used to match specific packet types,
> + * instead of using the @p inner_type field.
> + * This can be used to match any type of tagged packets.

Here as well, "specific" can be replaced with "QinQ" or "inner tagged".

> + * If the @p inner_type and @p has_more_vlan fields are not specified,
> + * then any tagged packets will match the pattern.

OK

Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add VLAN attributes to ETH and VLAN items

[Andrew Rybchenko]

On 10/14/20 9:53 PM, Dekel Peled wrote:
> This patch implements the change proposes in RFC [1], adding dedicated
> fields to ETH and VLAN items structs, to clearly define the required
> characteristic of a packet, and enable precise match criteria.
> Documentation is updated accordingly.
> 
> [1] https://mails.dpdk.org/archives/dev/2020-August/177536.html
> 
> Signed-off-by: Dekel Peled <dekelp@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst     | 14 ++++++++++++--
>  doc/guides/rel_notes/deprecation.rst   |  5 -----
>  doc/guides/rel_notes/release_20_11.rst |  7 +++++++
>  lib/librte_ethdev/rte_flow.h           | 18 +++++++++++++++---
>  4 files changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index f26a6c2..40230d3 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -908,12 +908,16 @@ order as on the wire.
>  If the ``type`` field contains a TPID value, then only tagged packets with the
>  specified TPID will match the pattern.
>  Otherwise, only untagged packets will match the pattern.

Is the above sentence still valid? It looks like no
if has_vlan is set in a mask and set in spec.

> -If the ``ETH`` item is the only item in the pattern, and the ``type`` field is
> -not specified, then both tagged and untagged packets will match the pattern.
> +The field ``has_vlan`` can be used to match specific packet types, instead
> +of using the ``type`` field.
> +This can be used to match any type of tagged packets.
> +If the ``type`` and ``has_vlan`` fields are not specified, then both tagged
> +and untagged packets will match the pattern.

Consider to highlight that has_vlan field in mask
controls if the field in spec is taken into account.
If the field is unset in mask, it could be either
tagged or untagged (if there is no VLAN item in spec).
If VLAN item is present, basically it is the same
as mask.has_vlan==1 and spec.has_vlan==1.

>  
>  - ``dst``: destination MAC.
>  - ``src``: source MAC.
>  - ``type``: EtherType or TPID.
> +- ``has_vlan``: packet header contains at least one VLAN.
>  - Default ``mask`` matches destination and source addresses only.
>  
>  Item: ``VLAN``
> @@ -926,9 +930,15 @@ The corresponding standard outer EtherType (TPID) values are
>  preceding pattern item.
>  If a ``VLAN`` item is present in the pattern, then only tagged packets will
>  match the pattern.
> +The field ``has_more_vlan`` can be used to match specific packet types,
> +instead of using the ``inner_type field``.
> +This can be used to match any type of tagged packets.
> +If the ``inner_type`` and ``has_more_vlan`` fields are not specified,
> +then any tagged packets will match the pattern.
>  
>  - ``tci``: tag control information.
>  - ``inner_type``: inner EtherType or TPID.
> +- ``has_more_vlan``: packet header contains at least one more VLAN, after this VLAN.
>  - Default ``mask`` matches the VID part of TCI only (lower 12 bits).
>  
>  Item: ``IPV4``
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 584e720..72011b0 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -154,11 +154,6 @@ Deprecation Notices
>    as deprecated in DPDK 20.11, along with the associated macros ``ETH_MIRROR_*``.
>    This API will be fully removed in DPDK 21.11.
>  
> -* ethdev: The ``struct rte_flow_item_eth`` and ``struct rte_flow_item_vlan``
> -  structs will be modified, to include an additional value, indicating existence
> -  or absence of a VLAN header following the current header, as proposed in RFC
> -  https://mails.dpdk.org/archives/dev/2020-August/177536.html.
> -
>  * ethdev: The ``struct rte_flow_item_ipv6`` struct will be modified to include
>    additional values, indicating existence or absence of IPv6 extension headers
>    following the IPv6 header, as proposed in RFC
> diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
> index 30db8f2..4932d82 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -292,6 +292,13 @@ API Changes
>  
>  * vhost: Moved vDPA APIs from experimental to stable.
>  
> +* ethdev: Added new field ``has_vlan`` to structure ``rte_flow_item_eth``,
> +  indicating that packet header contains at least one VLAN.
> +
> +* ethdev: Added new field ``has_more_vlan`` to structure
> +  ``rte_flow_item_vlan``, indicating that packet header contains at least one
> +  more VLAN, after this VLAN.
> +
>  * rawdev: Added a structure size parameter to the functions
>    ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
>    ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``,
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 3d5fb09..cb3bb5c 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -723,14 +723,18 @@ struct rte_flow_item_raw {
>   * If the @p type field contains a TPID value, then only tagged packets with the
>   * specified TPID will match the pattern.
>   * Otherwise, only untagged packets will match the pattern.
> - * If the @p ETH item is the only item in the pattern, and the @p type field
> - * is not specified, then both tagged and untagged packets will match the
> - * pattern.
> + * The field @p has_vlan can be used to match specific packet types, instead
> + * of using the @p type field.
> + * This can be used to match any type of tagged packets.
> + * If the @p type and @p has_vlan fields are not specified, then both tagged
> + * and untagged packets will match the pattern.

.. if there is no VLAN item in the pattern

>   */
>  struct rte_flow_item_eth {
>  	struct rte_ether_addr dst; /**< Destination MAC. */
>  	struct rte_ether_addr src; /**< Source MAC. */
>  	rte_be16_t type; /**< EtherType or TPID. */
> +	uint32_t has_vlan:1; /**< Packet header contains at least one VLAN. */
> +	uint32_t reserved:31; /**< Reserved, must be zero. */

Have you considered to make it uint16_t to keep the size of the
structure and have no holes?

>  };
>  
>  /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
> @@ -752,10 +756,18 @@ struct rte_flow_item_eth {
>   * the preceding pattern item.
>   * If a @p VLAN item is present in the pattern, then only tagged packets will
>   * match the pattern.
> + * The field @p has_more_vlan can be used to match specific packet types,
> + * instead of using the @p inner_type field.
> + * This can be used to match any type of tagged packets.
> + * If the @p inner_type and @p has_more_vlan fields are not specified,
> + * then any tagged packets will match the pattern.

... if there no VLAN items follow?

>   */
>  struct rte_flow_item_vlan {
>  	rte_be16_t tci; /**< Tag control information. */
>  	rte_be16_t inner_type; /**< Inner EtherType or TPID. */
> +	uint32_t has_more_vlan:1;
> +	/**< Packet header contains at least one more VLAN, after this VLAN. */
> +	uint32_t reserved:31; /**< Reserved, must be zero. */
>  };
>  
>  /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
> 

Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add VLAN attributes to ETH and VLAN items

[Dekel Peled]

Thanks, PSB.

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Thursday, October 15, 2020 1:35 PM
> To: Dekel Peled <dekelp@nvidia.com>; Ori Kam <orika@nvidia.com>;
> wenzhuo.lu@intel.com; beilei.xing@intel.com;
> bernard.iremonger@intel.com; mdr@ashroe.eu; nhorman@tuxdriver.com;
> NBU-Contact-Thomas Monjalon <thomas@monjalon.net>;
> ferruh.yigit@intel.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v4 1/2] ethdev: add VLAN attributes to ETH and VLAN
> items
> 
> On 10/14/20 9:53 PM, Dekel Peled wrote:
> > This patch implements the change proposes in RFC [1], adding dedicated
> > fields to ETH and VLAN items structs, to clearly define the required
> > characteristic of a packet, and enable precise match criteria.
> > Documentation is updated accordingly.
> >
> > [1]
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail
> > s.dpdk.org%2Farchives%2Fdev%2F2020-
> August%2F177536.html&amp;data=04%7C
> >
> 01%7Cdekelp%40nvidia.com%7C8fbd9db835b1498cbddf08d870f5f866%7C430
> 83d15
> >
> 727340c1b7db39efd9ccc17a%7C0%7C0%7C637383549029697005%7CUnknow
> n%7CTWFp
> >
> bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
> I6Mn
> >
> 0%3D%7C1000&amp;sdata=LiZ9UIHl0Kz18Ck94B6daV%2BhByfMyhZJDpYz7Zn
> NDL8%3D
> > &amp;reserved=0
> >
> > Signed-off-by: Dekel Peled <dekelp@nvidia.com>
> > ---
> >  doc/guides/prog_guide/rte_flow.rst     | 14 ++++++++++++--
> >  doc/guides/rel_notes/deprecation.rst   |  5 -----
> >  doc/guides/rel_notes/release_20_11.rst |  7 +++++++
> >  lib/librte_ethdev/rte_flow.h           | 18 +++++++++++++++---
> >  4 files changed, 34 insertions(+), 10 deletions(-)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index f26a6c2..40230d3 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -908,12 +908,16 @@ order as on the wire.
> >  If the ``type`` field contains a TPID value, then only tagged packets
> > with the  specified TPID will match the pattern.
> >  Otherwise, only untagged packets will match the pattern.
> 
> Is the above sentence still valid? It looks like no if has_vlan is set in a mask
> and set in spec.

IIUC in this sentence 'Otherwise' means 'type field contains Ethertype value and not TPID value'.
I agree it may be misleading, I'll remove this sentence.

> 
> > -If the ``ETH`` item is the only item in the pattern, and the ``type``
> > field is -not specified, then both tagged and untagged packets will match
> the pattern.
> > +The field ``has_vlan`` can be used to match specific packet types,
> > +instead of using the ``type`` field.
> > +This can be used to match any type of tagged packets.
> > +If the ``type`` and ``has_vlan`` fields are not specified, then both
> > +tagged and untagged packets will match the pattern.
> 
> Consider to highlight that has_vlan field in mask controls if the field in spec is
> taken into account.
> If the field is unset in mask, it could be either tagged or untagged (if there is
> no VLAN item in spec).
> If VLAN item is present, basically it is the same as mask.has_vlan==1 and
> spec.has_vlan==1.

Maybe I'm missing your intention here, but this is the standard functionality of spec and mask, it is not specific to this field.

> 
> >
> >  - ``dst``: destination MAC.
> >  - ``src``: source MAC.
> >  - ``type``: EtherType or TPID.
> > +- ``has_vlan``: packet header contains at least one VLAN.
> >  - Default ``mask`` matches destination and source addresses only.
> >
> >  Item: ``VLAN``
> > @@ -926,9 +930,15 @@ The corresponding standard outer EtherType
> (TPID)
> > values are  preceding pattern item.
> >  If a ``VLAN`` item is present in the pattern, then only tagged
> > packets will  match the pattern.
> > +The field ``has_more_vlan`` can be used to match specific packet
> > +types, instead of using the ``inner_type field``.
> > +This can be used to match any type of tagged packets.
> > +If the ``inner_type`` and ``has_more_vlan`` fields are not specified,
> > +then any tagged packets will match the pattern.
> >
> >  - ``tci``: tag control information.
> >  - ``inner_type``: inner EtherType or TPID.
> > +- ``has_more_vlan``: packet header contains at least one more VLAN,
> after this VLAN.
> >  - Default ``mask`` matches the VID part of TCI only (lower 12 bits).
> >
> >  Item: ``IPV4``
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index 584e720..72011b0 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -154,11 +154,6 @@ Deprecation Notices
> >    as deprecated in DPDK 20.11, along with the associated macros
> ``ETH_MIRROR_*``.
> >    This API will be fully removed in DPDK 21.11.
> >
> > -* ethdev: The ``struct rte_flow_item_eth`` and ``struct
> > rte_flow_item_vlan``
> > -  structs will be modified, to include an additional value,
> > indicating existence
> > -  or absence of a VLAN header following the current header, as
> > proposed in RFC
> > -
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail
> s.dpdk.org%2Farchives%2Fdev%2F2020-
> August%2F177536.html&amp;data=04%7C01%7Cdekelp%40nvidia.com%7C8f
> bd9db835b1498cbddf08d870f5f866%7C43083d15727340c1b7db39efd9ccc17a
> %7C0%7C0%7C637383549029697005%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1
> 000&amp;sdata=LiZ9UIHl0Kz18Ck94B6daV%2BhByfMyhZJDpYz7ZnNDL8%3D&
> amp;reserved=0.
> > -
> >  * ethdev: The ``struct rte_flow_item_ipv6`` struct will be modified to
> include
> >    additional values, indicating existence or absence of IPv6 extension
> headers
> >    following the IPv6 header, as proposed in RFC diff --git
> > a/doc/guides/rel_notes/release_20_11.rst
> > b/doc/guides/rel_notes/release_20_11.rst
> > index 30db8f2..4932d82 100644
> > --- a/doc/guides/rel_notes/release_20_11.rst
> > +++ b/doc/guides/rel_notes/release_20_11.rst
> > @@ -292,6 +292,13 @@ API Changes
> >
> >  * vhost: Moved vDPA APIs from experimental to stable.
> >
> > +* ethdev: Added new field ``has_vlan`` to structure
> > +``rte_flow_item_eth``,
> > +  indicating that packet header contains at least one VLAN.
> > +
> > +* ethdev: Added new field ``has_more_vlan`` to structure
> > +  ``rte_flow_item_vlan``, indicating that packet header contains at
> > +least one
> > +  more VLAN, after this VLAN.
> > +
> >  * rawdev: Added a structure size parameter to the functions
> >    ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``,
> >    ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``, diff
> > --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index 3d5fb09..cb3bb5c 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -723,14 +723,18 @@ struct rte_flow_item_raw {
> >   * If the @p type field contains a TPID value, then only tagged packets with
> the
> >   * specified TPID will match the pattern.
> >   * Otherwise, only untagged packets will match the pattern.
> > - * If the @p ETH item is the only item in the pattern, and the @p
> > type field
> > - * is not specified, then both tagged and untagged packets will match
> > the
> > - * pattern.
> > + * The field @p has_vlan can be used to match specific packet types,
> > + instead
> > + * of using the @p type field.
> > + * This can be used to match any type of tagged packets.
> > + * If the @p type and @p has_vlan fields are not specified, then both
> > + tagged
> > + * and untagged packets will match the pattern.
> 
> .. if there is no VLAN item in the pattern

IIUC this description is for the ETH item only, w/o dependency on other items.

> 
> >   */
> >  struct rte_flow_item_eth {
> >  	struct rte_ether_addr dst; /**< Destination MAC. */
> >  	struct rte_ether_addr src; /**< Source MAC. */
> >  	rte_be16_t type; /**< EtherType or TPID. */
> > +	uint32_t has_vlan:1; /**< Packet header contains at least one VLAN.
> */
> > +	uint32_t reserved:31; /**< Reserved, must be zero. */
> 
> Have you considered to make it uint16_t to keep the size of the structure
> and have no holes?

Yes, but left it as uint32_t because:
- compiler issues a warning on 16-bit bitfield.
- need a 1 bit flag for item attribute, to align with recent change in ipv6 item, and for strict control with specific mask.
 
> 
> >  };
> >
> >  /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */ @@ -752,10 +756,18
> @@
> > struct rte_flow_item_eth {
> >   * the preceding pattern item.
> >   * If a @p VLAN item is present in the pattern, then only tagged packets
> will
> >   * match the pattern.
> > + * The field @p has_more_vlan can be used to match specific packet
> > + types,
> > + * instead of using the @p inner_type field.
> > + * This can be used to match any type of tagged packets.
> > + * If the @p inner_type and @p has_more_vlan fields are not
> > + specified,
> > + * then any tagged packets will match the pattern.
> 
> ... if there no VLAN items follow?

IIUC this description is for the VLAN item only, w/o dependency on other items.

> 
> >   */
> >  struct rte_flow_item_vlan {
> >  	rte_be16_t tci; /**< Tag control information. */
> >  	rte_be16_t inner_type; /**< Inner EtherType or TPID. */
> > +	uint32_t has_more_vlan:1;
> > +	/**< Packet header contains at least one more VLAN, after this
> VLAN. */
> > +	uint32_t reserved:31; /**< Reserved, must be zero. */
> >  };
> >
> >  /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
> >

[dpdk-dev] [PATCH v4 2/2] app/testpmd: support VLAN attributes in ETH and VLAN items

[Dekel Peled]

rte_flow update introduced has_vlan field for ETH header item,
and field has_more_vlan for VLAN header item.
The new fields are used to clearly indicate packet tagging chrasteristics.
This patch updates testpmd CLI to support the new fields.

Signed-off-by: Dekel Peled <dekelp@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 32a9214..6831e7f 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -124,12 +124,14 @@ enum index {
 	ITEM_ETH_DST,
 	ITEM_ETH_SRC,
 	ITEM_ETH_TYPE,
+	ITEM_ETH_HAS_VLAN,
 	ITEM_VLAN,
 	ITEM_VLAN_TCI,
 	ITEM_VLAN_PCP,
 	ITEM_VLAN_DEI,
 	ITEM_VLAN_VID,
 	ITEM_VLAN_INNER_TYPE,
+	ITEM_VLAN_HAS_MORE_VLAN,
 	ITEM_IPV4,
 	ITEM_IPV4_TOS,
 	ITEM_IPV4_TTL,
@@ -882,6 +884,7 @@ struct parse_action_priv {
 	ITEM_ETH_DST,
 	ITEM_ETH_SRC,
 	ITEM_ETH_TYPE,
+	ITEM_ETH_HAS_VLAN,
 	ITEM_NEXT,
 	ZERO,
 };
@@ -892,6 +895,7 @@ struct parse_action_priv {
 	ITEM_VLAN_DEI,
 	ITEM_VLAN_VID,
 	ITEM_VLAN_INNER_TYPE,
+	ITEM_VLAN_HAS_MORE_VLAN,
 	ITEM_NEXT,
 	ZERO,
 };
@@ -2099,6 +2103,13 @@ static int comp_set_sample_index(struct context *, const struct token *,
 		.next = NEXT(item_eth, NEXT_ENTRY(UNSIGNED), item_param),
 		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_eth, type)),
 	},
+	[ITEM_ETH_HAS_VLAN] = {
+		.name = "has_vlan",
+		.help = "packet header contains VLAN",
+		.next = NEXT(item_eth, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_item_eth,
+					   has_vlan, 1)),
+	},
 	[ITEM_VLAN] = {
 		.name = "vlan",
 		.help = "match 802.1Q/ad VLAN tag",
@@ -2140,6 +2151,13 @@ static int comp_set_sample_index(struct context *, const struct token *,
 		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vlan,
 					     inner_type)),
 	},
+	[ITEM_VLAN_HAS_MORE_VLAN] = {
+		.name = "has_more_vlan",
+		.help = "packet header contains another VLAN",
+		.next = NEXT(item_vlan, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_item_vlan,
+					   has_more_vlan, 1)),
+	},
 	[ITEM_IPV4] = {
 		.name = "ipv4",
 		.help = "match IPv4 header",
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v4 2/2] app/testpmd: support VLAN attributes in ETH and VLAN items

[Ori Kam]

Hi Dekel,

> -----Original Message-----
> From: Dekel Peled <dekelp@nvidia.com>
> Sent: Wednesday, October 14, 2020 9:53 PM
> Cc: dev@dpdk.org
> Subject: [PATCH v4 2/2] app/testpmd: support VLAN attributes in ETH and VLAN
> items
> 
> rte_flow update introduced has_vlan field for ETH header item,
> and field has_more_vlan for VLAN header item.
> The new fields are used to clearly indicate packet tagging chrasteristics.
> This patch updates testpmd CLI to support the new fields.
> 
> Signed-off-by: Dekel Peled <dekelp@nvidia.com>
> ---


Acked-by: Ori Kam <orika@nvidia.com>
Thanks,
Ori

[dpdk-dev] [PATCH v5 0/2] support VLAN attributes in ETH and VLAN items

[Dekel Peled]

This series implements the change proposes in RFC [1], adding dedicated
fields to ETH and VLAN items structs, to clearly define the required
characteristic of a packet, and enable precise match criteria.
testpmd CLI is updated accordingly.
Documentation is updated accordingly.

[1] https://mails.dpdk.org/archives/dev/2020-August/177536.html

---
v2: fix checkpatch comment.
v3: updates description in comments and documentation.
v4: add testpmd patch with application support.
v5: update comments and documentation for clarity.
---

Dekel Peled (2):
  ethdev: add VLAN attributes to ETH and VLAN items
  app/testpmd: support VLAN attributes in ETH and VLAN items

 app/test-pmd/cmdline_flow.c            | 18 ++++++++++++++++++
 doc/guides/prog_guide/rte_flow.rst     | 13 ++++++++++---
 doc/guides/rel_notes/deprecation.rst   |  5 -----
 doc/guides/rel_notes/release_20_11.rst |  7 +++++++
 lib/librte_ethdev/rte_flow.h           | 17 +++++++++++++----
 5 files changed, 48 insertions(+), 12 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v5 1/2] ethdev: add VLAN attributes to ETH and VLAN items

[Dekel Peled]

This patch implements the change proposes in RFC [1], adding dedicated
fields to ETH and VLAN items structs, to clearly define the required
characteristic of a packet, and enable precise match criteria.
Documentation is updated accordingly.

[1] https://mails.dpdk.org/archives/dev/2020-August/177536.html

Signed-off-by: Dekel Peled <dekelp@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst     | 13 ++++++++++---
 doc/guides/rel_notes/deprecation.rst   |  5 -----
 doc/guides/rel_notes/release_20_11.rst |  7 +++++++
 lib/librte_ethdev/rte_flow.h           | 17 +++++++++++++----
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 55497c9..b93cc77 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -907,13 +907,15 @@ EtherType/TPID provided by the subsequent pattern item. This is the same
 order as on the wire.
 If the ``type`` field contains a TPID value, then only tagged packets with the
 specified TPID will match the pattern.
-Otherwise, only untagged packets will match the pattern.
-If the ``ETH`` item is the only item in the pattern, and the ``type`` field is
-not specified, then both tagged and untagged packets will match the pattern.
+The field ``has_vlan`` can be used to match any type of tagged packets,
+instead of using the ``type`` field.
+If the ``type`` and ``has_vlan`` fields are not specified, then both tagged
+and untagged packets will match the pattern.
 
 - ``dst``: destination MAC.
 - ``src``: source MAC.
 - ``type``: EtherType or TPID.
+- ``has_vlan``: packet header contains at least one VLAN.
 - Default ``mask`` matches destination and source addresses only.
 
 Item: ``VLAN``
@@ -926,9 +928,14 @@ The corresponding standard outer EtherType (TPID) values are
 preceding pattern item.
 If a ``VLAN`` item is present in the pattern, then only tagged packets will
 match the pattern.
+The field ``has_more_vlan`` can be used to match any type of tagged packets,
+instead of using the ``inner_type field``.
+If the ``inner_type`` and ``has_more_vlan`` fields are not specified,
+then any tagged packets will match the pattern.
 
 - ``tci``: tag control information.
 - ``inner_type``: inner EtherType or TPID.
+- ``has_more_vlan``: packet header contains at least one more VLAN, after this VLAN.
 - Default ``mask`` matches the VID part of TCI only (lower 12 bits).
 
 Item: ``IPV4``
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d1f5ed3..99520cd 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -154,11 +154,6 @@ Deprecation Notices
   as deprecated in DPDK 20.11, along with the associated macros ``ETH_MIRROR_*``.
   This API will be fully removed in DPDK 21.11.
 
-* ethdev: The ``struct rte_flow_item_eth`` and ``struct rte_flow_item_vlan``
-  structs will be modified, to include an additional value, indicating existence
-  or absence of a VLAN header following the current header, as proposed in RFC
-  https://mails.dpdk.org/archives/dev/2020-August/177536.html.
-
 * security: The API ``rte_security_session_create`` takes only single mempool
   for session and session private data. So the application need to create
   mempool for twice the number of sessions needed and will also lead to
diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index f8686a5..e371d39 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -321,6 +321,13 @@ API Changes
   As the data of ``uint8_t`` will be truncated when queue number under
   a TC is greater than 256.
 
+* ethdev: Added new field ``has_vlan`` to structure ``rte_flow_item_eth``,
+  indicating that packet header contains at least one VLAN.
+
+* ethdev: Added new field ``has_more_vlan`` to structure
+  ``rte_flow_item_vlan``, indicating that packet header contains at least one
+  more VLAN, after this VLAN.
+
 * vhost: Moved vDPA APIs from experimental to stable.
 
 * rawdev: Added a structure size parameter to the functions
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index d3e8d8a..4839528 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -728,15 +728,17 @@ struct rte_flow_item_raw {
  * same order as on the wire.
  * If the @p type field contains a TPID value, then only tagged packets with the
  * specified TPID will match the pattern.
- * Otherwise, only untagged packets will match the pattern.
- * If the @p ETH item is the only item in the pattern, and the @p type field
- * is not specified, then both tagged and untagged packets will match the
- * pattern.
+ * The field @p has_vlan can be used to match any type of tagged packets,
+ * instead of using the @p type field.
+ * If the @p type and @p has_vlan fields are not specified, then both tagged
+ * and untagged packets will match the pattern.
  */
 struct rte_flow_item_eth {
 	struct rte_ether_addr dst; /**< Destination MAC. */
 	struct rte_ether_addr src; /**< Source MAC. */
 	rte_be16_t type; /**< EtherType or TPID. */
+	uint32_t has_vlan:1; /**< Packet header contains at least one VLAN. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
@@ -758,10 +760,17 @@ struct rte_flow_item_eth {
  * the preceding pattern item.
  * If a @p VLAN item is present in the pattern, then only tagged packets will
  * match the pattern.
+ * The field @p has_more_vlan can be used to match any type of tagged packets,
+ * instead of using the @p inner_type field.
+ * If the @p inner_type and @p has_more_vlan fields are not specified,
+ * then any tagged packets will match the pattern.
  */
 struct rte_flow_item_vlan {
 	rte_be16_t tci; /**< Tag control information. */
 	rte_be16_t inner_type; /**< Inner EtherType or TPID. */
+	uint32_t has_more_vlan:1;
+	/**< Packet header contains at least one more VLAN, after this VLAN. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v5 1/2] ethdev: add VLAN attributes to ETH and VLAN items

[Ori Kam]

> -----Original Message-----
> From: Dekel Peled <dekelp@nvidia.com>
> Sent: Thursday, October 15, 2020 6:52 PM
> Subject: [PATCH v5 1/2] ethdev: add VLAN attributes to ETH and VLAN items
> 
> This patch implements the change proposes in RFC [1], adding dedicated
> fields to ETH and VLAN items structs, to clearly define the required
> characteristic of a packet, and enable precise match criteria.
> Documentation is updated accordingly.
> 
> [1]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dp
> dk.org%2Farchives%2Fdev%2F2020-
> August%2F177536.html&amp;data=02%7C01%7Corika%40nvidia.com%7C4325
> 4e0b28ab40e5940208d871225b10%7C43083d15727340c1b7db39efd9ccc17a%
> 7C0%7C0%7C637383739649652879&amp;sdata=Zg9cH%2BYP%2B%2Fd0YItuAq
> 9totU3ES40i6H2ELDi74ok%2FBI%3D&amp;reserved=0
> 
> Signed-off-by: Dekel Peled <dekelp@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst     | 13 ++++++++++---
>  doc/guides/rel_notes/deprecation.rst   |  5 -----
>  doc/guides/rel_notes/release_20_11.rst |  7 +++++++
>  lib/librte_ethdev/rte_flow.h           | 17 +++++++++++++----
>  4 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 55497c9..b93cc77 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -907,13 +907,15 @@ EtherType/TPID provided by the subsequent pattern
> item. This is the same
>  order as on the wire.
>  If the ``type`` field contains a TPID value, then only tagged packets with the
>  specified TPID will match the pattern.
> -Otherwise, only untagged packets will match the pattern.
> -If the ``ETH`` item is the only item in the pattern, and the ``type`` field is
> -not specified, then both tagged and untagged packets will match the pattern.
> +The field ``has_vlan`` can be used to match any type of tagged packets,
> +instead of using the ``type`` field.
> +If the ``type`` and ``has_vlan`` fields are not specified, then both tagged
> +and untagged packets will match the pattern.
> 
>  - ``dst``: destination MAC.
>  - ``src``: source MAC.
>  - ``type``: EtherType or TPID.
> +- ``has_vlan``: packet header contains at least one VLAN.
>  - Default ``mask`` matches destination and source addresses only.
> 
>  Item: ``VLAN``
> @@ -926,9 +928,14 @@ The corresponding standard outer EtherType (TPID)
> values are
>  preceding pattern item.
>  If a ``VLAN`` item is present in the pattern, then only tagged packets will
>  match the pattern.
> +The field ``has_more_vlan`` can be used to match any type of tagged packets,
> +instead of using the ``inner_type field``.
> +If the ``inner_type`` and ``has_more_vlan`` fields are not specified,
> +then any tagged packets will match the pattern.
> 
>  - ``tci``: tag control information.
>  - ``inner_type``: inner EtherType or TPID.
> +- ``has_more_vlan``: packet header contains at least one more VLAN, after
> this VLAN.
>  - Default ``mask`` matches the VID part of TCI only (lower 12 bits).
> 
>  Item: ``IPV4``
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index d1f5ed3..99520cd 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -154,11 +154,6 @@ Deprecation Notices
>    as deprecated in DPDK 20.11, along with the associated macros
> ``ETH_MIRROR_*``.
>    This API will be fully removed in DPDK 21.11.
> 
> -* ethdev: The ``struct rte_flow_item_eth`` and ``struct rte_flow_item_vlan``
> -  structs will be modified, to include an additional value, indicating existence
> -  or absence of a VLAN header following the current header, as proposed in
> RFC
> -
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dp
> dk.org%2Farchives%2Fdev%2F2020-
> August%2F177536.html&amp;data=02%7C01%7Corika%40nvidia.com%7C4325
> 4e0b28ab40e5940208d871225b10%7C43083d15727340c1b7db39efd9ccc17a%
> 7C0%7C0%7C637383739649652879&amp;sdata=Zg9cH%2BYP%2B%2Fd0YItuAq
> 9totU3ES40i6H2ELDi74ok%2FBI%3D&amp;reserved=0.
> -
>  * security: The API ``rte_security_session_create`` takes only single mempool
>    for session and session private data. So the application need to create
>    mempool for twice the number of sessions needed and will also lead to
> diff --git a/doc/guides/rel_notes/release_20_11.rst
> b/doc/guides/rel_notes/release_20_11.rst
> index f8686a5..e371d39 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -321,6 +321,13 @@ API Changes
>    As the data of ``uint8_t`` will be truncated when queue number under
>    a TC is greater than 256.
> 
> +* ethdev: Added new field ``has_vlan`` to structure ``rte_flow_item_eth``,
> +  indicating that packet header contains at least one VLAN.
> +
> +* ethdev: Added new field ``has_more_vlan`` to structure
> +  ``rte_flow_item_vlan``, indicating that packet header contains at least one
> +  more VLAN, after this VLAN.
> +
>  * vhost: Moved vDPA APIs from experimental to stable.
> 
>  * rawdev: Added a structure size parameter to the functions
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index d3e8d8a..4839528 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -728,15 +728,17 @@ struct rte_flow_item_raw {
>   * same order as on the wire.
>   * If the @p type field contains a TPID value, then only tagged packets with the
>   * specified TPID will match the pattern.
> - * Otherwise, only untagged packets will match the pattern.
> - * If the @p ETH item is the only item in the pattern, and the @p type field
> - * is not specified, then both tagged and untagged packets will match the
> - * pattern.
> + * The field @p has_vlan can be used to match any type of tagged packets,
> + * instead of using the @p type field.
> + * If the @p type and @p has_vlan fields are not specified, then both tagged
> + * and untagged packets will match the pattern.
>   */
>  struct rte_flow_item_eth {
>  	struct rte_ether_addr dst; /**< Destination MAC. */
>  	struct rte_ether_addr src; /**< Source MAC. */
>  	rte_be16_t type; /**< EtherType or TPID. */
> +	uint32_t has_vlan:1; /**< Packet header contains at least one VLAN. */
> +	uint32_t reserved:31; /**< Reserved, must be zero. */
>  };
> 
>  /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
> @@ -758,10 +760,17 @@ struct rte_flow_item_eth {
>   * the preceding pattern item.
>   * If a @p VLAN item is present in the pattern, then only tagged packets will
>   * match the pattern.
> + * The field @p has_more_vlan can be used to match any type of tagged
> packets,
> + * instead of using the @p inner_type field.
> + * If the @p inner_type and @p has_more_vlan fields are not specified,
> + * then any tagged packets will match the pattern.
>   */
>  struct rte_flow_item_vlan {
>  	rte_be16_t tci; /**< Tag control information. */
>  	rte_be16_t inner_type; /**< Inner EtherType or TPID. */
> +	uint32_t has_more_vlan:1;
> +	/**< Packet header contains at least one more VLAN, after this VLAN.
> */
> +	uint32_t reserved:31; /**< Reserved, must be zero. */
>  };
> 
>  /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
> --
> 1.8.3.1

Acked-by: Ori Kam <orika@nvidia.com>
Thanks,
Ori

[dpdk-dev] [PATCH v5 2/2] app/testpmd: support VLAN attributes in ETH and VLAN items

[Dekel Peled]

rte_flow update introduced has_vlan field for ETH header item,
and field has_more_vlan for VLAN header item.
The new fields are used to clearly indicate packet tagging chrasteristics.
This patch updates testpmd CLI to support the new fields.

Signed-off-by: Dekel Peled <dekelp@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 84bba0f..00c70a1 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -141,12 +141,14 @@ enum index {
 	ITEM_ETH_DST,
 	ITEM_ETH_SRC,
 	ITEM_ETH_TYPE,
+	ITEM_ETH_HAS_VLAN,
 	ITEM_VLAN,
 	ITEM_VLAN_TCI,
 	ITEM_VLAN_PCP,
 	ITEM_VLAN_DEI,
 	ITEM_VLAN_VID,
 	ITEM_VLAN_INNER_TYPE,
+	ITEM_VLAN_HAS_MORE_VLAN,
 	ITEM_IPV4,
 	ITEM_IPV4_TOS,
 	ITEM_IPV4_FRAGMENT_OFFSET,
@@ -936,6 +938,7 @@ struct parse_action_priv {
 	ITEM_ETH_DST,
 	ITEM_ETH_SRC,
 	ITEM_ETH_TYPE,
+	ITEM_ETH_HAS_VLAN,
 	ITEM_NEXT,
 	ZERO,
 };
@@ -946,6 +949,7 @@ struct parse_action_priv {
 	ITEM_VLAN_DEI,
 	ITEM_VLAN_VID,
 	ITEM_VLAN_INNER_TYPE,
+	ITEM_VLAN_HAS_MORE_VLAN,
 	ITEM_NEXT,
 	ZERO,
 };
@@ -2217,6 +2221,13 @@ static int comp_set_sample_index(struct context *, const struct token *,
 		.next = NEXT(item_eth, NEXT_ENTRY(UNSIGNED), item_param),
 		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_eth, type)),
 	},
+	[ITEM_ETH_HAS_VLAN] = {
+		.name = "has_vlan",
+		.help = "packet header contains VLAN",
+		.next = NEXT(item_eth, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_item_eth,
+					   has_vlan, 1)),
+	},
 	[ITEM_VLAN] = {
 		.name = "vlan",
 		.help = "match 802.1Q/ad VLAN tag",
@@ -2258,6 +2269,13 @@ static int comp_set_sample_index(struct context *, const struct token *,
 		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vlan,
 					     inner_type)),
 	},
+	[ITEM_VLAN_HAS_MORE_VLAN] = {
+		.name = "has_more_vlan",
+		.help = "packet header contains another VLAN",
+		.next = NEXT(item_vlan, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_item_vlan,
+					   has_more_vlan, 1)),
+	},
 	[ITEM_IPV4] = {
 		.name = "ipv4",
 		.help = "match IPv4 header",
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v5 0/2] support VLAN attributes in ETH and VLAN items

[Ferruh Yigit]

On 10/15/2020 4:51 PM, Dekel Peled wrote:
> This series implements the change proposes in RFC [1], adding dedicated
> fields to ETH and VLAN items structs, to clearly define the required
> characteristic of a packet, and enable precise match criteria.
> testpmd CLI is updated accordingly.
> Documentation is updated accordingly.
> 
> [1] https://mails.dpdk.org/archives/dev/2020-August/177536.html
> 
> ---
> v2: fix checkpatch comment.
> v3: updates description in comments and documentation.
> v4: add testpmd patch with application support.
> v5: update comments and documentation for clarity.
> ---
> 
> Dekel Peled (2):
>    ethdev: add VLAN attributes to ETH and VLAN items
>    app/testpmd: support VLAN attributes in ETH and VLAN items
> 

Series applied to dpdk-next-net/main, thanks.