Migrates QUIC sessions to a new network when old network is (about to be) disconnected.

net/quic/quic_stream_factory.cc

 // Copyright (c) 2012 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/quic/quic_stream_factory.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/location.h"
@@ skipping 38 matching lines @@
 #endif
 
 #if defined(USE_OPENSSL)
 #include <openssl/aead.h>
 #include "crypto/openssl_util.h"
 #else
 #include "base/cpu.h"
 #endif
 
 using std::min;
+using NetworkChangeNotifier::NetworkHandle;
 
 namespace net {
 
 namespace {
 
 enum CreateSessionFailure {
   CREATION_ERROR_CONNECTING_SOCKET,
   CREATION_ERROR_SETTING_RECEIVE_BUFFER,
   CREATION_ERROR_SETTING_SEND_BUFFER,
   CREATION_ERROR_MAX
 };
 
 // When a connection is idle for 30 seconds it will be closed.
 const int kIdleConnectionTimeoutSeconds = 30;
 
 // The maximum receive window sizes for QUIC sessions and streams.
 const int32 kQuicSessionMaxRecvWindowSize = 15 * 1024 * 1024;  // 15 MB
 const int32 kQuicStreamMaxRecvWindowSize = 6 * 1024 * 1024;    // 6 MB
 
 // Set the maximum number of undecryptable packets the connection will store.
 const int32 kMaxUndecryptablePackets = 100;
 
+// A default NetworkHandle.
+const NetworkHandle kDefaultNetworkHandle = -2;

[pauljensen, 2015/10/14 11:51:34]

This is a little sketchy for my liking.  I'd suggest either (listed in order of
increasing sketchyness):
1. add "bool bind_to_default_network" argument to
CreateAndConfigureDatagramSocket
2. overload kInvalidNetworkHandle to mean bind-to-default-network when passed to
CreateAndConfigureDatagramSocket
3. define this to kInvalidNetworkHandle-1

[Jana, 2015/10/16 23:11:00]

I think this is all sketchy :-) But, yeah, it seems reasonable to overload
kInvalid instead of creating a new value, so using option 2.

+
 void HistogramCreateSessionFailure(enum CreateSessionFailure error) {
   UMA_HISTOGRAM_ENUMERATION("Net.QuicSession.CreationError", error,
                             CREATION_ERROR_MAX);
 }
 
 bool IsEcdsaSupported() {
 #if defined(OS_WIN)
   if (base::win::GetVersion() < base::win::VERSION_VISTA)
     return false;
 #endif
@@ skipping 1039 matching lines @@
       list->Append(session->GetInfoAsValue(hosts));
     }
   }
   return list.Pass();
 }
 
 void QuicStreamFactory::ClearCachedStatesInCryptoConfig() {
   crypto_config_.ClearCachedStates();
 }
 
+// We still need OnIPAddressChanged, but not for Android >= L.
+// pauljensen: How do we do this?

[pauljensen, 2015/10/14 11:51:34]

Where do you register for events? (i.e. call
NetworkChangeNotifier::AddIPAddressObserver)?

I'd add a flag controlling all this new behavior and in the constructor I'd add
some:
   migrate_connections(some_quic_connection_migration_flag_thingy
#if defined(OS_ANDROID)
      && base::android::BuildInfo::GetInstance()->sdk_int() >=
         base::android::SDK_VERSION_LOLLIPOP
#endif

Where ever you call AddIPAddressObserver I'd change the code to:
  if (migrate_connections) {
    NetworkChangeNotifier::AddNetworkObserver(this);
  } else {
    NetworkChangeNotifier::AddIPAddressObserver(this);
  }

[Jana, 2015/10/16 23:11:00]

*Facepalm*. So I discovered that we do not actually register for events
currently. Fixing now.
Also, added code here to add NetworkObserver.

 void QuicStreamFactory::OnIPAddressChanged() {
   CloseAllSessions(ERR_NETWORK_CHANGED);
   set_require_confirmation(true);
 }
 
+void QuicStreamFactory::OnNetworkConnected(NetworkHandle network) {
+  // Nothing to do.
+}
+
+void QuicStreamFactory::OnNetworkMadeDefault(NetworkHandle network) {
+  // Nothing to do.
+}
+
+void QuicStreamFactory::OnNetworkDisconnected(NetworkHandle network) {
+  MigrateOrCloseSessions(network);
+}
+
+void QuicStreamFactory::OnNetworkSoonToDisconnect(NetworkHandle network) {
+  MigrateOrCloseSessions(network);
+}
+
+void QuicStreamFactory::MigrateOrCloseSessions(NetworkHandle network) {
+  // pauljensen: do we need the following "== network" check?

[pauljensen, 2015/10/14 11:51:34]

seems like a good idea

[Jana, 2015/10/16 23:11:00]

Acknowledged.

+  NetworkList network_list;
+  GetConnectedNetworks(&network_list);
+  if (network_list.isEmpty() || 
+      (network_list.size() == 1 && network_list[0] == network)) {
+    CloseAllSessions(ERR_NETWORK_CHANGED);
+    set_require_confirmation(true);
+    return;
+  }
+
+  // Find a new network that old sockets can be migrated to.
+  // pauljensen: do we need the following "== network" check?

[pauljensen, 2015/10/14 11:51:34]

seems like a good idea

[Jana, 2015/10/16 23:11:00]

Acknowledged.

+  NetworkHandle new_network = kInvalidNetworkHandle;
+  for (NetworkHandle n : network_list) {
+    if (n == network) {
+      continue;
+    }
+    new_network = n;
+  }
+
+  DCHECK_NE(kInvalidNetworkHandle, new_network);
+  DCHECK_NE(network, new_network);
+
+  for (QuicStreamFactory::SessionIdMap::iterator it = all_sessions_.begin();
+       it != all_sessions_.end(); ++it) {
+    QuicChromiumClientSession* session = it->first;
+    QuicServerId server_id = it->second;
+
+    if (session->GetNumOpenStreams() == 0) {
+      // If idle session, close session
+      active_sessions_.erase(server_id);
+      session->CloseSessionOnError(ERR_NETWORK_CHANGED, QUIC_INTERNAL_ERROR);
+    } else if (session->current_network() == new_network) {
+      // @@@ TODO (jri): store network and record in session when 
+      // BindToNetwork() is called.
+      // If session is already using |network|, move on.
+      continue;
+    } else {
+      // If session is active, 
+      // (i) make the session GO_AWAY,
+      // (ii) create a new socket (which should be bound to the new interface), 
+      // (iii) add a new PacketReader to the session to read from new socket, 
+      // (iv) replace the connection's PacketWriter with a new PacketWriter 
+      // that uses the new socket.
+      set_require_confirmation(true);
+      OnSessionGoingAway(session);
+      scoped_ptr<DatagramClientSocket> socket;
+      int rv = CreateAndConfigureDatagramSocket(&socket,
+                                                all_sessions[*session],
+                                                connection->peer_address(),
+                                                session->net_log(),
+                                                new_network);
+      if (rv != OK) {
+        session->CloseSessionOnError(ERR_NETWORK_CHANGED, QUIC_INTERNAL_ERROR);
+        continue;
+      }
+
+      if (!session->AddPacketReader(new QuicPacketReader(socket.get(), 
+                                                         clock_.get(),
+                                                         session,
+                                                         yield_after_packets_,
+                                                         yield_after_duration_,
+                                                         session->net_log()))) {
+        session->CloseSessionOnError(ERR_NETWORK_CHANGED, QUIC_INTERNAL_ERROR);
+        continue;
+      }
+
+      session->StartReading();
+      DefaultPacketWriterFactory packet_writer_factory(socket.get());
+      QuicConnection* connection = session->connection();
+      connection->SetQuicPacketWriter(packet_writer_factory.Create(connection),
+                                      /*owns_writer=*/true);
+
+      session->AddSocket(socket.Pass());
+    }
+  }
+}
+
 void QuicStreamFactory::OnSSLConfigChanged() {
   CloseAllSessions(ERR_CERT_DATABASE_CHANGED);
 }
 
 void QuicStreamFactory::OnCertAdded(const X509Certificate* cert) {
   CloseAllSessions(ERR_CERT_DATABASE_CHANGED);
 }
 
 void QuicStreamFactory::OnCACertChanged(const X509Certificate* cert) {
   // We should flush the sessions if we removed trust from a
@@ skipping 10 matching lines @@
 
 bool QuicStreamFactory::HasActiveSession(
     const QuicServerId& server_id) const {
   return ContainsKey(active_sessions_, server_id);
 }
 
 bool QuicStreamFactory::HasActiveJob(const QuicServerId& key) const {
   return ContainsKey(active_jobs_, key);
 }
 
-int QuicStreamFactory::CreateSession(const QuicServerId& server_id,
-                                     int cert_verify_flags,
-                                     scoped_ptr<QuicServerInfo> server_info,
-                                     const AddressList& address_list,
-                                     base::TimeTicks dns_resolution_end_time,
-                                     const BoundNetLog& net_log,
-                                     QuicChromiumClientSession** session) {
+int QuicStreamFactory::CreateAndConfigureDatagramSocket(
+    scoped_ptr<DatagramClientSocket>* new_socket,

[pauljensen, 2015/10/14 11:51:34]

http://dev.chromium.org/developers/smart-pointer-guidelines
says:
"You should especially never see scoped_ptr<>*"

[Jana, 2015/10/16 23:11:00]

Done.

+    const QuicServerId& server_id,
+    IPEndPoint addr,
+    const BoundNetLog& net_log,
+    NetworkHandle network) {
   bool enable_port_selection = enable_port_selection_;
   if (enable_port_selection &&
       ContainsKey(gone_away_aliases_, server_id)) {
     // Disable port selection when the server is going away.
     // There is no point in trying to return to the same server, if
     // that server is no longer handling requests.
     enable_port_selection = false;
     gone_away_aliases_.erase(server_id);
   }
 
-  QuicConnectionId connection_id = random_generator_->RandUint64();
-  IPEndPoint addr = *address_list.begin();
   scoped_refptr<PortSuggester> port_suggester =
       new PortSuggester(server_id.host_port_pair(), port_seed_);
   DatagramSocket::BindType bind_type = enable_port_selection ?
       DatagramSocket::RANDOM_BIND :  // Use our callback.
       DatagramSocket::DEFAULT_BIND;  // Use OS to randomize.
   scoped_ptr<DatagramClientSocket> socket(
       client_socket_factory_->CreateDatagramClientSocket(
           bind_type,
           base::Bind(&PortSuggester::SuggestPort, port_suggester),
           net_log.net_log(), net_log.source()));
 
   if (enable_non_blocking_io_ &&
       client_socket_factory_ == ClientSocketFactory::GetDefaultFactory()) {
 #if defined(OS_WIN)
     static_cast<UDPClientSocket*>(socket.get())->UseNonBlockingIO();
 #endif
   }
 
-  int rv = socket->Connect(addr);
+  // If caller leaves network unspecified, find current default.
+  if (network == kDefaultNetworkHandle) {
+    network = NetworkChangeNotifier::GetDefaultNetwork();

[pauljensen, 2015/10/14 11:51:34]

We may want to loop here in cases where the default changes quickly.  I thought
we were going to add DatagramClientSocket::BindToDefaultNetwork() that included
the looping?  So here we'd have something like:
if (migrate_connections) {
  int rv;
  if (network == kDefaultNetworkHandle) {
    rv = socket->BindToDefaultNetwork();
  } else {
    rv = socket->BindToNetwork(network);
  }
  if (rv != OK)
    return rv;
}

[Jana, 2015/10/16 23:11:00]

I've added a BindToDefaultNetwork method to DatagramClientSocket. I don't think
I understand how you see the looping work -- what if there's no default network
for a long time? Does it make sense to consider using a timer to wait "a short
while" and then give up?

+    if (network == kDefaultNetworkHandle) {
+      return ERR_INTERNET_DISCONNECTED;
+    }
+  }
 
+  new_socket->Reset(socket.get());
+
+  int rv = socket->BindToNetwork(network);
+  if (rv != OK) {
+    return rv;
+  }
+
+  rv = socket->Connect(addr);
   if (rv != OK) {
     HistogramCreateSessionFailure(CREATION_ERROR_CONNECTING_SOCKET);
     return rv;
   }
   UMA_HISTOGRAM_COUNTS("Net.QuicEphemeralPortsSuggested",
                        port_suggester->call_count());
   if (enable_port_selection) {
     DCHECK_LE(1u, port_suggester->call_count());
   } else {
     DCHECK_EQ(0u, port_suggester->call_count());
@@ skipping 15 matching lines @@
 
   socket->GetLocalAddress(&local_address_);
   if (check_persisted_supports_quic_ && http_server_properties_) {
     check_persisted_supports_quic_ = false;
     IPAddressNumber last_address;
     if (http_server_properties_->GetSupportsQuic(&last_address) &&
         last_address == local_address_.address()) {
       require_confirmation_ = false;
     }
   }
+  return OK;
+}
+
+int QuicStreamFactory::CreateSession(const QuicServerId& server_id,
+                                     int cert_verify_flags,
+                                     scoped_ptr<QuicServerInfo> server_info,
+                                     const AddressList& address_list,
+                                     base::TimeTicks dns_resolution_end_time,
+                                     const BoundNetLog& net_log,
+                                     QuicChromiumClientSession** session) {
+  scoped_ptr<DatagramClientSocket> socket;
+  int rv = CreateAndConfigureDatagramSocket(&socket,
+                                            server_id, 
+                                            *address_list.begin(), 
+                                            net_log,
+                                            kDefaultNetworkHandle);
+  if (rv != OK) {
+    return rv;
+  }
 
   DefaultPacketWriterFactory packet_writer_factory(socket.get());
-
   if (!helper_.get()) {
     helper_.reset(
         new QuicConnectionHelper(base::ThreadTaskRunnerHandle::Get().get(),
                                  clock_.get(), random_generator_));
   }
 
+  QuicConnectionId connection_id = random_generator_->RandUint64();
   QuicConnection* connection = new QuicConnection(
       connection_id, addr, helper_.get(), packet_writer_factory,
       true /* owns_writer */, Perspective::IS_CLIENT, server_id.is_https(),
       supported_versions_);
   connection->SetMaxPacketLength(max_packet_length_);
 
   InitializeCachedStateInCryptoConfig(server_id, server_info);
 
   QuicConfig config = config_;
   config.SetSocketReceiveBufferToSend(socket_receive_buffer_size_);
@@ skipping 178 matching lines @@
   // Since the session was active, there's no longer an
   // HttpStreamFactoryImpl::Job running which can mark it broken, unless the TCP
   // job also fails. So to avoid not using QUIC when we otherwise could, we mark
   // it as recently broken, which means that 0-RTT will be disabled but we'll
   // still race.
   http_server_properties_->MarkAlternativeServiceRecentlyBroken(
       alternative_service);
 }
 
 }  // namespace net