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

net/socket/tcp_socket_libevent.cc

Index: net/socket/tcp_socket_libevent.cc
diff --git a/net/socket/tcp_socket_libevent.cc b/net/socket/tcp_socket_libevent.cc
index f39611b603b63df6774e035017a7bbb60aef0750..29a7942a32f4d82c8bc8314623c3f514e9dc67d7 100644
--- a/net/socket/tcp_socket_libevent.cc
+++ b/net/socket/tcp_socket_libevent.cc
@@ -39,6 +39,9 @@ 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.
@@ -129,8 +132,9 @@ void CheckSupportAndMaybeEnableTCPFastOpen(bool user_enabled) {
 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_(FAST_OPEN_STATUS_UNKNOWN),
       logging_multiple_connect_attempts_(false),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_SOCKET)) {
   net_log_.BeginEvent(NetLog::TYPE_SOCKET_ALIVE,
@@ -139,9 +143,9 @@ TCPSocketLibevent::TCPSocketLibevent(NetLog* net_log,
 
 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_, FAST_OPEN_MAX_VALUE);
   }
 }
 
@@ -222,7 +226,7 @@ int TCPSocketLibevent::Connect(const IPEndPoint& address,
 
   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;
   }
@@ -239,7 +243,7 @@ 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_.
@@ -269,7 +273,7 @@ int TCPSocketLibevent::Read(IOBuffer* buf,
                  // ownership of buf to socket.
                  base::Unretained(this), make_scoped_refptr(buf), callback));
   if (rv >= 0)
-    RecordFastOpenStatus();
+    RecordTCPFastOpenStatus();
   if (rv != ERR_IO_PENDING)
     rv = HandleReadCompleted(buf, rv);
   return rv;
@@ -288,7 +292,8 @@ int TCPSocketLibevent::Write(IOBuffer* buf,
                  // 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_) {
     rv = TcpFastOpenWrite(buf, buf_len, write_callback);
   } else {
     rv = socket_->Write(buf, buf_len, write_callback);
@@ -415,7 +420,7 @@ bool TCPSocketLibevent::SetNoDelay(bool no_delay) {
 void TCPSocketLibevent::Close() {
   socket_.reset();
   tcp_fastopen_connected_ = false;
-  fast_open_status_ = FAST_OPEN_STATUS_UNKNOWN;
+  tcp_fastopen_status_ = FAST_OPEN_STATUS_UNKNOWN;

[Jana, 2014/09/19 02:18:16]

Why do we need to reset tcp_fastopen_status_ here?

[Randy Smith (Not in Mondays), 2014/09/22 16:05:13]

So I don't remember clearly why I did this, but looking at the code, I suspect I
was worried about the object being re-used; I don't see anything in the API
definition of the implementation that disallows that?

[Jana, 2014/09/22 17:04:56]

Maybe Matt can answer this question, but as I read it, I don't think that
happens... client_socket_factory creates TCPClientSockets, and a new TCPSocket
(which maps to TCPSocketLibevent for *ix) is created for each TCPClientSocket.
That said, I agree that I also don't see anything in the API definition of
TCPSocketLibevent that explicitly prevents an object from being reused...

If it is possible that a TCPSocketLibevent object is reused, then we'll have to
reset use_tcp_fastopen_ as well, and I'd want to double-check the code to make
sure that state is correctly reset.

[Randy Smith (Not in Mondays), 2014/09/22 17:42:47]

So I'm a fairly big believer in not making assumptions beyond what's codified in
the header file about how a class will be used.  If you wanted to stick a note
in the header file about not reusing the class and dump a DCHECK someplace in
the state transition to make sure it's not being re-used, I'm comfortable with
not resetting the state.

[Jana, 2014/09/22 23:16:04]

Yup, that sounds reasonable. I will try a DCHECK and check in with Matt as well.
If not, we'll have to reset more TCP FastOpen state as well.

 }
 
 bool TCPSocketLibevent::UsingTCPFastOpen() const {
@@ -423,7 +428,11 @@ bool TCPSocketLibevent::UsingTCPFastOpen() const {
 }
 
 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;
 }
 
@@ -556,7 +565,7 @@ void TCPSocketLibevent::ReadCompleted(const scoped_refptr<IOBuffer>& buf,
   // 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();
+  RecordTCPFastOpenStatus();
   callback.Run(HandleReadCompleted(buf.get(), rv));
 }
 
@@ -585,9 +594,16 @@ int TCPSocketLibevent::HandleWriteCompleted(IOBuffer* buf, int rv) const {
   if (rv < 0) {
     net_log_.AddEvent(NetLog::TYPE_SOCKET_WRITE_ERROR,
                       CreateNetLogSocketErrorCallback(rv, errno));
+    if (tcp_fastopen_write_attempted_ && !tcp_fastopen_connected_) {
+      // TCP FastOpen connect-with-write was attempted, but failed.
+      // Turn off TCP FastOpen for any subsequent connections.
+      g_tcp_fastopen_has_failed = true;
+      RecordTCPFastOpenStatus();
+    }
     return rv;
   }
 
+  tcp_fastopen_connected_ = true;
   base::StatsCounter write_bytes("tcp.write_bytes");
   write_bytes.Add(rv);
   net_log_.AddByteTransferEvent(NetLog::TYPE_SOCKET_BYTES_SENT, rv,
@@ -618,10 +634,10 @@ int TCPSocketLibevent::TcpFastOpenWrite(
                            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_ = FAST_OPEN_FAST_CONNECT_RETURN;
     return rv;
   }
 
@@ -639,19 +655,28 @@ int TCPSocketLibevent::TcpFastOpenWrite(
   }
 
   if (rv != ERR_IO_PENDING) {
-    fast_open_status_ = FAST_OPEN_ERROR;
+    tcp_fastopen_status_ = FAST_OPEN_ERROR;
     return rv;
   }
 
-  fast_open_status_ = FAST_OPEN_SLOW_CONNECT_RETURN;
+  tcp_fastopen_status_ = FAST_OPEN_SLOW_CONNECT_RETURN;
   return socket_->WaitForWrite(buf, buf_len, callback);
 }
 
-void TCPSocketLibevent::RecordFastOpenStatus() {
+void TCPSocketLibevent::RecordTCPFastOpenStatus() {
   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_);
+      (tcp_fastopen_status_ == FAST_OPEN_FAST_CONNECT_RETURN ||
+       tcp_fastopen_status_ == FAST_OPEN_SLOW_CONNECT_RETURN)) {
+    DCHECK_NE(FAST_OPEN_STATUS_UNKNOWN, tcp_fastopen_status_);
+
+    if (g_tcp_fastopen_has_failed && tcp_fastopen_write_attempted_) {
+      // Writing TCP FastOpen SYN+DATA or SYN-option failed.
+      tcp_fastopen_status_ =
+          (tcp_fastopen_status_ == FAST_OPEN_FAST_CONNECT_RETURN ?
+           FAST_OPEN_FAST_CONNECT_FAILED : FAST_OPEN_SLOW_CONNECT_FAILED);
+      return;
+    }
+
     bool getsockopt_success(false);
     bool server_acked_data(false);
 #if defined(TCP_INFO)
@@ -666,17 +691,17 @@ void TCPSocketLibevent::RecordFastOpenStatus() {
         (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 :
+      if (tcp_fastopen_status_ == FAST_OPEN_FAST_CONNECT_RETURN) {
+        tcp_fastopen_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 :
+        tcp_fastopen_status_ = (server_acked_data ? FAST_OPEN_NO_SYN_DATA_ACK :
                              FAST_OPEN_NO_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_ =
+          (tcp_fastopen_status_ == FAST_OPEN_FAST_CONNECT_RETURN ?
+           FAST_OPEN_SYN_DATA_FAILED : FAST_OPEN_NO_SYN_DATA_FAILED);
     }
   }
 }