Page MenuHomeFreeBSD
Differential D12492 Diff 33406 sys/netinet/tcp_usrreq.c

Changeset View

Standalone View

View Options

sys/netinet/tcp_usrreq.c

Show First 20 Lines • Show All 871 Lines • ▼ Show 20 Linesout:
return (error); return (error);
} }
/* /*
* Do a send by putting data in output queue and updating urgent * Do a send by putting data in output queue and updating urgent
* marker if URG set. Possibly send more data. Unlike the other * marker if URG set. Possibly send more data. Unlike the other
* pru_*() routines, the mbuf chains are our responsibility. We * pru_*() routines, the mbuf chains are our responsibility. We
* must either enqueue them or free them. The other pru_* routines * must either enqueue them or free them. The other pru_* routines
* generally are caller-frees. * generally are caller-frees. We must either add the mbuf chain
* to the socket send buffer or return an error, not both. We
* ensure that after adding the mbufs to the send buffer, we will
* not return any more errors. Errors from tcp_output are not
* passed back to the caller, but are instead deposited in
* so->so_error for the next call.
*/ */
static int static int
tcp_usr_send(struct socket *so, int flags, struct mbuf *m, tcp_usr_send(struct socket *so, int flags, struct mbuf *m,
struct sockaddr *nam, struct mbuf *control, struct thread *td) struct sockaddr *nam, struct mbuf *control, struct thread *td)
{ {
int error = 0; int error = 0;
int so_error = 0;
jtlUnsubmitted
Not Done
Inline Actions

There is no need to initialize this. It is set before its use in any branch where it is used.

jtl: There is no need to initialize this. It is set before its use in any branch where it is used.
struct inpcb *inp; struct inpcb *inp;
struct tcpcb *tp = NULL; struct tcpcb *tp = NULL;
#ifdef INET6 #ifdef INET6
int isipv6; int isipv6;
#endif #endif
TCPDEBUG0; TCPDEBUG0;
/* /*
Show All 21 Lines#ifdef INET6
isipv6 = nam && nam->sa_family == AF_INET6; isipv6 = nam && nam->sa_family == AF_INET6;
#endif /* INET6 */ #endif /* INET6 */
tp = intotcpcb(inp); tp = intotcpcb(inp);
TCPDEBUG1(); TCPDEBUG1();
if (control) { if (control) {
/* TCP doesn't do control messages (rights, creds, etc) */ /* TCP doesn't do control messages (rights, creds, etc) */
if (control->m_len) { if (control->m_len) {
m_freem(control); m_freem(control);
if (m) if (m)
jtlUnsubmitted
Not Done
Inline Actions

I believe this should have the same condition as above (m && (flags & PRUS_NOTREADY) == 0). @glebius can confirm. (This is relevant to another comment below.)

jtl: I believe this should have the same condition as above (m && (flags & PRUS_NOTREADY) == 0).
glebiusUnsubmitted
Not Done
Inline Actions

A control shall never be M_NOTREADY

glebius: A control shall never be M_NOTREADY
m_freem(m); m_freem(m);
error = EINVAL; error = EINVAL;
goto out; goto out;
} }
m_freem(control); /* empty control, just free it */ m_freem(control); /* empty control, just free it */
} }
if (!(flags & PRUS_OOB)) { if (!(flags & PRUS_OOB)) {
sbappendstream(&so->so_snd, m, flags);
if (nam && tp->t_state < TCPS_SYN_SENT) { if (nam && tp->t_state < TCPS_SYN_SENT) {
/* /*
* Do implied connect if not yet connected, * Do implied connect if not yet connected,
* initialize window to default value, and * initialize window to default value, and
* initialize maxseg using peer's cached MSS. * initialize maxseg using peer's cached MSS.
*/ */
#ifdef INET6 #ifdef INET6
if (isipv6) if (isipv6)
error = tcp6_connect(tp, nam, td); error = tcp6_connect(tp, nam, td);
#endif /* INET6 */ #endif /* INET6 */
#if defined(INET6) && defined(INET) #if defined(INET6) && defined(INET)
else else
#endif #endif
#ifdef INET #ifdef INET
error = tcp_connect(tp, nam, td); error = tcp_connect(tp, nam, td);
#endif #endif
if (error) if (error)
goto out; goto out;
jtlUnsubmitted
Not Done
Inline Actions

You need to free m in the error path. Given that this is now three times we need to do this, it might be worth combining the logic in one place. (See my comment above about the correct logic.)

jtl: You need to free m in the error path. Given that this is now three times we need to do this, it…
tp->snd_wnd = TTCP_CLIENT_SND_WND; tp->snd_wnd = TTCP_CLIENT_SND_WND;
tcp_mss(tp, -1); tcp_mss(tp, -1);
} }
sbappendstream(&so->so_snd, m, flags);
if (flags & PRUS_EOF) { if (flags & PRUS_EOF) {
/* /*
* Close the send side of the connection after * Close the send side of the connection after
* the data is sent. * the data is sent.
*/ */
INP_INFO_RLOCK_ASSERT(&V_tcbinfo); INP_INFO_RLOCK_ASSERT(&V_tcbinfo);
socantsendmore(so); socantsendmore(so);
tcp_usrclosed(tp); tcp_usrclosed(tp);
} }
if (!(inp->inp_flags & INP_DROPPED) && if (!(inp->inp_flags & INP_DROPPED) &&
!(flags & PRUS_NOTREADY)) { !(flags & PRUS_NOTREADY)) {
if (flags & PRUS_MORETOCOME) if (flags & PRUS_MORETOCOME)
tp->t_flags |= TF_MORETOCOME; tp->t_flags |= TF_MORETOCOME;
error = tp->t_fb->tfb_tcp_output(tp); so_error = tp->t_fb->tfb_tcp_output(tp);
/* next socket syscall call will return this error */
if (so_error && !so->so_error)
so->so_error = so_error;
if (flags & PRUS_MORETOCOME) if (flags & PRUS_MORETOCOME)
tp->t_flags &= ~TF_MORETOCOME; tp->t_flags &= ~TF_MORETOCOME;
} }
} else { } else {
/* /*
* XXXRW: PRUS_EOF not implemented with PRUS_OOB? * XXXRW: PRUS_EOF not implemented with PRUS_OOB?
*/ */
SOCKBUF_LOCK(&so->so_snd);
if (sbspace(&so->so_snd) < -512) {
SOCKBUF_UNLOCK(&so->so_snd);
m_freem(m);
error = ENOBUFS;
goto out;
}
/*
* According to RFC961 (Assigned Protocols),
* the urgent pointer points to the last octet
* of urgent data. We continue, however,
* to consider it to indicate the first octet
* of data past the urgent section.
* Otherwise, snd_up should be one lower.
*/
sbappendstream_locked(&so->so_snd, m, flags);
SOCKBUF_UNLOCK(&so->so_snd);
if (nam && tp->t_state < TCPS_SYN_SENT) { if (nam && tp->t_state < TCPS_SYN_SENT) {
/* /*
* Do implied connect if not yet connected, * Do implied connect if not yet connected,
* initialize window to default value, and * initialize window to default value, and
* initialize maxseg using peer's cached MSS. * initialize maxseg using peer's cached MSS.
*/ */
#ifdef INET6 #ifdef INET6
if (isipv6) if (isipv6)
error = tcp6_connect(tp, nam, td); error = tcp6_connect(tp, nam, td);
#endif /* INET6 */ #endif /* INET6 */
#if defined(INET6) && defined(INET) #if defined(INET6) && defined(INET)
else else
#endif #endif
#ifdef INET #ifdef INET
error = tcp_connect(tp, nam, td); error = tcp_connect(tp, nam, td);
#endif #endif
if (error) if (error)
goto out; goto out;
jtlUnsubmitted
Not Done
Inline Actions

You need to free m. See above.

jtl: You need to free m. See above.
tp->snd_wnd = TTCP_CLIENT_SND_WND; tp->snd_wnd = TTCP_CLIENT_SND_WND;
tcp_mss(tp, -1); tcp_mss(tp, -1);
} }
SOCKBUF_LOCK(&so->so_snd);
if (sbspace(&so->so_snd) < -512) {
SOCKBUF_UNLOCK(&so->so_snd);
m_freem(m);
error = ENOBUFS;
goto out;
}
/*
* According to RFC961 (Assigned Protocols),
* the urgent pointer points to the last octet
* of urgent data. We continue, however,
* to consider it to indicate the first octet
* of data past the urgent section.
* Otherwise, snd_up should be one lower.
*/
sbappendstream_locked(&so->so_snd, m, flags);
tp->snd_up = tp->snd_una + sbavail(&so->so_snd); tp->snd_up = tp->snd_una + sbavail(&so->so_snd);
SOCKBUF_UNLOCK(&so->so_snd);
if (!(flags & PRUS_NOTREADY)) { if (!(flags & PRUS_NOTREADY)) {
tp->t_flags |= TF_FORCEDATA; tp->t_flags |= TF_FORCEDATA;
error = tp->t_fb->tfb_tcp_output(tp); so_error = tp->t_fb->tfb_tcp_output(tp);
/* next socket syscall call will return this error */
jtlUnsubmitted
Not Done
Inline Actions

I understand why this makes sense in the sendfile case: sendfile actually checks so->so_error. However, I'm not sure this change is obviously good for ever other caller. Absent other changes, other callers of pru_send might need to wait for a further syscall or do further probing to discover the error.

In fact, I think what you are proposing is correctly categorized as an API change for the pru_send function. That requires modifying every caller to check so->so_error after the pru_send call and take appropriate action. (It might also require modifying other pru_send functions to behave similarly.)

jtl: I understand why this makes sense in the sendfile case: sendfile actually checks so->so_error.
if (so_error && !so->so_error)
so->so_error = so_error;
tp->t_flags &= ~TF_FORCEDATA; tp->t_flags &= ~TF_FORCEDATA;
} }
} }
out: out:
TCPDEBUG2((flags & PRUS_OOB) ? PRU_SENDOOB : TCPDEBUG2((flags & PRUS_OOB) ? PRU_SENDOOB :
((flags & PRUS_EOF) ? PRU_SEND_EOF : PRU_SEND)); ((flags & PRUS_EOF) ? PRU_SEND_EOF : PRU_SEND));
TCP_PROBE2(debug__user, tp, (flags & PRUS_OOB) ? PRU_SENDOOB : TCP_PROBE2(debug__user, tp, (flags & PRUS_OOB) ? PRU_SENDOOB :
((flags & PRUS_EOF) ? PRU_SEND_EOF : PRU_SEND)); ((flags & PRUS_EOF) ? PRU_SEND_EOF : PRU_SEND));
▲ Show 20 Lines • Show All 1,330 Lines • Show Last 20 Lines