Re: [PATCH net-next v2 11/14] net: bridge: mcast: track active state, prepare for outside lock reads

Ido Schimmel <[email protected]>
Newsgroups gmane.linux.network.bridge,gmane.linux.network,gmane.linux.kernel
Message-ID <20260208161126.GK154003@shredder>
On Fri, Feb 06, 2026 at 03:52:17AM +0100, Linus Lüssing wrote:
> @@ -1095,23 +1095,25 @@ static void br_ip6_multicast_update_active(struct net_bridge_mcast *brmctx,
>  {
>  #if IS_ENABLED(CONFIG_IPV6)
>  	if (force_inactive)
> -		brmctx->ip6_active = false;
> +		WRITE_ONCE(brmctx->ip6_active, false);
>  	else
> -		brmctx->ip6_active = br_ip6_multicast_querier_exists(brmctx);
> +		WRITE_ONCE(brmctx->ip6_active, br_ip6_multicast_querier_exists(brmctx));
>  #endif
>  }
>  
>  static void br_multicast_notify_active(struct net_bridge_mcast *brmctx,
>  				       bool ip4_active_old, bool ip6_active_old)
>  {
> -	if (brmctx->ip4_active == ip4_active_old &&
> -	    brmctx->ip6_active == ip6_active_old)
> +	int ip4_active = READ_ONCE(brmctx->ip4_active);
> +	int ip6_active = READ_ONCE(brmctx->ip6_active);

I believe that this is unnecessary since we are holding the lock and it
will confuse people reading the code. We only need the READ_ONCE() later
on in the data path

> +
> +	if (ip4_active == ip4_active_old &&
> +	    ip6_active == ip6_active_old)
>  		return;
>  
>  	br_info(brmctx->br, "mc_active changed, vid: %i: v4: %i->%i, v6: %i->%i\n",
>  		brmctx->vlan ? brmctx->vlan->vid : -1,
> -		ip4_active_old, brmctx->ip4_active,
> -		ip6_active_old, brmctx->ip6_active);
> +		ip4_active_old, ip4_active, ip6_active_old, ip6_active);
>  }
>  
>  /**
> @@ -1136,7 +1138,8 @@ static void br_multicast_notify_active(struct net_bridge_mcast *brmctx,
>   */
>  static void br_multicast_update_active(struct net_bridge_mcast *brmctx)
>  {
> -	bool ip4_active_old = brmctx->ip4_active, ip6_active_old = brmctx->ip6_active;
> +	bool ip4_active_old = READ_ONCE(brmctx->ip4_active);
> +	bool ip6_active_old = READ_ONCE(brmctx->ip6_active);

Same

>  	bool force_inactive = false;
>  
>  	lockdep_assert_held_once(&brmctx->br->multicast_lock);
> @@ -4266,13 +4269,13 @@ void br_multicast_ctx_init(struct net_bridge *br,
>  	brmctx->multicast_membership_interval = 260 * HZ;
>  
>  	brmctx->ip4_querier.port_ifidx = 0;
> -	brmctx->ip4_active = 0;
> +	WRITE_ONCE(brmctx->ip4_active, 0);
>  	seqcount_spinlock_init(&brmctx->ip4_querier.seq, &br->multicast_lock);
>  	brmctx->multicast_igmp_version = 2;
>  #if IS_ENABLED(CONFIG_IPV6)
>  	brmctx->multicast_mld_version = 1;
>  	brmctx->ip6_querier.port_ifidx = 0;
> -	brmctx->ip6_active = 0;
> +	WRITE_ONCE(brmctx->ip6_active, 0);
>  	seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock);
>  #endif
>  
> -- 
> 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.