* [RFC PATCH net] tcp: allow to use TCP Fastopen with MSG_ZEROCOPY
@ 2018-04-03 12:43 Alexey Kodanev
2018-04-04 13:22 ` Willem de Bruijn
0 siblings, 1 reply; 2+ messages in thread
From: Alexey Kodanev @ 2018-04-03 12:43 UTC (permalink / raw)
To: netdev; +Cc: Willem de Bruijn, Eric Dumazet, David Miller, Alexey Kodanev
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(-)
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [RFC PATCH net] tcp: allow to use TCP Fastopen with MSG_ZEROCOPY
2018-04-03 12:43 [RFC PATCH net] tcp: allow to use TCP Fastopen with MSG_ZEROCOPY Alexey Kodanev
@ 2018-04-04 13:22 ` Willem de Bruijn
0 siblings, 0 replies; 2+ messages in thread
From: Willem de Bruijn @ 2018-04-04 13:22 UTC (permalink / raw)
To: Alexey Kodanev
Cc: Network Development, Willem de Bruijn, Eric Dumazet,
David Miller, Soheil Hassas Yeganeh, Neal Cardwell
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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-04-04 13:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-03 12:43 [RFC PATCH net] tcp: allow to use TCP Fastopen with MSG_ZEROCOPY Alexey Kodanev
2018-04-04 13:22 ` Willem de Bruijn
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.