| Message ID | 1522759399-25693-1-git-send-email-alexey.kodanev@oracle.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [RFC,net] tcp: allow to use TCP Fastopen with MSG_ZEROCOPY | expand |
On Tue, Apr 3, 2018 at 2:43 PM, Alexey Kodanev <alexey.kodanev@oracle.com> wrote: > With TCP Fastopen we can have the following cases, which could also > use MSG_ZEROCOPY flag with send() and sendto(): > > * sendto() + MSG_FASTOPEN flag, sk state can be in TCP_CLOSE at > the start of tcp_sendmsg() > > * set socket option TCP_FASTOPEN_CONNECT, then connect() > and send(), sk state in TCP_SYN_SENT > > Currently, both cases with tcp_sendmsg() and MSG_ZEROCOPY flag results > to EINVAL error, because of the check for TCP_ESTABLISHED sk state in > the beginning of tcp_sendmsg(). > > Both conditions require two more checks there: !tp->fastopen_connect > and !(flags & MSG_FASTOPEN). It looks like we could remove the original > check altogether for this unlikely event instead. That way tcp_sendmsg() > without TFO should fail with EPIPE on sk_stream_wait_connect(), as > before the introduction of MSG_ZEROCOPY there. And work smoothly for > the TFO cases. > > Fixes: f214f915e7db ("tcp: enable MSG_ZEROCOPY") This patch adds MSG_ZEROCOPY support for TFO sockets. It is not a fix that needs to go to stable. > Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> > --- > > Is there something that I've overlooked and we can't use it here, and > we should handle this type of error, while using sendto() + TFO, > in userspace? > > net/ipv4/tcp.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index 9225610..768f02c 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -1193,11 +1193,6 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size) > flags = msg->msg_flags; > > if (flags & MSG_ZEROCOPY && size) { > - if (sk->sk_state != TCP_ESTABLISHED) { > - err = -EINVAL; > - goto out_err; > - } > - > skb = tcp_write_queue_tail(sk); > uarg = sock_zerocopy_realloc(sk, size, skb_zcopy(skb)); > if (!uarg) { > -- > 1.8.3.1 >
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 9225610..768f02c 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -1193,11 +1193,6 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size) flags = msg->msg_flags; if (flags & MSG_ZEROCOPY && size) { - if (sk->sk_state != TCP_ESTABLISHED) { - err = -EINVAL; - goto out_err; - } - skb = tcp_write_queue_tail(sk); uarg = sock_zerocopy_realloc(sk, size, skb_zcopy(skb)); if (!uarg) {
With TCP Fastopen we can have the following cases, which could also use MSG_ZEROCOPY flag with send() and sendto(): * sendto() + MSG_FASTOPEN flag, sk state can be in TCP_CLOSE at the start of tcp_sendmsg() * set socket option TCP_FASTOPEN_CONNECT, then connect() and send(), sk state in TCP_SYN_SENT Currently, both cases with tcp_sendmsg() and MSG_ZEROCOPY flag results to EINVAL error, because of the check for TCP_ESTABLISHED sk state in the beginning of tcp_sendmsg(). Both conditions require two more checks there: !tp->fastopen_connect and !(flags & MSG_FASTOPEN). It looks like we could remove the original check altogether for this unlikely event instead. That way tcp_sendmsg() without TFO should fail with EPIPE on sk_stream_wait_connect(), as before the introduction of MSG_ZEROCOPY there. And work smoothly for the TFO cases. Fixes: f214f915e7db ("tcp: enable MSG_ZEROCOPY") Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> --- Is there something that I've overlooked and we can't use it here, and we should handle this type of error, while using sendto() + TFO, in userspace? net/ipv4/tcp.c | 5 ----- 1 file changed, 5 deletions(-)