Notfiy NQE of QUIC RTT

net/quic/quic_connection_logger.cc

 // Copyright (c) 2013 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_connection_logger.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "net/base/net_util.h"
 #include "net/cert/cert_verify_result.h"
 #include "net/cert/x509_certificate.h"
 #include "net/log/net_log.h"
 #include "net/quic/crypto/crypto_handshake_message.h"
 #include "net/quic/crypto/crypto_protocol.h"
 #include "net/quic/quic_address_mismatch.h"
+#include "net/quic/quic_sent_packet_manager.h"
 #include "net/quic/quic_socket_address_coder.h"
 
 using base::StringPiece;
 using std::string;
 
 namespace net {
 
 namespace {
 
 // We have ranges-of-buckets in the cumulative histogram (covering 21 packet
@@ skipping 248 matching lines @@
 AddressFamily GetRealAddressFamily(const IPAddressNumber& address) {
   return IsIPv4Mapped(address) ? ADDRESS_FAMILY_IPV4 :
       GetAddressFamily(address);
 }
 
 }  // namespace
 
 QuicConnectionLogger::QuicConnectionLogger(
     QuicSpdySession* session,
     const char* const connection_description,
-    const BoundNetLog& net_log)
+    const BoundNetLog& net_log,
+    QuicConnection* connection,
+    scoped_ptr<SocketPerformanceWatcher> socket_performance_watcher)
     : net_log_(net_log),
       session_(session),
       last_received_packet_number_(0),
       last_received_packet_size_(0),
       previous_received_packet_size_(0),
       largest_received_packet_number_(0),
       largest_received_missing_packet_number_(0),
       num_out_of_order_received_packets_(0),
       num_out_of_order_large_received_packets_(0),
       num_packets_received_(0),
       num_truncated_acks_sent_(0),
       num_truncated_acks_received_(0),
       num_frames_received_(0),
       num_duplicate_frames_received_(0),
       num_incorrect_connection_ids_(0),
       num_undecryptable_packets_(0),
       num_duplicate_packets_(0),
       num_blocked_frames_received_(0),
       num_blocked_frames_sent_(0),
-      connection_description_(connection_description) {}
+      connection_description_(connection_description),
+      connection_(connection),
+      socket_performance_watcher_(socket_performance_watcher.Pass()) {}
 
 QuicConnectionLogger::~QuicConnectionLogger() {
   UMA_HISTOGRAM_COUNTS("Net.QuicSession.OutOfOrderPacketsReceived",
                        num_out_of_order_received_packets_);
   UMA_HISTOGRAM_COUNTS("Net.QuicSession.OutOfOrderLargePacketsReceived",
                        num_out_of_order_large_received_packets_);
   UMA_HISTOGRAM_COUNTS("Net.QuicSession.TruncatedAcksSent",
                        num_truncated_acks_sent_);
   UMA_HISTOGRAM_COUNTS("Net.QuicSession.TruncatedAcksReceived",
                        num_truncated_acks_received_);
@@ skipping 129 matching lines @@
     net_log_.AddEvent(
         NetLog::TYPE_QUIC_SESSION_PACKET_SENT,
         base::Bind(&NetLogQuicPacketSentCallback, serialized_packet,
                    level, transmission_type, packet.length(), sent_time));
   }  else {
     net_log_.AddEvent(
         NetLog::TYPE_QUIC_SESSION_PACKET_RETRANSMITTED,
         base::Bind(&NetLogQuicPacketRetransmittedCallback,
                    original_packet_number, serialized_packet.packet_number));
   }
+  // Notify socket performance watcher of the updated RTT value.
+  if (socket_performance_watcher_ && connection_) {

[Ryan Hamilton, 2015/09/17 01:59:49]

Why would we not have a connection?

[tbansal1, 2015/09/17 22:23:42]

Removed, I was just not sure, so tried to be on the safer side.

+    socket_performance_watcher_->OnUpdatedRTTAvailable(
+        base::TimeDelta::FromMicroseconds(connection_->sent_packet_manager()
+                                              .GetRttStats()
+                                              ->smoothed_rtt()
+                                              .ToMicroseconds()));
+  }
 }
 
 void QuicConnectionLogger::OnPacketReceived(const IPEndPoint& self_address,
                                             const IPEndPoint& peer_address,
                                             const QuicEncryptedPacket& packet) {
   if (local_address_from_self_.GetFamily() == ADDRESS_FAMILY_UNSPECIFIED) {
     local_address_from_self_ = self_address;
     UMA_HISTOGRAM_ENUMERATION("Net.QuicSession.ConnectionTypeFromSelf",
                               GetRealAddressFamily(self_address.address()),
                               ADDRESS_FAMILY_LAST);
   }
 
   previous_received_packet_size_ = last_received_packet_size_;
   last_received_packet_size_ = packet.length();
   net_log_.AddEvent(
       NetLog::TYPE_QUIC_SESSION_PACKET_RECEIVED,
       base::Bind(&NetLogQuicPacketCallback, &self_address, &peer_address,
                  packet.length()));
+
+  // Notify socket performance watcher of the updated RTT value.
+  if (socket_performance_watcher_ && connection_) {
+    socket_performance_watcher_->OnUpdatedRTTAvailable(
+        base::TimeDelta::FromMicroseconds(connection_->sent_packet_manager()
+                                              .GetRttStats()
+                                              ->smoothed_rtt()
+                                              .ToMicroseconds()));
+  }

[Ryan Hamilton, 2015/09/17 01:59:49]

I think smoothed_rtt() can return Zero. Is that OK?

[tbansal1, 2015/09/17 22:23:42]

Added check for zero.

 }

[Ryan Hamilton, 2015/09/17 01:59:49]

Looking at this more closely, I see that you're re-fetching RTT on every packet
sent and received. I wonder if it might be a better idea to do this only when
the RTT changes. To do this, I propose you add a new method to
QuicConnectionDebugVisitorInterface:

void OnRttChanged(QuicTime::Delta rtt) {}

You can then call this method from:

https://code.google.com/p/chromium/codesearch#chromium/src/net/quic/quic_conn...

And then implement it in QuicConnectionLogger to do the logic you have here.

Does that sound workable?  Looks good otherwise...

[tbansal1, 2015/09/17 22:23:42]

Done. That's much cleaner :)

 
 void QuicConnectionLogger::OnUnauthenticatedHeader(
     const QuicPacketHeader& header) {
   net_log_.AddEvent(
       NetLog::TYPE_QUIC_SESSION_UNAUTHENTICATED_PACKET_HEADER_RECEIVED,
       base::Bind(&NetLogQuicPacketHeaderCallback, &header));
 }
 
 void QuicConnectionLogger::OnIncorrectConnectionId(
     QuicConnectionId connection_id) {
@@ skipping 381 matching lines @@
       continue;
     }
     // Record some overlapping patterns, to get a better picture, since this is
     // not very expensive.
     if (i % 3 == 0)
       six_packet_histogram->Add(recent_6_mask);
   }
 }
 
 }  // namespace net