From nobody Mon Feb 9 13:35:21 2026 Received: from mail.aperture-lab.de (mail.aperture-lab.de [116.203.183.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DF7102BDC03; Fri, 6 Feb 2026 03:01:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=116.203.183.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770346901; cv=none; b=kkeQL4u5Ye5jJrwFCD0JCdMxzcnWV7fh95dzy77Om7PIyPJ4ePIkT4mRg5AUUeCSMPehUs7e6OPYjneFXug5x293bowaHnLGRqQJjvQabcXnzmInbvWwwRs3BuOAnHnVg2NMt8ytpctG4C44c82Z0Nde2o6qN/xv1CVVD/Vt92U= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770346901; c=relaxed/simple; bh=vaXBTLkB4Z2pRlYzMhVUcT8igRLyx9ydo/WCecReO9A=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=iLZtQf70CEplZEi9+oZfZMeos38oUWIVESWpNwBWo4GVYjuKcwSmwTI498yEO+ueblf+WiU47m2YXDj/0KspzfHZpwetJ+RsSXgYYlkeFBi6+jNd4mQZs9lNu8ZHXVOPCbQPQUBtlvbxnnZiQ/DV8yPz0rHVdbGt7lOycODH5a0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=c0d3.blue; spf=pass smtp.mailfrom=c0d3.blue; arc=none smtp.client-ip=116.203.183.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=c0d3.blue Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=c0d3.blue Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id C67D654CD34; Fri, 6 Feb 2026 04:01:38 +0100 (CET) From: =?UTF-8?q?Linus=20L=C3=BCssing?= To: bridge@lists.linux.dev Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Nikolay Aleksandrov , Ido Schimmel , Andrew Lunn , Simon Horman , Paolo Abeni , Jakub Kicinski , Eric Dumazet , "David S . Miller" , Kuniyuki Iwashima , Stanislav Fomichev , Xiao Liang , =?UTF-8?q?Linus=20L=C3=BCssing?= Subject: [PATCH net-next v2 03/14] net: bridge: mcast: avoid sleeping on bridge-down Date: Fri, 6 Feb 2026 03:52:09 +0100 Message-ID: <20260206030123.5430-4-linus.luessing@c0d3.blue> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260206030123.5430-1-linus.luessing@c0d3.blue> References: <20260206030123.5430-1-linus.luessing@c0d3.blue> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Last-TLS-Session-Version: TLSv1.3 We later want to use the multicast lock when setting the bridge interface up or down, to be able to atomically both check all conditions to toggle the multicast active state and to subsequently toggle it. While most variables we check / contexts we check from are serialized (toggled variables through netlink/sysfs) the timer_pending() check is not and might run in parallel. However so far we are not allowed to spinlock __br_multicast_stop() as its call to timer_delete_sync() might sleep. Therefore replacing the sleeping variant with the non-sleeping one. It is sufficient to only wait for any timer callback to finish when we are freeing the multicast context. Using the timer_shutdown() instead of the timer_delete() variant also allows us to detect that we are stopping from within the according timer callbacks, to retain the promise of the previous timer_delete_sync() calls that no multicast state is changed after these timer_{delete,shutdown}*() calls. And more importantly that we are not inadvertently rearming timers. This new check also makes the netif_running() check redundant/obsolete in these contexts. Signed-off-by: Linus L=C3=BCssing --- net/bridge/br_device.c | 4 ++ net/bridge/br_multicast.c | 108 ++++++++++++++++++++++++++------------ net/bridge/br_private.h | 5 ++ net/bridge/br_vlan.c | 5 ++ 4 files changed, 87 insertions(+), 35 deletions(-) diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index a818fdc22da9..d9d1227d5708 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -168,7 +168,9 @@ static int br_dev_open(struct net_device *dev) netdev_update_features(dev); netif_start_queue(dev); br_stp_enable_bridge(br); + spin_lock_bh(&br->multicast_lock); br_multicast_open(br); + spin_unlock_bh(&br->multicast_lock); =20 if (br_opt_get(br, BROPT_MULTICAST_ENABLED)) br_multicast_join_snoopers(br); @@ -191,7 +193,9 @@ static int br_dev_stop(struct net_device *dev) struct net_bridge *br =3D netdev_priv(dev); =20 br_stp_disable_bridge(br); + spin_lock_bh(&br->multicast_lock); br_multicast_stop(br); + spin_unlock_bh(&br->multicast_lock); =20 if (br_opt_get(br, BROPT_MULTICAST_ENABLED)) br_multicast_leave_snoopers(br); diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index dccae08b4f4c..f5a368dd20a3 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1665,6 +1665,14 @@ static void br_multicast_router_expired(struct net_b= ridge_mcast_port *pmctx, spin_unlock(&br->multicast_lock); } =20 +static bool br_multicast_stopping(struct net_bridge *br, + struct timer_list *timer) +{ + lockdep_assert_held_once(&br->multicast_lock); + + return !timer->function; +} + static void br_ip4_multicast_router_expired(struct timer_list *t) { struct net_bridge_mcast_port *pmctx =3D timer_container_of(pmctx, t, @@ -1700,7 +1708,8 @@ static void br_multicast_local_router_expired(struct = net_bridge_mcast *brmctx, struct timer_list *timer) { spin_lock(&brmctx->br->multicast_lock); - if (brmctx->multicast_router =3D=3D MDB_RTR_TYPE_DISABLED || + if (br_multicast_stopping(brmctx->br, timer) || + brmctx->multicast_router =3D=3D MDB_RTR_TYPE_DISABLED || brmctx->multicast_router =3D=3D MDB_RTR_TYPE_PERM || br_ip4_multicast_is_router(brmctx) || br_ip6_multicast_is_router(brmctx)) @@ -1730,10 +1739,11 @@ static void br_ip6_multicast_local_router_expired(s= truct timer_list *t) #endif =20 static void br_multicast_querier_expired(struct net_bridge_mcast *brmctx, - struct bridge_mcast_own_query *query) + struct bridge_mcast_own_query *query, + struct timer_list *timer) { spin_lock(&brmctx->br->multicast_lock); - if (!netif_running(brmctx->br->dev) || + if (br_multicast_stopping(brmctx->br, timer) || br_multicast_ctx_vlan_global_disabled(brmctx) || !br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED)) goto out; @@ -1749,7 +1759,7 @@ static void br_ip4_multicast_querier_expired(struct t= imer_list *t) struct net_bridge_mcast *brmctx =3D timer_container_of(brmctx, t, ip4_other_query.timer); =20 - br_multicast_querier_expired(brmctx, &brmctx->ip4_own_query); + br_multicast_querier_expired(brmctx, &brmctx->ip4_own_query, t); } =20 #if IS_ENABLED(CONFIG_IPV6) @@ -1758,7 +1768,7 @@ static void br_ip6_multicast_querier_expired(struct t= imer_list *t) struct net_bridge_mcast *brmctx =3D timer_container_of(brmctx, t, ip6_other_query.timer); =20 - br_multicast_querier_expired(brmctx, &brmctx->ip6_own_query); + br_multicast_querier_expired(brmctx, &brmctx->ip6_own_query, t); } #endif =20 @@ -4049,10 +4059,12 @@ int br_multicast_rcv(struct net_bridge_mcast **brmc= tx, } =20 static void br_multicast_query_expired(struct net_bridge_mcast *brmctx, - struct bridge_mcast_own_query *query) + struct bridge_mcast_own_query *query, + struct timer_list *timer) { spin_lock(&brmctx->br->multicast_lock); - if (br_multicast_ctx_vlan_disabled(brmctx)) + if (br_multicast_stopping(brmctx->br, timer) || + br_multicast_ctx_vlan_disabled(brmctx)) goto out; =20 if (query->startup_sent < brmctx->multicast_startup_query_count) @@ -4068,7 +4080,7 @@ static void br_ip4_multicast_query_expired(struct tim= er_list *t) struct net_bridge_mcast *brmctx =3D timer_container_of(brmctx, t, ip4_own_query.timer); =20 - br_multicast_query_expired(brmctx, &brmctx->ip4_own_query); + br_multicast_query_expired(brmctx, &brmctx->ip4_own_query, t); } =20 #if IS_ENABLED(CONFIG_IPV6) @@ -4077,7 +4089,7 @@ static void br_ip6_multicast_query_expired(struct tim= er_list *t) struct net_bridge_mcast *brmctx =3D timer_container_of(brmctx, t, ip6_own_query.timer); =20 - br_multicast_query_expired(brmctx, &brmctx->ip6_own_query); + br_multicast_query_expired(brmctx, &brmctx->ip6_own_query, t); } #endif =20 @@ -4120,29 +4132,30 @@ void br_multicast_ctx_init(struct net_bridge *br, seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock); #endif =20 - timer_setup(&brmctx->ip4_mc_router_timer, - br_ip4_multicast_local_router_expired, 0); - timer_setup(&brmctx->ip4_other_query.timer, - br_ip4_multicast_querier_expired, 0); - timer_setup(&brmctx->ip4_other_query.delay_timer, - br_multicast_query_delay_expired, 0); - timer_setup(&brmctx->ip4_own_query.timer, - br_ip4_multicast_query_expired, 0); + timer_setup(&brmctx->ip4_mc_router_timer, NULL, 0); + timer_setup(&brmctx->ip4_other_query.timer, NULL, 0); + timer_setup(&brmctx->ip4_other_query.delay_timer, NULL, 0); + timer_setup(&brmctx->ip4_own_query.timer, NULL, 0); #if IS_ENABLED(CONFIG_IPV6) - timer_setup(&brmctx->ip6_mc_router_timer, - br_ip6_multicast_local_router_expired, 0); - timer_setup(&brmctx->ip6_other_query.timer, - br_ip6_multicast_querier_expired, 0); - timer_setup(&brmctx->ip6_other_query.delay_timer, - br_multicast_query_delay_expired, 0); - timer_setup(&brmctx->ip6_own_query.timer, - br_ip6_multicast_query_expired, 0); + timer_setup(&brmctx->ip6_mc_router_timer, NULL, 0); + timer_setup(&brmctx->ip6_other_query.timer, NULL, 0); + timer_setup(&brmctx->ip6_other_query.delay_timer, NULL, 0); + timer_setup(&brmctx->ip6_own_query.timer, NULL, 0); #endif } =20 void br_multicast_ctx_deinit(struct net_bridge_mcast *brmctx) { - __br_multicast_stop(brmctx); + timer_shutdown_sync(&brmctx->ip4_mc_router_timer); + timer_shutdown_sync(&brmctx->ip4_other_query.timer); + timer_shutdown_sync(&brmctx->ip4_other_query.delay_timer); + timer_shutdown_sync(&brmctx->ip4_own_query.timer); +#if IS_ENABLED(CONFIG_IPV6) + timer_shutdown_sync(&brmctx->ip6_mc_router_timer); + timer_shutdown_sync(&brmctx->ip6_other_query.timer); + timer_shutdown_sync(&brmctx->ip6_other_query.delay_timer); + timer_shutdown_sync(&brmctx->ip6_own_query.timer); +#endif } =20 void br_multicast_init(struct net_bridge *br) @@ -4222,9 +4235,27 @@ void br_multicast_leave_snoopers(struct net_bridge *= br) br_ip6_multicast_leave_snoopers(br); } =20 +void br_multicast_reset_timer_cbs(struct net_bridge_mcast *brmctx) +{ + lockdep_assert_held_once(&brmctx->br->multicast_lock); + + brmctx->ip4_mc_router_timer.function =3D br_ip4_multicast_local_router_ex= pired; + brmctx->ip4_other_query.timer.function =3D br_ip4_multicast_querier_expir= ed; + brmctx->ip4_other_query.delay_timer.function =3D br_multicast_query_delay= _expired; + brmctx->ip4_own_query.timer.function =3D br_ip4_multicast_query_expired; +#if IS_ENABLED(CONFIG_IPV6) + brmctx->ip6_mc_router_timer.function =3D br_ip6_multicast_local_router_ex= pired; + brmctx->ip6_other_query.timer.function =3D br_ip6_multicast_querier_expir= ed; + brmctx->ip6_other_query.delay_timer.function =3D br_multicast_query_delay= _expired; + brmctx->ip6_own_query.timer.function =3D br_ip6_multicast_query_expired; +#endif +} + static void __br_multicast_open_query(struct net_bridge *br, struct bridge_mcast_own_query *query) { + lockdep_assert_held_once(&br->multicast_lock); + query->startup_sent =3D 0; =20 if (!br_opt_get(br, BROPT_MULTICAST_ENABLED)) @@ -4235,6 +4266,8 @@ static void __br_multicast_open_query(struct net_brid= ge *br, =20 static void __br_multicast_open(struct net_bridge_mcast *brmctx) { + br_multicast_reset_timer_cbs(brmctx); + __br_multicast_open_query(brmctx->br, &brmctx->ip4_own_query); #if IS_ENABLED(CONFIG_IPV6) __br_multicast_open_query(brmctx->br, &brmctx->ip6_own_query); @@ -4267,15 +4300,17 @@ void br_multicast_open(struct net_bridge *br) =20 static void __br_multicast_stop(struct net_bridge_mcast *brmctx) { - timer_delete_sync(&brmctx->ip4_mc_router_timer); - timer_delete_sync(&brmctx->ip4_other_query.timer); - timer_delete_sync(&brmctx->ip4_other_query.delay_timer); - timer_delete_sync(&brmctx->ip4_own_query.timer); + lockdep_assert_held_once(&brmctx->br->multicast_lock); + + timer_shutdown(&brmctx->ip4_mc_router_timer); + timer_shutdown(&brmctx->ip4_other_query.timer); + timer_shutdown(&brmctx->ip4_other_query.delay_timer); + timer_shutdown(&brmctx->ip4_own_query.timer); #if IS_ENABLED(CONFIG_IPV6) - timer_delete_sync(&brmctx->ip6_mc_router_timer); - timer_delete_sync(&brmctx->ip6_other_query.timer); - timer_delete_sync(&brmctx->ip6_other_query.delay_timer); - timer_delete_sync(&brmctx->ip6_own_query.timer); + timer_shutdown(&brmctx->ip6_mc_router_timer); + timer_shutdown(&brmctx->ip6_other_query.timer); + timer_shutdown(&brmctx->ip6_other_query.delay_timer); + timer_shutdown(&brmctx->ip6_own_query.timer); #endif } =20 @@ -4326,12 +4361,12 @@ void br_multicast_toggle_one_vlan(struct net_bridge= _vlan *vlan, bool on) =20 spin_lock_bh(&br->multicast_lock); vlan->priv_flags ^=3D BR_VLFLAG_MCAST_ENABLED; - spin_unlock_bh(&br->multicast_lock); =20 if (on) __br_multicast_open(&vlan->br_mcast_ctx); else __br_multicast_stop(&vlan->br_mcast_ctx); + spin_unlock_bh(&br->multicast_lock); } else { struct net_bridge_mcast *brmctx; =20 @@ -4389,6 +4424,7 @@ int br_multicast_toggle_vlan_snooping(struct net_brid= ge *br, bool on, if (!vg) return 0; =20 + spin_lock_bh(&br->multicast_lock); br_opt_toggle(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED, on); =20 /* disable/enable non-vlan mcast contexts based on vlan snooping */ @@ -4396,6 +4432,8 @@ int br_multicast_toggle_vlan_snooping(struct net_brid= ge *br, bool on, __br_multicast_stop(&br->multicast_ctx); else __br_multicast_open(&br->multicast_ctx); + spin_unlock_bh(&br->multicast_lock); + list_for_each_entry(p, &br->port_list, list) { if (on) br_multicast_disable_port_ctx(&p->multicast_ctx); diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 4ab6a1f58116..a181a27aa559 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -976,6 +976,7 @@ void br_multicast_disable_port(struct net_bridge_port *= port); void br_multicast_init(struct net_bridge *br); void br_multicast_join_snoopers(struct net_bridge *br); void br_multicast_leave_snoopers(struct net_bridge *br); +void br_multicast_reset_timer_cbs(struct net_bridge_mcast *brmctx); void br_multicast_open(struct net_bridge *br); void br_multicast_stop(struct net_bridge *br); void br_multicast_dev_del(struct net_bridge *br); @@ -1416,6 +1417,10 @@ static inline void br_multicast_leave_snoopers(struc= t net_bridge *br) { } =20 +static inline void br_multicast_reset_timer_cbs(struct net_bridge_mcast *b= rmctx) +{ +} + static inline void br_multicast_open(struct net_bridge *br) { } diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index ce72b837ff8e..7b46aa517c28 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -325,7 +325,12 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 f= lags, if (err && err !=3D -EOPNOTSUPP) goto out; } + br_multicast_ctx_init(br, v, &v->br_mcast_ctx); + + spin_lock_bh(&br->multicast_lock); + br_multicast_reset_timer_cbs(&v->br_mcast_ctx); + spin_unlock_bh(&br->multicast_lock); v->priv_flags |=3D BR_VLFLAG_GLOBAL_MCAST_ENABLED; } =20 --=20 2.51.0