On Sun, Aug 21, 2022 at 10:10:53PM +0300, Vitaliy Makkoveev wrote:
> On Sun, Aug 21, 2022 at 07:19:23PM +0200, Alexander Bluhm wrote:
> > When testing parallel UDP input, I see contention on the netlock
> > from the protocol timer. There is a check to avoid loop traversal
> > if igmp and mld6 are not active. Move the shortcut to avoid grabbing
> > netlock. This should be save as variables igmp_timers_are_running
> > and mld_timers_are_running are read atomically. In case we miss
> > an fast timer due to MP races, we just run it next time.
>
> Could we have a commentary like this before unlocked check? The rest of
> diff looks ok by me.
>
> > This should be save as variables igmp_timers_are_running
> > and mld_timers_are_running are read atomically. In case we miss
> > an fast timer due to MP races, we just run it next time.
I commited it with comment. I wonder if we should use read once.
It is an unprotected atomic acces. Read once makes this explicit.
The common prefix is mld6_ for mld6.c. Document that
igmp_timers_are_running and mld_timers_are_running are protected
by netlock.
ok?
bluhm
Index: netinet/igmp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/igmp.c,v
retrieving revision 1.79
diff -u -p -r1.79 igmp.c
--- netinet/igmp.c 21 Aug 2022 23:04:45 -0000 1.79
+++ netinet/igmp.c 22 Aug 2022 13:19:51 -0000
@@ -96,7 +96,7 @@
#define IP_MULTICASTOPTS 0
-int igmp_timers_are_running;
+int igmp_timers_are_running; /* [N] shortcut for fast timer */
static LIST_HEAD(, router_info) rti_head;
static struct mbuf *router_alert;
struct cpumem *igmpcounters;
@@ -532,7 +532,7 @@ igmp_fasttimo(void)
* Variable igmp_timers_are_running is read atomically. In case we
* miss a fast timer due to MP races, just run it next time.
*/
- if (!igmp_timers_are_running)
+ if (!READ_ONCE(igmp_timers_are_running))
return;
NET_LOCK();
Index: netinet6/mld6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/mld6.c,v
retrieving revision 1.57
diff -u -p -r1.57 mld6.c
--- netinet6/mld6.c 21 Aug 2022 23:04:45 -0000 1.57
+++ netinet6/mld6.c 22 Aug 2022 13:22:58 -0000
@@ -84,7 +84,7 @@
#include <netinet6/mld6_var.h>
static struct ip6_pktopts ip6_opts;
-static int mld_timers_are_running;
+int mld6_timers_are_running; /* [N] shortcut for fast timer */
/* XXX: These are necessary for KAME's link-local hack */
static struct in6_addr mld_all_nodes_linklocal =
IN6ADDR_LINKLOCAL_ALLNODES_INIT;
static struct in6_addr mld_all_routers_linklocal =
IN6ADDR_LINKLOCAL_ALLROUTERS_INIT;
@@ -99,7 +99,7 @@ mld6_init(void)
struct ip6_hbh *hbh = (struct ip6_hbh *)hbh_buf;
u_int16_t rtalert_code = htons((u_int16_t)IP6OPT_RTALERT_MLD);
- mld_timers_are_running = 0;
+ mld6_timers_are_running = 0;
/* ip6h_nxt will be fill in later */
hbh->ip6h_len = 0; /* (8 >> 3) - 1 */
@@ -136,7 +136,7 @@ mld6_start_listening(struct in6_multi *i
MLD_RANDOM_DELAY(MLD_V1_MAX_RI *
PR_FASTHZ);
in6m->in6m_state = MLD_IREPORTEDLAST;
- mld_timers_are_running = 1;
+ mld6_timers_are_running = 1;
}
}
@@ -266,7 +266,7 @@ mld6_input(struct mbuf *m, int off)
in6m->in6m_timer > timer) {
in6m->in6m_timer =
MLD_RANDOM_DELAY(timer);
- mld_timers_are_running = 1;
+ mld6_timers_are_running = 1;
}
}
}
@@ -330,15 +330,15 @@ mld6_fasttimeo(void)
/*
* Quick check to see if any work needs to be done, in order
* to minimize the overhead of fasttimo processing.
- * Variable mld_timers_are_running is read atomically. In case we
+ * Variable mld6_timers_are_running is read atomically. In case we
* miss a fast timer due to MP races, just run it next time.
*/
- if (!mld_timers_are_running)
+ if (!READ_ONCE(mld6_timers_are_running))
return;
NET_LOCK();
- mld_timers_are_running = 0;
+ mld6_timers_are_running = 0;
TAILQ_FOREACH(ifp, &ifnet, if_list)
mld6_checktimer(ifp);
@@ -363,7 +363,7 @@ mld6_checktimer(struct ifnet *ifp)
mld6_sendpkt(in6m, MLD_LISTENER_REPORT, NULL);
in6m->in6m_state = MLD_IREPORTEDLAST;
} else {
- mld_timers_are_running = 1;
+ mld6_timers_are_running = 1;
}
}
}
