From: Alexander Bluhm Subject: Re: sysctl(2): unlock divert{,6}_sysctl() To: Vitaliy Makkoveev Cc: tech@openbsd.org Date: Thu, 15 Aug 2024 16:42:06 +0200 On Thu, Aug 15, 2024 at 05:22:12PM +0300, Vitaliy Makkoveev wrote: > Introduce PR_MPSYSCTL flag to mark mp-safe (*pr_sysctl)() handlers and > unlock both divert_sysctl() and divert6_sysctl(). Unlock them together, > because they are identical and pretty simple: > > - DIVERTCTL_RECVSPACE and DIVERTCTL_SENDSPACE - atomically accessed > integers; > - DIVERTCTL_STATS - per-CPU counters; OK bluhm@ > Index: sys/kern/uipc_domain.c > =================================================================== > RCS file: /cvs/src/sys/kern/uipc_domain.c,v > diff -u -p -r1.67 uipc_domain.c > --- sys/kern/uipc_domain.c 14 Aug 2024 17:52:47 -0000 1.67 > +++ sys/kern/uipc_domain.c 15 Aug 2024 13:21:11 -0000 > @@ -237,14 +237,18 @@ net_sysctl(int *name, u_int namelen, voi > protocol = name[1]; > for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++) > if (pr->pr_protocol == protocol && pr->pr_sysctl) { > - size_t savelen = *oldlenp; > + size_t savelen; > int error; > > - if ((error = sysctl_vslock(oldp, savelen))) > - return (error); > + if ((pr->pr_flags & PR_MPSYSCTL) == 0) { > + savelen = *oldlenp; > + if ((error = sysctl_vslock(oldp, savelen))) > + return (error); > + } > error = (*pr->pr_sysctl)(name + 2, namelen - 2, > oldp, oldlenp, newp, newlen); > - sysctl_vsunlock(oldp, savelen); > + if ((pr->pr_flags & PR_MPSYSCTL) == 0) > + sysctl_vsunlock(oldp, savelen); > > return (error); > } > Index: sys/netinet/in_proto.c > =================================================================== > RCS file: /cvs/src/sys/netinet/in_proto.c,v > diff -u -p -r1.107 in_proto.c > --- sys/netinet/in_proto.c 26 Jul 2024 14:38:20 -0000 1.107 > +++ sys/netinet/in_proto.c 15 Aug 2024 13:21:12 -0000 > @@ -354,7 +354,7 @@ const struct protosw inetsw[] = { > .pr_type = SOCK_RAW, > .pr_domain = &inetdomain, > .pr_protocol = IPPROTO_DIVERT, > - .pr_flags = PR_ATOMIC|PR_ADDR|PR_MPSOCKET, > + .pr_flags = PR_ATOMIC|PR_ADDR|PR_MPSOCKET|PR_MPSYSCTL, > .pr_ctloutput = rip_ctloutput, > .pr_usrreqs = &divert_usrreqs, > .pr_init = divert_init, > Index: sys/netinet/ip_divert.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_divert.c,v > diff -u -p -r1.96 ip_divert.c > --- sys/netinet/ip_divert.c 12 Jul 2024 19:50:35 -0000 1.96 > +++ sys/netinet/ip_divert.c 15 Aug 2024 13:21:12 -0000 > @@ -41,17 +41,22 @@ > > #include > > +/* > + * Locks used to protect data: > + * a atomic > + */ > + > struct inpcbtable divbtable; > struct cpumem *divcounters; > > #ifndef DIVERT_SENDSPACE > #define DIVERT_SENDSPACE (65536 + 100) > #endif > -u_int divert_sendspace = DIVERT_SENDSPACE; > +u_int divert_sendspace = DIVERT_SENDSPACE; /* [a] */ > #ifndef DIVERT_RECVSPACE > #define DIVERT_RECVSPACE (65536 + 100) > #endif > -u_int divert_recvspace = DIVERT_RECVSPACE; > +u_int divert_recvspace = DIVERT_RECVSPACE; /* [a] */ > > #ifndef DIVERTHASHSIZE > #define DIVERTHASHSIZE 128 > @@ -271,7 +276,8 @@ divert_attach(struct socket *so, int pro > if (error) > return error; > > - error = soreserve(so, divert_sendspace, divert_recvspace); > + error = soreserve(so, atomic_load_int(&divert_sendspace), > + atomic_load_int(&divert_recvspace)); > if (error) > return error; > > @@ -346,8 +352,6 @@ int > divert_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, > size_t newlen) > { > - int error; > - > /* All sysctl names at this level are terminal. */ > if (namelen != 1) > return (ENOTDIR); > @@ -356,12 +360,9 @@ divert_sysctl(int *name, u_int namelen, > case DIVERTCTL_STATS: > return (divert_sysctl_divstat(oldp, oldlenp, newp)); > default: > - NET_LOCK(); > - error = sysctl_bounded_arr(divertctl_vars, > - nitems(divertctl_vars), name, namelen, oldp, oldlenp, newp, > - newlen); > - NET_UNLOCK(); > - return (error); > + return (sysctl_bounded_arr(divertctl_vars, > + nitems(divertctl_vars), name, namelen, oldp, oldlenp, > + newp, newlen)); > } > /* NOTREACHED */ > } > Index: sys/netinet6/in6_proto.c > =================================================================== > RCS file: /cvs/src/sys/netinet6/in6_proto.c,v > diff -u -p -r1.117 in6_proto.c > --- sys/netinet6/in6_proto.c 26 Jul 2024 14:38:20 -0000 1.117 > +++ sys/netinet6/in6_proto.c 15 Aug 2024 13:21:12 -0000 > @@ -289,7 +289,7 @@ const struct protosw inet6sw[] = { > .pr_type = SOCK_RAW, > .pr_domain = &inet6domain, > .pr_protocol = IPPROTO_DIVERT, > - .pr_flags = PR_ATOMIC|PR_ADDR|PR_MPSOCKET, > + .pr_flags = PR_ATOMIC|PR_ADDR|PR_MPSOCKET|PR_MPSYSCTL, > .pr_ctloutput = rip6_ctloutput, > .pr_usrreqs = &divert6_usrreqs, > .pr_init = divert6_init, > Index: sys/netinet6/ip6_divert.c > =================================================================== > RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v > diff -u -p -r1.96 ip6_divert.c > --- sys/netinet6/ip6_divert.c 12 Jul 2024 19:50:35 -0000 1.96 > +++ sys/netinet6/ip6_divert.c 15 Aug 2024 13:21:12 -0000 > @@ -44,17 +44,22 @@ > > #include > > +/* > + * Locks used to protect data: > + * a atomic > + */ > + > struct inpcbtable divb6table; > struct cpumem *div6counters; > > #ifndef DIVERT_SENDSPACE > #define DIVERT_SENDSPACE (65536 + 100) > #endif > -u_int divert6_sendspace = DIVERT_SENDSPACE; > +u_int divert6_sendspace = DIVERT_SENDSPACE; /* [a] */ > #ifndef DIVERT_RECVSPACE > #define DIVERT_RECVSPACE (65536 + 100) > #endif > -u_int divert6_recvspace = DIVERT_RECVSPACE; > +u_int divert6_recvspace = DIVERT_RECVSPACE; /* [a] */ > > #ifndef DIVERTHASHSIZE > #define DIVERTHASHSIZE 128 > @@ -279,7 +284,8 @@ divert6_attach(struct socket *so, int pr > if (error) > return (error); > > - error = soreserve(so, divert6_sendspace, divert6_recvspace); > + error = soreserve(so, atomic_load_int(&divert6_sendspace), > + atomic_load_int(&divert6_recvspace)); > if (error) > return (error); > > @@ -322,8 +328,6 @@ int > divert6_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, > void *newp, size_t newlen) > { > - int error; > - > /* All sysctl names at this level are terminal. */ > if (namelen != 1) > return (ENOTDIR); > @@ -332,12 +336,9 @@ divert6_sysctl(int *name, u_int namelen, > case DIVERT6CTL_STATS: > return (divert6_sysctl_div6stat(oldp, oldlenp, newp)); > default: > - NET_LOCK(); > - error = sysctl_bounded_arr(divert6ctl_vars, > + return (sysctl_bounded_arr(divert6ctl_vars, > nitems(divert6ctl_vars), name, namelen, oldp, oldlenp, > - newp, newlen); > - NET_UNLOCK(); > - return (error); > + newp, newlen)); > } > /* NOTREACHED */ > } > Index: sys/sys/protosw.h > =================================================================== > RCS file: /cvs/src/sys/sys/protosw.h,v > diff -u -p -r1.67 protosw.h > --- sys/sys/protosw.h 12 Jul 2024 19:50:35 -0000 1.67 > +++ sys/sys/protosw.h 15 Aug 2024 13:21:12 -0000 > @@ -131,6 +131,7 @@ struct protosw { > #define PR_SPLICE 0x0040 /* socket splicing is possible */ > #define PR_MPINPUT 0x0080 /* input runs with shared netlock */ > #define PR_MPSOCKET 0x0100 /* socket uses shared netlock */ > +#define PR_MPSYSCTL 0x0200 /* mp-safe sysctl(2) handler */ > > /* > * The arguments to usrreq are: