Download raw body.
domain protocol timer unlock
On Mon, Jul 29, 2024 at 04:25:56PM +0200, Alexander Bluhm wrote:
> On Sat, May 18, 2024 at 12:03:32PM +0200, Claudio Jeker wrote:
> > Stupid compilers are broken. Some of these transformations they do are
> > just not side-effect free. So no you are not too paranoid. You are not
> > paranoid enough since the code would probably need some memory barriers as
> > well (at least on the producer side). Right now I think it is fine but if
> > someone changes the NET_LOCK in the timeout even a shared NET_LOCK then we
> > are in trouble.
>
> Here is a variant with atomic operations and memory barriers for
> igmp_timers_are_running and mld6_timers_are_running. The memory
> barriers should not be necessary as long net lock protects us. But
> we want to reduce net lock, so we can to it correctly right now.
>
> This diff is independent of the question whether all these timeouts
> should use the timeout(9) framework. My gaol is to remove kernel
> lock from network code. So I have to make multicast MP good enough.
Previous diff had a misplaced membar_consumer().
> ok?
anyone?
bluhm
Index: kern/uipc_domain.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_domain.c,v
diff -u -p -r1.65 uipc_domain.c
--- kern/uipc_domain.c 11 Jan 2024 14:15:11 -0000 1.65
+++ kern/uipc_domain.c 6 Aug 2024 13:03:01 -0000
@@ -90,8 +90,10 @@ domaininit(void)
max_linkhdr = 64;
max_hdr = max_linkhdr + max_protohdr;
- timeout_set_proc(&pffast_timeout, pffasttimo, &pffast_timeout);
- timeout_set_proc(&pfslow_timeout, pfslowtimo, &pfslow_timeout);
+ timeout_set_flags(&pffast_timeout, pffasttimo, &pffast_timeout,
+ KCLOCK_NONE, TIMEOUT_PROC | TIMEOUT_MPSAFE);
+ timeout_set_flags(&pfslow_timeout, pfslowtimo, &pfslow_timeout,
+ KCLOCK_NONE, TIMEOUT_PROC | TIMEOUT_MPSAFE);
timeout_add(&pffast_timeout, 1);
timeout_add(&pfslow_timeout, 1);
}
Index: netinet/igmp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/igmp.c,v
diff -u -p -r1.83 igmp.c
--- netinet/igmp.c 16 Sep 2023 09:33:27 -0000 1.83
+++ netinet/igmp.c 6 Aug 2024 13:21:46 -0000
@@ -96,12 +96,12 @@
#define IP_MULTICASTOPTS 0
-int igmp_timers_are_running; /* [N] shortcut for fast timer */
+int igmp_timers_are_running; /* [a] shortcut for fast timer */
static LIST_HEAD(, router_info) rti_head;
static struct mbuf *router_alert;
struct cpumem *igmpcounters;
-void igmp_checktimer(struct ifnet *);
+int igmp_checktimer(struct ifnet *);
void igmp_sendpkt(struct ifnet *, struct in_multi *, int, in_addr_t);
int rti_fill(struct in_multi *);
struct router_info * rti_find(struct ifnet *);
@@ -228,7 +228,7 @@ igmp_input_if(struct ifnet *ifp, struct
struct in_multi *inm;
struct router_info *rti;
struct in_ifaddr *ia;
- int timer;
+ int timer, running = 0;
igmplen = ntohs(ip->ip_len) - iphlen;
@@ -300,7 +300,7 @@ igmp_input_if(struct ifnet *ifp, struct
inm->inm_state = IGMP_DELAYING_MEMBER;
inm->inm_timer = IGMP_RANDOM_DELAY(
IGMP_MAX_HOST_REPORT_DELAY * PR_FASTHZ);
- igmp_timers_are_running = 1;
+ running = 1;
}
}
} else {
@@ -341,7 +341,7 @@ igmp_input_if(struct ifnet *ifp, struct
IGMP_DELAYING_MEMBER;
inm->inm_timer =
IGMP_RANDOM_DELAY(timer);
- igmp_timers_are_running = 1;
+ running = 1;
break;
case IGMP_SLEEPING_MEMBER:
inm->inm_state =
@@ -475,6 +475,11 @@ igmp_input_if(struct ifnet *ifp, struct
}
+ if (running) {
+ membar_producer();
+ atomic_store_int(&igmp_timers_are_running, running);
+ }
+
/*
* Pass all valid IGMP packets up to any process(es) listening
* on a raw IGMP socket.
@@ -485,7 +490,7 @@ igmp_input_if(struct ifnet *ifp, struct
void
igmp_joingroup(struct in_multi *inm, struct ifnet *ifp)
{
- int i;
+ int i, running = 0;
inm->inm_state = IGMP_IDLE_MEMBER;
@@ -496,9 +501,14 @@ igmp_joingroup(struct in_multi *inm, str
inm->inm_state = IGMP_DELAYING_MEMBER;
inm->inm_timer = IGMP_RANDOM_DELAY(
IGMP_MAX_HOST_REPORT_DELAY * PR_FASTHZ);
- igmp_timers_are_running = 1;
+ running = 1;
} else
inm->inm_timer = 0;
+
+ if (running) {
+ membar_producer();
+ atomic_store_int(&igmp_timers_are_running, running);
+ }
}
void
@@ -525,6 +535,7 @@ void
igmp_fasttimo(void)
{
struct ifnet *ifp;
+ int running = 0;
/*
* Quick check to see if any work needs to be done, in order
@@ -533,23 +544,29 @@ igmp_fasttimo(void)
* lock intentionally. In case it is not set due to MP races, we may
* miss to check the timers. Then run the loop at next fast timeout.
*/
- if (!igmp_timers_are_running)
+ if (!atomic_load_int(&igmp_timers_are_running))
return;
+ membar_consumer();
NET_LOCK();
- igmp_timers_are_running = 0;
- TAILQ_FOREACH(ifp, &ifnetlist, if_list)
- igmp_checktimer(ifp);
+ TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
+ if (igmp_checktimer(ifp))
+ running = 1;
+ }
+
+ membar_producer();
+ atomic_store_int(&igmp_timers_are_running, running);
NET_UNLOCK();
}
-void
+int
igmp_checktimer(struct ifnet *ifp)
{
struct in_multi *inm;
struct ifmaddr *ifma;
+ int running = 0;
NET_ASSERT_LOCKED();
@@ -570,9 +587,11 @@ igmp_checktimer(struct ifnet *ifp)
inm->inm_state = IGMP_IDLE_MEMBER;
}
} else {
- igmp_timers_are_running = 1;
+ running = 1;
}
}
+
+ return (running);
}
void
Index: netinet6/icmp6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
diff -u -p -r1.254 icmp6.c
--- netinet6/icmp6.c 14 Jul 2024 18:53:39 -0000 1.254
+++ netinet6/icmp6.c 6 Aug 2024 13:03:01 -0000
@@ -1198,7 +1198,6 @@ icmp6_reflect(struct mbuf **mp, size_t o
void
icmp6_fasttimo(void)
{
-
mld6_fasttimeo();
}
Index: netinet6/mld6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/mld6.c,v
diff -u -p -r1.62 mld6.c
--- netinet6/mld6.c 13 Feb 2024 12:22:09 -0000 1.62
+++ netinet6/mld6.c 6 Aug 2024 13:21:03 -0000
@@ -85,9 +85,9 @@
#include <netinet6/mld6_var.h>
static struct ip6_pktopts ip6_opts;
-int mld6_timers_are_running; /* [N] shortcut for fast timer */
+int mld6_timers_are_running; /* [a] shortcut for fast timer */
-void mld6_checktimer(struct ifnet *);
+int mld6_checktimer(struct ifnet *);
static void mld6_sendpkt(struct in6_multi *, int, const struct in6_addr *);
void
@@ -118,6 +118,7 @@ mld6_start_listening(struct in6_multi *i
{
/* XXX: These are necessary for KAME's link-local hack */
struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT;
+ int running = 0;
/*
* RFC2710 page 10:
@@ -138,7 +139,12 @@ mld6_start_listening(struct in6_multi *i
MLD_RANDOM_DELAY(MLD_V1_MAX_RI *
PR_FASTHZ);
in6m->in6m_state = MLD_IREPORTEDLAST;
- mld6_timers_are_running = 1;
+ running = 1;
+ }
+
+ if (running) {
+ membar_producer();
+ atomic_store_int(&mld6_timers_are_running, running);
}
}
@@ -169,6 +175,7 @@ mld6_input(struct mbuf *m, int off)
struct in6_multi *in6m;
struct ifmaddr *ifma;
int timer; /* timer value in the MLD query header */
+ int running = 0;
/* XXX: These are necessary for KAME's link-local hack */
struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT;
@@ -272,7 +279,7 @@ mld6_input(struct mbuf *m, int off)
in6m->in6m_timer > timer) {
in6m->in6m_timer =
MLD_RANDOM_DELAY(timer);
- mld6_timers_are_running = 1;
+ running = 1;
}
}
}
@@ -323,8 +330,13 @@ mld6_input(struct mbuf *m, int off)
#endif
break;
}
- if_put(ifp);
+ if (running) {
+ membar_producer();
+ atomic_store_int(&mld6_timers_are_running, running);
+ }
+
+ if_put(ifp);
m_freem(m);
}
@@ -332,6 +344,7 @@ void
mld6_fasttimeo(void)
{
struct ifnet *ifp;
+ int running;
/*
* Quick check to see if any work needs to be done, in order
@@ -340,23 +353,29 @@ mld6_fasttimeo(void)
* lock intentionally. In case it is not set due to MP races, we may
* miss to check the timers. Then run the loop at next fast timeout.
*/
- if (!mld6_timers_are_running)
+ if (!atomic_load_int(&mld6_timers_are_running))
return;
+ membar_consumer();
NET_LOCK();
- mld6_timers_are_running = 0;
- TAILQ_FOREACH(ifp, &ifnetlist, if_list)
- mld6_checktimer(ifp);
+ TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
+ if (mld6_checktimer(ifp))
+ running = 1;
+ }
+
+ membar_producer();
+ atomic_store_int(&mld6_timers_are_running, running);
NET_UNLOCK();
}
-void
+int
mld6_checktimer(struct ifnet *ifp)
{
struct in6_multi *in6m;
struct ifmaddr *ifma;
+ int running = 0;
NET_ASSERT_LOCKED();
@@ -370,9 +389,11 @@ mld6_checktimer(struct ifnet *ifp)
mld6_sendpkt(in6m, MLD_LISTENER_REPORT, NULL);
in6m->in6m_state = MLD_IREPORTEDLAST;
} else {
- mld6_timers_are_running = 1;
+ running = 1;
}
}
+
+ return (running);
}
static void
domain protocol timer unlock