RE: [PATCH net 1/5] net: stmmac: Remove VLAN perfect matching dead code

[Ovidiu Panait]

Hi Maxime,

> 
> Hi Ovidiu,
> 
> On 7/29/26 11:51, Ovidiu Panait wrote:
> > stmmac_vlan_update() falls back to "perfect matching" when the VLAN hash
> > filter is unavailable (!priv->dma_cap.vlhash). This fallback has been
> > unreachable in normal operation since its introduction in
> > commit c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if
> > HASH is not available") because the NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER
> > features are advertised only when priv->dma_cap.vlhash is true.
> >
> > The fallback is also duplicating the code in vlan_add_hw_rx_fltr(),
> which
> > is always available since stmmac_get_num_vlan() returns at least 1.
> 
> It's not exactly dead code, there's an stmmac selftest for that, see :
> 
> https://elixir.bootlin.com/linux/v7.2-
> rc4/source/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c#L973
> 
> It does exactly what you say, it fakes the fact that we don't support
> vlhash,
> then exercise the perfect matching path.
> 
> But is it correct to drop that feature altogether ? Or should we just
> relax the conditions for NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER to be
> advertised ?
> 

We are not dropping the perfect matching feature by removing this code.
Basically, there are two perfect matching code paths that duplicate the
same logic and which diverged over time:

- vlan_update_hash() has the "if (perfect_match)" path that has never run on real
  hardware (unless running the selftest) because NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER
  is only advertised if the core supports VLAN hash filtering.

- vlan_add_hw_rx_fltr() also has a perfect matching codepath, which calls into
  vlan_write_single(). This runs unconditionally and overwrites what
  vlan_update_hash() did anyway, as it runs after stmmac_vlan_update():

stmmac_vlan_rx_add_vid():
...
	ret = stmmac_vlan_update(priv, num_double_vlans);
	if (ret) {
		clear_bit(vid, priv->active_vlans);
		goto err_pm_put;
	}

	if (priv->hw->num_vlan) {
		ret = stmmac_add_hw_vlan_rx_fltr(priv, ndev, priv->hw, proto, vid);
		if (ret) {
			clear_bit(vid, priv->active_vlans);
			stmmac_vlan_update(priv, priv->num_double_vlans);
			goto err_pm_put;
		}
	}

This path is also more correct because it takes into account cores that support
extended VLAN filters.

Therefore, I think removing the perfect matching code in vlan_update_hash() is
the right choice here.

Ovidiu

> Maxime