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

[PATCH] 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 hash ops before stmmac_vlan_update().

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. 
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6827c99bde8c..bc09439ec00b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6869,6 +6869,9 @@ static int stmmac_vlan_restore(struct stmmac_priv *priv)
 	if (priv->hw->num_vlan)
 		stmmac_restore_hw_vlan_rx_fltr(priv, priv->dev, priv->hw);
 
+	if (!priv->hw->vlan || !priv->hw->vlan->update_vlan_hash)
+		return 0;
+
 	ret = stmmac_vlan_update(priv, priv->num_double_vlans);
 	if (ret)
 		netdev_err(priv->dev, "Failed to restore VLANs\n");

---
base-commit: 1c9982b4961334c1edb0745a04cabd34bc2de675
change-id: 20260314-vlan-restore-error-f8b3a1c7f50a

Best regards,
-- 
Michal Piekos <michal.piekos@mmpsystems.pl>

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

[Ovidiu Panait]

Hi Michal,

> 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 hash ops before stmmac_vlan_update().
> 
> 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.
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6827c99bde8c..bc09439ec00b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6869,6 +6869,9 @@ static int stmmac_vlan_restore(struct stmmac_priv
> *priv)
>         if (priv->hw->num_vlan)
>                 stmmac_restore_hw_vlan_rx_fltr(priv, priv->dev, priv->hw);
> 
> +       if (!priv->hw->vlan || !priv->hw->vlan->update_vlan_hash)
> +               return 0;
> +

The stmmac_restore_hw_vlan_rx_fltr() call above also goes through the
priv->hw->vlan pointer, so maybe for consistency, the check for empty
VLAN ops should be made at the top of the function.

Another alternative would be to replace the NETIF_F_VLAN_FEATURES check
at the top of stmmac_vlan_update() function to check if VLAN HW filtering
is supported instead (NETIF_F_HW_VLAN_CTAG_FILTER and
NETIF_F_HW_VLAN_STAG_FILTER).

The restore only deals with the HW filters anyway and the vlan ops are
always defined in that case (stmmac_vlan_rx_add_vid() and
stmmac_vlan_rx_kill_vid() rely on this being true as well, as they also
call stmmac_vlan_update() unconditionally).

Ovidiu

>         ret = stmmac_vlan_update(priv, priv->num_double_vlans);
>         if (ret)
>                 netdev_err(priv->dev, "Failed to restore VLANs\n");
> 
> ---
> base-commit: 1c9982b4961334c1edb0745a04cabd34bc2de675
> change-id: 20260314-vlan-restore-error-f8b3a1c7f50a
> 
> Best regards,
> --
> Michal Piekos <michal.piekos@mmpsystems.pl>

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

[Michal Piekos]

On Tue, Mar 17, 2026 at 12:36:42PM +0000, Ovidiu Panait wrote:
> Hi Michal,
> 
> > 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 hash ops before stmmac_vlan_update().
> > 
> > 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.
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 6827c99bde8c..bc09439ec00b 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -6869,6 +6869,9 @@ static int stmmac_vlan_restore(struct stmmac_priv
> > *priv)
> >         if (priv->hw->num_vlan)
> >                 stmmac_restore_hw_vlan_rx_fltr(priv, priv->dev, priv->hw);
> > 
> > +       if (!priv->hw->vlan || !priv->hw->vlan->update_vlan_hash)
> > +               return 0;
> > +
> 
> The stmmac_restore_hw_vlan_rx_fltr() call above also goes through the
> priv->hw->vlan pointer, so maybe for consistency, the check for empty
> VLAN ops should be made at the top of the function.
> 
> Another alternative would be to replace the NETIF_F_VLAN_FEATURES check
> at the top of stmmac_vlan_update() function to check if VLAN HW filtering
> is supported instead (NETIF_F_HW_VLAN_CTAG_FILTER and
> NETIF_F_HW_VLAN_STAG_FILTER).
> 
> The restore only deals with the HW filters anyway and the vlan ops are
> always defined in that case (stmmac_vlan_rx_add_vid() and
> stmmac_vlan_rx_kill_vid() rely on this being true as well, as they also
> call stmmac_vlan_update() unconditionally).
> 
> Ovidiu

Thank you for the comment. 
Your suggested solution to check VLAN HW filtering flags is much better.
I'll go for it for v2.

Michal

> 
> >         ret = stmmac_vlan_update(priv, priv->num_double_vlans);
> >         if (ret)
> >                 netdev_err(priv->dev, "Failed to restore VLANs\n");
> > 
> > ---
> > base-commit: 1c9982b4961334c1edb0745a04cabd34bc2de675
> > change-id: 20260314-vlan-restore-error-f8b3a1c7f50a
> > 
> > Best regards,
> > --
> > Michal Piekos <michal.piekos@mmpsystems.pl>
>