From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: igmp mld6 timeout membar
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Sat, 28 Mar 2026 19:59:37 +0300

On Wed, Mar 25, 2026 at 08:32:26PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> We had some membars in igmp and mld6 timer code that are not
> necessary.  There is no need to synchronise memory access.  The
> important things run within locks that have their own barriers.
> 
> These checks exists only to avoid locking without work.
> Also take a shortcut if the router list is empty.
> 
> ok?
> 

ok mvs

> bluhm
> 
> Index: netinet/igmp.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/igmp.c,v
> diff -u -p -r1.97 igmp.c
> --- netinet/igmp.c	22 Mar 2026 23:14:00 -0000	1.97
> +++ netinet/igmp.c	25 Mar 2026 17:43:49 -0000
> @@ -532,10 +532,8 @@ igmp_input_if(struct ifnet *ifp, struct 
>  
>  	}
>  
> -	if (running) {
> -		membar_producer();
> -		atomic_store_int(&igmp_timers_are_running, running);
> -	}
> +	if (running)
> +		atomic_store_int(&igmp_timers_are_running, 1);
>  
>  	/*
>  	 * Pass all valid IGMP packets up to any process(es) listening
> @@ -567,10 +565,8 @@ igmp_joingroup(struct in_multi *inm, str
>  	} else
>  		inm->inm_timer = 0;
>  
> -	if (running) {
> -		membar_producer();
> -		atomic_store_int(&igmp_timers_are_running, running);
> -	}
> +	if (running)
> +		atomic_store_int(&igmp_timers_are_running, 1);
>  }
>  
>  void
> @@ -614,7 +610,7 @@ igmp_fasttimo(void)
>  	 */
>  	if (!atomic_load_int(&igmp_timers_are_running))
>  		return;
> -	membar_consumer();
> +	atomic_store_int(&igmp_timers_are_running, 0);
>  
>  	NET_LOCK_SHARED();
>  
> @@ -624,9 +620,6 @@ igmp_fasttimo(void)
>  			running = 1;
>  	}
>  
> -	membar_producer();
> -	atomic_store_int(&igmp_timers_are_running, running);
> -
>  	while (!STAILQ_EMPTY(&pktlist)) {
>  		struct igmp_pktinfo *pkt;
>  
> @@ -637,6 +630,9 @@ igmp_fasttimo(void)
>  	}
>  
>  	NET_UNLOCK_SHARED();
> +
> +	if (running)
> +		atomic_store_int(&igmp_timers_are_running, 1);
>  }
>  
>  int
> @@ -683,6 +679,9 @@ void
>  igmp_slowtimo(void)
>  {
>  	struct router_info *rti;
> +
> +	if (LIST_EMPTY(&rti_head))
> +		return;
>  
>  	mtx_enter(&igmp_mtx);
>  	LIST_FOREACH(rti, &rti_head, rti_list) {
> Index: netinet6/mld6.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/mld6.c,v
> diff -u -p -r1.74 mld6.c
> --- netinet6/mld6.c	22 Mar 2026 23:14:00 -0000	1.74
> +++ netinet6/mld6.c	25 Mar 2026 17:43:49 -0000
> @@ -144,10 +144,8 @@ mld6_start_listening(struct in6_multi *i
>  		running = 1;
>  	}
>  
> -	if (running) {
> -		membar_producer();
> -		atomic_store_int(&mld6_timers_are_running, running);
> -	}
> +	if (running)
> +		atomic_store_int(&mld6_timers_are_running, 1);
>  }
>  
>  void
> @@ -367,10 +365,8 @@ mld6_input(struct mbuf *m, int off)
>  		break;
>  	}
>  
> -	if (running) {
> -		membar_producer();
> -		atomic_store_int(&mld6_timers_are_running, running);
> -	}
> +	if (running)
> +		atomic_store_int(&mld6_timers_are_running, 1);
>  
>  	if_put(ifp);
>  	m_freem(m);
> @@ -392,7 +388,7 @@ mld6_fasttimo(void)
>  	 */
>  	if (!atomic_load_int(&mld6_timers_are_running))
>  		return;
> -	membar_consumer();
> +	atomic_store_int(&mld6_timers_are_running, 0);
>  
>  	NET_LOCK_SHARED();
>  
> @@ -402,9 +398,6 @@ mld6_fasttimo(void)
>  			running = 1;
>  	}
>  
> -	membar_producer();
> -	atomic_store_int(&mld6_timers_are_running, running);
> -
>  	while (!STAILQ_EMPTY(&pktlist)) {
>  		struct mld6_pktinfo *pkt;
>  
> @@ -415,6 +408,9 @@ mld6_fasttimo(void)
>  	}
>  
>  	NET_UNLOCK_SHARED();
> +
> +	if (running)
> +		atomic_store_int(&mld6_timers_are_running, 1);
>  }
>  
>  int
>