Log QUIC public reset client address mismatch to a histogram.

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 "base/bind.h"
 #include "base/callback.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "net/base/net_log.h"
 #include "net/quic/crypto/crypto_handshake_message.h"
+#include "net/quic/crypto/crypto_protocol.h"
+#include "net/quic/quic_socket_address_coder.h"
 
+using base::StringPiece;
 using std::string;
 
 namespace net {
 
 namespace {
 
 base::Value* NetLogQuicPacketCallback(const IPEndPoint* self_address,
                                       const IPEndPoint* peer_address,
                                       size_t packet_size,
                                       NetLog::LogLevel /* log_level */) {
@@ skipping 162 matching lines @@
   dict->SetInteger("quic_error", error);
   dict->SetBoolean("from_peer", from_peer);
   return dict;
 }
 
 void UpdatePacketGapSentHistogram(size_t num_consecutive_missing_packets) {
   UMA_HISTOGRAM_COUNTS("Net.QuicSession.PacketGapSent",
                        num_consecutive_missing_packets);
 }
 
+void UpdatePublicResetAddressMismatchHistogram(
+    const IPEndPoint& server_hello_address,
+    const IPEndPoint& public_reset_address) {
+  enum {
+    kAddressAndPortMatch_base = 0,
+    kAddressAndPortMatch_v4_v4 = 0,

[wtc, 2014/02/13 02:04:10]

Is it correct for the histogram enumeration to start from 0?

[jar (doing other things), 2014/02/13 03:24:48]

Perfectly correct.

+    kAddressAndPortMatch_v6_v6 = 1,
+    kAddressAndPortMatch_v4_v6 = 2,
+    kAddressAndPortMatch_v6_v4 = 3,
+
+    // The addresses match, but the ports don't match.
+    kPortMismatch_base = 4,
+    kPortMismatch_v4_v4 = 4,
+    kPortMismatch_v6_v6 = 5,
+    kPortMismatch_v4_v6 = 6,
+    kPortMismatch_v6_v4 = 7,
+
+    // The addresses don't match.
+    kAddressMismatch_base = 8,
+    kAddressMismatch_v4_v4 = 8,
+    kAddressMismatch_v6_v6 = 9,
+    kAddressMismatch_v4_v6 = 10,
+    kAddressMismatch_v6_v4 = 11,

[Ryan Hamilton, 2014/02/13 04:44:05]

An alternative to consider which might make the code simpler would be for sample
to be effectively a bit mask encoded as:

<address_matches><port_matches><reset_is_v4><shlo_is_v4>

I think this would allow you to incrementally build up sample via:

if (first_ipv4)
  sample |= 1 << 0;
if (second_ipv4)
  sample |= 1 << 1;
if (server_hello_address.address() == public_reset_address.address())
  sample |= 1 << 2;
if (server_hello_address.port() == public_reset_address.port())
  sample |= 1 << 4;

Or something like that...

[wtc, 2014/02/14 00:53:57]

Done. (Not in the way you suggested though.)

+
+    kBoundaryValue,
+  };
+
+  // TODO(wtc): should we report these two cases?

[jar (doing other things), 2014/02/13 03:24:48]

This is fine IMO.  You are seemingly talking to and older server, that does not
support the feature... so you can't (yet) report the results in the histogram. 

Suggest deleting the comment (or adding a comment that the server doesn't
support this... so we can't report).

[wtc, 2014/02/14 00:53:57]

Done.

+  if (server_hello_address.address().empty() ||
+      public_reset_address.address().empty()) {
+    return;
+  }
+
+  bool first_ipv4 =
+    (server_hello_address.address().size() == kIPv4AddressSize);
+  bool second_ipv4 =
+    (public_reset_address.address().size() == kIPv4AddressSize);
+  int offset;
+  if (first_ipv4 == second_ipv4) {
+    if (first_ipv4) {
+      offset = 0;  // v4_v4
+    } else {
+      offset = 1;  // v6_v6
+    }
+  } else {
+    if (first_ipv4) {
+      offset = 2;  // v4_v6
+    } else {
+      offset = 3;  // v6_v4
+    }
+  }
+
+  int sample;
+  if (server_hello_address.address() != public_reset_address.address()) {
+    sample = kAddressMismatch_base;
+  } else if (server_hello_address.port() != public_reset_address.port()) {
+    sample = kPortMismatch_base;
+  } else {
+    sample = kAddressAndPortMatch_base;
+  }
+  sample += offset;
+  HISTOGRAM_ENUMERATION("Net.QuicSession.PublicResetAddressMismatch",

[wtc, 2014/02/13 02:04:10]

Is this histogram name OK? All the histograms in this file are under
Net.QuicSession, so I also put this one there.

[jar (doing other things), 2014/02/13 03:24:48]

Critical is the "UMA" prefix, as otherwise the histograms won't upload (in
retrospect.. I shouldn't have made UMA upload the default).  You need to use:

UMA_HISTOGRAM_ENUMERATION()

The actual histogram name looks just fine.

[wtc, 2014/02/14 00:53:57]

Done.

+                        sample, kBoundaryValue);
+}
+
 }  // namespace
 
 QuicConnectionLogger::QuicConnectionLogger(const BoundNetLog& net_log)
     : net_log_(net_log),
       last_received_packet_sequence_number_(0),
       largest_received_packet_sequence_number_(0),
       largest_received_missing_packet_sequence_number_(0),
       out_of_order_recieved_packet_count_(0),
       num_truncated_acks_sent_(0),
       num_truncated_acks_received_(0) {
@@ skipping 178 matching lines @@
 void QuicConnectionLogger::OnConnectionCloseFrame(
     const QuicConnectionCloseFrame& frame) {
   net_log_.AddEvent(
       NetLog::TYPE_QUIC_SESSION_CONNECTION_CLOSE_FRAME_RECEIVED,
       base::Bind(&NetLogQuicConnectionCloseFrameCallback, &frame));
 }
 
 void QuicConnectionLogger::OnPublicResetPacket(
     const QuicPublicResetPacket& packet) {
   net_log_.AddEvent(NetLog::TYPE_QUIC_SESSION_PUBLIC_RESET_PACKET_RECEIVED);
+  UpdatePublicResetAddressMismatchHistogram(client_address_,
+                                            packet.client_address);
 }
 
 void QuicConnectionLogger::OnVersionNegotiationPacket(
     const QuicVersionNegotiationPacket& packet) {
   net_log_.AddEvent(
       NetLog::TYPE_QUIC_SESSION_VERSION_NEGOTIATION_PACKET_RECEIVED,
       base::Bind(&NetLogQuicVersionNegotiationPacketCallback, &packet));
 }
 
 void QuicConnectionLogger::OnRevivedPacket(
     const QuicPacketHeader& revived_header,
     base::StringPiece payload) {
   net_log_.AddEvent(
       NetLog::TYPE_QUIC_SESSION_PACKET_HEADER_REVIVED,
       base::Bind(&NetLogQuicPacketHeaderCallback, &revived_header));
 }
 
 void QuicConnectionLogger::OnCryptoHandshakeMessageReceived(
     const CryptoHandshakeMessage& message) {
   net_log_.AddEvent(
       NetLog::TYPE_QUIC_SESSION_CRYPTO_HANDSHAKE_MESSAGE_RECEIVED,
       base::Bind(&NetLogQuicCryptoHandshakeMessageCallback, &message));
+
+  if (message.tag() == kSHLO) {
+    StringPiece address;
+    QuicSocketAddressCoder decoder;
+    if (message.GetStringPiece(kCADR, &address) &&
+        decoder.Decode(address.data(), address.size())) {
+      client_address_ = IPEndPoint(decoder.ip(), decoder.port());
+    }
+  }
 }
 
 void QuicConnectionLogger::OnCryptoHandshakeMessageSent(
     const CryptoHandshakeMessage& message) {
   net_log_.AddEvent(
       NetLog::TYPE_QUIC_SESSION_CRYPTO_HANDSHAKE_MESSAGE_SENT,
       base::Bind(&NetLogQuicCryptoHandshakeMessageCallback, &message));
 }
 
 void QuicConnectionLogger::OnConnectionClosed(QuicErrorCode error,
                                               bool from_peer) {
   net_log_.AddEvent(
       NetLog::TYPE_QUIC_SESSION_CLOSED,
       base::Bind(&NetLogQuicOnConnectionClosedCallback, error, from_peer));
 }
 
 void QuicConnectionLogger::OnSuccessfulVersionNegotiation(
     const QuicVersion& version) {
   string quic_version = QuicVersionToString(version);
   net_log_.AddEvent(NetLog::TYPE_QUIC_SESSION_VERSION_NEGOTIATED,
                     NetLog::StringCallback("version", &quic_version));
 }
 
 }  // namespace net