DPDK patches and discussions
 help / color / mirror / Atom feed
From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
To: Dekel Peled <dekelp@nvidia.com>,
	orika@nvidia.com, wenzhuo.lu@intel.com, beilei.xing@intel.com,
	bernard.iremonger@intel.com, mdr@ashroe.eu,
	nhorman@tuxdriver.com, thomas@monjalon.net,
	ferruh.yigit@intel.com
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add VLAN attributes to ETH and VLAN items
Date: Thu, 15 Oct 2020 13:34:39 +0300
Message-ID: <35851923-ae1f-bc27-5e16-5d736f98318b@oktetlabs.ru> (raw)
In-Reply-To: <6363b9417eea68914d23bbd636567977e5b0eda8.1602700693.git.dekelp@nvidia.com>

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. */
> 


  parent reply	other threads:[~2020-10-15 10:34 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-01 18:49 [dpdk-dev] [PATCH] " Dekel Peled
2020-10-01 19:01 ` [dpdk-dev] [PATCH v2] " Dekel Peled
2020-10-07 18:21   ` [dpdk-dev] [PATCH v3] " Dekel Peled
2020-10-02 12:39 ` [dpdk-dev] [PATCH] " Maxime Leroy
2020-10-02 14:40   ` Thomas Monjalon
2020-10-05  9:37   ` Dekel Peled
2020-10-07 11:52     ` Maxime Leroy
2020-10-07 12:13       ` Ori Kam
2020-10-07 14:01         ` Dekel Peled
2020-10-14 18:53 ` [dpdk-dev] [PATCH v4 0/2] support VLAN attributes in " Dekel Peled
2020-10-14 18:53   ` [dpdk-dev] [PATCH v4 1/2] ethdev: add VLAN attributes to " Dekel Peled
2020-10-14 20:13     ` Thomas Monjalon
2020-10-15 10:34     ` Andrew Rybchenko [this message]
2020-10-15 15:06       ` Dekel Peled
2020-10-14 18:53   ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: support VLAN attributes in " Dekel Peled
2020-10-15  6:09     ` Ori Kam
2020-10-15 15:51 ` [dpdk-dev] [PATCH v5 0/2] " Dekel Peled
2020-10-15 15:51   ` [dpdk-dev] [PATCH v5 1/2] ethdev: add VLAN attributes to " Dekel Peled
2020-10-15 16:11     ` Ori Kam
2020-10-15 15:51   ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: support VLAN attributes in " Dekel Peled
2020-10-15 23:18   ` [dpdk-dev] [PATCH v5 0/2] " Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=35851923-ae1f-bc27-5e16-5d736f98318b@oktetlabs.ru \
    --to=andrew.rybchenko@oktetlabs.ru \
    --cc=beilei.xing@intel.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dekelp@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=mdr@ashroe.eu \
    --cc=nhorman@tuxdriver.com \
    --cc=orika@nvidia.com \
    --cc=thomas@monjalon.net \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git