Bug 1430768 - Refine TFO telemetry. r=mcmanus
authorDragana Damjanovic dd.mozilla@gmail.com
Wed, 17 Jan 2018 06:47:00 +0200
changeset 454295 652042ebfff8ac76c3d57e69db364c0309fa52e2
parent 454294 eac2d7ff2c83a0fddf16bb70f5c732ed7a369f62
child 454296 33571130ef67d4d9105dae116fd469dcdf6e0fd9
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1430768
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1430768 - Refine TFO telemetry. r=mcmanus
netwerk/base/TCPFastOpenLayer.h
netwerk/base/nsSocketTransport2.cpp
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/protocol/http/nsHttpConnectionMgr.cpp
--- a/netwerk/base/TCPFastOpenLayer.h
+++ b/netwerk/base/TCPFastOpenLayer.h
@@ -61,16 +61,23 @@ typedef enum {
   // The following 4 are set when the recovery connection fails as well.
   TFO_FAILED_CONNECTION_REFUSED_NO_TFO_FAILED_TOO,
   TFO_FAILED_NET_TIMEOUT_NO_TFO_FAILED_TOO,
   TFO_FAILED_UNKNOW_ERROR_NO_TFO_FAILED_TOO,
   TFO_FAILED_BACKUP_CONNECTION_NO_TFO_FAILED_TOO,
   TFO_BACKUP_CONN, // This is a backup conn, for a halfOpenSock that was used
                    // TFO.
   TFO_INIT_FAILED, // nsHalfOpenSocket::SetupConn failed.
+  TFO_UNKNOWN_RESOLVING, // There is a high number of TFO_UNKNOWN state reported.
+                         // Let's split them depending on the socket transport state:
+                         // TFO_UNKNOWN_RESOLVING, TFO_UNKNOWN_RESOLVED,
+                         // TFO_UNKNOWN_CONNECTING and TFO_UNKNOWN_CONNECTED..
+  TFO_UNKNOWN_RESOLVED,
+  TFO_UNKNOWN_CONNECTING,
+  TFO_UNKNOWN_CONNECTED,
   TFO_FAILED,
   TFO_HTTP // TFO is disabled for non-secure connections.
 } TFOResult;
 
 nsresult AttachTCPFastOpenIOLayer(PRFileDesc *fd);
 
 // Get the result of TCP Fast Open.
 void TCPFastOpenFinish(PRFileDesc *fd, PRErrorCode &err,
--- a/netwerk/base/nsSocketTransport2.cpp
+++ b/netwerk/base/nsSocketTransport2.cpp
@@ -1558,16 +1558,20 @@ nsSocketTransport::InitiateSocket()
         }
 
         // If we have still some data buffered this data must be flush before
         // mOutput.OnSocketReady(NS_OK) is called in
         // nsSocketTransport::OnSocketReady, partially to keep socket status
         // event in order.
         mFastOpenLayerHasBufferedData = TCPFastOpenGetCurrentBufferSize(fd);
 
+        MOZ_ASSERT((mFastOpenStatus == TFO_NOT_TRIED) ||
+                   (mFastOpenStatus == TFO_DISABLED) ||
+                   (mFastOpenStatus == TFO_DATA_SENT) ||
+                   (mFastOpenStatus == TFO_TRIED));
         mFastOpenCallback->SetFastOpenStatus(mFastOpenStatus);
         SOCKET_LOG(("called StartFastOpen - code=%d; fastOpen is %s "
                     "supported.\n", code,
                     fastOpenNotSupported ? "not" : ""));
         SOCKET_LOG(("TFO status %d\n", mFastOpenStatus));
 
         if (fastOpenNotSupported) {
           // When TCP_FastOpen is turned off on the local host
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -127,21 +127,22 @@ nsHttpConnection::~nsHttpConnection()
     }
     if (mForceSendTimer) {
         mForceSendTimer->Cancel();
         mForceSendTimer = nullptr;
     }
 
     if ((mFastOpenStatus != TFO_FAILED) &&
         (mFastOpenStatus != TFO_HTTP) &&
-        ((mFastOpenStatus != TFO_DISABLED) ||
+        (((mFastOpenStatus > TFO_DISABLED_CONNECT) && (mFastOpenStatus < TFO_BACKUP_CONN)) ||
          gHttpHandler->UseFastOpen())) {
         // TFO_FAILED will be reported in the replacement connection with more
         // details.
         // Otherwise report only if TFO is enabled and supported.
+        // If TFO is disabled, report only connections ha cause it to be disabled, e.g. TFO_FAILED_NET_TIMEOUT, etc.
         Telemetry::Accumulate(Telemetry::TCP_FAST_OPEN_3, mFastOpenStatus);
     }
 }
 
 nsresult
 nsHttpConnection::Init(nsHttpConnectionInfo *info,
                        uint16_t maxHangTime,
                        nsISocketTransport *transport,
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -4421,17 +4421,29 @@ nsHalfOpenSocket::OnOutputStreamReady(ns
         // Collect telemetry for backup connection being faster than primary
         // connection. We want to collect this telemetry only for cases where
         // TFO is not used.
         mBackupConnStatsSet = true;
         Telemetry::Accumulate(Telemetry::NETWORK_HTTP_BACKUP_CONN_WON_1,
                               (out == mBackupStreamOut));
     }
 
-    nsresult rv =  SetupConn(out, false);
+    if (mFastOpenStatus == TFO_UNKNOWN) {
+        MOZ_ASSERT(out == mStreamOut);
+        if (mPrimaryStreamStatus == NS_NET_STATUS_RESOLVING_HOST) {
+            mFastOpenStatus = TFO_UNKNOWN_RESOLVING;
+        } else if (mPrimaryStreamStatus == NS_NET_STATUS_RESOLVED_HOST) {
+            mFastOpenStatus = TFO_UNKNOWN_RESOLVED;
+        } else if (mPrimaryStreamStatus == NS_NET_STATUS_CONNECTING_TO) {
+            mFastOpenStatus = TFO_UNKNOWN_CONNECTING;
+        } else if (mPrimaryStreamStatus == NS_NET_STATUS_CONNECTED_TO) {
+            mFastOpenStatus = TFO_UNKNOWN_CONNECTED;
+        }
+    }
+    nsresult rv = SetupConn(out, false);
     if (mEnt) {
         mEnt->mDoNotDestroy = false;
     }
     return rv;
 }
 
 bool
 nsHttpConnectionMgr::
@@ -4476,32 +4488,34 @@ nsHalfOpenSocket::FastOpenEnabled()
 
 nsresult
 nsHttpConnectionMgr::
 nsHalfOpenSocket::StartFastOpen()
 {
     MOZ_ASSERT(mStreamOut);
     MOZ_ASSERT(!mBackupTransport);
     MOZ_ASSERT(mEnt);
+    MOZ_ASSERT(mFastOpenStatus == TFO_UNKNOWN);
 
     LOG(("nsHalfOpenSocket::StartFastOpen [this=%p]\n",
          this));
 
     RefPtr<nsHalfOpenSocket> deleteProtector(this);
 
     mFastOpenInProgress = true;
     mEnt->mDoNotDestroy = true;
     // Remove this HalfOpen from mEnt->mHalfOpens.
     // The new connection will take care of closing this HalfOpen from now on!
     if (!mEnt->mHalfOpens.RemoveElement(this)) {
         MOZ_ASSERT(false, "HalfOpen is not in mHalfOpens!");
         mSocketTransport->SetFastOpenCallback(nullptr);
         CancelBackupTimer();
         mFastOpenInProgress = false;
         Abandon();
+        mFastOpenStatus = TFO_INIT_FAILED;
         return NS_ERROR_ABORT;
     }
 
     MOZ_ASSERT(gHttpHandler->ConnMgr()->mNumHalfOpenConns);
     if (gHttpHandler->ConnMgr()->mNumHalfOpenConns) { // just in case
         gHttpHandler->ConnMgr()->mNumHalfOpenConns--;
     }
 
@@ -4525,16 +4539,17 @@ nsHalfOpenSocket::StartFastOpen()
         // will remove reference to this HalfOpen as well.
         mSocketTransport->SetFastOpenCallback(nullptr);
         CancelBackupTimer();
         mFastOpenInProgress = false;
 
         // The connection is responsible to take care of the halfOpen so we
         // need to clean it up.
         Abandon();
+        mFastOpenStatus = TFO_INIT_FAILED;
     } else {
         LOG(("nsHalfOpenSocket::StartFastOpen [this=%p conn=%p]\n",
              this, mConnectionNegotiatingFastOpen.get()));
 
         mEnt->mHalfOpenFastOpenBackups.AppendElement(this);
         // SetupBackupTimer should setup timer which will hold a ref to this
         // halfOpen. It will failed only if it cannot create timer. Anyway just
         // to be sure I will add this deleteProtector!!!
@@ -4571,16 +4586,20 @@ nsHalfOpenSocket::SetFastOpenConnected(n
     // in case nsHttpConnection::Activate fails will be done in StartFastOpen.
     // Also OnMsgReclaimConnection can decided that we do not need this
     // transaction and cancel it as well.
     // In all other cases mConnectionNegotiatingFastOpen must not be nullptr.
     if (!mConnectionNegotiatingFastOpen) {
         return;
     }
 
+    MOZ_ASSERT((mFastOpenStatus == TFO_NOT_TRIED) || 
+               (mFastOpenStatus == TFO_DATA_SENT) ||
+               (mFastOpenStatus == TFO_TRIED));
+
     RefPtr<nsHalfOpenSocket> deleteProtector(this);
 
     mEnt->mDoNotDestroy = true;
 
     // Delete 2 points of entry to FastOpen function so that we do not reenter.
     mEnt->mHalfOpenFastOpenBackups.RemoveElement(this);
     mSocketTransport->SetFastOpenCallback(nullptr);
 
@@ -4630,17 +4649,18 @@ nsHalfOpenSocket::SetFastOpenConnected(n
         gHttpHandler->ConnMgr()->StartedConnect();
 
         // Restore callbacks.
         mStreamOut->AsyncWait(this, 0, 0, nullptr);
         mSocketTransport->SetEventSink(this, nullptr);
         mSocketTransport->SetSecurityCallbacks(this);
         mStreamIn->AsyncWait(nullptr, 0, 0, nullptr);
 
-        if (aError == NS_ERROR_CONNECTION_REFUSED) {
+        if ((aError == NS_ERROR_CONNECTION_REFUSED) ||
+            (aError == NS_ERROR_PROXY_CONNECTION_REFUSED)) {
             mFastOpenStatus = TFO_FAILED_CONNECTION_REFUSED;
         } else if (aError == NS_ERROR_NET_TIMEOUT) {
             mFastOpenStatus = TFO_FAILED_NET_TIMEOUT;
         } else {
             mFastOpenStatus = TFO_FAILED_UNKNOW_ERROR;
         }
 
     } else {
@@ -4812,18 +4832,20 @@ nsHalfOpenSocket::SetupConn(nsIAsyncOutp
     }
 
     if (NS_FAILED(rv)) {
         LOG(("nsHalfOpenSocket::SetupConn "
              "conn->init (%p) failed %" PRIx32 "\n",
              conn.get(), static_cast<uint32_t>(rv)));
 
         // Set TFO status.
-        if (mFastOpenStatus == TFO_HTTP) {
-            conn->SetFastOpenStatus(TFO_HTTP);
+        if ((mFastOpenStatus == TFO_HTTP) ||
+            (mFastOpenStatus == TFO_DISABLED) ||
+            (mFastOpenStatus == TFO_DISABLED_CONNECT)) {
+            conn->SetFastOpenStatus(mFastOpenStatus);
         } else {
             conn->SetFastOpenStatus(TFO_INIT_FAILED);
         }
         return rv;
     }
 
     // This half-open socket has created a connection.  This flag excludes it
     // from counter of actual connections used for checking limits.
@@ -4924,17 +4946,18 @@ nsHalfOpenSocket::SetupConn(nsIAsyncOutp
         if (NS_SUCCEEDED(rv) && (idx != -1)) {
             mConnectionNegotiatingFastOpen = conn;
         } else {
             conn->SetFastOpen(false);
             conn->SetFastOpenStatus(TFO_INIT_FAILED);
         }
     } else {
         conn->SetFastOpenStatus(mFastOpenStatus);
-        if (mFastOpenStatus != TFO_HTTP) {
+        if ((mFastOpenStatus != TFO_HTTP) && (mFastOpenStatus != TFO_DISABLED) &&
+            (mFastOpenStatus != TFO_DISABLED_CONNECT)) {
             mFastOpenStatus = TFO_BACKUP_CONN; // Set this to TFO_BACKUP_CONN
                                                // so that if a backup
                                                // connection is established we
                                                // do not report values twice.
         }
     }
 
     // If this halfOpenConn was speculative, but at the end the conn got a