Hi,
Why do we need NET_RLOCK_IN_SOFTNET and NET_RLOCK_IN_IOCTL macros
which do the same?
I would prefer to call both of them _SHARED. Also the W and R are
hard to see, I think NET_ASSERT_LOCKED_EXCLUSIVE is more obvious.
Some comments were pretty outdated.
ok?
bluhm
Index: net/if.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v
retrieving revision 1.661
diff -u -p -r1.661 if.c
--- net/if.c 5 Aug 2022 13:57:16 -0000 1.661
+++ net/if.c 5 Aug 2022 20:36:54 -0000
@@ -869,22 +869,16 @@ if_input_process(struct ifnet *ifp, stru
enqueue_randomness(ml_len(ml) ^ (uintptr_t)MBUF_LIST_FIRST(ml));
/*
- * We grab the NET_LOCK() before processing any packet to
- * ensure there's no contention on the routing table lock.
- *
- * Without it we could race with a userland thread to insert
- * a L2 entry in ip{6,}_output(). Such race would result in
- * one of the threads sleeping *inside* the IP output path.
- *
- * Since we have a NET_LOCK() we also use it to serialize access
- * to PF globals, pipex globals, unicast and multicast addresses
- * lists and the socket layer.
+ * We grab the shared netlock for packet processing in the softnet
+ * threads. Packets can regrab the exclusive lock via queues.
+ * ioctl, sysctl, and socket syscall may use shared lock if they
+ * read only or MP safe. Usually they hold the exclusive net lock.
*/
- NET_RLOCK_IN_SOFTNET();
+ NET_LOCK_SHARED();
while ((m = ml_dequeue(ml)) != NULL)
(*ifp->if_input)(ifp, m);
- NET_RUNLOCK_IN_SOFTNET();
+ NET_UNLOCK_SHARED();
}
void
@@ -2423,27 +2417,27 @@ ifioctl_get(u_long cmd, caddr_t data)
switch(cmd) {
case SIOCGIFCONF:
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
error = ifconf(data);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
return (error);
case SIOCIFGCLONERS:
error = if_clone_list((struct if_clonereq *)data);
return (error);
case SIOCGIFGMEMB:
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
error = if_getgroupmembers(data);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
return (error);
case SIOCGIFGATTR:
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
error = if_getgroupattribs(data);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
return (error);
case SIOCGIFGLIST:
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
error = if_getgrouplist(data);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
return (error);
}
@@ -2451,7 +2445,7 @@ ifioctl_get(u_long cmd, caddr_t data)
if (ifp == NULL)
return (ENXIO);
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
switch(cmd) {
case SIOCGIFFLAGS:
@@ -2519,7 +2513,7 @@ ifioctl_get(u_long cmd, caddr_t data)
panic("invalid ioctl %lu", cmd);
}
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
if_put(ifp);
Index: net/pf_ioctl.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.384
diff -u -p -r1.384 pf_ioctl.c
--- net/pf_ioctl.c 28 Jul 2022 12:27:29 -0000 1.384
+++ net/pf_ioctl.c 5 Aug 2022 20:36:54 -0000
@@ -3244,12 +3244,12 @@ pf_sysctl(void *oldp, size_t *oldlenp, v
{
struct pf_status pfs;
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
PF_LOCK();
memcpy(&pfs, &pf_status, sizeof(struct pf_status));
pfi_update_status(pfs.ifname, &pfs);
PF_UNLOCK();
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
return sysctl_rdstruct(oldp, oldlenp, newp, &pfs, sizeof(pfs));
}
Index: netinet/in.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in.c,v
retrieving revision 1.174
diff -u -p -r1.174 in.c
--- netinet/in.c 5 May 2022 08:43:37 -0000 1.174
+++ netinet/in.c 5 Aug 2022 20:36:54 -0000
@@ -569,7 +569,7 @@ in_ioctl_get(u_long cmd, caddr_t data, s
return (error);
}
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
if (ifa->ifa_addr->sa_family != AF_INET)
@@ -620,7 +620,7 @@ in_ioctl_get(u_long cmd, caddr_t data, s
}
err:
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
return (error);
}
Index: netinet/in_pcb.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.268
diff -u -p -r1.268 in_pcb.c
--- netinet/in_pcb.c 28 Jun 2022 09:32:27 -0000 1.268
+++ netinet/in_pcb.c 5 Aug 2022 20:36:54 -0000
@@ -674,7 +674,7 @@ in_pcbnotifyall(struct inpcbtable *table
struct in_addr faddr;
u_int rdomain;
- NET_ASSERT_WLOCKED();
+ NET_ASSERT_LOCKED_EXCLUSIVE();
if (dst->sa_family != AF_INET)
return;
Index: netinet/ip_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.376
diff -u -p -r1.376 ip_input.c
--- netinet/ip_input.c 4 Aug 2022 18:05:09 -0000 1.376
+++ netinet/ip_input.c 5 Aug 2022 20:36:54 -0000
@@ -233,8 +233,8 @@ ip_init(void)
/*
* Enqueue packet for local delivery. Queuing is used as a boundary
- * between the network layer (input/forward path) running with shared
- * NET_RLOCK_IN_SOFTNET() and the transport layer needing it exclusively.
+ * between the network layer (input/forward path) running with
+ * NET_LOCK_SHARED() and the transport layer needing it exclusively.
*/
int
ip_ours(struct mbuf **mp, int *offp, int nxt, int af)
@@ -254,6 +254,7 @@ ip_ours(struct mbuf **mp, int *offp, int
/*
* Dequeue and process locally delivered packets.
+ * This is called with exclusive NET_LOCK().
*/
void
ipintr(void)
@@ -696,7 +697,7 @@ ip_deliver(struct mbuf **mp, int *offp,
int nest = 0;
#endif /* INET6 */
- NET_ASSERT_WLOCKED();
+ NET_ASSERT_LOCKED_EXCLUSIVE();
/* pf might have modified stuff, might have to chksum */
switch (af) {
Index: netinet/ip_ipsp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v
retrieving revision 1.272
diff -u -p -r1.272 ip_ipsp.c
--- netinet/ip_ipsp.c 14 Jul 2022 13:52:10 -0000 1.272
+++ netinet/ip_ipsp.c 5 Aug 2022 20:36:54 -0000
@@ -644,7 +644,7 @@ tdb_walk(u_int rdomain, int (*walker)(st
* traversing the tdb_hnext list. Create a new tdb_walk list with
* exclusive netlock protection.
*/
- NET_ASSERT_WLOCKED();
+ NET_ASSERT_LOCKED_EXCLUSIVE();
SIMPLEQ_INIT(&tdblist);
mtx_enter(&tdb_sadb_mtx);
Index: netinet/ip_mroute.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_mroute.c,v
retrieving revision 1.135
diff -u -p -r1.135 ip_mroute.c
--- netinet/ip_mroute.c 5 May 2022 13:57:40 -0000 1.135
+++ netinet/ip_mroute.c 5 Aug 2022 20:36:54 -0000
@@ -266,16 +266,16 @@ mrt_ioctl(struct socket *so, u_long cmd,
else
switch (cmd) {
case SIOCGETVIFCNT:
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
error = get_vif_cnt(inp->inp_rtableid,
(struct sioc_vif_req *)data);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
break;
case SIOCGETSGCNT:
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
error = get_sg_cnt(inp->inp_rtableid,
(struct sioc_sg_req *)data);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
break;
default:
error = ENOTTY;
Index: netinet/raw_ip.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/raw_ip.c,v
retrieving revision 1.128
diff -u -p -r1.128 raw_ip.c
--- netinet/raw_ip.c 15 May 2022 09:12:20 -0000 1.128
+++ netinet/raw_ip.c 5 Aug 2022 20:36:54 -0000
@@ -152,7 +152,7 @@ rip_input(struct mbuf **mp, int *offp, i
}
}
#endif
- NET_ASSERT_WLOCKED();
+ NET_ASSERT_LOCKED_EXCLUSIVE();
SIMPLEQ_INIT(&inpcblist);
mtx_enter(&rawcbtable.inpt_mtx);
TAILQ_FOREACH(inp, &rawcbtable.inpt_queue, inp_queue) {
Index: netinet/udp_usrreq.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.279
diff -u -p -r1.279 udp_usrreq.c
--- netinet/udp_usrreq.c 26 Jun 2022 15:50:21 -0000 1.279
+++ netinet/udp_usrreq.c 5 Aug 2022 20:36:54 -0000
@@ -364,7 +364,7 @@ udp_input(struct mbuf **mp, int *offp, i
* Locate pcb(s) for datagram.
* (Algorithm copied from raw_intr().)
*/
- NET_ASSERT_WLOCKED();
+ NET_ASSERT_LOCKED_EXCLUSIVE();
SIMPLEQ_INIT(&inpcblist);
mtx_enter(&udbtable.inpt_mtx);
TAILQ_FOREACH(inp, &udbtable.inpt_queue, inp_queue) {
Index: netinet6/in6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.246
diff -u -p -r1.246 in6.c
--- netinet6/in6.c 25 Feb 2022 23:51:04 -0000 1.246
+++ netinet6/in6.c 5 Aug 2022 20:36:54 -0000
@@ -418,7 +418,7 @@ in6_ioctl_get(u_long cmd, caddr_t data,
return (error);
}
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
if (sa6 != NULL) {
error = in6_check_embed_scope(sa6, ifp->if_index);
@@ -512,7 +512,7 @@ in6_ioctl_get(u_long cmd, caddr_t data,
}
err:
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
return (error);
}
Index: netinet6/in6_pcb.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6_pcb.c,v
retrieving revision 1.117
diff -u -p -r1.117 in6_pcb.c
--- netinet6/in6_pcb.c 14 Apr 2022 14:10:22 -0000 1.117
+++ netinet6/in6_pcb.c 5 Aug 2022 20:36:54 -0000
@@ -375,7 +375,7 @@ in6_pcbnotify(struct inpcbtable *table,
u_int32_t flowinfo;
u_int rdomain;
- NET_ASSERT_WLOCKED();
+ NET_ASSERT_LOCKED_EXCLUSIVE();
if ((unsigned)cmd >= PRC_NCMDS)
return;
Index: netinet6/ip6_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.249
diff -u -p -r1.249 ip6_input.c
--- netinet6/ip6_input.c 24 Jul 2022 22:38:25 -0000 1.249
+++ netinet6/ip6_input.c 5 Aug 2022 22:26:17 -0000
@@ -169,8 +169,8 @@ ip6_init(void)
/*
* Enqueue packet for local delivery. Queuing is used as a boundary
- * between the network layer (input/forward path) running with shared
- * NET_RLOCK_IN_SOFTNET() and the transport layer needing it exclusively.
+ * between the network layer (input/forward path) running with
+ * NET_LOCK_SHARED() and the transport layer needing it exclusively.
*/
int
ip6_ours(struct mbuf **mp, int *offp, int nxt, int af)
@@ -186,6 +186,7 @@ ip6_ours(struct mbuf **mp, int *offp, in
/*
* Dequeue and process locally delivered packets.
+ * This is called with exclusive NET_LOCK().
*/
void
ip6intr(void)
Index: netinet6/ip6_mroute.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_mroute.c,v
retrieving revision 1.131
diff -u -p -r1.131 ip6_mroute.c
--- netinet6/ip6_mroute.c 5 May 2022 13:57:41 -0000 1.131
+++ netinet6/ip6_mroute.c 5 Aug 2022 20:36:54 -0000
@@ -247,16 +247,16 @@ mrt6_ioctl(struct socket *so, u_long cmd
switch (cmd) {
case SIOCGETSGCNT_IN6:
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
error = get_sg6_cnt((struct sioc_sg_req6 *)data,
inp->inp_rtableid);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
break;
case SIOCGETMIFCNT_IN6:
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
error = get_mif6_cnt((struct sioc_mif_req6 *)data,
inp->inp_rtableid);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
break;
default:
error = ENOTTY;
Index: netinet6/nd6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.242
diff -u -p -r1.242 nd6.c
--- netinet6/nd6.c 28 Jul 2022 13:10:37 -0000 1.242
+++ netinet6/nd6.c 5 Aug 2022 20:36:54 -0000
@@ -1022,9 +1022,9 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
switch (cmd) {
case SIOCGIFINFO_IN6:
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
ndi->ndi = *ND_IFINFO(ifp);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
return (0);
case SIOCGNBRINFO_IN6:
{
@@ -1032,7 +1032,7 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
struct in6_addr nb_addr = nbi->addr; /* make local for safety */
time_t expire;
- NET_RLOCK_IN_IOCTL();
+ NET_LOCK_SHARED();
/*
* XXX: KAME specific hack for scoped addresses
* XXXX: for other scopes than link-local?
@@ -1049,7 +1049,7 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
if (rt == NULL ||
(ln = (struct llinfo_nd6 *)rt->rt_llinfo) == NULL) {
rtfree(rt);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
return (EINVAL);
}
expire = ln->ln_rt->rt_expire;
@@ -1064,7 +1064,7 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
nbi->expire = expire;
rtfree(rt);
- NET_RUNLOCK_IN_IOCTL();
+ NET_UNLOCK_SHARED();
return (0);
}
}
Index: netinet6/raw_ip6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/raw_ip6.c,v
retrieving revision 1.147
diff -u -p -r1.147 raw_ip6.c
--- netinet6/raw_ip6.c 23 Mar 2022 00:16:07 -0000 1.147
+++ netinet6/raw_ip6.c 5 Aug 2022 20:36:54 -0000
@@ -164,7 +164,7 @@ rip6_input(struct mbuf **mp, int *offp,
}
}
#endif
- NET_ASSERT_WLOCKED();
+ NET_ASSERT_LOCKED_EXCLUSIVE();
SIMPLEQ_INIT(&inpcblist);
mtx_enter(&rawin6pcbtable.inpt_mtx);
TAILQ_FOREACH(in6p, &rawin6pcbtable.inpt_queue, inp_queue) {
Index: sys/systm.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/systm.h,v
retrieving revision 1.157
diff -u -p -r1.157 systm.h
--- sys/systm.h 12 Jul 2022 17:12:31 -0000 1.157
+++ sys/systm.h 5 Aug 2022 20:36:54 -0000
@@ -323,23 +323,17 @@ extern struct rwlock netlock;
#define NET_UNLOCK() do { rw_exit_write(&netlock); } while (0)
/*
- * Reader version of NET_LOCK() to be used in "softnet" thread only.
-
+ * Reader version of NET_LOCK().
* The "softnet" thread should be the only thread processing packets
* without holding an exclusive lock. This is done to allow read-only
* ioctl(2) to not block.
- */
-#define NET_RLOCK_IN_SOFTNET() do { rw_enter_read(&netlock); } while
(0)
-#define NET_RUNLOCK_IN_SOFTNET()do { rw_exit_read(&netlock); } while (0)
-
-/*
- * Reader version of NET_LOCK() to be used in ioctl/sysctl path only.
- *
- * Can be grabbed instead of the exclusive version when no field
+ * Shared lock can be grabbed instead of the exclusive version if no field
* protected by the NET_LOCK() is modified by the ioctl/sysctl.
+ * Socket system call can use shared netlock if it has additional locks
+ * to protect socket and pcb data structures.
*/
-#define NET_RLOCK_IN_IOCTL() do { rw_enter_read(&netlock); } while
(0)
-#define NET_RUNLOCK_IN_IOCTL() do { rw_exit_read(&netlock); } while (0)
+#define NET_LOCK_SHARED() do { rw_enter_read(&netlock); } while
(0)
+#define NET_UNLOCK_SHARED() do { rw_exit_read(&netlock); } while (0)
#ifdef DIAGNOSTIC
@@ -357,7 +351,7 @@ do {
\
splassert_fail(RW_READ, _s, __func__); \
} while (0)
-#define NET_ASSERT_WLOCKED()
\
+#define NET_ASSERT_LOCKED_EXCLUSIVE()
\
do { \
int _s = rw_status(&netlock); \
if ((splassert_ctl > 0) && (_s != RW_WRITE)) \
@@ -365,9 +359,9 @@ do {
\
} while (0)
#else /* DIAGNOSTIC */
-#define NET_ASSERT_UNLOCKED() do {} while (0)
-#define NET_ASSERT_LOCKED() do {} while (0)
-#define NET_ASSERT_WLOCKED() do {} while (0)
+#define NET_ASSERT_UNLOCKED() do {} while (0)
+#define NET_ASSERT_LOCKED() do {} while (0)
+#define NET_ASSERT_LOCKED_EXCLUSIVE() do {} while (0)
#endif /* !DIAGNOSTIC */
__returns_twice int setjmp(label_t *);
