Re: [PATCH net v6 3/5] net: stmmac: Rename double VLAN references to svlan

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The self-test renamed to "SVLAN Filtering (perf)"
  (stmmac_test_svlanfilt_perfect), and likewise…

--

net: stmmac: Rename double VLAN references to svlan

This patch renames the "double VLAN" identifiers in the stmmac driver to
"svlan". It covers the VLAN hash callbacks, the num_double_vlans counter
and the selftests. The ESVL and DOVLTC bits control S-VLAN tag
processing, not the inner-tag double VLAN feature. No functional change
is intended.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> index c25dc9f89270..c485217ba880 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c

[ ... ]

> -static int stmmac_test_dvlanfilt_perfect(struct stmmac_priv *priv)
> +static int stmmac_test_svlanfilt_perfect(struct stmmac_priv *priv)
>  {
>  	int ret, prev_cap = priv->dma_cap.vlhash;
>  
> @@ -1171,7 +1171,7 @@ static int stmmac_test_dvlanfilt_perfect(struct stmmac_priv *priv)
>  		return -EOPNOTSUPP;
>  
>  	priv->dma_cap.vlhash = 0;
> -	ret = __stmmac_test_dvlanfilt(priv);
> +	ret = __stmmac_test_svlanfilt(priv);
>  	priv->dma_cap.vlhash = prev_cap;

[Severity: Low]
Does the "SVLAN Filtering (perf)" test still exercise perfect VLAN
filtering?

This test forces perfect-match mode by clearing priv->dma_cap.vlhash
before it calls __stmmac_test_svlanfilt(). An earlier patch in this
series, "net: stmmac: Remove VLAN perfect matching dead code", removed
the fallback in stmmac_vlan_update() that used to read vlhash:

    if (!priv->dma_cap.vlhash) {
        pmatch = vid;
        hash = 0;
    }

At this commit, the only reads of dma_cap.vlhash outside the selftests
are the probe-time feature check in __stmmac_dvr_probe() and the
debugfs dump. The filtering path never reads it:

__stmmac_test_svlanfilt()
  vlan_vid_add()
    stmmac_vlan_rx_add_vid()
      stmmac_vlan_update()
        stmmac_update_vlan_hash(priv, priv->hw, hash, is_svlan);

stmmac_vlan_update() always computes a non-zero hash for the active VID.
vlan_update_hash() and dwxgmac2_update_vlan_hash() then do:

    value |= VLAN_VTHM | VLAN_ETV;

so hash matching stays enabled throughout the "(perf)" run.

That seems to make stmmac_test_svlanfilt_perfect() a repeat of
stmmac_test_svlanfilt(). It could pass even if perfect filtering via
stmmac_add_hw_vlan_rx_fltr() were broken. The same applies to
stmmac_test_vlanfilt_perfect().

This patch renames the test but keeps it in this state, and the
remaining patches in the series do not touch the selftests. Should
these two "(perf)" tests be reworked or dropped?

>  
>  	return ret;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928203441.34876-1-ovidiu.panait.rb%40renesas.com