mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nicolai Buchwitz <nb@tipi-net.de>
To: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, shuah@kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH net-next 2/2] selftests: drv-net: Add VLAN test
Date: Tue, 15 Sep 2026 10:16:06 +0200	[thread overview]
Message-ID: <62229b50d8cafb952fa234d46bc78782@tipi-net.de> (raw)
In-Reply-To: <20260913143926.77366-3-ovidiu.panait.rb@renesas.com>

Hi Ovidiu

On 13.9.2026 16:39, 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.
> 
> Three VLAN configurations are covered, each with hardware RX VLAN
> stripping enabled and disabled (via the rx-vlan-offload and
> rx-vlan-stag-hw-parse features):
> - a single 802.1q VLAN interface
> - a single 802.1ad VLAN interface
> - an 802.1q VLAN stacked on top of an 802.1ad interface
> 
> The "hw" test variants enable the RX VLAN stripping features supported
> by the device (rx-vlan-offload and rx-vlan-stag-hw-parse), the "sw"
> test variants disable all of them. A test is xfailed if the requested
> configuration is not possible.
> 
> VLAN insertion offloads are not tested for now.
> 
> NETIF=end0 LOCAL_V4=172.16.0.2 REMOTE_V4=172.16.0.3 \
> REMOTE_TYPE=ssh REMOTE_ARGS=root@172.16.0.3 \
> run_kselftest.sh -t drivers/net/hw:vlan.py
>  TAP version 13
>  1..1
>  # timeout set to 0
>  # selftests: drivers/net/hw: vlan.py
>  # # Interface: end0, driver: st_gmac
>  # TAP version 13
>  # 1..6
>  # ok 1 vlan.test.8021q_hw
>  # ok 2 vlan.test.8021q_sw
>  # ok 3 vlan.test.8021ad_hw
>  # ok 4 vlan.test.8021ad_sw
>  # ok 5 vlan.test.qinq_hw
>  # ok 6 vlan.test.qinq_sw
>  # # Totals: pass:6 fail:0 xfail:0 xpass:0 skip:0 error:0
>  ok 1 selftests: drivers/net/hw: vlan.py
>  # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
> 
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>

> [...]

> +def _setup(cfg, outer_proto, inner_proto, hw_strip):
> +    """Configure VLAN stripping and create the VLAN interfaces."""
> +
> +    names = ["rx-vlan-offload"]
> +    if outer_proto == "802.1ad":
> +        names.append("rx-vlan-stag-hw-parse")
> +
> +    feat = ethtool(f"-k {cfg.ifname}", json=True)[0]
> +    feats = {}
> +    for name in names:
> +        # hw: skip the features the device does not support
> +        # sw: request all of them off
> +        if hw_strip and feat[name]["fixed"] and not 
> feat[name]["active"]:
> +            continue
> +        feats[name] = hw_strip
> +
> +    if hw_strip and not feats:
> +        raise KsftXfailEx("Device does not support RX VLAN stripping")
> +
> +    set_ethtool_feat(cfg.ifname, feat, feats)

If a driver refuses to set the feature, set_ethtool_feat() would not 
fail
and the test would be false positive.

I did a quick search and at least intel iavf could run into this:

    drivers/net/ethernet/intel/iavf/iavf_main.c:iavf_fix_strip_features() 
{
         [...]
         if ((netdev->features & NETIF_F_RXFCS) && is_vlan_strip) {
                 requested_features &= ~vlan_strip;
         [...]
    }

IMHO the test should check if the feature actually got enabled. This
could be wrapped in set_ethtool_feat() or be checked afterwards. I would
lean towards the first one.

> [...]

Thanks,
Nicolai

  reply	other threads:[~2026-09-15  8:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-13 14:39 [PATCH net-next 0/2] " Ovidiu Panait
2026-09-13 14:39 ` [PATCH net-next 1/2] selftests: drv-net: Move _set_ethtool_feat() into lib Ovidiu Panait
2026-09-15  7:57   ` Nicolai Buchwitz
2026-09-13 14:39 ` [PATCH net-next 2/2] selftests: drv-net: Add VLAN test Ovidiu Panait
2026-09-15  8:16   ` Nicolai Buchwitz [this message]
2026-09-17  2:26 ` [PATCH net-next 0/2] " Jakub Kicinski

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=62229b50d8cafb952fa234d46bc78782@tipi-net.de \
    --to=nb@tipi-net.de \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=ovidiu.panait.rb@renesas.com \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.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

all inboxes | Powered by JetHome®