Re: [PATCH v2] net: stmmac: skip VLAN restore when VLAN hash ops are missing

[Michal Piekos]

On Thu, Mar 26, 2026 at 08:07:33AM +0000, Ovidiu Panait wrote:
> Hi,
> 
> > 
> > Hi,
> > 
> > On 21/03/2026 06:38, Michal Piekos wrote:
> > > stmmac_vlan_restore() unconditionally calls stmmac_vlan_update() when
> > > NETIF_F_VLAN_FEATURES is set. On platforms where priv->hw->vlan (or
> > > ->update_vlan_hash) is not provided, stmmac_update_vlan_hash() returns
> > > -EINVAL via stmmac_do_void_callback(), resulting in a spurious
> > > "Failed to restore VLANs" error even when no VLAN filtering is in use.
> > >
> > > Check presence of VLAN HW FILTER flags before stmmac_vlan_update().
> > >
> > > Tested on Orange Pi Zero 3.
> > >
> > > Fixes: bd7ad51253a7 ("net: stmmac: Fix VLAN HW state restore")
> > > Signed-off-by: Michal Piekos <michal.piekos@xxxxxxxxxxxxx>
> > > ---
> > > This patch fixes a noisy "Failed to restore VLANs" message on platforms
> > > where stmmac VLAN hash ops are not implemented.
> > > stmmac_vlan_restore() calls stmmac_vlan_update() without checking for
> > > VLAN hash ops presence which results in -EINVAL.
> > 
> > I've been seeing the same message on socfpga. My two cents on that is
> > that this error messages doesn't bring anything to the table anyways.
> > 
> > As Russell explains, it's either triggered when the vlan op isn't
> > implemented (the stmmac callback macro stuff turns that into a -EINVAL),
> > or when some capabilities arent present. All in all, it's always stuff
> > that users can't really do anything about, as it's HW limitations, I
> > think we can simply discard this message.
> > 
> > Also, nothing actually checks what stmmac_vlan_restore() returns, so we
> > might as well return void ?
> > 
> 
> I think this is the best solution until the VLAN capabilities handling is
> cleaned up.
> 
> Michal, please let me know if you will be handling this in v3 or I should
> send a fix for it.
> 

I will provide the fix for it latest during the weekend.

Michal

> Thanks,
> Ovidiu
> 
> > Maxime
> > 
> > > ---
> > > Changes in v2:
> > > - Replace check for hash ops with check for HW FILTER flags
> > > - Link to v1: https://lore.kernel.org/r/20260314-vlan-restore-error-v1-
> > 1-4fc6c3e2115f@xxxxxxxxxxxxx
> > > ---
> > >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > index 6827c99bde8c..cfc0ce9cec9c 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > @@ -6863,7 +6863,8 @@ static int stmmac_vlan_restore(struct stmmac_priv
> > *priv)
> > >  {
> > >  	int ret;
> > >
> > > -	if (!(priv->dev->features & NETIF_F_VLAN_FEATURES))
> > > +	if (!(priv->dev->features &
> > > +	      (NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER)))
> > >  		return 0;
> > >
> > >  	if (priv->hw->num_vlan)
> > >
> > > ---
> > > base-commit: 42bddab0563fe67882b2722620a66dd98c8dbf33
> > > change-id: 20260314-vlan-restore-error-f8b3a1c7f50a
> > >
> > > Best regards,
>