Use `sb_mtx' instead of `inp_mtx' in receive path for inet sockets.
authormvs <mvs@openbsd.org>
Sun, 11 Feb 2024 18:14:26 +0000 (18:14 +0000)
committermvs <mvs@openbsd.org>
Sun, 11 Feb 2024 18:14:26 +0000 (18:14 +0000)
In soreceve(), we only touch `so_rcv' socket buffer, which has it's own
`sb_mtx' mutex(9) for protection. So, we can avoid solock() in this
path - it's enough to hold `sb_mtx' in soreceive() and around
corresponding sbappend*(). But not right now :)

This time we use shared netlock for some inet sockets in the soreceive()
path. To protect `so_rcv' buffer we use `inp_mtx' mutex(9) and the
pru_lock() to acquire this mutex(9) in socket layer. But the `inp_mtx'
mutex belongs to the PCB. We initialize socket before PCB, tcp(4)
sockets could exist without PCB, so use `sb_mtx' mutex(9) to protect
sockbuf stuff.

This diff mechanically replaces `inp_mtx' by `sb_mtx' in the receive
path. Only for sockets which already use `inp_mtx'. All other sockets
left as is. They will be converted later.

Since the `sb_mtx' is optional, the new SB_MTXLOCK flag introduced. If
this flag is set on `sb_flags', the `sb_mtx' mutex(9) should be taken.
New sb_mtx_lock() and sb_mtx_unlock() was introduced to hide this check.
They are temporary and will be replaced by mtx_enter() when all this
area will be converted to `sb_mtx' mutex(9).

Also, the new sbmtxassertlocked() function introduced to throw
corresponding assertion for SB_MTXLOCK marked buffers. This time only
sbappendaddr() calls it. This function is also temporary and will be
replaced by MTX_ASSERT_LOCKED() later.

ok bluhm

sys/kern/uipc_socket.c
sys/kern/uipc_socket2.c
sys/netinet/ip_divert.c
sys/netinet/ip_mroute.c
sys/netinet/raw_ip.c
sys/netinet/udp_usrreq.c
sys/netinet6/ip6_divert.c
sys/netinet6/ip6_mroute.c
sys/netinet6/raw_ip6.c
sys/sys/socketvar.h

index 86dbd16..9a2bda2 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uipc_socket.c,v 1.317 2024/02/05 20:21:38 mvs Exp $   */
+/*     $OpenBSD: uipc_socket.c,v 1.318 2024/02/11 18:14:26 mvs Exp $   */
 /*     $NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $        */
 
 /*
@@ -150,6 +150,18 @@ soalloc(const struct domain *dp, int wait)
        TAILQ_INIT(&so->so_q0);
        TAILQ_INIT(&so->so_q);
 
+       switch (dp->dom_family) {
+       case AF_INET:
+       case AF_INET6:
+               switch (dp->dom_protosw->pr_type) {
+               case SOCK_DGRAM:
+               case SOCK_RAW:
+                       so->so_rcv.sb_flags |= SB_MTXLOCK;
+                       break;
+               }
+               break;
+       }
+
        return (so);
 }
 
@@ -825,7 +837,7 @@ restart:
                sounlock_shared(so);
                return (error);
        }
-       pru_lock(so);
+       sb_mtx_lock(&so->so_rcv);
 
        m = so->so_rcv.sb_mb;
 #ifdef SOCKET_SPLICE
@@ -889,7 +901,7 @@ restart:
                SBLASTRECORDCHK(&so->so_rcv, "soreceive sbwait 1");
                SBLASTMBUFCHK(&so->so_rcv, "soreceive sbwait 1");
                sbunlock(so, &so->so_rcv);
-               pru_unlock(so);
+               sb_mtx_unlock(&so->so_rcv);
                error = sbwait(so, &so->so_rcv);
                if (error) {
                        sounlock_shared(so);
@@ -961,13 +973,13 @@ dontblock:
                        sbsync(&so->so_rcv, nextrecord);
                        if (controlp) {
                                if (pr->pr_domain->dom_externalize) {
-                                       pru_unlock(so);
+                                       sb_mtx_unlock(&so->so_rcv);
                                        sounlock_shared(so);
                                        error =
                                            (*pr->pr_domain->dom_externalize)
                                            (cm, controllen, flags);
                                        solock_shared(so);
-                                       pru_lock(so);
+                                       sb_mtx_lock(&so->so_rcv);
                                }
                                *controlp = cm;
                        } else {
@@ -1041,11 +1053,11 @@ dontblock:
                        SBLASTRECORDCHK(&so->so_rcv, "soreceive uiomove");
                        SBLASTMBUFCHK(&so->so_rcv, "soreceive uiomove");
                        resid = uio->uio_resid;
-                       pru_unlock(so);
+                       sb_mtx_unlock(&so->so_rcv);
                        sounlock_shared(so);
                        uio_error = uiomove(mtod(m, caddr_t) + moff, len, uio);
                        solock_shared(so);
-                       pru_lock(so);
+                       sb_mtx_lock(&so->so_rcv);
                        if (uio_error)
                                uio->uio_resid = resid - len;
                } else
@@ -1127,14 +1139,14 @@ dontblock:
                                break;
                        SBLASTRECORDCHK(&so->so_rcv, "soreceive sbwait 2");
                        SBLASTMBUFCHK(&so->so_rcv, "soreceive sbwait 2");
-                       pru_unlock(so);
+                       sb_mtx_unlock(&so->so_rcv);
                        error = sbwait(so, &so->so_rcv);
                        if (error) {
                                sbunlock(so, &so->so_rcv);
                                sounlock_shared(so);
                                return (0);
                        }
-                       pru_lock(so);
+                       sb_mtx_lock(&so->so_rcv);
                        if ((m = so->so_rcv.sb_mb) != NULL)
                                nextrecord = m->m_nextpkt;
                }
@@ -1168,7 +1180,7 @@ dontblock:
            (flags & MSG_EOR) == 0 &&
            (so->so_rcv.sb_state & SS_CANTRCVMORE) == 0) {
                sbunlock(so, &so->so_rcv);
-               pru_unlock(so);
+               sb_mtx_unlock(&so->so_rcv);
                goto restart;
        }
 
@@ -1179,7 +1191,7 @@ dontblock:
                *flagsp |= flags;
 release:
        sbunlock(so, &so->so_rcv);
-       pru_unlock(so);
+       sb_mtx_unlock(&so->so_rcv);
        sounlock_shared(so);
        return (error);
 }
index cc809f6..9322cd2 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uipc_socket2.c,v 1.142 2024/02/05 20:21:38 mvs Exp $  */
+/*     $OpenBSD: uipc_socket2.c,v 1.143 2024/02/11 18:14:26 mvs Exp $  */
 /*     $NetBSD: uipc_socket2.c,v 1.11 1996/02/04 02:17:55 christos Exp $       */
 
 /*
@@ -504,6 +504,16 @@ sosleep_nsec(struct socket *so, void *ident, int prio, const char *wmesg,
        return ret;
 }
 
+void
+sbmtxassertlocked(struct socket *so, struct sockbuf *sb)
+{
+       if (sb->sb_flags & SB_MTXLOCK) {
+               if (splassert_ctl > 0 && mtx_owned(&sb->sb_mtx) == 0)
+                       splassert_fail(0, RW_WRITE, __func__);
+       } else
+               soassertlocked(so);
+}
+
 /*
  * Wait for data to arrive at/drain from a socket buffer.
  */
@@ -925,7 +935,7 @@ sbappendaddr(struct socket *so, struct sockbuf *sb, const struct sockaddr *asa,
        struct mbuf *m, *n, *nlast;
        int space = asa->sa_len;
 
-       soassertlocked(so);
+       sbmtxassertlocked(so, sb);
 
        if (m0 && (m0->m_flags & M_PKTHDR) == 0)
                panic("sbappendaddr");
index 593bfcf..d5ee300 100644 (file)
@@ -1,4 +1,4 @@
-/*      $OpenBSD: ip_divert.c,v 1.93 2024/02/03 22:50:09 mvs Exp $ */
+/*      $OpenBSD: ip_divert.c,v 1.94 2024/02/11 18:14:26 mvs Exp $ */
 
 /*
  * Copyright (c) 2009 Michele Marchetto <michele@openbsd.org>
@@ -241,14 +241,14 @@ divert_packet(struct mbuf *m, int dir, u_int16_t divert_port)
                in_proto_cksum_out(m, NULL);
        }
 
-       mtx_enter(&inp->inp_mtx);
        so = inp->inp_socket;
+       mtx_enter(&so->so_rcv.sb_mtx);
        if (sbappendaddr(so, &so->so_rcv, sintosa(&sin), m, NULL) == 0) {
-               mtx_leave(&inp->inp_mtx);
+               mtx_leave(&so->so_rcv.sb_mtx);
                divstat_inc(divs_fullsock);
                goto bad;
        }
-       mtx_leave(&inp->inp_mtx);
+       mtx_leave(&so->so_rcv.sb_mtx);
        sorwakeup(so);
 
        in_pcbunref(inp);
index e318493..3e9b00e 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_mroute.c,v 1.140 2023/12/06 09:27:17 bluhm Exp $   */
+/*     $OpenBSD: ip_mroute.c,v 1.141 2024/02/11 18:14:26 mvs Exp $     */
 /*     $NetBSD: ip_mroute.c,v 1.85 2004/04/26 01:31:57 matt Exp $      */
 
 /*
@@ -1051,12 +1051,11 @@ int
 socket_send(struct socket *so, struct mbuf *mm, struct sockaddr_in *src)
 {
        if (so != NULL) {
-               struct inpcb *inp = sotoinpcb(so);
                int ret;
 
-               mtx_enter(&inp->inp_mtx);
+               mtx_enter(&so->so_rcv.sb_mtx);
                ret = sbappendaddr(so, &so->so_rcv, sintosa(src), mm, NULL);
-               mtx_leave(&inp->inp_mtx);
+               mtx_leave(&so->so_rcv.sb_mtx);
 
                if (ret != 0) {
                        sorwakeup(so);
index 5f8047c..0075382 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: raw_ip.c,v 1.155 2024/02/03 22:50:09 mvs Exp $        */
+/*     $OpenBSD: raw_ip.c,v 1.156 2024/02/11 18:14:26 mvs Exp $        */
 /*     $NetBSD: raw_ip.c,v 1.25 1996/02/18 18:58:33 christos Exp $     */
 
 /*
@@ -220,24 +220,24 @@ rip_input(struct mbuf **mp, int *offp, int proto, int af)
                else
                        n = m_copym(m, 0, M_COPYALL, M_NOWAIT);
                if (n != NULL) {
+                       struct socket *so = inp->inp_socket;
                        int ret;
 
                        if (inp->inp_flags & INP_CONTROLOPTS ||
-                           inp->inp_socket->so_options & SO_TIMESTAMP)
+                           so->so_options & SO_TIMESTAMP)
                                ip_savecontrol(inp, &opts, ip, n);
 
-                       mtx_enter(&inp->inp_mtx);
-                       ret = sbappendaddr(inp->inp_socket,
-                           &inp->inp_socket->so_rcv,
+                       mtx_enter(&so->so_rcv.sb_mtx);
+                       ret = sbappendaddr(so, &so->so_rcv,
                            sintosa(&ripsrc), n, opts);
-                       mtx_leave(&inp->inp_mtx);
+                       mtx_leave(&so->so_rcv.sb_mtx);
 
                        if (ret == 0) {
                                /* should notify about lost packet */
                                m_freem(n);
                                m_freem(opts);
                        } else
-                               sorwakeup(inp->inp_socket);
+                               sorwakeup(so);
                }
                in_pcbunref(inp);
        }
index 18b0ae9..65f6a74 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: udp_usrreq.c,v 1.317 2024/02/03 22:50:09 mvs Exp $    */
+/*     $OpenBSD: udp_usrreq.c,v 1.318 2024/02/11 18:14:26 mvs Exp $    */
 /*     $NetBSD: udp_usrreq.c,v 1.28 1996/03/16 23:54:03 christos Exp $ */
 
 /*
@@ -697,15 +697,15 @@ udp_sbappend(struct inpcb *inp, struct mbuf *m, struct ip *ip,
 #endif
        m_adj(m, hlen);
 
-       mtx_enter(&inp->inp_mtx);
+       mtx_enter(&so->so_rcv.sb_mtx);
        if (sbappendaddr(so, &so->so_rcv, srcaddr, m, opts) == 0) {
-               mtx_leave(&inp->inp_mtx);
+               mtx_leave(&so->so_rcv.sb_mtx);
                udpstat_inc(udps_fullsock);
                m_freem(m);
                m_freem(opts);
                return;
        }
-       mtx_leave(&inp->inp_mtx);
+       mtx_leave(&so->so_rcv.sb_mtx);
 
        sorwakeup(so);
 }
index 4b75c3c..c7b3e3e 100644 (file)
@@ -1,4 +1,4 @@
-/*      $OpenBSD: ip6_divert.c,v 1.93 2024/02/11 01:27:45 bluhm Exp $ */
+/*      $OpenBSD: ip6_divert.c,v 1.94 2024/02/11 18:14:27 mvs Exp $ */
 
 /*
  * Copyright (c) 2009 Michele Marchetto <michele@openbsd.org>
@@ -249,14 +249,14 @@ divert6_packet(struct mbuf *m, int dir, u_int16_t divert_port)
                in6_proto_cksum_out(m, NULL);
        }
 
-       mtx_enter(&inp->inp_mtx);
        so = inp->inp_socket;
+       mtx_enter(&so->so_rcv.sb_mtx);
        if (sbappendaddr(so, &so->so_rcv, sin6tosa(&sin6), m, NULL) == 0) {
-               mtx_leave(&inp->inp_mtx);
+               mtx_leave(&so->so_rcv.sb_mtx);
                div6stat_inc(div6s_fullsock);
                goto bad;
        }
-       mtx_leave(&inp->inp_mtx);
+       mtx_leave(&so->so_rcv.sb_mtx);
        sorwakeup(so);
 
        in_pcbunref(inp);
index 759a677..9af9ed5 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip6_mroute.c,v 1.139 2024/02/03 22:50:09 mvs Exp $    */
+/*     $OpenBSD: ip6_mroute.c,v 1.140 2024/02/11 18:14:27 mvs Exp $    */
 /*     $NetBSD: ip6_mroute.c,v 1.59 2003/12/10 09:28:38 itojun Exp $   */
 /*     $KAME: ip6_mroute.c,v 1.45 2001/03/25 08:38:51 itojun Exp $     */
 
@@ -856,17 +856,16 @@ int
 socket6_send(struct socket *so, struct mbuf *mm, struct sockaddr_in6 *src)
 {
        if (so != NULL) {
-               struct inpcb *inp = sotoinpcb(so);
                int ret;
 
-               mtx_enter(&inp->inp_mtx);
+               mtx_enter(&so->so_rcv.sb_mtx);
                ret = sbappendaddr(so, &so->so_rcv, sin6tosa(src), mm, NULL);
-               if (ret != 0)
-                       sorwakeup(so);
-               mtx_leave(&inp->inp_mtx);
+               mtx_leave(&so->so_rcv.sb_mtx);
 
-               if (ret != 0)
+               if (ret != 0) {
+                       sorwakeup(so);
                        return 0;
+               }
        }
        m_freem(mm);
        return -1;
index 4ae5ff2..653c300 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: raw_ip6.c,v 1.180 2024/02/03 22:50:09 mvs Exp $       */
+/*     $OpenBSD: raw_ip6.c,v 1.181 2024/02/11 18:14:27 mvs Exp $       */
 /*     $KAME: raw_ip6.c,v 1.69 2001/03/04 15:55:44 itojun Exp $        */
 
 /*
@@ -263,6 +263,7 @@ rip6_input(struct mbuf **mp, int *offp, int proto, int af)
                else
                        n = m_copym(m, 0, M_COPYALL, M_NOWAIT);
                if (n != NULL) {
+                       struct socket *so = inp->inp_socket;
                        int ret;
 
                        if (inp->inp_flags & IN6P_CONTROLOPTS)
@@ -270,11 +271,10 @@ rip6_input(struct mbuf **mp, int *offp, int proto, int af)
                        /* strip intermediate headers */
                        m_adj(n, *offp);
 
-                       mtx_enter(&inp->inp_mtx);
-                       ret = sbappendaddr(inp->inp_socket,
-                           &inp->inp_socket->so_rcv,
+                       mtx_enter(&so->so_rcv.sb_mtx);
+                       ret = sbappendaddr(so, &so->so_rcv,
                            sin6tosa(&rip6src), n, opts);
-                       mtx_leave(&inp->inp_mtx);
+                       mtx_leave(&so->so_rcv.sb_mtx);
 
                        if (ret == 0) {
                                /* should notify about lost packet */
@@ -282,7 +282,7 @@ rip6_input(struct mbuf **mp, int *offp, int proto, int af)
                                m_freem(opts);
                                rip6stat_inc(rip6s_fullsock);
                        } else
-                               sorwakeup(inp->inp_socket);
+                               sorwakeup(so);
                }
                in_pcbunref(inp);
        }
index f57773e..f1922f6 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: socketvar.h,v 1.122 2024/02/03 22:50:09 mvs Exp $     */
+/*     $OpenBSD: socketvar.h,v 1.123 2024/02/11 18:14:27 mvs Exp $     */
 /*     $NetBSD: socketvar.h,v 1.18 1996/02/09 18:25:38 christos Exp $  */
 
 /*-
@@ -134,6 +134,7 @@ struct socket {
 #define        SB_ASYNC        0x10            /* ASYNC I/O, need signals */
 #define        SB_SPLICE       0x20            /* buffer is splice source or drain */
 #define        SB_NOINTR       0x40            /* operations not interruptible */
+#define SB_MTXLOCK     0x80            /* use sb_mtx for sockbuf protection */
 
        void    (*so_upcall)(struct socket *so, caddr_t arg, int waitf);
        caddr_t so_upcallarg;           /* Arg for above */
@@ -197,6 +198,22 @@ sorele(struct socket *so)
 #define isspliced(so)          ((so)->so_sp && (so)->so_sp->ssp_socket)
 #define issplicedback(so)      ((so)->so_sp && (so)->so_sp->ssp_soback)
 
+static inline void
+sb_mtx_lock(struct sockbuf *sb)
+{
+       if (sb->sb_flags & SB_MTXLOCK)
+               mtx_enter(&sb->sb_mtx);
+}
+
+static inline void
+sb_mtx_unlock(struct sockbuf *sb)
+{
+       if (sb->sb_flags & SB_MTXLOCK)
+               mtx_leave(&sb->sb_mtx);
+}
+
+void   sbmtxassertlocked(struct socket *so, struct sockbuf *);
+
 /*
  * Do we need to notify the other side when I/O is possible?
  */