Expose packet loss counts from QUIC to NetworkQualityEstimator

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 <utility>
 
@@ skipping 485 matching lines @@
 }
 
 void QuicConnectionLogger::OnProtocolVersionMismatch(
     QuicVersion received_version) {
   // TODO(rtenneti): Add logging.
 }
 
 void QuicConnectionLogger::OnPacketHeader(const QuicPacketHeader& header) {
   net_log_.AddEvent(NetLog::TYPE_QUIC_SESSION_PACKET_AUTHENTICATED);
   ++num_packets_received_;
+
+  const QuicPacketCount previous_largest_received_packet_number =
+      largest_received_packet_number_;
   if (largest_received_packet_number_ < header.packet_number) {
     QuicPacketNumber delta =
         header.packet_number - largest_received_packet_number_;
     if (delta > 1) {
       // There is a gap between the largest packet previously received and
       // the current packet.  This indicates either loss, or out-of-order
       // delivery.
       UMA_HISTOGRAM_COUNTS("Net.QuicSession.PacketGapReceived",
                            static_cast<base::HistogramBase::Sample>(delta - 1));
     }
     largest_received_packet_number_ = header.packet_number;
   }
   if (header.packet_number < received_packets_.size()) {
     received_packets_[static_cast<size_t>(header.packet_number)] = true;
   }
   if (header.packet_number < last_received_packet_number_) {
     ++num_out_of_order_received_packets_;
     if (previous_received_packet_size_ < last_received_packet_size_)
       ++num_out_of_order_large_received_packets_;
     UMA_HISTOGRAM_COUNTS(
         "Net.QuicSession.OutOfOrderGapReceived",
         static_cast<base::HistogramBase::Sample>(last_received_packet_number_ -
                                                  header.packet_number));
   }
   last_received_packet_number_ = header.packet_number;
+  ReportPacketCounts(previous_largest_received_packet_number);
+}
+
+void QuicConnectionLogger::ReportPacketCounts(
+    QuicPacketCount previous_largest_received_packet_number) const {
+  DCHECK_GE(largest_received_packet_number_,
+            previous_largest_received_packet_number);
+  if (!socket_performance_watcher_)
+    return;
+
+  size_t packets_lost = 0;
+  size_t packets_received_in_order = 0;
+  size_t packets_received_out_of_order = 0;
+
+  const size_t diff =
+      largest_received_packet_number_ - previous_largest_received_packet_number;
+
+  if (diff == 0) {
+    // The received packet number has a lower sequence number than the expected
+    // sequence number.
+    packets_received_out_of_order = 1;
+  } else if (diff == 1) {
+    // The received packet number has the same sequence number as the expected
+    // sequence number.
+    packets_received_in_order = 1;
+  } else {
+    // The received packet number has higher sequence number as the expected
+    // sequence number. Report all packets with intermediate sequence numbers
+    // as lost.
+    packets_lost = diff - 1;

[Ryan Hamilton, 2016/03/01 18:20:32]

This is a very aggressive definition of loss. Reordering in the network is
expected, and this definition would end up confusing out of order deliver with
loss. Are you sure this is the right metric?

[tbansal1, 2016/03/01 19:05:36]

This API also reports p_r_out_of_order count. So, packets that were delivered
later (due to reorder) would get reported to the callee (NQE). At the callee,
value of packets_lost - p_r_out_of_order should give an estimate of how many
packets are currently considered as lost.

I did not put this logic here because missing packets may give an early
indication of packet losses.

[Ryan Hamilton, 2016/03/02 04:26:49]

If you have a variable called "packets_lost" but in order to compute the number
of lost packets you need to subtract out_of_order from packets_lost, then
packets_lost is the wrong name for that variable, I think. Perhaps you can come
up with a better name?
 
Please do not do this. Reordering is common. This "early warning" is not
advisable.

Also, we have plans is QUIC for the sender to intentionally send gaps. For
example after sending packets 1-100, the sender might jump to 500. (Long story).
I'm concerned that your calculations here might do the wrong thing.

If you wanted to measure up-stream packet loss, we can do that explicitly, of
course.

[tbansal1, 2016/03/03 02:39:05]

I will change the name, and send out the CL. In the meantime, question for you:
If sender is going to skip sequence numbers, surely there must be some way
control signal for receiver to be notified of that? Otherwise, how would the
receiver report the list of missing_packets in QuicAckFrame to the sender?

On 2016/03/02 04:26:49, Ryan Hamilton wrote:

[Ryan Hamilton, 2016/03/03 04:24:24]

The receiver will see packets 1, 2, ... 100, 500, 501, 502
and so it will report packets 101-499 as missing in the ack frame.

[tbansal1, 2016/03/03 23:34:46]

Done.

+    packets_received_in_order = 1;
+  }
+  socket_performance_watcher_->OnUpdatedPacketCountAvailable(
+      packets_lost, packets_received_in_order, packets_received_out_of_order);
 }
 
 void QuicConnectionLogger::OnStreamFrame(const QuicStreamFrame& frame) {
   net_log_.AddEvent(NetLog::TYPE_QUIC_SESSION_STREAM_FRAME_RECEIVED,
                     base::Bind(&NetLogQuicStreamFrameCallback, &frame));
 }
 
 void QuicConnectionLogger::OnAckFrame(const QuicAckFrame& frame) {
   net_log_.AddEvent(NetLog::TYPE_QUIC_SESSION_ACK_FRAME_RECEIVED,
                     base::Bind(&NetLogQuicAckFrameCallback, &frame));
@@ skipping 227 matching lines @@
     bit_mask_of_packets >>= 1;
     DCHECK_LT(range_start + bits_so_far, kBoundingSampleInCumulativeHistogram);
     histogram->Add(range_start + bits_so_far);
     range_start += i + 1;
   }
 }
 
 float QuicConnectionLogger::ReceivedPacketLossRate() const {
   if (largest_received_packet_number_ <= num_packets_received_)
     return 0.0f;
-  float num_received = largest_received_packet_number_ - num_packets_received_;
-  return num_received / largest_received_packet_number_;
+  float num_lost = largest_received_packet_number_ - num_packets_received_;
+  return num_lost / largest_received_packet_number_;
 }
 
 void QuicConnectionLogger::OnRttChanged(QuicTime::Delta rtt) const {
   // Notify socket performance watcher of the updated RTT value.
   if (!socket_performance_watcher_)
     return;
 
   int64_t microseconds = rtt.ToMicroseconds();
   if (microseconds != 0) {
     socket_performance_watcher_->OnUpdatedRTTAvailable(
@@ skipping 84 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