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

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

[Michal Piekos]

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@mmpsystems.pl>
---
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. 
---
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@mmpsystems.pl
---
 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,
-- 
Michal Piekos <michal.piekos@mmpsystems.pl>

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

[Paolo Abeni]

On 3/21/26 6:38 AM, 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@mmpsystems.pl>
> ---
> 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. 
> ---
> 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@mmpsystems.pl
> ---
>  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)
Adding Russell.

It's not obvious to me that with this change the
restore_hw_vlan_rx_fltr() and vlan_update() callback are still invoked
in all the relevant driver/features permutation.

/P

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

[Russell King (Oracle)]

On Tue, Mar 24, 2026 at 01:14:29PM +0100, Paolo Abeni wrote:
> On 3/21/26 6:38 AM, 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@mmpsystems.pl>
> > ---
> > 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. 
> > ---
> > 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@mmpsystems.pl
> > ---
> >  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)
> Adding Russell.
> 
> It's not obvious to me that with this change the
> restore_hw_vlan_rx_fltr() and vlan_update() callback are still invoked
> in all the relevant driver/features permutation.

I think there's a few questions here.

When CONFIG_VLAN_8021Q is enabled, then we set the filter features
if the vlhash dma capability is set:

        if (priv->dma_cap.vlhash) {
                ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
                ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
        }

Only dwmac4 and dwxgmac2 have the ability to set this feature (and
thus be synthesised with the feature enabled.) From what I understand,
when present, these features are permanently enabled (as they're not
set in hw_Features).

However, in stmmac_vlan_update() there is:

        if (!priv->dma_cap.vlhash) {
                if (count > 2) /* VID = 0 always passes filter */
                        return -EOPNOTSUPP;

                pmatch = vid;
                hash = 0;
        }

So, it is clear that the intention is that stmmac_vlan_update() will
be called even when the NETIF_F_HW_VLAN_*TAG_FILTER features are not
present - consider a core that implements the VLAN ops, but has
priv->dma_cap.vlhash false.

It looks like vlan support was added in dwmac v4.0 and later cores
(from the hwif table.) As the vlan ops are NULL before that, the
horrid stmmac_do_void_callback() crud will return -EINVAL (yuck). The
method it calls is a void function, so the only possible return values
are zero if implemented, or -EINVAL if not.

stmmac_vlan_update() itself only ever returns zero if the netif
isn't running, or the above return value. So, an error returned from
stmmac_vlan_update() just means that the driver has no support for
programming the vlan configuration.

Looking at the implementations for the case where vlhash is false,
(where stmmac_update_vlan_hash() will be called with hash=0 and
perfect_match with a vid, vlan_update_hash() still write
to the VLAN_TAG register to configure vlan handling. I haven't
looked up exactly what it does.

The other possibility is dwxgmac2_update_vlan_hash(). This looks
similar.

So, I don't think that the correct solution is to only call this
function when one or more of NETIF_F_HW_VLAN_*TAG_FILTER feature flags
are set - clearly the code is written to handle the case where the
update_vlan_hash method is populated but vlhash is false and these
feature flags are clear.

Now I'm wondering whether the code in the STMMAC_VLAN_TAG_USED block
is correct:

        /* Both mac100 and gmac support receive VLAN tag detection */
        ndev->features |= NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_STAG_RX;
        if (dwmac_is_xmac(priv->plat->core_type)) {
                ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX;
                priv->hw->hw_vlan_en = true;
        }
        if (priv->dma_cap.vlhash) {
                ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
                ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
        }
        if (priv->dma_cap.vlins)
                ndev->features |= NETIF_F_HW_VLAN_CTAG_TX;

The test for dwmac_is_xmac() matches for dwmac4 and dwxgmac cores. If
this is true, then we will have the vlan ops populated, and it means
that NETIF_F_HW_VLAN_CTAG_RX can be configured by the user rather than
being always-enabled. Is this correct? I'm not sure - I haven't dug
into enough of the documentation for this yet. However, I suggest
that we need to always call stmmac_update_vlan_hash() for these cores.

So, I'm coming to the conclusion that we either need a test for
dwmac_is_xmac() and not the feature flags, or maybe we just get rid
of the "Failed to restore VLANs" error print and make
stmmac_vlan_restore() return void (nothing uses the return value.)

In summary, I'm not sure what the correct approach is - please reach
out to Ovidiu Panait <ovidiu.panait.rb@renesas.com> who recently added
this code, but I'm fairly sure that this solution is incorrect.

Really, Ovidiu Panait should be reviewing this patch as his recent
change introduced the problematical error print.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

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

[Ovidiu Panait]

Hi,

> 
> On Tue, Mar 24, 2026 at 01:14:29PM +0100, Paolo Abeni wrote:
> > On 3/21/26 6:38 AM, 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@mmpsystems.pl>
> > > ---
> > > 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.
> > > ---
> > > 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@mmpsystems.pl
> > > ---
> > >  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)
> > Adding Russell.
> >
> > It's not obvious to me that with this change the
> > restore_hw_vlan_rx_fltr() and vlan_update() callback are still invoked
> > in all the relevant driver/features permutation.
> 
> I think there's a few questions here.
> 
> When CONFIG_VLAN_8021Q is enabled, then we set the filter features
> if the vlhash dma capability is set:
> 
>         if (priv->dma_cap.vlhash) {
>                 ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>                 ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
>         }
> 

The VLAN core will call into the ndo_vlan_rx_add_vid and ndo_vlan_rx_kill_vid
callbacks in the driver only if NETIF_F_HW_VLAN_CTAG_FILTER or
NETIF_F_HW_VLAN_STAG_FILTER features are advertised. For stmmac, the HW
filtering features are currently advertised only if priv->dma_cap.vlhash is
set.

stmmac_vlan_update() is only called from ndo_vlan_rx_add_vid and
ndo_vlan_rx_kill_vid callbacks to set up hash filtering, so on the
restore path it should also be called only if HW filtering is enabled.
Otherwise there is nothing to restore.

> Only dwmac4 and dwxgmac2 have the ability to set this feature (and
> thus be synthesised with the feature enabled.) From what I understand,
> when present, these features are permanently enabled (as they're not
> set in hw_Features).
> 
> However, in stmmac_vlan_update() there is:
> 
>         if (!priv->dma_cap.vlhash) {
>                 if (count > 2) /* VID = 0 always passes filter */
>                         return -EOPNOTSUPP;
> 
>                 pmatch = vid;
>                 hash = 0;
>         }
> 
> So, it is clear that the intention is that stmmac_vlan_update() will
> be called even when the NETIF_F_HW_VLAN_*TAG_FILTER features are not
> present - consider a core that implements the VLAN ops, but has
> priv->dma_cap.vlhash false.
> 

My understanding is that the code in stmmac_vlan_update() that checks for
!priv->dma_cap.vlhash is dead code, this condition cannot be true inside
the ndo_vlan_rx_add_vid and ndo_vlan_rx_kill_vid callbacks because the
callbacks are only called when priv->dma_cap.vlhash is true.

This seems to be a long-standing issue in the VLAN code.

> It looks like vlan support was added in dwmac v4.0 and later cores
> (from the hwif table.) As the vlan ops are NULL before that, the
> horrid stmmac_do_void_callback() crud will return -EINVAL (yuck). The
> method it calls is a void function, so the only possible return values
> are zero if implemented, or -EINVAL if not.
> 
> stmmac_vlan_update() itself only ever returns zero if the netif
> isn't running, or the above return value. So, an error returned from
> stmmac_vlan_update() just means that the driver has no support for
> programming the vlan configuration.
> 
> Looking at the implementations for the case where vlhash is false,
> (where stmmac_update_vlan_hash() will be called with hash=0 and
> perfect_match with a vid, vlan_update_hash() still write
> to the VLAN_TAG register to configure vlan handling. I haven't
> looked up exactly what it does.
> 
> The other possibility is dwxgmac2_update_vlan_hash(). This looks
> similar.
> 
> So, I don't think that the correct solution is to only call this
> function when one or more of NETIF_F_HW_VLAN_*TAG_FILTER feature flags
> are set - clearly the code is written to handle the case where the
> update_vlan_hash method is populated but vlhash is false and these
> feature flags are clear.
> 

Even though the original intent seems to have been to allow for
stmmac_vlan_update() to work with priv->dma_cap.vlhash = false, in practice
it doesn't work because stmmac_vlan_update() is only called when
priv->dma_cap.vlhash = true.

> Now I'm wondering whether the code in the STMMAC_VLAN_TAG_USED block
> is correct:
> 
>         /* Both mac100 and gmac support receive VLAN tag detection */
>         ndev->features |= NETIF_F_HW_VLAN_CTAG_RX |
> NETIF_F_HW_VLAN_STAG_RX;
>         if (dwmac_is_xmac(priv->plat->core_type)) {
>                 ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX;
>                 priv->hw->hw_vlan_en = true;
>         }
>         if (priv->dma_cap.vlhash) {
>                 ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>                 ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
>         }
>         if (priv->dma_cap.vlins)
>                 ndev->features |= NETIF_F_HW_VLAN_CTAG_TX;
> 
> The test for dwmac_is_xmac() matches for dwmac4 and dwxgmac cores. If
> this is true, then we will have the vlan ops populated, and it means
> that NETIF_F_HW_VLAN_CTAG_RX can be configured by the user rather than
> being always-enabled. Is this correct? I'm not sure - I haven't dug
> into enough of the documentation for this yet. However, I suggest
> that we need to always call stmmac_update_vlan_hash() for these cores.
> 
> So, I'm coming to the conclusion that we either need a test for
> dwmac_is_xmac() and not the feature flags, or maybe we just get rid
> of the "Failed to restore VLANs" error print and make
> stmmac_vlan_restore() return void (nothing uses the return value.)
> 
> In summary, I'm not sure what the correct approach is - please reach
> out to Ovidiu Panait <ovidiu.panait.rb@renesas.com> who recently added
> this code, but I'm fairly sure that this solution is incorrect.
> 
> Really, Ovidiu Panait should be reviewing this patch as his recent
> change introduced the problematical error print.
> 

I reviewed the v1 for this patch ([1]), and given the existing VLAN
implementation and for a targeted fix, I think checking HW filtering
features is correct. The restore only takes care of restoring the HW
filters, so it should run only when we have HW filters support.

When the HW filters should be enabled, though, I think is a separate
issue that requires some refactoring of the VLAN implementation, to
make it more precise, as Russell pointed out.

I only have access to a board that uses the DWMAC4 IP, but I'll have
a look in the documentation I have to check how to improve VLAN
features handling. 

[1] https://lore.kernel.org/all/20260314-vlan-restore-error-v1-1-4fc6c3e2115f@mmpsystems.pl/

Ovidiu

> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

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

[Russell King (Oracle)]

On Tue, Mar 24, 2026 at 03:37:24PM +0000, Ovidiu Panait wrote:
> Hi,
> 
> My understanding is that the code in stmmac_vlan_update() that checks for
> !priv->dma_cap.vlhash is dead code, this condition cannot be true inside
> the ndo_vlan_rx_add_vid and ndo_vlan_rx_kill_vid callbacks because the
> callbacks are only called when priv->dma_cap.vlhash is true.
> 
> This seems to be a long-standing issue in the VLAN code.

If you're certain that there's dead code here, please consider
submitting patches to clean it up thereby making the code easier
to understand.

> I only have access to a board that uses the DWMAC4 IP, but I'll have
> a look in the documentation I have to check how to improve VLAN
> features handling. 

Definitely please do! Having more effort to clean up parts of stmmac
would be very welcome (but please bear in mind that I have had quite
a large patch set, but that's reducing now.)

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

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

[Maxime Chevallier]

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@mmpsystems.pl>
> ---
> 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 ?

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@mmpsystems.pl
> ---
>  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,

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

[Ovidiu Panait]

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@mmpsystems.pl>
> > ---
> > 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.

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@mmpsystems.pl
> > ---
> >  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,

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@mmpsystems.pl>
> > > ---
> > > 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@mmpsystems.pl
> > > ---
> > >  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,
>