On Tue, Aug 23, 2022 at 11:47:30AM +0300, Vitaliy Makkoveev wrote: > 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.
This may make sense. But we should have a look at all functions to see if they should do the NULL check. The old behavior was to return EOPNOTSUPP. > > (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. > > Maybe. But we should look at all funtions to make a decision what return type we want. > 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. Get this in as it is. First finish the conversion, then consider guenther@'s comments. After everything has been converted, we will have a better view what is needed. bluhm > > On Mon, Aug 22, 2022 at 1:40 PM Vitaliy Makkoveev <[email protected]> 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 *); > > > > > >
