On Sat, Aug 13, 2022 at 12:04:57PM +0200, Alexander Bluhm wrote:
> On Mon, Aug 08, 2022 at 03:48:57PM +0300, Vitaliy Makkoveev wrote:
> > We use if (error != 0) or if (m == NULL) idioms in the network stack, so
> > I used them too in the trivial places.
>
> I nearly got them consistent in netinet/netinet6
>
> $ grep 'if (error)' netinet*/* | wc
> 106 347 3398
> $ grep 'if (error != 0)' netinet*/* | wc
> 5 25 182
>
No problem, personally I prefer "if (error)" style.
> > Also I like to hide ugly "(*so->so_proto->pr_usrreqs->pru_usrreq)" calls
> > within corresponding pru_*() wrappers.
>
> That makes code more readable. It helps to see which parameters
> are relevant.
>
[skip]
> > So, for the first step, just move existing user requests handlers
> > pointers.
>
> These are two mechanical steps. pru_*() wrappers and pr_usrreqs.
> Only one of them would be easier to review. But anyway, I looked
> at both of them.
>
I propose to commit pru_*() hunks first.
>
> > +static inline int
> > +pru_attach(const struct protosw *prp, struct socket *so, int proto)
> > +{
> > + return (*prp->pr_usrreqs->pru_attach)(so, proto);
> > +}
>
> All callers have so->so_proto == prp. So you can avoid the prp
> parameter and use so->so_proto instead.
>
Fixed.
> > +static inline int
> > +pru_sense(struct socket *so, struct stat *ub)
> > +{
> > + return (*so->so_proto->pr_usrreqs->pru_usrreq)(so,
> > + PRU_SENSE, (struct mbuf *)ub, NULL, NULL, curproc);
> > +}
>
> > +static inline int
> > +pru_sockaddr(struct socket *so, struct mbuf *addr)
> > +{
> > + return (*so->so_proto->pr_usrreqs->pru_usrreq)(so,
> > + PRU_SOCKADDR, NULL, addr, NULL, curproc);
> > +}
> > +
> > +static inline int
> > +pru_peeraddr(struct socket *so, struct mbuf *addr)
> > +{
> > + return (*so->so_proto->pr_usrreqs->pru_usrreq)(so,
> > + PRU_PEERADDR, NULL, addr, NULL, curproc);
> > +}
>
> pru_sense, pru_sockaddr, pru_peeraddr used struct proc *p, you
> changed it to curproc. Maybe they are always the same, but this
> change should not be hidden in such a large diff.
>
Actually, the existing callers pass the `curproc' to PRU_SENSE,
PRU_SOCKADDR and PRU_PEERADDR requests, but no one of existing sockets
uses passed 'proc *' arg for them. We could use NULL instead of
`curproc', but I want to be conservative.
> > --- sys/sys/socketvar.h 15 Jul 2022 17:20:24 -0000 1.106
> > +++ sys/sys/socketvar.h 8 Aug 2022 12:42:41 -0000
> > @@ -32,6 +32,9 @@
> > * @(#)socketvar.h 8.1 (Berkeley) 6/2/93
> > */
> >
> > +#ifndef _SYS_SOCKETVAR_H_
> > +#define _SYS_SOCKETVAR_H_
> > +
> > #include <sys/selinfo.h> /* for struct selinfo */
> > #include <sys/queue.h>
> > #include <sys/sigio.h> /* for struct sigio_ref
> > */
> > @@ -370,3 +373,5 @@ void sbcheck(struct socket *, struct soc
> > #endif /* SOCKBUF_DEBUG */
> >
> > #endif /* _KERNEL */
> > +
> > +#endif /* _SYS_SOCKETVAR_H_ */
>
> Is _SYS_SOCKETVAR_H_ related to this diff?
This hunk is required when sys/socketvar.h and sys/protosw.h included
together or when they are both included through other headers. We need
to have 'socket' structure definition before pru_*() wrappers. We could
move pru_*() wrappers to sys/socketvar.h, but it also need to include
sys/protosw.h because it's required to have 'protosw' structure
definition too.
Index: sys/kern/sys_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/sys_socket.c,v
retrieving revision 1.51
diff -u -p -r1.51 sys_socket.c
--- sys/kern/sys_socket.c 20 Jun 2022 01:39:44 -0000 1.51
+++ sys/kern/sys_socket.c 13 Aug 2022 18:26:39 -0000
@@ -138,8 +138,7 @@ soo_ioctl(struct file *fp, u_long cmd, c
if (IOCGROUP(cmd) == 'r')
return (EOPNOTSUPP);
KERNEL_LOCK();
- error = ((*so->so_proto->pr_usrreq)(so, PRU_CONTROL,
- (struct mbuf *)cmd, (struct mbuf *)data, NULL, p));
+ error = pru_control(so, cmd, data, NULL, p);
KERNEL_UNLOCK();
break;
}
@@ -161,8 +160,7 @@ soo_stat(struct file *fp, struct stat *u
ub->st_mode |= S_IWUSR | S_IWGRP | S_IWOTH;
ub->st_uid = so->so_euid;
ub->st_gid = so->so_egid;
- (void) ((*so->so_proto->pr_usrreq)(so, PRU_SENSE,
- (struct mbuf *)ub, NULL, NULL, p));
+ (void)pru_sense(so, ub);
sounlock(so);
return (0);
}
Index: sys/kern/uipc_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.280
diff -u -p -r1.280 uipc_socket.c
--- sys/kern/uipc_socket.c 25 Jul 2022 07:28:22 -0000 1.280
+++ sys/kern/uipc_socket.c 13 Aug 2022 18:26:39 -0000
@@ -195,7 +195,7 @@ socreate(int dom, struct socket **aso, i
so->so_rcv.sb_timeo_nsecs = INFSLP;
solock(so);
- error = (*prp->pr_attach)(so, proto);
+ error = pru_attach(so, proto);
if (error) {
so->so_state |= SS_NOFDREF;
/* sofree() calls sounlock(). */
@@ -210,12 +210,8 @@ socreate(int dom, struct socket **aso, i
int
sobind(struct socket *so, struct mbuf *nam, struct proc *p)
{
- int error;
-
soassertlocked(so);
-
- error = (*so->so_proto->pr_usrreq)(so, PRU_BIND, NULL, nam, NULL, p);
- return (error);
+ return pru_bind(so, nam, p);
}
int
@@ -231,8 +227,7 @@ solisten(struct socket *so, int backlog)
if (isspliced(so) || issplicedback(so))
return (EOPNOTSUPP);
#endif /* SOCKET_SPLICE */
- error = (*so->so_proto->pr_usrreq)(so, PRU_LISTEN, NULL, NULL, NULL,
- curproc);
+ error = pru_listen(so);
if (error)
return (error);
if (TAILQ_FIRST(&so->so_q) == NULL)
@@ -392,8 +387,7 @@ soclose(struct socket *so, int flags)
drop:
if (so->so_pcb) {
int error2;
- KASSERT(so->so_proto->pr_detach);
- error2 = (*so->so_proto->pr_detach)(so);
+ error2 = pru_detach(so);
if (error == 0)
error = error2;
}
@@ -444,8 +438,7 @@ soabort(struct socket *so)
{
soassertlocked(so);
- return (*so->so_proto->pr_usrreq)(so, PRU_ABORT, NULL, NULL, NULL,
- curproc);
+ return pru_abort(so);
}
int
@@ -460,8 +453,7 @@ soaccept(struct socket *so, struct mbuf
so->so_state &= ~SS_NOFDREF;
if ((so->so_state & SS_ISDISCONNECTED) == 0 ||
(so->so_proto->pr_flags & PR_ABRTACPTDIS) == 0)
- error = (*so->so_proto->pr_usrreq)(so, PRU_ACCEPT, NULL,
- nam, NULL, curproc);
+ error = pru_accept(so, nam);
else
error = ECONNABORTED;
return (error);
@@ -487,8 +479,7 @@ soconnect(struct socket *so, struct mbuf
(error = sodisconnect(so))))
error = EISCONN;
else
- error = (*so->so_proto->pr_usrreq)(so, PRU_CONNECT,
- NULL, nam, NULL, curproc);
+ error = pru_connect(so, nam);
return (error);
}
@@ -502,8 +493,7 @@ soconnect2(struct socket *so1, struct so
else
solock(so1);
- error = (*so1->so_proto->pr_usrreq)(so1, PRU_CONNECT2, NULL,
- (struct mbuf *)so2, NULL, curproc);
+ error = pru_connect2(so1, so2);
if (persocket)
sounlock(so2);
@@ -522,8 +512,7 @@ sodisconnect(struct socket *so)
return (ENOTCONN);
if (so->so_state & SS_ISDISCONNECTING)
return (EALREADY);
- error = (*so->so_proto->pr_usrreq)(so, PRU_DISCONNECT, NULL, NULL,
- NULL, curproc);
+ error = pru_disconnect(so);
return (error);
}
@@ -654,9 +643,10 @@ restart:
so->so_state &= ~SS_ISSENDING;
if (top && so->so_options & SO_ZEROIZE)
top->m_flags |= M_ZEROIZE;
- error = (*so->so_proto->pr_usrreq)(so,
- (flags & MSG_OOB) ? PRU_SENDOOB : PRU_SEND,
- top, addr, control, curproc);
+ if (flags & MSG_OOB)
+ error = pru_sendoob(so, top, addr, control);
+ else
+ error = pru_send(so, top, addr, control);
clen = 0;
control = NULL;
top = NULL;
@@ -819,8 +809,7 @@ soreceive(struct socket *so, struct mbuf
if (flags & MSG_OOB) {
m = m_get(M_WAIT, MT_DATA);
solock(so);
- error = (*pr->pr_usrreq)(so, PRU_RCVOOB, m,
- (struct mbuf *)(long)(flags & MSG_PEEK), NULL, curproc);
+ error = pru_rcvoob(so, m, flags & MSG_PEEK);
sounlock(so);
if (error)
goto bad;
@@ -1170,8 +1159,7 @@ dontblock:
SBLASTRECORDCHK(&so->so_rcv, "soreceive 4");
SBLASTMBUFCHK(&so->so_rcv, "soreceive 4");
if (pr->pr_flags & PR_WANTRCVD && so->so_pcb)
- (*pr->pr_usrreq)(so, PRU_RCVD, NULL,
- (struct mbuf *)(long)flags, NULL, curproc);
+ pru_rcvd(so, flags);
}
if (orig_resid == uio->uio_resid && orig_resid &&
(flags & MSG_EOR) == 0 && (so->so_state & SS_CANTRCVMORE) == 0) {
@@ -1193,7 +1181,6 @@ release:
int
soshutdown(struct socket *so, int how)
{
- const struct protosw *pr = so->so_proto;
int error = 0;
solock(so);
@@ -1205,8 +1192,7 @@ soshutdown(struct socket *so, int how)
sorflush(so);
/* FALLTHROUGH */
case SHUT_WR:
- error = (*pr->pr_usrreq)(so, PRU_SHUTDOWN, NULL, NULL, NULL,
- curproc);
+ error = pru_shutdown(so);
break;
default:
error = EINVAL;
@@ -1538,8 +1524,7 @@ somove(struct socket *so, int wait)
if (m == NULL) {
sbdroprecord(so, &so->so_rcv);
if (so->so_proto->pr_flags & PR_WANTRCVD && so->so_pcb)
- (so->so_proto->pr_usrreq)(so, PRU_RCVD, NULL,
- NULL, NULL, NULL);
+ pru_rcvd(so, 0);
goto nextpkt;
}
@@ -1645,8 +1630,7 @@ somove(struct socket *so, int wait)
/* Send window update to source peer as receive buffer has changed. */
if (so->so_proto->pr_flags & PR_WANTRCVD && so->so_pcb)
- (so->so_proto->pr_usrreq)(so, PRU_RCVD, NULL,
- NULL, NULL, NULL);
+ pru_rcvd(so, 0);
/* Receive buffer did shrink by len bytes, adjust oob. */
state = so->so_state;
@@ -1674,8 +1658,7 @@ somove(struct socket *so, int wait)
} else if (oobmark) {
o = m_split(m, oobmark, wait);
if (o) {
- error = (*sosp->so_proto->pr_usrreq)(sosp,
- PRU_SEND, m, NULL, NULL, NULL);
+ error = pru_send(sosp, m, NULL, NULL);
if (error) {
if (sosp->so_state & SS_CANTSENDMORE)
error = EPIPE;
@@ -1692,8 +1675,7 @@ somove(struct socket *so, int wait)
if (o) {
o->m_len = 1;
*mtod(o, caddr_t) = *mtod(m, caddr_t);
- error = (*sosp->so_proto->pr_usrreq)(sosp, PRU_SENDOOB,
- o, NULL, NULL, NULL);
+ error = pru_sendoob(sosp, o, NULL, NULL);
if (error) {
if (sosp->so_state & SS_CANTSENDMORE)
error = EPIPE;
@@ -1714,8 +1696,7 @@ somove(struct socket *so, int wait)
/* Append all remaining data to drain socket. */
if (so->so_rcv.sb_cc == 0 || maxreached)
sosp->so_state &= ~SS_ISSENDING;
- error = (*sosp->so_proto->pr_usrreq)(sosp, PRU_SEND, m, NULL, NULL,
- NULL);
+ error = pru_send(sosp, m, NULL, NULL);
if (error) {
if (sosp->so_state & SS_CANTSENDMORE)
error = EPIPE;
Index: sys/kern/uipc_socket2.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.126
diff -u -p -r1.126 uipc_socket2.c
--- sys/kern/uipc_socket2.c 25 Jul 2022 07:28:22 -0000 1.126
+++ sys/kern/uipc_socket2.c 13 Aug 2022 18:26:39 -0000
@@ -238,7 +238,7 @@ sonewconn(struct socket *head, int conns
sounlock(head);
}
- error = (*so->so_proto->pr_attach)(so, 0);
+ error = pru_attach(so, 0);
if (persocket) {
sounlock(so);
Index: sys/kern/uipc_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.199
diff -u -p -r1.199 uipc_syscalls.c
--- sys/kern/uipc_syscalls.c 18 Jul 2022 04:42:37 -0000 1.199
+++ sys/kern/uipc_syscalls.c 13 Aug 2022 18:26:40 -0000
@@ -1100,7 +1100,7 @@ sys_getsockname(struct proc *p, void *v,
}
m = m_getclr(M_WAIT, MT_SONAME);
solock(so);
- error = (*so->so_proto->pr_usrreq)(so, PRU_SOCKADDR, NULL, m, NULL, p);
+ error = pru_sockaddr(so, m);
sounlock(so);
if (error)
goto bad;
@@ -1147,7 +1147,7 @@ sys_getpeername(struct proc *p, void *v,
goto bad;
m = m_getclr(M_WAIT, MT_SONAME);
solock(so);
- error = (*so->so_proto->pr_usrreq)(so, PRU_PEERADDR, NULL, m, NULL, p);
+ error = pru_peeraddr(so, m);
sounlock(so);
if (error)
goto bad;
Index: sys/net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.662
diff -u -p -r1.662 if.c
--- sys/net/if.c 6 Aug 2022 15:57:58 -0000 1.662
+++ sys/net/if.c 13 Aug 2022 18:26:40 -0000
@@ -2360,9 +2360,7 @@ forceup:
break;
/* FALLTHROUGH */
default:
- error = ((*so->so_proto->pr_usrreq)(so, PRU_CONTROL,
- (struct mbuf *) cmd, (struct mbuf *) data,
- (struct mbuf *) ifp, p));
+ error = pru_control(so, cmd, data, ifp, p);
if (error != EOPNOTSUPP)
break;
switch (cmd) {
Index: sys/nfs/nfs_socket.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_socket.c,v
retrieving revision 1.142
diff -u -p -r1.142 nfs_socket.c
--- sys/nfs/nfs_socket.c 6 Jun 2022 14:45:41 -0000 1.142
+++ sys/nfs/nfs_socket.c 13 Aug 2022 18:26:40 -0000
@@ -1177,11 +1177,9 @@ nfs_timer(void *arg)
nmp->nm_sent < nmp->nm_cwnd) &&
(m = m_copym(rep->r_mreq, 0, M_COPYALL, M_DONTWAIT))){
if ((nmp->nm_flag & NFSMNT_NOCONN) == 0)
- error = (*so->so_proto->pr_usrreq)(so, PRU_SEND,
- m, NULL, NULL, curproc);
+ error = pru_send(so, m, NULL, NULL);
else
- error = (*so->so_proto->pr_usrreq)(so, PRU_SEND,
- m, nmp->nm_nam, NULL, curproc);
+ error = pru_send(so, m, nmp->nm_nam, NULL);
if (error) {
if (NFSIGNORE_SOERROR(nmp->nm_soflags, error))
so->so_error = 0;
Index: sys/sys/protosw.h
===================================================================
RCS file: /cvs/src/sys/sys/protosw.h,v
retrieving revision 1.35
diff -u -p -r1.35 protosw.h
--- sys/sys/protosw.h 25 Feb 2022 23:51:04 -0000 1.35
+++ sys/sys/protosw.h 13 Aug 2022 18:26:40 -0000
@@ -227,6 +227,11 @@ char *prcorequests[] = {
#endif
#ifdef _KERNEL
+
+#include <sys/socketvar.h>
+#include <sys/systm.h>
+
+struct ifnet;
struct sockaddr;
const struct protosw *pffindproto(int, int, int);
const struct protosw *pffindtype(int, int);
@@ -239,5 +244,133 @@ extern const struct protosw inetsw[];
extern u_char ip6_protox[];
extern const struct protosw inet6sw[];
#endif /* INET6 */
+
+static inline int
+pru_attach(struct socket *so, int proto)
+{
+ return (*so->so_proto->pr_attach)(so, proto);
+}
+
+static inline int
+pru_detach(struct socket *so)
+{
+ return (*so->so_proto->pr_detach)(so);
+}
+
+static inline int
+pru_bind(struct socket *so, struct mbuf *nam, struct proc *p)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_BIND, NULL, nam, NULL, p);
+}
+
+static inline int
+pru_listen(struct socket *so)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_LISTEN, NULL, NULL, NULL, curproc);
+}
+
+static inline int
+pru_connect(struct socket *so, struct mbuf *nam)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_CONNECT, NULL, nam, NULL, curproc);
+}
+
+static inline int
+pru_accept(struct socket *so, struct mbuf *nam)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_ACCEPT, NULL, nam, NULL, curproc);
+}
+
+static inline int
+pru_disconnect(struct socket *so)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_DISCONNECT, NULL, NULL, NULL, curproc);
+}
+
+static inline int
+pru_shutdown(struct socket *so)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_SHUTDOWN, NULL, NULL, NULL, curproc);
+}
+
+static inline int
+pru_rcvd(struct socket *so, int flags)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_RCVD, NULL, (struct mbuf *)(long)flags, NULL, curproc);
+}
+
+static inline int
+pru_send(struct socket *so, struct mbuf *top, struct mbuf *addr,
+ struct mbuf *control)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_SEND, top, addr, control, curproc);
+}
+
+static inline int
+pru_abort(struct socket *so)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_ABORT, NULL, NULL, NULL, curproc);
+}
+
+static inline int
+pru_control(struct socket *so, u_long cmd, caddr_t data,
+ struct ifnet *ifp, struct proc *p)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_CONTROL, (struct mbuf *)cmd, (struct mbuf *)data,
+ (struct mbuf *)ifp, p);
+}
+
+static inline int
+pru_sense(struct socket *so, struct stat *ub)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_SENSE, (struct mbuf *)ub, NULL, NULL, curproc);
+}
+
+static inline int
+pru_rcvoob(struct socket *so, struct mbuf *m, int flags)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_RCVOOB, m, (struct mbuf *)(long)flags, NULL, curproc);
+}
+
+static inline int
+pru_sendoob(struct socket *so, struct mbuf *top, struct mbuf *addr,
+ struct mbuf *control)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_SENDOOB, top, addr, control, curproc);
+}
+
+static inline int
+pru_sockaddr(struct socket *so, struct mbuf *addr)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_SOCKADDR, NULL, addr, NULL, curproc);
+}
+
+static inline int
+pru_peeraddr(struct socket *so, struct mbuf *addr)
+{
+ return (*so->so_proto->pr_usrreq)(so,
+ PRU_PEERADDR, NULL, addr, NULL, curproc);
+}
+
+static inline int
+pru_connect2(struct socket *so1, struct socket *so2)
+{
+ return (*so1->so_proto->pr_usrreq)(so1,
+ PRU_CONNECT2, NULL, (struct mbuf *)so2, NULL, curproc);
+}
#endif
Index: sys/sys/socketvar.h
===================================================================
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.106
diff -u -p -r1.106 socketvar.h
--- sys/sys/socketvar.h 15 Jul 2022 17:20:24 -0000 1.106
+++ sys/sys/socketvar.h 13 Aug 2022 18:26:40 -0000
@@ -32,6 +32,9 @@
* @(#)socketvar.h 8.1 (Berkeley) 6/2/93
*/
+#ifndef _SYS_SOCKETVAR_H_
+#define _SYS_SOCKETVAR_H_
+
#include <sys/selinfo.h> /* for struct selinfo */
#include <sys/queue.h>
#include <sys/sigio.h> /* for struct sigio_ref */
@@ -370,3 +373,5 @@ void sbcheck(struct socket *, struct soc
#endif /* SOCKBUF_DEBUG */
#endif /* _KERNEL */
+
+#endif /* _SYS_SOCKETVAR_H_ */
