Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
igmp mld6 timeout useless membar
To:
tech@openbsd.org
Date:
Fri, 2 Jan 2026 17:18:43 +0100

Download raw body.

Thread
  • Alexander Bluhm:

    igmp mld6 timeout useless membar

Hi,

The membar in igmp_fasttimo() and mld6_fasttimo() is not necessary.
All data structures are protected by locks.  The interegr flag
igmp_timers_are_running is just an optmization to skip useless list
traversal.  The worst that might happen by reordering of instructions
is that the first timeout is skipped.

Another race can be prevented by resetting igmp_timers_are_running
before the list lock is taken.

In igmp_slowtimo() we should also avoid grabbing the mutex if the
list is empty.  Again the worst thing that might happen by this
lock-free access is a skipped periodic timer.

ok?

bluhm

Index: netinet/igmp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/igmp.c,v
diff -u -p -r1.94 igmp.c
--- netinet/igmp.c	2 Jan 2026 13:13:29 -0000	1.94
+++ netinet/igmp.c	2 Jan 2026 14:45:29 -0000
@@ -533,10 +533,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
@@ -565,10 +563,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
@@ -608,7 +604,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();
 
@@ -617,10 +613,10 @@ igmp_fasttimo(void)
 			running = 1;
 	}
 
-	membar_producer();
-	atomic_store_int(&igmp_timers_are_running, running);
-
 	NET_UNLOCK_SHARED();
+
+	if (running)
+		atomic_store_int(&igmp_timers_are_running, 1);
 }
 
 int
@@ -660,6 +656,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.71 mld6.c
--- netinet6/mld6.c	2 Jan 2026 13:13:29 -0000	1.71
+++ netinet6/mld6.c	2 Jan 2026 14:51:48 -0000
@@ -142,10 +142,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
@@ -337,10 +335,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);
@@ -361,7 +357,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();
 
@@ -370,10 +366,10 @@ mld6_fasttimo(void)
 			running = 1;
 	}
 
-	membar_producer();
-	atomic_store_int(&mld6_timers_are_running, running);
-
 	NET_UNLOCK_SHARED();
+
+	if (running)
+		atomic_store_int(&mld6_timers_are_running, 1);
 }
 
 int