DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: dev@dpdk.org, techboard@dpdk.org
Subject: Re: [RFC PATCH] doc: clarify VLAN and QinQ stripping behaviour
Date: Mon, 14 Jul 2025 08:06:54 -0700	[thread overview]
Message-ID: <20250714080654.3c1feeea@hermes.local> (raw)
In-Reply-To: <20250714133014.44597-1-bruce.richardson@intel.com>

On Mon, 14 Jul 2025 14:30:14 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> The behaviour of VLAN tag stripping Rx offloads is unclear in DPDK, and
> not very well documented. Even the documentation that does exist appears
> contradictory.
> 
> For example, the doxygen docs for the mbuf flag
> RTE_MBUF_F_RX_QINQ_STRIPPED says:
> 
>   "If RTE_MBUF_F_RX_QINQ_STRIPPED is set and RTE_MBUF_F_RX_VLAN_STRIPPED
>   is unset, only the outer VLAN is removed from packet data,..."
> 
> but the docs for RTE_MBUF_F_RX_QINQ says:
> 
>   "If the flag RTE_MBUF_F_RX_QINQ_STRIPPED is also present, both VLANs
>   headers have been stripped from mbuf data, ..."
> 
> Without a good definition of what the correct behaviour is, it's not
> possible to assess and ensure conformance across drivers. Update the
> documentation for NIC features, ethdev and mbuf library to all report
> the same information: that VLAN strip feature is stripping one flag, and
> QinQ strip feature is removing two.
> 
> Summary of VLAN and QinQ stripping behaviour as reported in docs after
> this patch:
> 
> +-------------------+----------------------+----------------------------+
> | Input Traffic     | VLAN-strip on        | QinQ strip on              |
> +===================+======================+============================+
> | Single VLAN pkts  | Tag in vlan_tci      | Tag in vlan_tci            |
> +-------------------+----------------------+----------------------------+
> | Double VLAN pkts  | Outer tag in vlan_tci| Outer tag in vlan_tci_outer|
> |                   |                      | Inner tag in vlan_tci      |
> +-------------------+----------------------+----------------------------+
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  doc/guides/nics/features.rst | 21 +++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h      | 20 ++++++++++++++++++++
>  lib/mbuf/rte_mbuf_core.h     | 36 ++++++++++++++++++++----------------
>  3 files changed, 61 insertions(+), 16 deletions(-)
> 
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index a075c057ec..c57ea8e08f 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -483,6 +483,11 @@ VLAN offload
>  ------------
>  
>  Supports VLAN offload to hardware.
> +This includes both VLAN stripping on Rx and VLAN insertion on Tx.
> +
> +On Rx VLAN strip always strips one VLAN tag if available.
> +If multiple VLAN tags are present, it strips the outer tag.
> +The stripped VLAN TCI is saved in mbuf->vlan_tci.
>  
>  * **[uses]       rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_VLAN_STRIP,RTE_ETH_RX_OFFLOAD_VLAN_FILTER,RTE_ETH_RX_OFFLOAD_VLAN_EXTEND``.
>  * **[uses]       rte_eth_txconf,rte_eth_txmode**: ``offloads:RTE_ETH_TX_OFFLOAD_VLAN_INSERT``.
> @@ -501,6 +506,22 @@ QinQ offload
>  ------------
>  
>  Supports QinQ (queue in queue) offload.
> +This includes both QinQ stripping on Rx and QinQ insertion on Tx.
> +
> +On Rx, QinQ strip strips two VLAN tags if present.
> +If only one tag is present, it behaves as VLAN strip.
> +Specifying both VLAN strip and QinQ strip is equivalent to QinQ strip alone.
> +
> +Summary of VLAN and QinQ stripping behavior:
> +
> ++----------------------+----------------------+------------------------------+
> +| Input Traffic        | VLAN-strip on        | QinQ strip on                |
> ++======================+======================+==============================+
> +| Single VLAN packets  | Tag in vlan_tci      | Tag in vlan_tci              |
> ++----------------------+----------------------+------------------------------+
> +| Double VLAN packets  | Outer tag in vlan_tci| Outer tag in vlan_tci_outer  |
> +|                      |                      | Inner tag in vlan_tci        |
> ++----------------------+----------------------+------------------------------+
>  
>  * **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_QINQ_STRIP``.
>  * **[uses]     rte_eth_txconf,rte_eth_txmode**: ``offloads:RTE_ETH_TX_OFFLOAD_QINQ_INSERT``.
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index f9fb6ae549..ccf0cf5e30 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1552,11 +1552,31 @@ struct rte_eth_conf {
>  /**
>   * Rx offload capabilities of a device.
>   */
> +/**
> + * VLAN strip offload.
> + *
> + * When enabled, strips one VLAN tag if available.
> + * If multiple VLAN tags are present, it strips the outer tag.
> + * The stripped VLAN TCI is saved in mbuf->vlan_tci and RTE_MBUF_F_RX_VLAN_STRIPPED flag is set.
> + */
>  #define RTE_ETH_RX_OFFLOAD_VLAN_STRIP       RTE_BIT64(0)
>  #define RTE_ETH_RX_OFFLOAD_IPV4_CKSUM       RTE_BIT64(1)
>  #define RTE_ETH_RX_OFFLOAD_UDP_CKSUM        RTE_BIT64(2)
>  #define RTE_ETH_RX_OFFLOAD_TCP_CKSUM        RTE_BIT64(3)
>  #define RTE_ETH_RX_OFFLOAD_TCP_LRO          RTE_BIT64(4)
> +/**
> + * QinQ strip offload.
> + *
> + * When enabled, strips two VLAN tags if present.
> + * If only one tag is present, it behaves as VLAN strip.
> + * The stripped VLAN TCIs are saved in mbuf fields and appropriate RTE_MBUF_F_RX_* flags are set.
> + *
> + * For single VLAN packets: Tag is saved in mbuf->vlan_tci (same as VLAN strip)
> + * For double VLAN packets: Outer tag is saved in mbuf->vlan_tci_outer,
> + *                          Inner tag is saved in mbuf->vlan_tci
> + *
> + * Note: Specifying both VLAN strip and QinQ strip is equivalent to QinQ strip alone.
> + */
>  #define RTE_ETH_RX_OFFLOAD_QINQ_STRIP       RTE_BIT64(5)
>  #define RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM RTE_BIT64(6)
>  #define RTE_ETH_RX_OFFLOAD_MACSEC_STRIP     RTE_BIT64(7)
> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> index a0df265b5d..23aed8ec69 100644
> --- a/lib/mbuf/rte_mbuf_core.h
> +++ b/lib/mbuf/rte_mbuf_core.h
> @@ -63,10 +63,17 @@ extern "C" {
>  #define RTE_MBUF_F_RX_OUTER_IP_CKSUM_BAD (1ULL << 5)
>  
>  /**
> - * A vlan has been stripped by the hardware and its tci is saved in
> - * mbuf->vlan_tci. This can only happen if vlan stripping is enabled
> - * in the RX configuration of the PMD.
> - * When RTE_MBUF_F_RX_VLAN_STRIPPED is set, RTE_MBUF_F_RX_VLAN must also be set.
> + * A vlan has been stripped by the hardware and its tci is saved in mbuf->vlan_tci.
> + * This can only happen if vlan or QinQ stripping is enabled in the RX configuration of the PMD.
> + *
> + * NOTE:
> + * - If VLAN stripping is enabled, but not QinQ, the tag stripped will be the outer
> + *   VLAN tag of a QinQ packet.
> + * - If QinQ stripping is enabled, then the outer VLAN tag is stripped and saved in
> + *   mbuf->vlan_tci_outer (indicated by the presence of flag @ref RTE_MBUF_F_RX_QINQ_STRIPPED),
> + *   while the inner VLAN tag is stripped and saved in mbuf->vlan_tci.
> + *
> + * When @ref RTE_MBUF_F_RX_VLAN_STRIPPED is set, @ref RTE_MBUF_F_RX_VLAN must also be set.
>   */
>  #define RTE_MBUF_F_RX_VLAN_STRIPPED (1ULL << 6)
>  
> @@ -113,19 +120,16 @@ extern "C" {
>  #define RTE_MBUF_F_RX_FDIR_FLX      (1ULL << 14)
>  
>  /**
> - * The outer VLAN has been stripped by the hardware and its TCI is
> - * saved in mbuf->vlan_tci_outer.
> - * This can only happen if VLAN stripping is enabled in the Rx
> - * configuration of the PMD.
> - * When RTE_MBUF_F_RX_QINQ_STRIPPED is set, the flags RTE_MBUF_F_RX_VLAN
> - * and RTE_MBUF_F_RX_QINQ must also be set.
> + * Two VLANs have been stripped from the packet by hardware and are
> + * reported in the vlan_tci and vlan_tci_outer fields.
> + *
> + * When this flag is set:
> + * - The outer VLAN has been stripped by the hardware and it is saved in mbuf->vlan_tci_outer.
> + * - The inner VLAN has also been stripped by the hardware and it is saved in mbuf->vlan_tci.
>   *
> - * - If both RTE_MBUF_F_RX_QINQ_STRIPPED and RTE_MBUF_F_RX_VLAN_STRIPPED are
> - *   set, the 2 VLANs have been stripped by the hardware and their TCIs are
> - *   saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
> - * - If RTE_MBUF_F_RX_QINQ_STRIPPED is set and RTE_MBUF_F_RX_VLAN_STRIPPED
> - *   is unset, only the outer VLAN is removed from packet data, but both tci
> - *   are saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
> + * When @ref RTE_MBUF_F_RX_QINQ_STRIPPED is set, the flags @ref RTE_MBUF_F_RX_VLAN,
> + * @ref RTE_MBUF_F_RX_VLAN_STRIPPED,
> + * and @ref RTE_MBUF_F_RX_QINQ must also be set.
>   */
>  #define RTE_MBUF_F_RX_QINQ_STRIPPED (1ULL << 15)
>  

Since QINQ strip without VLAN strip would be undefined, it should be blocked
at ethdev layer. Something like:

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index dd7c00bc94..0d51c7cf82 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4542,6 +4542,18 @@ rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask)
 	if (mask == 0)
 		return ret;
 
+	/*
+	 * QinQ offloading dtoes no make sense without also stripping
+	 * outer tag.
+	 */
+	if ((dev_offloads & RTE_ETH_QINQ_STRIP_OFFLOAD) &&
+	    !(dev_offloads & RTE_ETH_VLAN_STRIP_OFFLOAD)) {
+		RTE_ETHDEV_LOG_LINE(ERR,
+			    "Ethdev port_id=%u requested QinQ strip without VLAN strip",
+			    port_id);
+		return -EINVAL;
+	}
+
 	ret = rte_eth_dev_info_get(port_id, &dev_info);
 	if (ret != 0)
 		return ret;

  reply	other threads:[~2025-07-14 15:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-14 13:30 Bruce Richardson
2025-07-14 15:06 ` Stephen Hemminger [this message]
2025-07-14 15:11   ` Bruce Richardson
2025-07-14 16:33 ` Morten Brørup
2025-07-14 16:49   ` Bruce Richardson
2025-07-14 20:09 ` Dean Marx
2025-07-14 21:41   ` Patrick Robb

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=20250714080654.3c1feeea@hermes.local \
    --to=stephen@networkplumber.org \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=techboard@dpdk.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).