Date: Tue, 3 Sep 2013 06:10:01 GMT From: Yuri <yuri@rawbw.com> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/181741: [PATCH] Packet loss when 'control' messages are present with large data (sendmsg(2)) Message-ID: <201309030610.r836A17N056917@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/181741; it has been noted by GNATS.
From: Yuri <yuri@rawbw.com>
To: bug-followup@FreeBSD.org
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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201309030610.r836A17N056917>