Netdev List
 help / color / mirror / Atom feed
* [PATCH net v2] net: bridge: vlan: fix bugs caused by switchdev deletion errors
@ 2026-09-14 10:52 Nikolay Aleksandrov
  2026-09-14 15:00 ` Ido Schimmel
  2026-09-16  1:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-14 10:52 UTC (permalink / raw)
  To: netdev
  Cc: idosch, davem, edumazet, kuba, pabeni, horms, vivien.didelot,
	petrm, jiri, bridge, Nikolay Aleksandrov

Allowing switchdev to prevent vlan deletion and error out in __vlan_del
could cause multiple different issues - inconsistent state, memory leaks
when flushing, NULL pointer dereference on bridge error when flushing.
It doesn't make sense to allow it to stop __vlan_del, so log the error
and continue with software vlan deletion. This is also consistent with
8021q behaviour.

Suggested-by: Ido Schimmel <idosch@nvidia.com>
Fixes: bf361ad38165 ("net: bridge: check __vlan_vid_del for error")
Fixes: 5454f5c28eca ("net: bridge: vlan: check for errors from __vlan_del in __vlan_flush")
Fixes: 2594e9064a57 ("bridge: vlan: add per-vlan struct and move to rhashtables")
Fixes: 9c86ce2c1ae3 ("net: bridge: Notify about bridge VLANs")
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
v2: this is a new patch, don't fail vlan deletion as suggested by Ido
v1: https://lore.kernel.org/netdev/20260911100645.1360386-1-razor@blackwall.org/

 to Sashiko (from local clone check):
 [Severity: Critical]
 Could this introduce a regression where net/bridge/br_vlan.c:__vlan_del()
 leaves stale VLAN membership in hardware after tearing down the software
 VLAN?

 For the port branch, __vlan_vid_del() has already converted -EOPNOTSUPP to
 success.  An error reaching this warning is therefore a substantive offload
 deletion failure.

 Nik: No, a driver should not block VLAN deletion and this behaviour is in
      line with 8021q. Also blocking it could leave incosistent state. It
      is best to print a warning and allow software vlan deletion to
      proceed.

 net/bridge/br_vlan.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 1e0e436629ec..92b3cb621a26 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -387,12 +387,12 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags,
 	goto out;
 }
 
-static int __vlan_del(struct net_bridge_vlan *v)
+static void __vlan_del(struct net_bridge_vlan *v)
 {
 	struct net_bridge_vlan *masterv = v;
 	struct net_bridge_vlan_group *vg;
 	struct net_bridge_port *p = NULL;
-	int err = 0;
+	int err;
 
 	if (br_vlan_is_master(v)) {
 		vg = br_vlan_group(v->br);
@@ -406,12 +406,16 @@ static int __vlan_del(struct net_bridge_vlan *v)
 	if (p) {
 		err = __vlan_vid_del(p->dev, p->br, v);
 		if (err)
-			goto out;
+			br_warn(p->br,
+				"port %u(%s) failed to delete vlan %u from switchdev: %pe\n",
+				(unsigned int)p->port_no, p->dev->name,
+				v->vid, ERR_PTR(err));
 	} else {
 		err = br_switchdev_port_vlan_del(v->br->dev, v->vid);
 		if (err && err != -EOPNOTSUPP)
-			goto out;
-		err = 0;
+			br_warn(v->br,
+				"failed to delete bridge vlan %u from switchdev: %pe\n",
+				v->vid, ERR_PTR(err));
 	}
 
 	if (br_vlan_should_use(v)) {
@@ -431,8 +435,6 @@ static int __vlan_del(struct net_bridge_vlan *v)
 	}
 
 	br_vlan_put_master(masterv);
-out:
-	return err;
 }
 
 static void __vlan_group_free(struct net_bridge_vlan_group *vg)
@@ -449,7 +451,6 @@ static void __vlan_flush(const struct net_bridge *br,
 {
 	struct net_bridge_vlan *vlan, *tmp;
 	u16 v_start = 0, v_end = 0;
-	int err;
 
 	__vlan_delete_pvid(vg, vg->pvid);
 	list_for_each_entry_safe(vlan, tmp, &vg->vlan_list, vlist) {
@@ -463,13 +464,7 @@ static void __vlan_flush(const struct net_bridge *br,
 		}
 		v_end = vlan->vid;
 
-		err = __vlan_del(vlan);
-		if (err) {
-			br_err(br,
-			       "port %u(%s) failed to delete vlan %d: %pe\n",
-			       (unsigned int) p->port_no, p->dev->name,
-			       vlan->vid, ERR_PTR(err));
-		}
+		__vlan_del(vlan);
 	}
 
 	/* notify about the last/whole vlan range */
@@ -837,8 +832,9 @@ int br_vlan_delete(struct net_bridge *br, u16 vid)
 	br_fdb_delete_by_port(br, NULL, vid, 0);
 
 	vlan_tunnel_info_del(vg, v);
+	__vlan_del(v);
 
-	return __vlan_del(v);
+	return 0;
 }
 
 void br_vlan_flush(struct net_bridge *br)
@@ -1368,8 +1364,9 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
 		return -ENOENT;
 	br_fdb_find_delete_local(port->br, port, port->dev->dev_addr, vid);
 	br_fdb_delete_by_port(port->br, port, vid, 0);
+	__vlan_del(v);
 
-	return __vlan_del(v);
+	return 0;
 }
 
 void nbp_vlan_flush(struct net_bridge_port *port)
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] net: bridge: vlan: fix bugs caused by switchdev deletion errors
  2026-09-14 10:52 [PATCH net v2] net: bridge: vlan: fix bugs caused by switchdev deletion errors Nikolay Aleksandrov
@ 2026-09-14 15:00 ` Ido Schimmel
  2026-09-16  1:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Ido Schimmel @ 2026-09-14 15:00 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: netdev, davem, edumazet, kuba, pabeni, horms, vivien.didelot,
	petrm, jiri, bridge

On Mon, Sep 14, 2026 at 01:52:58PM +0300, Nikolay Aleksandrov wrote:
> Allowing switchdev to prevent vlan deletion and error out in __vlan_del
> could cause multiple different issues - inconsistent state, memory leaks
> when flushing, NULL pointer dereference on bridge error when flushing.
> It doesn't make sense to allow it to stop __vlan_del, so log the error
> and continue with software vlan deletion. This is also consistent with
> 8021q behaviour.
> 
> Suggested-by: Ido Schimmel <idosch@nvidia.com>
> Fixes: bf361ad38165 ("net: bridge: check __vlan_vid_del for error")
> Fixes: 5454f5c28eca ("net: bridge: vlan: check for errors from __vlan_del in __vlan_flush")
> Fixes: 2594e9064a57 ("bridge: vlan: add per-vlan struct and move to rhashtables")
> Fixes: 9c86ce2c1ae3 ("net: bridge: Notify about bridge VLANs")
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] net: bridge: vlan: fix bugs caused by switchdev deletion errors
  2026-09-14 10:52 [PATCH net v2] net: bridge: vlan: fix bugs caused by switchdev deletion errors Nikolay Aleksandrov
  2026-09-14 15:00 ` Ido Schimmel
@ 2026-09-16  1:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-16  1:40 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: netdev, idosch, davem, edumazet, kuba, pabeni, horms,
	vivien.didelot, petrm, jiri, bridge

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 14 Sep 2026 13:52:58 +0300 you wrote:
> Allowing switchdev to prevent vlan deletion and error out in __vlan_del
> could cause multiple different issues - inconsistent state, memory leaks
> when flushing, NULL pointer dereference on bridge error when flushing.
> It doesn't make sense to allow it to stop __vlan_del, so log the error
> and continue with software vlan deletion. This is also consistent with
> 8021q behaviour.
> 
> [...]

Here is the summary with links:
  - [net,v2] net: bridge: vlan: fix bugs caused by switchdev deletion errors
    https://git.kernel.org/netdev/net/c/2842ce397dd0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-16  1:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14 10:52 [PATCH net v2] net: bridge: vlan: fix bugs caused by switchdev deletion errors Nikolay Aleksandrov
2026-09-14 15:00 ` Ido Schimmel
2026-09-16  1:40 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox