On Mon, Aug 22, 2022 at 11:08:07PM -0900, Philip Guenther wrote:
> Since pru_rcvd() is only invoked if the protocol has the PR_WANTRCVD flag
> set, there should be no need to test whether the callback is set: a
> protocol without the callback MUST NOT have PR_WANTRCVD.
>
> (I guess this could, alternatively, go the other direction and eliminate
> PR_WANTRCVD and use the presence of the callback to decide whether the
> protocol needs anything to be done.)
I don't want this. At least the per buffer locking could require relock
in the PRU_RCVD path, and I don't to do it within handler or pru_rcvd()
wrapper.
>
> Side note: pru_rcvd() (and the pru_rcvd implementations) should have a
> return type of void.
>
Since the TCP socket could exist without PCB, and we want to return
error in this case, all callbacks should return int. In other hand, we
always do `so_pcb' NULL check before call pru_rcvd() and we don't
interesting in the pru_rcvd() return value.
Also PRU_RCVD is not the only request where return value type could
be changed to void, at least PRU_SHUTDOWN handlers have no error path,
except the same case for TCP sockets.
Anyway, as I said in the one of preceding split diff for unix sockets
case, this time I want only split existing (*pru_usrreq)() handlers, and
leave possible refactoring to the future. The split diffs are mostly
mechanical, but huge enough, and I don't want to make them harder.
So I want to commit this as is.
>
> Philip Guenther
>
>
>
> On Mon, Aug 22, 2022 at 1:40 PM Vitaliy Makkoveev <m...@openbsd.org> wrote:
>
> > Another one.
> >
> > Since we never use `flags' arg within handlers, remove it from the
> > pru_rcvd() args.
> >
> > Index: sys/sys/protosw.h
> > ===================================================================
> > RCS file: /cvs/src/sys/sys/protosw.h,v
> > retrieving revision 1.43
> > diff -u -p -r1.43 protosw.h
> > --- sys/sys/protosw.h 22 Aug 2022 21:18:48 -0000 1.43
> > +++ sys/sys/protosw.h 22 Aug 2022 22:27:08 -0000
> > @@ -72,6 +72,7 @@ struct pr_usrreqs {
> > int (*pru_accept)(struct socket *, struct mbuf *);
> > int (*pru_disconnect)(struct socket *);
> > int (*pru_shutdown)(struct socket *);
> > + int (*pru_rcvd)(struct socket *);
> > };
> >
> > struct protosw {
> > @@ -314,10 +315,11 @@ pru_shutdown(struct socket *so)
> > }
> >
> > static inline int
> > -pru_rcvd(struct socket *so, int flags)
> > +pru_rcvd(struct socket *so)
> > {
> > - return (*so->so_proto->pr_usrreqs->pru_usrreq)(so,
> > - PRU_RCVD, NULL, (struct mbuf *)(long)flags, NULL, curproc);
> > + if (so->so_proto->pr_usrreqs->pru_rcvd)
> > + return (*so->so_proto->pr_usrreqs->pru_rcvd)(so);
> > + return (EOPNOTSUPP);
> > }
> >
> > static inline int
> > Index: sys/kern/uipc_socket.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> > retrieving revision 1.284
> > diff -u -p -r1.284 uipc_socket.c
> > --- sys/kern/uipc_socket.c 21 Aug 2022 16:22:17 -0000 1.284
> > +++ sys/kern/uipc_socket.c 22 Aug 2022 22:27:08 -0000
> > @@ -1156,7 +1156,7 @@ dontblock:
> > SBLASTRECORDCHK(&so->so_rcv, "soreceive 4");
> > SBLASTMBUFCHK(&so->so_rcv, "soreceive 4");
> > if (pr->pr_flags & PR_WANTRCVD && so->so_pcb)
> > - pru_rcvd(so, flags);
> > + pru_rcvd(so);
> > }
> > if (orig_resid == uio->uio_resid && orig_resid &&
> > (flags & MSG_EOR) == 0 && (so->so_state & SS_CANTRCVMORE) ==
> > 0) {
> > @@ -1521,7 +1521,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)
> > - pru_rcvd(so, 0);
> > + pru_rcvd(so);
> > goto nextpkt;
> > }
> >
> > @@ -1627,7 +1627,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)
> > - pru_rcvd(so, 0);
> > + pru_rcvd(so);
> >
> > /* Receive buffer did shrink by len bytes, adjust oob. */
> > state = so->so_state;
> > Index: sys/kern/uipc_usrreq.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> > retrieving revision 1.174
> > diff -u -p -r1.174 uipc_usrreq.c
> > --- sys/kern/uipc_usrreq.c 22 Aug 2022 21:18:48 -0000 1.174
> > +++ sys/kern/uipc_usrreq.c 22 Aug 2022 22:27:08 -0000
> > @@ -136,6 +136,7 @@ const struct pr_usrreqs uipc_usrreqs = {
> > .pru_accept = uipc_accept,
> > .pru_disconnect = uipc_disconnect,
> > .pru_shutdown = uipc_shutdown,
> > + .pru_rcvd = uipc_rcvd,
> > };
> >
> > void
> > @@ -243,32 +244,6 @@ uipc_usrreq(struct socket *so, int req,
> > }
> > break;
> >
> > - case PRU_RCVD:
> > - switch (so->so_type) {
> > -
> > - case SOCK_DGRAM:
> > - panic("uipc 1");
> > - /*NOTREACHED*/
> > -
> > - case SOCK_STREAM:
> > - case SOCK_SEQPACKET:
> > - if ((so2 = unp_solock_peer(so)) == NULL)
> > - break;
> > - /*
> > - * Adjust backpressure on sender
> > - * and wakeup any waiting to write.
> > - */
> > - so2->so_snd.sb_mbcnt = so->so_rcv.sb_mbcnt;
> > - so2->so_snd.sb_cc = so->so_rcv.sb_cc;
> > - sowwakeup(so2);
> > - sounlock(so2);
> > - break;
> > -
> > - default:
> > - panic("uipc 2");
> > - }
> > - break;
> > -
> > case PRU_SEND:
> > if (control) {
> > sounlock(so);
> > @@ -567,6 +542,37 @@ uipc_shutdown(struct socket *so)
> >
> > socantsendmore(so);
> > unp_shutdown(unp);
> > + return (0);
> > +}
> > +
> > +int
> > +uipc_rcvd(struct socket *so)
> > +{
> > + struct socket *so2;
> > +
> > + switch (so->so_type) {
> > + case SOCK_DGRAM:
> > + panic("uipc 1");
> > + /*NOTREACHED*/
> > +
> > + case SOCK_STREAM:
> > + case SOCK_SEQPACKET:
> > + if ((so2 = unp_solock_peer(so)) == NULL)
> > + break;
> > + /*
> > + * Adjust backpressure on sender
> > + * and wakeup any waiting to write.
> > + */
> > + so2->so_snd.sb_mbcnt = so->so_rcv.sb_mbcnt;
> > + so2->so_snd.sb_cc = so->so_rcv.sb_cc;
> > + sowwakeup(so2);
> > + sounlock(so2);
> > + break;
> > +
> > + default:
> > + panic("uipc 2");
> > + }
> > +
> > return (0);
> > }
> >
> > Index: sys/net/pfkeyv2.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> > retrieving revision 1.241
> > diff -u -p -r1.241 pfkeyv2.c
> > --- sys/net/pfkeyv2.c 22 Aug 2022 21:18:48 -0000 1.241
> > +++ sys/net/pfkeyv2.c 22 Aug 2022 22:27:08 -0000
> > @@ -396,7 +396,6 @@ pfkeyv2_usrreq(struct socket *so, int re
> > break;
> >
> > case PRU_RCVOOB:
> > - case PRU_RCVD:
> > case PRU_SENDOOB:
> > error = EOPNOTSUPP;
> > break;
> > Index: sys/net/rtsock.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/rtsock.c,v
> > retrieving revision 1.341
> > diff -u -p -r1.341 rtsock.c
> > --- sys/net/rtsock.c 22 Aug 2022 21:18:48 -0000 1.341
> > +++ sys/net/rtsock.c 22 Aug 2022 22:27:08 -0000
> > @@ -116,6 +116,7 @@ int route_usrreq(struct socket *, int, s
> > struct mbuf *, struct proc *);
> > int route_disconnect(struct socket *);
> > int route_shutdown(struct socket *);
> > +int route_rcvd(struct socket *);
> > void route_input(struct mbuf *m0, struct socket *, sa_family_t);
> > int route_arp_conflict(struct rtentry *, struct rt_addrinfo *);
> > int route_cleargateway(struct rtentry *, void *, unsigned int);
> > @@ -255,17 +256,6 @@ route_usrreq(struct socket *so, int req,
> > nam->m_len = route_src.sa_len;
> > break;
> >
> > - case PRU_RCVD:
> > - /*
> > - * If we are in a FLUSH state, check if the buffer is
> > - * empty so that we can clear the flag.
> > - */
> > - if (((rop->rop_flags & ROUTECB_FLAG_FLUSH) != 0) &&
> > - ((sbspace(rop->rop_socket, &rop->rop_socket->so_rcv) ==
> > - rop->rop_socket->so_rcv.sb_hiwat)))
> > - rop->rop_flags &= ~ROUTECB_FLAG_FLUSH;
> > - break;
> > -
> > case PRU_RCVOOB:
> > case PRU_SENDOOB:
> > error = EOPNOTSUPP;
> > @@ -375,6 +365,25 @@ route_shutdown(struct socket *so)
> > }
> >
> > int
> > +route_rcvd(struct socket *so)
> > +{
> > + struct rtpcb *rop = sotortpcb(so);
> > +
> > + soassertlocked(so);
> > +
> > + /*
> > + * If we are in a FLUSH state, check if the buffer is
> > + * empty so that we can clear the flag.
> > + */
> > + if (((rop->rop_flags & ROUTECB_FLAG_FLUSH) != 0) &&
> > + ((sbspace(rop->rop_socket, &rop->rop_socket->so_rcv) ==
> > + rop->rop_socket->so_rcv.sb_hiwat)))
> > + rop->rop_flags &= ~ROUTECB_FLAG_FLUSH;
> > +
> > + return (0);
> > +}
> > +
> > +int
> > route_ctloutput(int op, struct socket *so, int level, int optname,
> > struct mbuf *m)
> > {
> > @@ -2415,6 +2424,7 @@ const struct pr_usrreqs route_usrreqs =
> > .pru_detach = route_detach,
> > .pru_disconnect = route_disconnect,
> > .pru_shutdown = route_shutdown,
> > + .pru_rcvd = route_rcvd,
> > };
> >
> > const struct protosw routesw[] = {
> > Index: sys/netinet/ip_divert.c
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet/ip_divert.c,v
> > retrieving revision 1.76
> > diff -u -p -r1.76 ip_divert.c
> > --- sys/netinet/ip_divert.c 22 Aug 2022 21:18:48 -0000 1.76
> > +++ sys/netinet/ip_divert.c 22 Aug 2022 22:27:08 -0000
> > @@ -294,7 +294,6 @@ divert_usrreq(struct socket *so, int req
> > case PRU_SLOWTIMO:
> > case PRU_PROTORCV:
> > case PRU_PROTOSEND:
> > - case PRU_RCVD:
> > case PRU_RCVOOB:
> > error = EOPNOTSUPP;
> > break;
> > Index: sys/netinet/raw_ip.c
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet/raw_ip.c,v
> > retrieving revision 1.137
> > diff -u -p -r1.137 raw_ip.c
> > --- sys/netinet/raw_ip.c 22 Aug 2022 21:18:48 -0000 1.137
> > +++ sys/netinet/raw_ip.c 22 Aug 2022 22:27:08 -0000
> > @@ -537,7 +537,6 @@ rip_usrreq(struct socket *so, int req, s
> > * Not supported.
> > */
> > case PRU_SENDOOB:
> > - case PRU_RCVD:
> > case PRU_RCVOOB:
> > error = EOPNOTSUPP;
> > break;
> > Index: sys/netinet/tcp_usrreq.c
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
> > retrieving revision 1.194
> > diff -u -p -r1.194 tcp_usrreq.c
> > --- sys/netinet/tcp_usrreq.c 22 Aug 2022 21:18:48 -0000 1.194
> > +++ sys/netinet/tcp_usrreq.c 22 Aug 2022 22:27:08 -0000
> > @@ -121,6 +121,7 @@ const struct pr_usrreqs tcp_usrreqs = {
> > .pru_accept = tcp_accept,
> > .pru_disconnect = tcp_disconnect,
> > .pru_shutdown = tcp_shutdown,
> > + .pru_rcvd = tcp_rcvd,
> > };
> >
> > static int pr_slowhz = PR_SLOWHZ;
> > @@ -225,21 +226,6 @@ tcp_usrreq(struct socket *so, int req, s
> > break;
> >
> > /*
> > - * After a receive, possibly send window update to peer.
> > - */
> > - case PRU_RCVD:
> > - /*
> > - * soreceive() calls this function when a user receives
> > - * ancillary data on a listening socket. We don't call
> > - * tcp_output in such a case, since there is no header
> > - * template for a listening socket and hence the kernel
> > - * will panic.
> > - */
> > - if ((so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING)) != 0)
> > - (void) tcp_output(tp);
> > - break;
> > -
> > - /*
> > * Do a send by putting data in output queue and updating urgent
> > * marker if URG set. Possibly send more data.
> > */
> > @@ -912,6 +898,40 @@ out:
> > if (otp)
> > tcp_trace(TA_USER, ostate, tp, otp, NULL, PRU_SHUTDOWN, 0);
> > return (error);
> > +}
> > +
> > +/*
> > + * After a receive, possibly send window update to peer.
> > + */
> > +int
> > +tcp_rcvd(struct socket *so)
> > +{
> > + struct inpcb *inp;
> > + struct tcpcb *tp;
> > + int error;
> > + short ostate;
> > +
> > + soassertlocked(so);
> > +
> > + if ((error = tcp_sogetpcb(so, &inp, &tp)))
> > + return (error);
> > +
> > + if (so->so_options & SO_DEBUG)
> > + ostate = tp->t_state;
> > +
> > + /*
> > + * soreceive() calls this function when a user receives
> > + * ancillary data on a listening socket. We don't call
> > + * tcp_output in such a case, since there is no header
> > + * template for a listening socket and hence the kernel
> > + * will panic.
> > + */
> > + if ((so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING)) != 0)
> > + (void) tcp_output(tp);
> > +
> > + if (so->so_options & SO_DEBUG)
> > + tcp_trace(TA_USER, ostate, tp, tp, NULL, PRU_RCVD, 0);
> > + return (0);
> > }
> >
> > /*
> > Index: sys/netinet/tcp_var.h
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet/tcp_var.h,v
> > retrieving revision 1.147
> > diff -u -p -r1.147 tcp_var.h
> > --- sys/netinet/tcp_var.h 22 Aug 2022 21:18:48 -0000 1.147
> > +++ sys/netinet/tcp_var.h 22 Aug 2022 22:27:08 -0000
> > @@ -720,6 +720,7 @@ int tcp_connect(struct socket *, struct
> > int tcp_accept(struct socket *, struct mbuf *);
> > int tcp_disconnect(struct socket *);
> > int tcp_shutdown(struct socket *);
> > +int tcp_rcvd(struct socket *);
> > void tcp_xmit_timer(struct tcpcb *, int);
> > void tcpdropoldhalfopen(struct tcpcb *, u_int16_t);
> > void tcp_sack_option(struct tcpcb *,struct tcphdr *,u_char *,int);
> > Index: sys/netinet/udp_usrreq.c
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
> > retrieving revision 1.289
> > diff -u -p -r1.289 udp_usrreq.c
> > --- sys/netinet/udp_usrreq.c 22 Aug 2022 21:18:48 -0000 1.289
> > +++ sys/netinet/udp_usrreq.c 22 Aug 2022 22:27:08 -0000
> > @@ -1164,7 +1164,6 @@ udp_usrreq(struct socket *so, int req, s
> > case PRU_SLOWTIMO:
> > case PRU_PROTORCV:
> > case PRU_PROTOSEND:
> > - case PRU_RCVD:
> > case PRU_RCVOOB:
> > error = EOPNOTSUPP;
> > break;
> > Index: sys/netinet6/ip6_divert.c
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
> > retrieving revision 1.75
> > diff -u -p -r1.75 ip6_divert.c
> > --- sys/netinet6/ip6_divert.c 22 Aug 2022 21:18:48 -0000 1.75
> > +++ sys/netinet6/ip6_divert.c 22 Aug 2022 22:27:08 -0000
> > @@ -300,7 +300,6 @@ divert6_usrreq(struct socket *so, int re
> > case PRU_SLOWTIMO:
> > case PRU_PROTORCV:
> > case PRU_PROTOSEND:
> > - case PRU_RCVD:
> > case PRU_RCVOOB:
> > error = EOPNOTSUPP;
> > break;
> > Index: sys/netinet6/raw_ip6.c
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet6/raw_ip6.c,v
> > retrieving revision 1.157
> > diff -u -p -r1.157 raw_ip6.c
> > --- sys/netinet6/raw_ip6.c 22 Aug 2022 21:18:48 -0000 1.157
> > +++ sys/netinet6/raw_ip6.c 22 Aug 2022 22:27:08 -0000
> > @@ -654,7 +654,6 @@ rip6_usrreq(struct socket *so, int req,
> > * Not supported.
> > */
> > case PRU_SENDOOB:
> > - case PRU_RCVD:
> > case PRU_RCVOOB:
> > error = EOPNOTSUPP;
> > break;
> > Index: sys/sys/unpcb.h
> > ===================================================================
> > RCS file: /cvs/src/sys/sys/unpcb.h,v
> > retrieving revision 1.33
> > diff -u -p -r1.33 unpcb.h
> > --- sys/sys/unpcb.h 22 Aug 2022 21:18:48 -0000 1.33
> > +++ sys/sys/unpcb.h 22 Aug 2022 22:27:08 -0000
> > @@ -119,6 +119,7 @@ int uipc_connect(struct socket *, struct
> > int uipc_accept(struct socket *, struct mbuf *);
> > int uipc_disconnect(struct socket *);
> > int uipc_shutdown(struct socket *);
> > +int uipc_rcvd(struct socket *);
> >
> > void unp_init(void);
> > int unp_bind(struct unpcb *, struct mbuf *, struct proc *);
> >
> >
