The following reply was made to PR kern/181741; it has been noted by GNATS.
From: Yuri <[email protected]> To: [email protected] Cc: Subject: Re: kern/181741: [PATCH] Packet loss when 'control' messages are present with large data (sendmsg(2)) Date: Mon, 02 Sep 2013 23:03:41 -0700 This is a multi-part message in MIME format. --------------060206060307010809000308 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Here is the updated patch. --------------060206060307010809000308 Content-Type: text/plain; charset=UTF-8; name="patch-10-net-control-loss-3.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="patch-10-net-control-loss-3.patch" Index: kern/uipc_sockbuf.c =================================================================== --- kern/uipc_sockbuf.c (revision 255139) +++ kern/uipc_sockbuf.c (working copy) @@ -688,23 +688,16 @@ return (retval); } -int +void sbappendcontrol_locked(struct sockbuf *sb, struct mbuf *m0, struct mbuf *control) { - struct mbuf *m, *n, *mlast; - int space; + struct mbuf *m, *mlast; SOCKBUF_LOCK_ASSERT(sb); - if (control == 0) - panic("sbappendcontrol_locked"); - space = m_length(control, &n) + m_length(m0, NULL); + m_last(control)->m_next = m0; /* concatenate data to control */ - if (space > sbspace(sb)) - return (0); - n->m_next = m0; /* concatenate data to control */ - SBLASTRECORDCHK(sb); for (m = control; m->m_next; m = m->m_next) @@ -717,18 +710,14 @@ SBLASTMBUFCHK(sb); SBLASTRECORDCHK(sb); - return (1); } -int +void sbappendcontrol(struct sockbuf *sb, struct mbuf *m0, struct mbuf *control) { - int retval; - SOCKBUF_LOCK(sb); - retval = sbappendcontrol_locked(sb, m0, control); + sbappendcontrol_locked(sb, m0, control); SOCKBUF_UNLOCK(sb); - return (retval); } /* Index: kern/uipc_socket.c =================================================================== --- kern/uipc_socket.c (revision 255139) +++ kern/uipc_socket.c (working copy) @@ -1102,6 +1102,11 @@ KASSERT(so->so_proto->pr_flags & PR_ATOMIC, ("sosend_dgram: !PR_ATOMIC")); + if (so->so_proto->pr_usrreqs->pru_finalizecontrol && + (error = (*so->so_proto->pr_usrreqs->pru_finalizecontrol)(so, + flags, &control, td))) + goto out; + if (uio != NULL) resid = uio->uio_resid; else @@ -1166,10 +1171,14 @@ * problem and need fixing. */ space = sbspace(&so->so_snd); + SOCKBUF_UNLOCK(&so->so_snd); if (flags & MSG_OOB) space += 1024; + if (clen > space) { + error = EMSGSIZE; + goto out; + } space -= clen; - SOCKBUF_UNLOCK(&so->so_snd); if (resid > space) { error = EMSGSIZE; goto out; @@ -1269,6 +1278,11 @@ int clen = 0, error, dontroute; int atomic = sosendallatonce(so) || top; + if (so->so_proto->pr_usrreqs->pru_finalizecontrol && + (error = (*so->so_proto->pr_usrreqs->pru_finalizecontrol)(so, + flags, &control, td))) + goto out; + if (uio != NULL) resid = uio->uio_resid; else @@ -1361,6 +1375,10 @@ goto restart; } SOCKBUF_UNLOCK(&so->so_snd); + if (clen > space) { + error = EMSGSIZE; + goto release; + } space -= clen; do { if (uio == NULL) { Index: kern/uipc_usrreq.c =================================================================== --- kern/uipc_usrreq.c (revision 255139) +++ kern/uipc_usrreq.c (working copy) @@ -290,7 +290,7 @@ static void unp_internalize_fp(struct file *); static int unp_externalize(struct mbuf *, struct mbuf **, int); static int unp_externalize_fp(struct file *); -static struct mbuf *unp_addsockcred(struct thread *, struct mbuf *); +static int unp_addsockcred(struct mbuf **, struct thread *); static void unp_process_defers(void * __unused, int); /* @@ -782,6 +782,43 @@ } static int +uipc_finalizecontrol(struct socket *so, int flags, struct mbuf **pcontrol, + struct thread *td) +{ + struct unpcb *unp, *unp2; + int error = 0; + + unp = sotounpcb(so); + KASSERT(unp != NULL, ("uipc_finalizecontrol: unp == NULL")); + + unp2 = unp->unp_conn; + + if (*pcontrol != NULL && (error = unp_internalize(pcontrol, td))) + return (error); + + /* Lockless read, ignore when not connected. */ + if (unp2 && unp2->unp_flags & UNP_WANTCRED) { + switch (so->so_type) { + case SOCK_SEQPACKET: + case SOCK_STREAM: + /* Credentials are passed only once on streams */ + UNP_PCB_LOCK(unp2); + if (unp2->unp_flags & UNP_WANTCRED) { + unp2->unp_flags &= ~UNP_WANTCRED; + error = unp_addsockcred(pcontrol, td); + } + UNP_PCB_UNLOCK(unp2); + break; + case SOCK_DGRAM: + error = unp_addsockcred(pcontrol, td); + break; + } + } + + return (error); +} + +static int uipc_rcvd(struct socket *so, int flags) { struct unpcb *unp, *unp2; @@ -845,8 +882,6 @@ error = EOPNOTSUPP; goto release; } - if (control != NULL && (error = unp_internalize(&control, td))) - goto release; if ((nam != NULL) || (flags & PRUS_EOF)) UNP_LINK_WLOCK(); else @@ -880,9 +915,6 @@ error = ENOTCONN; break; } - /* Lockless read. */ - if (unp2->unp_flags & UNP_WANTCRED) - control = unp_addsockcred(td, control); UNP_PCB_LOCK(unp); if (unp->unp_addr != NULL) from = (struct sockaddr *)unp->unp_addr; @@ -949,14 +981,6 @@ so2 = unp2->unp_socket; UNP_PCB_LOCK(unp2); SOCKBUF_LOCK(&so2->so_rcv); - if (unp2->unp_flags & UNP_WANTCRED) { - /* - * Credentials are passed only once on SOCK_STREAM - * and SOCK_SEQPACKET. - */ - unp2->unp_flags &= ~UNP_WANTCRED; - control = unp_addsockcred(td, control); - } /* * Send to paired receive port, and then reduce send buffer * hiwater marks to maintain backpressure. Wake up readers. @@ -964,9 +988,9 @@ switch (so->so_type) { case SOCK_STREAM: if (control != NULL) { - if (sbappendcontrol_locked(&so2->so_rcv, m, - control)) - control = NULL; + sbappendcontrol_locked(&so2->so_rcv, m, + control); + control = NULL; } else sbappend_locked(&so2->so_rcv, m); break; @@ -981,6 +1005,7 @@ break; } } + m = NULL; /* * XXXRW: While fine for SOCK_STREAM, this conflates maximum @@ -1004,7 +1029,6 @@ SOCKBUF_UNLOCK(&so->so_snd); unp2->unp_cc = sbcc; UNP_PCB_UNLOCK(unp2); - m = NULL; break; default: @@ -1114,6 +1138,7 @@ .pru_disconnect = uipc_disconnect, .pru_listen = uipc_listen, .pru_peeraddr = uipc_peeraddr, + .pru_finalizecontrol = uipc_finalizecontrol, .pru_rcvd = uipc_rcvd, .pru_send = uipc_send, .pru_sense = uipc_sense, @@ -1136,6 +1161,7 @@ .pru_disconnect = uipc_disconnect, .pru_listen = uipc_listen, .pru_peeraddr = uipc_peeraddr, + .pru_finalizecontrol = uipc_finalizecontrol, .pru_rcvd = uipc_rcvd, .pru_send = uipc_send, .pru_sense = uipc_sense, @@ -1158,6 +1184,7 @@ .pru_disconnect = uipc_disconnect, .pru_listen = uipc_listen, .pru_peeraddr = uipc_peeraddr, + .pru_finalizecontrol = uipc_finalizecontrol, .pru_rcvd = uipc_rcvd, .pru_send = uipc_send, .pru_sense = uipc_sense, @@ -1747,7 +1774,7 @@ SCM_RIGHTS, SOL_SOCKET); if (*controlp == NULL) { FILEDESC_XUNLOCK(fdesc); - error = E2BIG; + error = ENOBUFS; unp_freerights(fdep, newfds); goto next; } @@ -1928,7 +1955,7 @@ SCM_RIGHTS, SOL_SOCKET); if (*controlp == NULL) { FILEDESC_SUNLOCK(fdesc); - error = E2BIG; + error = ENOBUFS; goto out; } fdp = data; @@ -1992,9 +2019,10 @@ return (error); } -static struct mbuf * -unp_addsockcred(struct thread *td, struct mbuf *control) +static int +unp_addsockcred(struct mbuf **pcontrol, struct thread *td) { + struct mbuf *control = *pcontrol; struct mbuf *m, *n, *n_prev; struct sockcred *sc; const struct cmsghdr *cm; @@ -2004,7 +2032,7 @@ ngroups = MIN(td->td_ucred->cr_ngroups, CMGROUP_MAX); m = sbcreatecontrol(NULL, SOCKCREDSIZE(ngroups), SCM_CREDS, SOL_SOCKET); if (m == NULL) - return (control); + return (ENOBUFS); sc = (struct sockcred *) CMSG_DATA(mtod(m, struct cmsghdr *)); sc->sc_uid = td->td_ucred->cr_ruid; @@ -2038,7 +2066,8 @@ /* Prepend it to the head. */ m->m_next = control; - return (m); + *pcontrol = m; + return (0); } static struct unpcb * Index: sys/protosw.h =================================================================== --- sys/protosw.h (revision 255139) +++ sys/protosw.h (working copy) @@ -201,6 +201,8 @@ int (*pru_listen)(struct socket *so, int backlog, struct thread *td); int (*pru_peeraddr)(struct socket *so, struct sockaddr **nam); + int (*pru_finalizecontrol)(struct socket *so, int flags, struct mbuf **pcontrol, + struct thread *td); int (*pru_rcvd)(struct socket *so, int flags); int (*pru_rcvoob)(struct socket *so, struct mbuf *m, int flags); int (*pru_send)(struct socket *so, int flags, struct mbuf *m, Index: sys/sockbuf.h =================================================================== --- sys/sockbuf.h (revision 255139) +++ sys/sockbuf.h (working copy) @@ -127,9 +127,9 @@ struct mbuf *m0, struct mbuf *control); int sbappendaddr_locked(struct sockbuf *sb, const struct sockaddr *asa, struct mbuf *m0, struct mbuf *control); -int sbappendcontrol(struct sockbuf *sb, struct mbuf *m0, +void sbappendcontrol(struct sockbuf *sb, struct mbuf *m0, struct mbuf *control); -int sbappendcontrol_locked(struct sockbuf *sb, struct mbuf *m0, +void sbappendcontrol_locked(struct sockbuf *sb, struct mbuf *m0, struct mbuf *control); void sbappendrecord(struct sockbuf *sb, struct mbuf *m0); void sbappendrecord_locked(struct sockbuf *sb, struct mbuf *m0); --------------060206060307010809000308-- _______________________________________________ [email protected] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-bugs To unsubscribe, send any mail to "[email protected]"
