QUIC - disable QUIC under recent pathological connection errors.

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/cpu.h"
@@ skipping 547 matching lines @@
     bool enable_port_selection,
     bool always_require_handshake_confirmation,
     bool disable_connection_pooling,
     float load_server_info_timeout_srtt_multiplier,
     bool enable_connection_racing,
     bool enable_non_blocking_io,
     bool disable_disk_cache,
     bool prefer_aes,
     int max_number_of_lossy_connections,
     float packet_loss_threshold,
+    int max_epitaphs,
+    int threshold_public_resets_post_handshake,
+    int threshold_timeouts_streams_open,
     int socket_receive_buffer_size,
     const QuicTagVector& connection_options)
     : require_confirmation_(true),
       host_resolver_(host_resolver),
       client_socket_factory_(client_socket_factory),
       http_server_properties_(http_server_properties),
       transport_security_state_(transport_security_state),
       quic_server_info_factory_(nullptr),
       quic_crypto_client_stream_factory_(quic_crypto_client_stream_factory),
       random_generator_(random_generator),
       clock_(clock),
       max_packet_length_(max_packet_length),
       config_(InitializeQuicConfig(connection_options)),
       supported_versions_(supported_versions),
       enable_port_selection_(enable_port_selection),
       always_require_handshake_confirmation_(
           always_require_handshake_confirmation),
       disable_connection_pooling_(disable_connection_pooling),
       load_server_info_timeout_srtt_multiplier_(
           load_server_info_timeout_srtt_multiplier),
       enable_connection_racing_(enable_connection_racing),
       enable_non_blocking_io_(enable_non_blocking_io),
       disable_disk_cache_(disable_disk_cache),
       prefer_aes_(prefer_aes),
       max_number_of_lossy_connections_(max_number_of_lossy_connections),
       packet_loss_threshold_(packet_loss_threshold),
+      max_epitaphs_(max_epitaphs),
+      public_resets_post_handshake_(0),
+      timeouts_streams_open_(0),
+      max_public_resets_post_handshake_(0),
+      max_timeouts_streams_open_(0),
+      threshold_timeouts_streams_open_(threshold_timeouts_streams_open),
+      threshold_public_resets_post_handshake_(
+          threshold_public_resets_post_handshake),
       socket_receive_buffer_size_(socket_receive_buffer_size),
       port_seed_(random_generator_->RandUint64()),
       check_persisted_supports_quic_(true),
       task_runner_(nullptr),
       weak_factory_(this) {
   DCHECK(transport_security_state_);
   crypto_config_.set_user_agent_id(user_agent_id);
   crypto_config_.AddCanonicalSuffix(".c.youtube.com");
   crypto_config_.AddCanonicalSuffix(".googlevideo.com");
   crypto_config_.AddCanonicalSuffix(".googleusercontent.com");
   crypto_config_.SetProofVerifier(
       new ProofVerifierChromium(cert_verifier, transport_security_state));
   // TODO(rtenneti): http://crbug.com/487355. Temporary fix for b/20760730 until
   // channel_id_service is supported in cronet.
   if (channel_id_service) {
     crypto_config_.SetChannelIDSource(
         new ChannelIDSourceChromium(channel_id_service));
   }
   base::CPU cpu;
   bool has_aes_hardware_support = cpu.has_aesni() && cpu.has_avx();
   UMA_HISTOGRAM_BOOLEAN("Net.QuicSession.PreferAesGcm",
                         has_aes_hardware_support);
   if (has_aes_hardware_support || prefer_aes_)
     crypto_config_.PreferAesGcm();
   if (!IsEcdsaSupported())
     crypto_config_.DisableEcdsa();
 }
 
 QuicStreamFactory::~QuicStreamFactory() {
+  // TODO: need a separate max to track this.

[Ryan Hamilton, 2015/06/30 18:55:32]

TODO:(ckrasic) since you have the context?

[Buck, 2015/07/01 19:06:19]

Removed, the comment was obsolete.

+  if (max_public_resets_post_handshake_ > 0) {
+    UMA_HISTOGRAM_CUSTOM_COUNTS(
+        "Net.QuicStreamFactory.PublicResetsPostHandshake",
+        public_resets_post_handshake_, 0, max_epitaphs_, max_epitaphs_ / 2);
+  }
+  if (max_timeouts_streams_open_ > 0) {
+    UMA_HISTOGRAM_CUSTOM_COUNTS("Net.QuicStreamFactory.TimeoutsStreamsOpen",
+                                public_resets_post_handshake_, 0, max_epitaphs_,
+                                max_epitaphs_ / 2);
+  }

[Ryan Hamilton, 2015/06/30 18:55:32]

I think this is too late for histograms to be of much use. By the time the
network stack is being shut down, the histograms services is already dead. This
won't crash, but I don't think it'll end up logging anything :/  Perhaps log
when the total number of connections is > some value?

[Buck, 2015/07/01 19:06:19]

Ok.  I've moved these to where max_* get incremented.  So the histograms will be
cumulative now.  I think it will still give me what I need which is data guide
setting the thresholds.

   CloseAllSessions(ERR_ABORTED);
   while (!all_sessions_.empty()) {
     delete all_sessions_.begin()->first;
     all_sessions_.erase(all_sessions_.begin());
   }
   while (!active_jobs_.empty()) {
     const QuicServerId server_id = active_jobs_.begin()->first;
     STLDeleteElements(&(active_jobs_[server_id]));
     active_jobs_.erase(server_id);
   }
@@ skipping 181 matching lines @@
   STLDeleteElements(&(active_jobs_[server_id]));
   active_jobs_.erase(server_id);
   job_requests_map_.erase(server_id);
 }
 
 scoped_ptr<QuicHttpStream> QuicStreamFactory::CreateFromSession(
     QuicClientSession* session) {
   return scoped_ptr<QuicHttpStream>(new QuicHttpStream(session->GetWeakPtr()));
 }
 
+QuicErrorCode QuicStreamFactory::QuicDisabledReason(uint16 port) {
+  if (max_number_of_lossy_connections_ > 0 &&
+      number_of_lossy_connections_[port] >= max_number_of_lossy_connections_) {
+    return QUIC_BAD_PACKET_LOSS_RATE;
+  }
+  if (threshold_public_resets_post_handshake_ > 0 &&
+      public_resets_post_handshake_ >=
+          threshold_public_resets_post_handshake_) {
+    return QUIC_PUBLIC_RESETS_POST_HANDSHAKE;
+  }
+  if (threshold_timeouts_streams_open_ > 0 &&
+      timeouts_streams_open_ >= threshold_timeouts_streams_open_) {
+    return QUIC_TIMEOUTS_STREAMS_OPEN;
+  }
+  return QUIC_NO_ERROR;
+}
+
 bool QuicStreamFactory::IsQuicDisabled(uint16 port) {
-  return max_number_of_lossy_connections_ > 0 &&
-         number_of_lossy_connections_[port] >= max_number_of_lossy_connections_;
+  return (QuicDisabledReason(port) != QUIC_NO_ERROR);

[Ryan Hamilton, 2015/06/30 18:55:32]

nit: no need for the extra ()s

[Buck, 2015/07/01 19:06:19]

Done.

 }
 
 bool QuicStreamFactory::OnHandshakeConfirmed(QuicClientSession* session,
                                              float packet_loss_rate) {
   DCHECK(session);
   uint16 port = session->server_id().port();
   if (packet_loss_rate < packet_loss_threshold_) {
     number_of_lossy_connections_[port] = 0;
     return false;
   }
@@ skipping 10 matching lines @@
 
   // Collect data for port 443 for packet loss events.
   if (port == 443 && max_number_of_lossy_connections_ > 0) {
     UMA_HISTOGRAM_SPARSE_SLOWLY(
         base::StringPrintf("Net.QuicStreamFactory.BadPacketLossEvents%d",
                            max_number_of_lossy_connections_),
         std::min(number_of_lossy_connections_[port],
                  max_number_of_lossy_connections_));
   }
 
-  bool is_quic_disabled = IsQuicDisabled(port);
-  if (is_quic_disabled) {
+  QuicErrorCode error_code = QuicDisabledReason(port);
+  if (error_code != QUIC_NO_ERROR) {
     // Close QUIC connection if Quic is disabled for this port.
-    session->CloseSessionOnErrorAndNotifyFactoryLater(
-        ERR_ABORTED, QUIC_BAD_PACKET_LOSS_RATE);
+    session->CloseSessionOnErrorAndNotifyFactoryLater(ERR_ABORTED, error_code);
 
     // If this bad packet loss rate disabled the QUIC, then record it.
-    if (!was_quic_disabled)
+    if (!was_quic_disabled && error_code == QUIC_BAD_PACKET_LOSS_RATE)
       UMA_HISTOGRAM_SPARSE_SLOWLY("Net.QuicStreamFactory.QuicIsDisabled", port);
+    return true;
   }
-  return is_quic_disabled;
+  return false;

[Ryan Hamilton, 2015/06/30 18:55:32]

Your call, but I might write this as:

if (error_code == NO_ERROR)
  return true;

// Close QUIC ...
..

(Just to remove a level of nesting now that we're using different explicit
return statements.

[Buck, 2015/07/01 19:06:19]

Done.

 }
 
 void QuicStreamFactory::OnIdleSession(QuicClientSession* session) {
 }
 
 void QuicStreamFactory::OnSessionGoingAway(QuicClientSession* session) {
   const AliasSet& aliases = session_aliases_[session];
   for (AliasSet::const_iterator it = aliases.begin(); it != aliases.end();
        ++it) {
     DCHECK(active_sessions_.count(*it));
@@ skipping 12 matching lines @@
     const IpAliasKey ip_alias_key(session->connection()->peer_address(),
                                   aliases.begin()->is_https());
     ip_aliases_[ip_alias_key].erase(session);
     if (ip_aliases_[ip_alias_key].empty()) {
       ip_aliases_.erase(ip_alias_key);
     }
   }
   session_aliases_.erase(session);
 }
 
+void QuicStreamFactory::MaybeDisableQuic(QuicClientSession* session) {
+  DCHECK(session);
+  uint16 port = session->server_id().port();
+  if (IsQuicDisabled(port))
+    return;
+
+  // Expire the oldest epitaph if appropriate.  This enforces that we
+  // only consider the max_epitaphs_ most recent sessions.
+  QuicClientSessionEpitaph epitaph;
+  if ((int)epitaphs_.size() == max_epitaphs_) {

[Ryan Hamilton, 2015/06/30 18:55:32]

nit: static_cast

[Buck, 2015/07/01 19:06:19]

Done.

+    epitaph = epitaphs_.front();
+    epitaphs_.pop_front();
+    if (epitaph == QUIC_EPITAPH_PUBLIC_RESET_POST_HANDSHAKE) {
+      public_resets_post_handshake_--;
+    } else if (epitaph == QUIC_EPITAPH_TIMEOUT_STREAMS_OPEN) {
+      timeouts_streams_open_--;
+    }
+  }
+  epitaph = session->epitaph();
+  epitaphs_.push_back(epitaph);
+  if (epitaph == QUIC_EPITAPH_PUBLIC_RESET_POST_HANDSHAKE) {
+    public_resets_post_handshake_++;
+  } else if (epitaph == QUIC_EPITAPH_TIMEOUT_STREAMS_OPEN) {
+    timeouts_streams_open_++;
+  }
+  max_timeouts_streams_open_ =
+      std::max(timeouts_streams_open_, max_timeouts_streams_open_);
+  max_public_resets_post_handshake_ = std::max(
+      public_resets_post_handshake_, max_public_resets_post_handshake_);

[Ryan Hamilton, 2015/06/30 18:55:31]

I'm not following the logic of resetting max_ here. Can you add a comment? (I'm
sure it makes sense)

[Buck, 2015/07/01 19:06:19]

I changed this so it is hopefully more direct and understandable.

[Ryan Hamilton, 2015/07/06 18:25:41]

Oh, I see. I was confusing max_disabled_reasons_ with max_<particular_reason>_.
*facepalm*

+  if (IsQuicDisabled(port)) {
+    UMA_HISTOGRAM_ENUMERATION("Net.QuicStreamFactory.DisabledReasons", epitaph,
+                              QUIC_EPITAPH_MAX);
+  }
+}
+
 void QuicStreamFactory::OnSessionClosed(QuicClientSession* session) {
   DCHECK_EQ(0u, session->GetNumOpenStreams());
+  MaybeDisableQuic(session);
   OnSessionGoingAway(session);
   delete session;
   all_sessions_.erase(session);
 }
 
 void QuicStreamFactory::OnSessionConnectTimeout(
     QuicClientSession* session) {
   const AliasSet& aliases = session_aliases_[session];
   for (AliasSet::const_iterator it = aliases.begin(); it != aliases.end();
        ++it) {
@@ skipping 359 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