Adds TCP FastOpen blackhole recovery code to avoid getting persistently blackholed.

net/socket/tcp_socket_libevent.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/tcp_socket.h"
 
 #include <errno.h>
 #include <netinet/tcp.h>
 #include <sys/socket.h>
 
@@ skipping 21 matching lines @@
 
 namespace net {
 
 namespace {
 
 // True if OS supports TCP FastOpen.
 bool g_tcp_fastopen_supported = false;
 // True if TCP FastOpen is user-enabled for all connections.
 // TODO(jri): Change global variable to param in HttpNetworkSession::Params.
 bool g_tcp_fastopen_user_enabled = false;
+// True if TCP FastOpen connect-with-write has failed at least once.
+bool g_tcp_fastopen_has_failed = false;
+
 
 // SetTCPNoDelay turns on/off buffering in the kernel. By default, TCP sockets
 // will wait up to 200ms for more data to complete a packet before transmitting.
 // After calling this function, the kernel will not wait. See TCP_NODELAY in
 // `man 7 tcp`.
 bool SetTCPNoDelay(int fd, bool no_delay) {
   int on = no_delay ? 1 : 0;
   int error = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on));
   return error == 0;
 }
@@ skipping 70 matching lines @@
       base::WorkerPool::GetTaskRunner(/*task_is_slow=*/false).get(),
       FROM_HERE,
       base::Bind(SystemSupportsTCPFastOpen),
       base::Bind(RegisterTCPFastOpenIntentAndSupport, user_enabled));
 #endif
 }
 
 TCPSocketLibevent::TCPSocketLibevent(NetLog* net_log,
                                      const NetLog::Source& source)
     : use_tcp_fastopen_(false),
+      tcp_fastopen_write_attempted_(false),
       tcp_fastopen_connected_(false),
-      fast_open_status_(FAST_OPEN_STATUS_UNKNOWN),
+      tcp_fastopen_status_(TCP_FASTOPEN_STATUS_UNKNOWN),
       logging_multiple_connect_attempts_(false),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_SOCKET)) {
   net_log_.BeginEvent(NetLog::TYPE_SOCKET_ALIVE,
                       source.ToEventParametersCallback());
 }
 
 TCPSocketLibevent::~TCPSocketLibevent() {
   net_log_.EndEvent(NetLog::TYPE_SOCKET_ALIVE);
-  if (tcp_fastopen_connected_) {
+  if (tcp_fastopen_write_attempted_) {
     UMA_HISTOGRAM_ENUMERATION("Net.TcpFastOpenSocketConnection",
-                              fast_open_status_, FAST_OPEN_MAX_VALUE);
+                              tcp_fastopen_status_, TCP_FASTOPEN_MAX_VALUE);
   }
 }
 
 int TCPSocketLibevent::Open(AddressFamily family) {
   DCHECK(!socket_);
   socket_.reset(new SocketLibevent);
   int rv = socket_->Open(ConvertAddressFamily(family));
   if (rv != OK)
     socket_.reset();
   return rv;
@@ skipping 60 matching lines @@
 
   net_log_.BeginEvent(NetLog::TYPE_TCP_CONNECT_ATTEMPT,
                       CreateNetLogIPEndPointCallback(&address));
 
   SockaddrStorage storage;
   if (!address.ToSockAddr(storage.addr, &storage.addr_len))
     return ERR_ADDRESS_INVALID;
 
   if (use_tcp_fastopen_) {
     // With TCP FastOpen, we pretend that the socket is connected.
-    DCHECK(!tcp_fastopen_connected_);
+    DCHECK(!tcp_fastopen_write_attempted_);
     socket_->SetPeerAddress(storage);
     return OK;
   }
 
   int rv = socket_->Connect(storage,
                             base::Bind(&TCPSocketLibevent::ConnectCompleted,
                                        base::Unretained(this), callback));
   if (rv != ERR_IO_PENDING)
     rv = HandleConnectCompleted(rv);
   return rv;
 }
 
 bool TCPSocketLibevent::IsConnected() const {
   if (!socket_)
     return false;
 
-  if (use_tcp_fastopen_ && !tcp_fastopen_connected_ &&
+  if (use_tcp_fastopen_ && !tcp_fastopen_write_attempted_ &&
       socket_->HasPeerAddress()) {
     // With TCP FastOpen, we pretend that the socket is connected.
     // This allows GetPeerAddress() to return peer_address_.
     return true;
   }
 
   return socket_->IsConnected();
 }
 
 bool TCPSocketLibevent::IsConnectedAndIdle() const {
   // TODO(wtc): should we also handle the TCP FastOpen case here,
   // as we do in IsConnected()?
   return socket_ && socket_->IsConnectedAndIdle();
 }
 
 int TCPSocketLibevent::Read(IOBuffer* buf,
                             int buf_len,
                             const CompletionCallback& callback) {
   DCHECK(socket_);
   DCHECK(!callback.is_null());
 
   int rv = socket_->Read(
       buf, buf_len,
       base::Bind(&TCPSocketLibevent::ReadCompleted,
                  // Grab a reference to |buf| so that ReadCompleted() can still
                  // use it when Read() completes, as otherwise, this transfers
                  // ownership of buf to socket.
                  base::Unretained(this), make_scoped_refptr(buf), callback));
-  if (rv >= 0)
-    RecordFastOpenStatus();
   if (rv != ERR_IO_PENDING)
     rv = HandleReadCompleted(buf, rv);
   return rv;
 }
 
 int TCPSocketLibevent::Write(IOBuffer* buf,
                              int buf_len,
                              const CompletionCallback& callback) {
   DCHECK(socket_);
   DCHECK(!callback.is_null());
 
   CompletionCallback write_callback =
       base::Bind(&TCPSocketLibevent::WriteCompleted,
                  // Grab a reference to |buf| so that WriteCompleted() can still
                  // use it when Write() completes, as otherwise, this transfers
                  // ownership of buf to socket.
                  base::Unretained(this), make_scoped_refptr(buf), callback);
   int rv;
-  if (use_tcp_fastopen_ && !tcp_fastopen_connected_) {
+
+  if (use_tcp_fastopen_ && !tcp_fastopen_write_attempted_) {
+    DCHECK(!g_tcp_fastopen_has_failed);
     rv = TcpFastOpenWrite(buf, buf_len, write_callback);
   } else {
     rv = socket_->Write(buf, buf_len, write_callback);
   }
 
   if (rv != ERR_IO_PENDING)
     rv = HandleWriteCompleted(buf, rv);
   return rv;
 }
 
@@ skipping 105 matching lines @@
   return SetTCPKeepAlive(socket_->socket_fd(), enable, delay);
 }
 
 bool TCPSocketLibevent::SetNoDelay(bool no_delay) {
   DCHECK(socket_);
   return SetTCPNoDelay(socket_->socket_fd(), no_delay);
 }
 
 void TCPSocketLibevent::Close() {
   socket_.reset();
+
+  // Reset TCP FastOpen state.

[mmenke, 2014/09/24 14:46:11]

Should we have:

if (tcp_fastopen_write_attempted_) {
  UMA_HISTOGRAM_ENUMERATION("Net.TcpFastOpenSocketConnection"
                            fast_open_status_, FAST_OPEN_MAX_VALUE);
}

And then just call Close() in the destructor?

[Jana, 2014/09/25 16:10:16]

Good suggestion -- Done.

+  use_tcp_fastopen_ = false;
   tcp_fastopen_connected_ = false;
-  fast_open_status_ = FAST_OPEN_STATUS_UNKNOWN;
+  tcp_fastopen_write_attempted_ = false;
+  tcp_fastopen_status_ = TCP_FASTOPEN_STATUS_UNKNOWN;
 }
 
 bool TCPSocketLibevent::UsingTCPFastOpen() const {
   return use_tcp_fastopen_;
 }
 
 void TCPSocketLibevent::EnableTCPFastOpenIfSupported() {
-  if (IsTCPFastOpenSupported())
+  // Do not enable TCP FastOpen if it had previously failed.
+  // This check conservatively avoids middleboxes that may blackhole
+  // TCP FastOpen SYN+Data packets; on such a failure, subsequent sockets
+  // should not use TCP FastOpen.
+  if (IsTCPFastOpenSupported() && !g_tcp_fastopen_has_failed)
     use_tcp_fastopen_ = true;
 }
 
 bool TCPSocketLibevent::IsValid() const {
   return socket_ != NULL && socket_->socket_fd() != kInvalidSocket;
 }
 
 void TCPSocketLibevent::StartLoggingMultipleConnectAttempts(
     const AddressList& addresses) {
   if (!logging_multiple_connect_attempts_) {
@@ skipping 109 matching lines @@
 
   net_log_.EndEvent(NetLog::TYPE_TCP_CONNECT,
                     CreateNetLogSourceAddressCallback(storage.addr,
                                                       storage.addr_len));
 }
 
 void TCPSocketLibevent::ReadCompleted(const scoped_refptr<IOBuffer>& buf,
                                       const CompletionCallback& callback,
                                       int rv) {
   DCHECK_NE(ERR_IO_PENDING, rv);
-  // Records TCP FastOpen status regardless of error in asynchronous case.
-  // TODO(rdsmith,jri): Change histogram name to indicate it could be called on
-  // error.
-  RecordFastOpenStatus();
   callback.Run(HandleReadCompleted(buf.get(), rv));
 }
 
 int TCPSocketLibevent::HandleReadCompleted(IOBuffer* buf, int rv) {
+  if (tcp_fastopen_write_attempted_ && !tcp_fastopen_connected_ &&
+      !g_tcp_fastopen_has_failed) {

[mmenke, 2014/09/24 14:46:11]

The !g_tcp_fastopen_has_failed still seems kinda weird here.  Suppose this
happens:

* Try to connect socket A using Tcp Fast Open
* Try to connect socket B using Tcp Fast Open
* Socket A ends up here with a failure.  g_tcp_fastopen_has_failed is set to
false.
* Socket B ends up here (With a success or a failure):  We now have
tcp_fastopen_write_attempted_ is true, tcp_fastopen_connected_ is false,
g_tcp_fastopen_has_failed is true, and tcp_fastopen_status_ is
TCP_FASTOPEN_FAST_CONNECT_RETURN or TCP_FASTOPEN_SLOW_CONNECT_RETURN,
presumably.  We don't update tcp_fastopen_status_ for socket B because of the
failure, but we then record the histogram on close/destruction for socket
B...And the value we record isn't actually in any way comparable to the
histogram we record for socket A, since we don't call RecordTCPFastOpenStatus().

[Jana, 2014/09/25 16:10:16]

Hmm -- good point. I had the g_tcp_fastopen_has_failed check as a redundant one,
but as you point out in your example, it prevents a perfectly good histogram
event from being recorded correctly. Also in light of my new histogram
additions, it makes sense to simply allow all failures to go through and record
themselves. I'm removing the check for g_tcp_fastopen_has_failed here.

+    // A TCP FastOpen connect-with-write was attempted. This read was a
+    // subsequent read, which either succeeded or failed. If the read
+    // succeeded, the socket is considered connected via TCP FastOpen.
+    // If the read failed, TCP FastOpen is (conservatively) turned off for all
+    // subsequent connections. TCP FastOpen status is recorded in both cases.
+    // TODO(rdsmith,jri): Change histogram name to indicate it could be called
+    // on error.
+    if (rv >= 0)
+      tcp_fastopen_connected_ = true;
+    else
+      g_tcp_fastopen_has_failed = true;
+    RecordTCPFastOpenStatus();
+  }
+
   if (rv < 0) {
     net_log_.AddEvent(NetLog::TYPE_SOCKET_READ_ERROR,
                       CreateNetLogSocketErrorCallback(rv, errno));
     return rv;
   }
 
   base::StatsCounter read_bytes("tcp.read_bytes");
   read_bytes.Add(rv);
   net_log_.AddByteTransferEvent(NetLog::TYPE_SOCKET_BYTES_RECEIVED, rv,
                                 buf->data());
   return rv;
 }
 
 void TCPSocketLibevent::WriteCompleted(const scoped_refptr<IOBuffer>& buf,
                                        const CompletionCallback& callback,
-                                       int rv) const {
+                                       int rv) {
   DCHECK_NE(ERR_IO_PENDING, rv);
   callback.Run(HandleWriteCompleted(buf.get(), rv));
 }
 
-int TCPSocketLibevent::HandleWriteCompleted(IOBuffer* buf, int rv) const {
+int TCPSocketLibevent::HandleWriteCompleted(IOBuffer* buf, int rv) {
   if (rv < 0) {
+    if (tcp_fastopen_write_attempted_ && !tcp_fastopen_connected_) {
+      // TCP FastOpen connect-with-write was attempted, and the write failed
+      // for unknown reasons. Record status and (conservatively) turn off
+      // TCP FastOpen for all subsequent connections.
+      tcp_fastopen_status_ = TCP_FASTOPEN_ERROR;
+      g_tcp_fastopen_has_failed = true;
+    }
     net_log_.AddEvent(NetLog::TYPE_SOCKET_WRITE_ERROR,
                       CreateNetLogSocketErrorCallback(rv, errno));
     return rv;
   }
 
   base::StatsCounter write_bytes("tcp.write_bytes");
   write_bytes.Add(rv);
   net_log_.AddByteTransferEvent(NetLog::TYPE_SOCKET_BYTES_SENT, rv,
                                 buf->data());
   return rv;
@@ skipping 15 matching lines @@
   // for system support on startup, but users may dynamically disable TCP Fast
   // Open via sysctl.
   flags |= MSG_NOSIGNAL;
 #endif // defined(OS_LINUX)
   rv = HANDLE_EINTR(sendto(socket_->socket_fd(),
                            buf->data(),
                            buf_len,
                            flags,
                            storage.addr,
                            storage.addr_len));
-  tcp_fastopen_connected_ = true;
+  tcp_fastopen_write_attempted_ = true;
 
   if (rv >= 0) {
-    fast_open_status_ = FAST_OPEN_FAST_CONNECT_RETURN;
+    tcp_fastopen_status_ = TCP_FASTOPEN_FAST_CONNECT_RETURN;
     return rv;
   }
 
   DCHECK_NE(EPIPE, errno);
 
   // If errno == EINPROGRESS, that means the kernel didn't have a cookie
   // and would block. The kernel is internally doing a connect() though.
   // Remap EINPROGRESS to EAGAIN so we treat this the same as our other
   // asynchronous cases. Note that the user buffer has not been copied to
   // kernel space.
   if (errno == EINPROGRESS) {
     rv = ERR_IO_PENDING;
   } else {
     rv = MapSystemError(errno);
   }
 
   if (rv != ERR_IO_PENDING) {
-    fast_open_status_ = FAST_OPEN_ERROR;
+    // TCP FastOpen connect-with-write was attempted, and the write failed for
+    // unknown reasons. Record status and (conservatively) turn off
+    // TCP FastOpen for all subsequent connections.
+    tcp_fastopen_status_ = TCP_FASTOPEN_ERROR;
+    g_tcp_fastopen_has_failed = true;
     return rv;
   }
 
-  fast_open_status_ = FAST_OPEN_SLOW_CONNECT_RETURN;
+  tcp_fastopen_status_ = TCP_FASTOPEN_SLOW_CONNECT_RETURN;
   return socket_->WaitForWrite(buf, buf_len, callback);
 }
 
-void TCPSocketLibevent::RecordFastOpenStatus() {
-  if (use_tcp_fastopen_ &&
-      (fast_open_status_ == FAST_OPEN_FAST_CONNECT_RETURN ||
-       fast_open_status_ == FAST_OPEN_SLOW_CONNECT_RETURN)) {
-    DCHECK_NE(FAST_OPEN_STATUS_UNKNOWN, fast_open_status_);
-    bool getsockopt_success(false);
-    bool server_acked_data(false);
+void TCPSocketLibevent::RecordTCPFastOpenStatus() {

[mmenke, 2014/09/24 14:46:11]

Maybe UpdateTCPFastOpenStatusAfterRead?

[Jana, 2014/09/25 16:10:16]

Done.

+  if (!use_tcp_fastopen_ ||
+      (tcp_fastopen_status_ != TCP_FASTOPEN_FAST_CONNECT_RETURN &&
+       tcp_fastopen_status_ != TCP_FASTOPEN_SLOW_CONNECT_RETURN)) {
+    // Not using TCP FastOpen, or status has been finalized.
+    return;
+  }
+  DCHECK_NE(TCP_FASTOPEN_STATUS_UNKNOWN, tcp_fastopen_status_);
+
+  if (tcp_fastopen_write_attempted_ && !tcp_fastopen_connected_) {
+    // TCP FastOpen connect-with-write was attempted, and failed.
+    tcp_fastopen_status_ =
+      (tcp_fastopen_status_ == TCP_FASTOPEN_FAST_CONNECT_RETURN ?
+       TCP_FASTOPEN_FAST_CONNECT_FAILED : TCP_FASTOPEN_SLOW_CONNECT_FAILED);

[mmenke, 2014/09/24 14:46:11]

nit:  4 space indent.

[Jana, 2014/09/25 16:10:16]

Done.

+    return;
+  }
+
+  bool getsockopt_success(false);
+  bool server_acked_data(false);
 #if defined(TCP_INFO)
-    // Probe to see the if the socket used TCP FastOpen.
-    tcp_info info;
-    socklen_t info_len = sizeof(tcp_info);
-    getsockopt_success =
-        getsockopt(socket_->socket_fd(), IPPROTO_TCP, TCP_INFO,
-                   &info, &info_len) == 0 &&
-        info_len == sizeof(tcp_info);
-    server_acked_data = getsockopt_success &&
-        (info.tcpi_options & TCPI_OPT_SYN_DATA);
+  // Probe to see the if the socket used TCP FastOpen.
+  tcp_info info;
+  socklen_t info_len = sizeof(tcp_info);
+  getsockopt_success = getsockopt(socket_->socket_fd(), IPPROTO_TCP, TCP_INFO,
+                                  &info, &info_len) == 0 &&
+                       info_len == sizeof(tcp_info);
+  server_acked_data = getsockopt_success &&
+                      (info.tcpi_options & TCPI_OPT_SYN_DATA);
 #endif
-    if (getsockopt_success) {
-      if (fast_open_status_ == FAST_OPEN_FAST_CONNECT_RETURN) {
-        fast_open_status_ = (server_acked_data ? FAST_OPEN_SYN_DATA_ACK :
-                             FAST_OPEN_SYN_DATA_NACK);
-      } else {
-        fast_open_status_ = (server_acked_data ? FAST_OPEN_NO_SYN_DATA_ACK :
-                             FAST_OPEN_NO_SYN_DATA_NACK);
-      }
+
+  if (getsockopt_success) {
+    if (tcp_fastopen_status_ == TCP_FASTOPEN_FAST_CONNECT_RETURN) {
+      tcp_fastopen_status_ = (server_acked_data ?
+                              TCP_FASTOPEN_SYN_DATA_ACK :
+                              TCP_FASTOPEN_SYN_DATA_NACK);
     } else {
-      fast_open_status_ = (fast_open_status_ == FAST_OPEN_FAST_CONNECT_RETURN ?
-                           FAST_OPEN_SYN_DATA_FAILED :
-                           FAST_OPEN_NO_SYN_DATA_FAILED);
+      tcp_fastopen_status_ = (server_acked_data ?
+                              TCP_FASTOPEN_NO_SYN_DATA_ACK :
+                              TCP_FASTOPEN_NO_SYN_DATA_NACK);
     }
+  } else {
+    tcp_fastopen_status_ =
+        (tcp_fastopen_status_ == TCP_FASTOPEN_FAST_CONNECT_RETURN ?
+         TCP_FASTOPEN_SYN_DATA_GETSOCKOPT_FAILED :
+         TCP_FASTOPEN_NO_SYN_DATA_GETSOCKOPT_FAILED);
   }
 }
 
 }  // namespace net