New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 687452 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Dec 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Update TCP fastopen on CrOS kernels

Project Member Reported by sonnyrao@chromium.org, Feb 1 2017

Issue description

We want to pull in the following upstream commits to enable TCP fast open functionality for CrOS


065263f40f09 net/tcp-fastopen: refactor cookie check logic
25776aa94340 net: Remove __sk_dst_reset() in tcp_v6_connect()
19f6d3f3c842 net/tcp-fastopen: Add new API support
3979ad7e82df net/tcp-fastopen: make connect()'s return case more consistent with non-TFO

 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 8 2017

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/68e09f4b2d27fb97e23c533b9663ff615694fc55

commit 68e09f4b2d27fb97e23c533b9663ff615694fc55
Author: Wei Wang <weiwan@google.com>
Date: Wed Feb 08 04:14:14 2017

UPSTREAM: net/tcp-fastopen: refactor cookie check logic

Refactor the cookie check logic in tcp_send_syn_data() into a function.
This function will be called else where in later changes.

BUG= chromium:687452 
TEST=build/boot on Kevin

Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 065263f40f0972d5f1cd294bb0242bd5aa5f06b2)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: I8969787f0c31f9552819abbcac9ef5dee32808d5
Reviewed-on: https://chromium-review.googlesource.com/435506
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Wei Wang <weiwan@google.com>

[modify] https://crrev.com/68e09f4b2d27fb97e23c533b9663ff615694fc55/net/ipv4/tcp_output.c
[modify] https://crrev.com/68e09f4b2d27fb97e23c533b9663ff615694fc55/net/ipv4/tcp_fastopen.c
[modify] https://crrev.com/68e09f4b2d27fb97e23c533b9663ff615694fc55/include/net/tcp.h

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/94c3b157f5c050784da0785b51046c079adf97b9

commit 94c3b157f5c050784da0785b51046c079adf97b9
Author: Wei Wang <weiwan@google.com>
Date: Wed Feb 08 04:14:15 2017

UPSTREAM: net: Remove __sk_dst_reset() in tcp_v6_connect()

Remove __sk_dst_reset() in the failure handling because __sk_dst_reset()
will eventually get called when sk is released. No need to handle it in
the protocol specific connect call.
This is also to make the code path consistent with ipv4.

BUG= chromium:687452 
TEST=build/boot on Kevin

Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 25776aa943401662617437841b3d3ea4693ee98a)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: I5493bf7fd3f4c76bbfe5bcba611b6cb863526090
Reviewed-on: https://chromium-review.googlesource.com/435507
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Wei Wang <weiwan@google.com>

[modify] https://crrev.com/94c3b157f5c050784da0785b51046c079adf97b9/net/ipv6/tcp_ipv6.c

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/512ce30bbb02eb9b4749052c9df226aeb1d48889

commit 512ce30bbb02eb9b4749052c9df226aeb1d48889
Author: Andrey Vagin <avagin@openvz.org>
Date: Wed Feb 08 04:14:16 2017

UPSTREAM: tcp: add an ability to dump and restore window parameters

We found that sometimes a restored tcp socket doesn't work.

A reason of this bug is incorrect window parameters and in this case
tcp_acceptable_seq() returns tcp_wnd_end(tp) instead of tp->snd_nxt. The
other side drops packets with this seq, because seq is less than
tp->rcv_nxt ( tcp_sequence() ).

Data from a send queue is sent only if there is enough space in a
window, so when we restore unacked data, we need to expand a window to
fit this data.

This was in a first version of this patch:
"tcp: extend window to fit all restored unacked data in a send queue"

Then Alexey recommended me to restore window parameters instead of
adjusted them according with data in a sent queue. This sounds resonable.

rcv_wnd has to be restored, because it was reported to another side
and the offered window is never shrunk.
One of reasons why we need to restore snd_wnd was described above.

BUG= chromium:687452 
TEST=build/boot on Kevin

Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit b1ed4c4fa9a5ccf325184fd90edc50978ef6e33a)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: Id9ee029021e2cccc1e1bb0ed29840c9953578a90
Reviewed-on: https://chromium-review.googlesource.com/435508
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Wei Wang <weiwan@google.com>

[modify] https://crrev.com/512ce30bbb02eb9b4749052c9df226aeb1d48889/include/uapi/linux/tcp.h
[modify] https://crrev.com/512ce30bbb02eb9b4749052c9df226aeb1d48889/net/ipv4/tcp.c

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/026bd6878514de5109ac2a9e769d7ddf6291631a

commit 026bd6878514de5109ac2a9e769d7ddf6291631a
Author: Wei Wang <weiwan@google.com>
Date: Wed Feb 08 04:14:18 2017

BACKPORT: net/tcp-fastopen: Add new API support

This patch adds a new socket option, TCP_FASTOPEN_CONNECT, as an
alternative way to perform Fast Open on the active side (client). Prior
to this patch, a client needs to replace the connect() call with
sendto(MSG_FASTOPEN). This can be cumbersome for applications who want
to use Fast Open: these socket operations are often done in lower layer
libraries used by many other applications. Changing these libraries
and/or the socket call sequences are not trivial. A more convenient
approach is to perform Fast Open by simply enabling a socket option when
the socket is created w/o changing other socket calls sequence:
  s = socket()
    create a new socket
  setsockopt(s, IPPROTO_TCP, TCP_FASTOPEN_CONNECT );
    newly introduced sockopt
    If set, new functionality described below will be used.
    Return ENOTSUPP if TFO is not supported or not enabled in the
    kernel.

  connect()
    With cookie present, return 0 immediately.
    With no cookie, initiate 3WHS with TFO cookie-request option and
    return -1 with errno = EINPROGRESS.

  write()/sendmsg()
    With cookie present, send out SYN with data and return the number of
    bytes buffered.
    With no cookie, and 3WHS not yet completed, return -1 with errno =
    EINPROGRESS.
    No MSG_FASTOPEN flag is needed.

  read()
    Return -1 with errno = EWOULDBLOCK/EAGAIN if connect() is called but
    write() is not called yet.
    Return -1 with errno = EWOULDBLOCK/EAGAIN if connection is
    established but no msg is received yet.
    Return number of bytes read if socket is established and there is
    msg received.

The new API simplifies life for applications that always perform a write()
immediately after a successful connect(). Such applications can now take
advantage of Fast Open by merely making one new setsockopt() call at the time
of creating the socket. Nothing else about the application's socket call
sequence needs to change.

BUG= chromium:687452 
TEST=build/boot on Kevin

Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 19f6d3f3c8422d65b5e3d2162e30ef07c6e21ea2)
[SR: conflicts because we're missing TCP chronograph support and
     rate limit support]
Conflicts:
	include/linux/tcp.h
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: Iebfb1a6065bf4b975189c6c6b77270e22b001344
Reviewed-on: https://chromium-review.googlesource.com/435509
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Wei Wang <weiwan@google.com>

[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/net/ipv4/tcp.c
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/net/ipv4/af_inet.c
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/net/ipv6/tcp_ipv6.c
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/include/linux/tcp.h
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/include/net/tcp.h
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/include/net/inet_sock.h
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/include/uapi/linux/tcp.h
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/net/ipv4/tcp_ipv4.c
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/net/ipv4/tcp_fastopen.c

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/24f4ced7687378865ae304737c9911cdf756913f

commit 24f4ced7687378865ae304737c9911cdf756913f
Author: Willy Tarreau <w@1wt.eu>
Date: Wed Feb 08 04:14:19 2017

UPSTREAM: net/tcp-fastopen: make connect()'s return case more consistent with non-TFO

Without TFO, any subsequent connect() call after a successful one returns
-1 EISCONN. The last API update ensured that __inet_stream_connect() can
return -1 EINPROGRESS in response to sendmsg() when TFO is in use to
indicate that the connection is now in progress. Unfortunately since this
function is used both for connect() and sendmsg(), it has the undesired
side effect of making connect() now return -1 EINPROGRESS as well after
a successful call, while at the same time poll() returns POLLOUT. This
can confuse some applications which happen to call connect() and to
check for -1 EISCONN to ensure the connection is usable, and for which
EINPROGRESS indicates a need to poll, causing a loop.

This problem was encountered in haproxy where a call to connect() is
precisely used in certain cases to confirm a connection's readiness.
While arguably haproxy's behaviour should be improved here, it seems
important to aim at a more robust behaviour when the goal of the new
API is to make it easier to implement TFO in existing applications.

This patch simply ensures that we preserve the same semantics as in
the non-TFO case on the connect() syscall when using TFO, while still
returning -1 EINPROGRESS on sendmsg(). For this we simply tell
__inet_stream_connect() whether we're doing a regular connect() or in
fact connecting for a sendmsg() call.

BUG= chromium:687452 
TEST=build/boot on Kevin

Cc: Wei Wang <weiwan@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 3979ad7e82dfe3fb94a51c3915e64ec64afa45c3)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: I668d700a70339763d61c8908e5561054a83d19c2
Reviewed-on: https://chromium-review.googlesource.com/435510
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Wei Wang <weiwan@google.com>

[modify] https://crrev.com/24f4ced7687378865ae304737c9911cdf756913f/include/net/inet_common.h
[modify] https://crrev.com/24f4ced7687378865ae304737c9911cdf756913f/net/ipv4/af_inet.c
[modify] https://crrev.com/24f4ced7687378865ae304737c9911cdf756913f/net/ipv4/tcp.c

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/68e09f4b2d27fb97e23c533b9663ff615694fc55

commit 68e09f4b2d27fb97e23c533b9663ff615694fc55
Author: Wei Wang <weiwan@google.com>
Date: Wed Feb 08 04:14:14 2017

UPSTREAM: net/tcp-fastopen: refactor cookie check logic

Refactor the cookie check logic in tcp_send_syn_data() into a function.
This function will be called else where in later changes.

BUG= chromium:687452 
TEST=build/boot on Kevin

Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 065263f40f0972d5f1cd294bb0242bd5aa5f06b2)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: I8969787f0c31f9552819abbcac9ef5dee32808d5
Reviewed-on: https://chromium-review.googlesource.com/435506
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Wei Wang <weiwan@google.com>

[modify] https://crrev.com/68e09f4b2d27fb97e23c533b9663ff615694fc55/net/ipv4/tcp_output.c
[modify] https://crrev.com/68e09f4b2d27fb97e23c533b9663ff615694fc55/net/ipv4/tcp_fastopen.c
[modify] https://crrev.com/68e09f4b2d27fb97e23c533b9663ff615694fc55/include/net/tcp.h

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/94c3b157f5c050784da0785b51046c079adf97b9

commit 94c3b157f5c050784da0785b51046c079adf97b9
Author: Wei Wang <weiwan@google.com>
Date: Wed Feb 08 04:14:15 2017

UPSTREAM: net: Remove __sk_dst_reset() in tcp_v6_connect()

Remove __sk_dst_reset() in the failure handling because __sk_dst_reset()
will eventually get called when sk is released. No need to handle it in
the protocol specific connect call.
This is also to make the code path consistent with ipv4.

BUG= chromium:687452 
TEST=build/boot on Kevin

Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 25776aa943401662617437841b3d3ea4693ee98a)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: I5493bf7fd3f4c76bbfe5bcba611b6cb863526090
Reviewed-on: https://chromium-review.googlesource.com/435507
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Wei Wang <weiwan@google.com>

[modify] https://crrev.com/94c3b157f5c050784da0785b51046c079adf97b9/net/ipv6/tcp_ipv6.c

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/512ce30bbb02eb9b4749052c9df226aeb1d48889

commit 512ce30bbb02eb9b4749052c9df226aeb1d48889
Author: Andrey Vagin <avagin@openvz.org>
Date: Wed Feb 08 04:14:16 2017

UPSTREAM: tcp: add an ability to dump and restore window parameters

We found that sometimes a restored tcp socket doesn't work.

A reason of this bug is incorrect window parameters and in this case
tcp_acceptable_seq() returns tcp_wnd_end(tp) instead of tp->snd_nxt. The
other side drops packets with this seq, because seq is less than
tp->rcv_nxt ( tcp_sequence() ).

Data from a send queue is sent only if there is enough space in a
window, so when we restore unacked data, we need to expand a window to
fit this data.

This was in a first version of this patch:
"tcp: extend window to fit all restored unacked data in a send queue"

Then Alexey recommended me to restore window parameters instead of
adjusted them according with data in a sent queue. This sounds resonable.

rcv_wnd has to be restored, because it was reported to another side
and the offered window is never shrunk.
One of reasons why we need to restore snd_wnd was described above.

BUG= chromium:687452 
TEST=build/boot on Kevin

Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit b1ed4c4fa9a5ccf325184fd90edc50978ef6e33a)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: Id9ee029021e2cccc1e1bb0ed29840c9953578a90
Reviewed-on: https://chromium-review.googlesource.com/435508
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Wei Wang <weiwan@google.com>

[modify] https://crrev.com/512ce30bbb02eb9b4749052c9df226aeb1d48889/include/uapi/linux/tcp.h
[modify] https://crrev.com/512ce30bbb02eb9b4749052c9df226aeb1d48889/net/ipv4/tcp.c

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/026bd6878514de5109ac2a9e769d7ddf6291631a

commit 026bd6878514de5109ac2a9e769d7ddf6291631a
Author: Wei Wang <weiwan@google.com>
Date: Wed Feb 08 04:14:18 2017

BACKPORT: net/tcp-fastopen: Add new API support

This patch adds a new socket option, TCP_FASTOPEN_CONNECT, as an
alternative way to perform Fast Open on the active side (client). Prior
to this patch, a client needs to replace the connect() call with
sendto(MSG_FASTOPEN). This can be cumbersome for applications who want
to use Fast Open: these socket operations are often done in lower layer
libraries used by many other applications. Changing these libraries
and/or the socket call sequences are not trivial. A more convenient
approach is to perform Fast Open by simply enabling a socket option when
the socket is created w/o changing other socket calls sequence:
  s = socket()
    create a new socket
  setsockopt(s, IPPROTO_TCP, TCP_FASTOPEN_CONNECT );
    newly introduced sockopt
    If set, new functionality described below will be used.
    Return ENOTSUPP if TFO is not supported or not enabled in the
    kernel.

  connect()
    With cookie present, return 0 immediately.
    With no cookie, initiate 3WHS with TFO cookie-request option and
    return -1 with errno = EINPROGRESS.

  write()/sendmsg()
    With cookie present, send out SYN with data and return the number of
    bytes buffered.
    With no cookie, and 3WHS not yet completed, return -1 with errno =
    EINPROGRESS.
    No MSG_FASTOPEN flag is needed.

  read()
    Return -1 with errno = EWOULDBLOCK/EAGAIN if connect() is called but
    write() is not called yet.
    Return -1 with errno = EWOULDBLOCK/EAGAIN if connection is
    established but no msg is received yet.
    Return number of bytes read if socket is established and there is
    msg received.

The new API simplifies life for applications that always perform a write()
immediately after a successful connect(). Such applications can now take
advantage of Fast Open by merely making one new setsockopt() call at the time
of creating the socket. Nothing else about the application's socket call
sequence needs to change.

BUG= chromium:687452 
TEST=build/boot on Kevin

Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 19f6d3f3c8422d65b5e3d2162e30ef07c6e21ea2)
[SR: conflicts because we're missing TCP chronograph support and
     rate limit support]
Conflicts:
	include/linux/tcp.h
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: Iebfb1a6065bf4b975189c6c6b77270e22b001344
Reviewed-on: https://chromium-review.googlesource.com/435509
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Wei Wang <weiwan@google.com>

[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/net/ipv4/tcp.c
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/net/ipv4/af_inet.c
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/net/ipv6/tcp_ipv6.c
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/include/linux/tcp.h
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/include/net/tcp.h
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/include/net/inet_sock.h
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/include/uapi/linux/tcp.h
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/net/ipv4/tcp_ipv4.c
[modify] https://crrev.com/026bd6878514de5109ac2a9e769d7ddf6291631a/net/ipv4/tcp_fastopen.c

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/24f4ced7687378865ae304737c9911cdf756913f

commit 24f4ced7687378865ae304737c9911cdf756913f
Author: Willy Tarreau <w@1wt.eu>
Date: Wed Feb 08 04:14:19 2017

UPSTREAM: net/tcp-fastopen: make connect()'s return case more consistent with non-TFO

Without TFO, any subsequent connect() call after a successful one returns
-1 EISCONN. The last API update ensured that __inet_stream_connect() can
return -1 EINPROGRESS in response to sendmsg() when TFO is in use to
indicate that the connection is now in progress. Unfortunately since this
function is used both for connect() and sendmsg(), it has the undesired
side effect of making connect() now return -1 EINPROGRESS as well after
a successful call, while at the same time poll() returns POLLOUT. This
can confuse some applications which happen to call connect() and to
check for -1 EISCONN to ensure the connection is usable, and for which
EINPROGRESS indicates a need to poll, causing a loop.

This problem was encountered in haproxy where a call to connect() is
precisely used in certain cases to confirm a connection's readiness.
While arguably haproxy's behaviour should be improved here, it seems
important to aim at a more robust behaviour when the goal of the new
API is to make it easier to implement TFO in existing applications.

This patch simply ensures that we preserve the same semantics as in
the non-TFO case on the connect() syscall when using TFO, while still
returning -1 EINPROGRESS on sendmsg(). For this we simply tell
__inet_stream_connect() whether we're doing a regular connect() or in
fact connecting for a sendmsg() call.

BUG= chromium:687452 
TEST=build/boot on Kevin

Cc: Wei Wang <weiwan@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 3979ad7e82dfe3fb94a51c3915e64ec64afa45c3)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: I668d700a70339763d61c8908e5561054a83d19c2
Reviewed-on: https://chromium-review.googlesource.com/435510
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Wei Wang <weiwan@google.com>

[modify] https://crrev.com/24f4ced7687378865ae304737c9911cdf756913f/include/net/inet_common.h
[modify] https://crrev.com/24f4ced7687378865ae304737c9911cdf756913f/net/ipv4/af_inet.c
[modify] https://crrev.com/24f4ced7687378865ae304737c9911cdf756913f/net/ipv4/tcp.c

Status: WontFix (was: Assigned)
We're removing FastOpen.

Sign in to add a comment