Re: [PATCH net-next v3 14/14] net: bridge: mcast: add inactive state assertions

Nikolay Aleksandrov <[email protected]>
Newsgroups gmane.linux.network.bridge,gmane.linux.network,gmane.linux.kernel
Message-ID <aaWPUbG3_X4s8T0R@penguin>
On Mon, Mar 02, 2026 at 06:40:08AM +0100, Linus Lüssing wrote:
> To avoid packetloss and as it is very hard from a user's perspective to
> debug multicast snooping related issues it is even more crucial to properly
> switch from an active to an inactive multicast snooping state than the
> other way around.
> 
> Therefore adding a few kernel warnings if any of our assertions to be in
> an inactive state would fail.
> 
> Signed-off-by: Linus Lüssing <[email protected]>
> ---
>  net/bridge/br_multicast.c | 43 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 4a1005bb68f1..106d6dd78328 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1421,10 +1421,29 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge_mcast *brmctx,
>  	return NULL;
>  }
>  
> +static void br_ip4_multicast_assert_inactive(struct net_bridge_mcast *brmctx)
> +{
> +	WARN_ON_ONCE(br_multicast_snooping_active(brmctx, htons(ETH_P_IP), NULL));
> +}
> +
> +static void br_ip6_multicast_assert_inactive(struct net_bridge_mcast *brmctx)
> +{
> +	WARN_ON_ONCE(br_multicast_snooping_active(brmctx, htons(ETH_P_IPV6), NULL));
> +}
> +
> +static void br_multicast_assert_inactive(struct net_bridge_mcast *brmctx)
> +{
> +	br_ip4_multicast_assert_inactive(brmctx);
> +	br_ip6_multicast_assert_inactive(brmctx);
> +}
> +

since brmctx can be const in br_multicast_snooping_active(), you can constify
it in all these helpers as well

>  static void br_multicast_toggle_enabled(struct net_bridge *br, bool on)
>  {
>  	br_opt_toggle(br, BROPT_MULTICAST_ENABLED, on);
>  	br_multicast_update_active(&br->multicast_ctx);
> +
> +	if (!on)
> +		br_multicast_assert_inactive(&br->multicast_ctx);
>  }
>  
>  struct net_bridge_mdb_entry *br_multicast_new_group(struct net_bridge *br,
> @@ -1894,9 +1913,7 @@ static void br_multicast_querier_expired(struct net_bridge_mcast *brmctx,
>  					 struct bridge_mcast_own_query *query,
>  					 struct timer_list *timer)
>  {
> -	spin_lock(&brmctx->br->multicast_lock);
> -	if (br_multicast_is_stopping(brmctx->br, timer) ||
> -	    br_multicast_ctx_vlan_global_disabled(brmctx) ||
> +	if (br_multicast_ctx_vlan_global_disabled(brmctx) ||
>  	    !br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED))
>  		goto out;
>  
> @@ -1907,7 +1924,6 @@ static void br_multicast_querier_expired(struct net_bridge_mcast *brmctx,
>  	 * if our own querier is disabled, too
>  	 */
>  	br_multicast_update_active(brmctx);
> -	spin_unlock(&brmctx->br->multicast_lock);
>  }
>  
>  static void br_ip4_multicast_querier_expired(struct timer_list *t)
> @@ -1915,7 +1931,16 @@ static void br_ip4_multicast_querier_expired(struct timer_list *t)
>  	struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t,
>  							     ip4_other_query.timer);
>  
> +	spin_lock(&brmctx->br->multicast_lock);
> +	if (br_multicast_is_stopping(brmctx->br, t))
> +		goto out;
> +
>  	br_multicast_querier_expired(brmctx, &brmctx->ip4_own_query, t);
> +
> +	if (!brmctx->multicast_querier)
> +		br_ip4_multicast_assert_inactive(brmctx);
> +out:
> +	spin_unlock(&brmctx->br->multicast_lock);
>  }
>  
>  #if IS_ENABLED(CONFIG_IPV6)
> @@ -1924,7 +1949,16 @@ static void br_ip6_multicast_querier_expired(struct timer_list *t)
>  	struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t,
>  							     ip6_other_query.timer);
>  
> +	spin_lock(&brmctx->br->multicast_lock);
> +	if (br_multicast_is_stopping(brmctx->br, t))
> +		goto out;
> +
>  	br_multicast_querier_expired(brmctx, &brmctx->ip6_own_query, t);
> +
> +	if (!brmctx->multicast_querier)
> +		br_ip6_multicast_assert_inactive(brmctx);
> +out:
> +	spin_unlock(&brmctx->br->multicast_lock);
>  }
>  #endif
>  
> @@ -4502,6 +4536,7 @@ static void __br_multicast_stop(struct net_bridge_mcast *brmctx)
>  
>  	/* bridge interface is down, set multicast state to inactive */
>  	br_multicast_update_active(brmctx);
> +	br_multicast_assert_inactive(brmctx);
>  }
>  
>  void br_multicast_update_vlan_mcast_ctx(struct net_bridge_vlan *v, u8 state)
> -- 
> 2.51.0
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.