Re: [PATCH net v3 5/5] selftests: drv-net: Add VLAN test

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 25 Aug 2026 16:45:22 +0000 Ovidiu Panait wrote:
> Add a test that validates ping traffic over VLAN interfaces. It aims
> to catch drivers which mishandle hardware VLAN tag stripping, in
> particular QinQ.

I take it back, sorry, looks like AI found something to complain about:

> +def _setup(cfg, outer_proto, inner_proto, hw_strip):
> +    """Configure VLAN stripping and create the VLAN interfaces."""
> +
> +    feat = ethtool(f"-k {cfg.ifname}", json=True)[0]
> +    set_ethtool_feat(cfg.ifname, feat, {"rx-vlan-offload": hw_strip})

Does toggling rx-vlan-offload here actually change anything for the
802.1ad and QinQ variants?
In net/ethtool/common.c, netdev_features_strings maps the two RX VLAN
parse features to distinct bits:
	[NETIF_F_HW_VLAN_CTAG_RX_BIT] =  "rx-vlan-hw-parse",
	...
	[NETIF_F_HW_VLAN_STAG_RX_BIT] =  "rx-vlan-stag-hw-parse",
rx-vlan-offload is the alias for rx-vlan-hw-parse, i.e.
NETIF_F_HW_VLAN_CTAG_RX (TPID 0x8100). Stripping of an 802.1ad S-tag
(TPID 0x88a8) is controlled by NETIF_F_HW_VLAN_STAG_RX, which this test
never reads or sets.
For the st_gmac/stmmac driver used in the commit message log,
stmmac_rx_vlan() gates each TPID on its own feature bit:
	if ((vlan_proto == htons(ETH_P_8021Q) &&
	     dev->features & NETIF_F_HW_VLAN_CTAG_RX) ||
	    (vlan_proto == htons(ETH_P_8021AD) &&
	     dev->features & NETIF_F_HW_VLAN_STAG_RX)) {
and STAG_RX is only advertised for DWMAC_CORE_XGMAC, so on other cores
the outer-tag stripping state is unaffected by the flag the test flips.
Also, set_ethtool_feat() in tools/testing/selftests/drivers/net/lib/py/feat.py
only programs the feature names the caller passes, and _setup() applies
them to cfg.ifname only, never to cfg.remote. The TX counterparts
(tx-vlan-hw-insert / tx-vlan-stag-hw-insert) are left untouched as well.
Would it make sense to include the corresponding STAG feature in the dict
passed to set_ethtool_feat() for the 802.1ad and QinQ variants (xfail or
skip when the bit is fixed), or otherwise document the limitation?

> +
> +    _vlan_setup(cfg.ifname, LOCAL_IP, outer_proto, inner_proto)
> +    _vlan_setup(cfg.remote_ifname, REMOTE_IP, outer_proto, inner_proto,
> +                host=cfg.remote)
> +
> +
> +def _vlan_variants():
> +    """Generator that yields the VLAN protocols and the stripping mode."""
> +
> +    yield KsftNamedVariant("8021q_hw", "802.1q", None, True)
> +    yield KsftNamedVariant("8021q_sw", "802.1q", None, False)
> +    yield KsftNamedVariant("8021ad_hw", "802.1ad", None, True)
> +    yield KsftNamedVariant("8021ad_sw", "802.1ad", None, False)
> +    yield KsftNamedVariant("qinq_hw", "802.1ad", "802.1q", True)
> +    yield KsftNamedVariant("qinq_sw", "802.1ad", "802.1q", False)

Following on from the above: do the 8021ad_hw/8021ad_sw and
qinq_hw/qinq_sw pairs end up running the same outer-tag configuration
twice on devices that implement S-tag stripping separately from C-tag
stripping?
If so, the commit message statement
  "Three VLAN configurations are covered, each with hardware VLAN
   stripping enabled and disabled"
and the module docstring entries for 8021ad_hw/8021ad_sw and
qinq_hw/qinq_sw would describe coverage the test does not establish, and
the QinQ mis-stripping case the test aims to catch would pass in both
variants.