On Mon, Jul 14, 2025 at 08:06:54AM -0700, Stephen Hemminger wrote:
> On Mon, 14 Jul 2025 14:30:14 +0100
> Bruce Richardson <bruce.richard...@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.richard...@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;
Ok, that seems reasonable enough to enforce.
/Bruce
