Log QUIC public reset client address mismatch to a histogram.

net/quic/quic_connection_logger.cc

Index: net/quic/quic_connection_logger.cc
===================================================================
--- net/quic/quic_connection_logger.cc	(revision 250874)
+++ net/quic/quic_connection_logger.cc	(working copy)
@@ -12,7 +12,10 @@
 #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 {
@@ -195,6 +198,71 @@
                        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)
@@ -393,6 +461,8 @@
 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(
@@ -415,6 +485,15 @@
   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(