Bug 725587 - Firefox jumps randomly from IPv6 to IPv4 and vice versa in dual-stack environment, r=mcmanus

netwerk/base/public/nsISocketTransport.idl

@@ -160,6 +160,12 @@ interface nsISocketTransport : nsITransport
      */
     const unsigned long NO_PERMANENT_STORAGE = (1 << 3);
 
+    /**
+     * If set, we will skip all IPv4 addresses the host may have and only
+     * connect to IPv6 ones.
+     */
+    const unsigned long DISABLE_IPV4 = (1 << 4);
+
     /**
      * Socket QoS/ToS markings. Valid values are IPTOS_DSCP_AFxx or
      * IPTOS_CLASS_CSx (or IPTOS_DSCP_EF, but currently no supported

netwerk/base/src/nsSocketTransport2.cpp

@@ -922,6 +922,12 @@ nsSocketTransport::ResolveHost()
         dnsFlags = nsIDNSService::RESOLVE_BYPASS_CACHE;
     if (mConnectionFlags & nsSocketTransport::DISABLE_IPV6)
         dnsFlags |= nsIDNSService::RESOLVE_DISABLE_IPV6;
+    if (mConnectionFlags & nsSocketTransport::DISABLE_IPV4)
+        dnsFlags |= nsIDNSService::RESOLVE_DISABLE_IPV4;
+
+    NS_ASSERTION(!(dnsFlags & nsIDNSService::RESOLVE_DISABLE_IPV6) ||
+                 !(dnsFlags & nsIDNSService::RESOLVE_DISABLE_IPV4),
+                 "Setting both RESOLVE_DISABLE_IPV6 and RESOLVE_DISABLE_IPV4");
 
     SendStatus(NS_NET_STATUS_RESOLVING_HOST);
     rv = dns->AsyncResolve(SocketHost(), dnsFlags, this, nullptr,
@@ -1261,12 +1267,12 @@ nsSocketTransport::RecoverFromError()
 
     bool tryAgain = false;
 
-    if (mConnectionFlags & DISABLE_IPV6 &&
+    if (mConnectionFlags & (DISABLE_IPV6 | DISABLE_IPV4) &&
         mCondition == NS_ERROR_UNKNOWN_HOST &&
         mState == STATE_RESOLVING &&
         !mProxyTransparentResolvesHost) {
         SOCKET_LOG(("  trying lookup again with both ipv4/ipv6 enabled\n"));
-        mConnectionFlags &= ~DISABLE_IPV6;
+        mConnectionFlags &= ~(DISABLE_IPV6 | DISABLE_IPV4);
         tryAgain = true;
     }
 
@@ -1279,14 +1285,15 @@ nsSocketTransport::RecoverFromError()
             SOCKET_LOG(("  trying again with next ip address\n"));
             tryAgain = true;
         }
-        else if (mConnectionFlags & DISABLE_IPV6) {
+        else if (mConnectionFlags & (DISABLE_IPV6 | DISABLE_IPV4)) {
             // Drop state to closed.  This will trigger new round of DNS
             // resolving bellow.
-            // XXX Here should idealy be set now non-existing flag DISABLE_IPV4
-            SOCKET_LOG(("  failed to connect all ipv4 hosts,"
+            // XXX Could be optimized to only switch the flags to save duplicate
+            // connection attempts.
+            SOCKET_LOG(("  failed to connect all ipv4-only or ipv6-only hosts,"
                         " trying lookup/connect again with both ipv4/ipv6\n"));
             mState = STATE_CLOSED;
-            mConnectionFlags &= ~DISABLE_IPV6;
+            mConnectionFlags &= ~(DISABLE_IPV6 | DISABLE_IPV4);
             tryAgain = true;
         }
     }

netwerk/dns/nsDNSService2.cpp

@@ -851,6 +851,9 @@ nsDNSService::GetAFForLookup(const nsACString &host, uint32_t flags)
         } while (*end);
     }
 
+    if ((af != PR_AF_INET) && (flags & RESOLVE_DISABLE_IPV4))
+        af = PR_AF_INET6;
+
     return af;
 }
 

netwerk/dns/nsIDNSService.idl

@@ -133,4 +133,9 @@ interface nsIDNSService : nsISupports
      * asyncResolve.
      */
     const unsigned long RESOLVE_OFFLINE = (1 << 6);
+
+    /**
+     * If set, only IPv6 addresses will be returned from resolve/asyncResolve.
+     */
+    const unsigned long RESOLVE_DISABLE_IPV4 = (1 << 7);
 };

netwerk/protocol/http/nsHttpChannel.cpp

@@ -4455,8 +4455,10 @@ nsHttpChannel::BeginConnect()
     // Force-Reload should reset the persistent connection pool for this host
     if (mLoadFlags & LOAD_FRESH_CONNECTION) {
         // just the initial document resets the whole pool
-        if (mLoadFlags & LOAD_INITIAL_DOCUMENT_URI)
+        if (mLoadFlags & LOAD_INITIAL_DOCUMENT_URI) {
             gHttpHandler->ConnMgr()->ClosePersistentConnections();
+            gHttpHandler->ConnMgr()->ResetIPFamillyPreference(mConnectionInfo);
+        }
         // each sub resource gets a fresh connection
         mCaps &= ~(NS_HTTP_ALLOW_KEEPALIVE | NS_HTTP_ALLOW_PIPELINING);
     }

netwerk/protocol/http/nsHttpConnectionMgr.cpp

@@ -2519,8 +2519,13 @@ nsHalfOpenSocket::SetupStreams(nsISocketTransport **transport,
     // IPv6 on the backup connection gives them a much better user experience
     // with dual-stack hosts, though they still pay the 250ms delay for each new
     // connection. This strategy is also known as "happy eyeballs".
-    if (isBackup && gHttpHandler->FastFallbackToIPv4())
+    if (mEnt->mPreferIPv6) {
+        tmpFlags |= nsISocketTransport::DISABLE_IPV4;
+    }
+    else if (mEnt->mPreferIPv4 ||
+             (isBackup && gHttpHandler->FastFallbackToIPv4())) {
         tmpFlags |= nsISocketTransport::DISABLE_IPV6;
+    }
 
     socketTransport->SetConnectionFlags(tmpFlags);
 
@@ -2712,6 +2717,7 @@ nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream *out)
     LOG(("nsHalfOpenSocket::OnOutputStreamReady "
          "Created new nshttpconnection %p\n", conn.get()));
 
+    PRNetAddr peeraddr;
     nsCOMPtr<nsIInterfaceRequestor> callbacks;
     mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks));
     if (out == mStreamOut) {
@@ -2722,6 +2728,9 @@ nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream *out)
                         callbacks,
                         PR_MillisecondsToInterval(rtt.ToMilliseconds()));
 
+        if (NS_SUCCEEDED(mSocketTransport->GetPeerAddr(&peeraddr)))
+            mEnt->RecordIPFamilyPreference(peeraddr.raw.family);
+
         // The nsHttpConnection object now owns these streams and sockets
         mStreamOut = nullptr;
         mStreamIn = nullptr;
@@ -2735,6 +2744,9 @@ nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream *out)
                         callbacks,
                         PR_MillisecondsToInterval(rtt.ToMilliseconds()));
 
+        if (NS_SUCCEEDED(mBackupTransport->GetPeerAddr(&peeraddr)))
+            mEnt->RecordIPFamilyPreference(peeraddr.raw.family);
+
         // The nsHttpConnection object now owns these streams and sockets
         mBackupStreamOut = nullptr;
         mBackupStreamIn = nullptr;
@@ -2947,6 +2959,8 @@ nsConnectionEntry::nsConnectionEntry(nsHttpConnectionInfo *ci)
     , mUsingSpdy(false)
     , mTestedSpdy(false)
     , mSpdyPreferred(false)
+    , mPreferIPv4(false)
+    , mPreferIPv6(false)
 {
     NS_ADDREF(mConnInfo);
     if (gHttpHandler->GetPipelineAggressive()) {
@@ -3102,7 +3116,8 @@ nsConnectionEntry::SetYellowConnection(nsHttpConnection *conn)
 }
 
 void
-nsHttpConnectionMgr::nsConnectionEntry::OnYellowComplete()
+nsHttpConnectionMgr::
+nsConnectionEntry::OnYellowComplete()
 {
     if (mPipelineState == PS_YELLOW) {
         if (mYellowGoodEvents && !mYellowBadEvents) {
@@ -3125,7 +3140,8 @@ nsHttpConnectionMgr::nsConnectionEntry::OnYellowComplete()
 }
 
 void
-nsHttpConnectionMgr::nsConnectionEntry::CreditPenalty()
+nsHttpConnectionMgr::
+nsConnectionEntry::CreditPenalty()
 {
     if (mLastCreditTime.IsNull())
         return;
@@ -3221,8 +3237,17 @@ nsHttpConnectionMgr::GetConnectionData(nsTArray<mozilla::net::HttpRetParams> *aA
     return true;
 }
 
+void
+nsHttpConnectionMgr::ResetIPFamillyPreference(nsHttpConnectionInfo *ci)
+{
+    nsConnectionEntry *ent = LookupConnectionEntry(ci, nullptr, nullptr);
+    if (ent)
+        ent->ResetIPFamilyPreference();
+}
+
 uint32_t
-nsHttpConnectionMgr::nsConnectionEntry::UnconnectedHalfOpens()
+nsHttpConnectionMgr::
+nsConnectionEntry::UnconnectedHalfOpens()
 {
     uint32_t unconnectedHalfOpens = 0;
     for (uint32_t i = 0; i < mHalfOpens.Length(); ++i) {
@@ -3248,3 +3273,22 @@ nsConnectionEntry::RemoveHalfOpen(nsHalfOpenSocket *halfOpen)
         // altering the pending q vector from an arbitrary stack
         gHttpHandler->ConnMgr()->ProcessPendingQ(mConnInfo);
 }
+
+void
+nsHttpConnectionMgr::
+nsConnectionEntry::RecordIPFamilyPreference(uint16_t family)
+{
+  if (family == PR_AF_INET && !mPreferIPv6)
+    mPreferIPv4 = true;
+
+  if (family == PR_AF_INET6 && !mPreferIPv4)
+    mPreferIPv6 = true;
+}
+
+void
+nsHttpConnectionMgr::
+nsConnectionEntry::ResetIPFamilyPreference()
+{
+  mPreferIPv4 = false;
+  mPreferIPv6 = false;
+}

netwerk/protocol/http/nsHttpConnectionMgr.h

@@ -225,6 +225,9 @@ public:
     bool     SupportsPipelining(nsHttpConnectionInfo *);
 
     bool GetConnectionData(nsTArray<mozilla::net::HttpRetParams> *);
+
+    void ResetIPFamillyPreference(nsHttpConnectionInfo *);
+
 private:
     virtual ~nsHttpConnectionMgr();
 
@@ -343,6 +346,20 @@ private:
         bool mTestedSpdy;
 
         bool mSpdyPreferred;
+
+        // Flags to remember our happy-eyeballs decision.
+        // Reset only by Ctrl-F5 reload.
+        // True when we've first connected an IPv4 server for this host,
+        // initially false.
+        bool mPreferIPv4 : 1;
+        // True when we've first connected an IPv6 server for this host,
+        // initially false.
+        bool mPreferIPv6 : 1;
+
+        // Set the IP family preference flags according the connected family
+        void RecordIPFamilyPreference(uint16_t family);
+        // Resets all flags to their default values
+        void ResetIPFamilyPreference();
     };
 
     // nsConnectionHandle