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

[Nikolay Aleksandrov <nikolay@nvidia.com>]

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 <linus.luessing@c0d3.blue>
> ---
>  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
>