Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions noq-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1871,8 +1871,6 @@ impl Connection {

builder.finish_and_track(now, self, path_id, PadDatagram::ToSize(probe_size));

self.path_stats.get_mut(path_id).sent_plpmtud_probes += 1;

Some(self.build_transmit(path_id, transmit))
}

Expand Down Expand Up @@ -3356,8 +3354,10 @@ impl Connection {
if packet_too_old || largest_acked_packet_pn >= packet + packet_threshold {
// The packet should be declared lost.
if Some(packet) == in_flight_mtu_probe {
// Lost MTU probes are not included in `lost_packets`, because they
// should not trigger a congestion control response
// MTU probes are handled separately: they should not trigger
// retransmission or a congestion control response. They are still
// counted in the `lost_packets`/`lost_bytes` stats (in
// `handle_lost_packets`), consistent with `sent_packets`.
lost_mtu_probe = in_flight_mtu_probe;
} else {
lost_packets.push(packet);
Expand Down Expand Up @@ -3561,7 +3561,12 @@ impl Connection {
.unwrap()
.remove_in_flight(&info);
self.path_data_mut(path_id).mtud.on_probe_lost();
self.path_stats.get_mut(path_id).lost_plpmtud_probes += 1;
let path_stats = self.path_stats.get_mut(path_id);
path_stats.lost_plpmtud_probes += 1;
// MTUD probes are also counted in the general lost_packets/lost_bytes
// counters, consistent with sent_packets/sent_bytes counting all packets.
path_stats.lost_packets += 1;
path_stats.lost_bytes += info.size as u64;
}
}

Expand Down
9 changes: 9 additions & 0 deletions noq-proto/src/connection/packet_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,15 @@ impl<'a, 'b> PacketBuilder<'a, 'b> {
false => 0,
};

let is_mtud_probe =
conn.path_data(path_id).mtud.in_flight_mtu_probe() == Some(packet_number);
{
let path_stats = conn.path_stats.get_mut(path_id);
path_stats.sent_packets += 1;
path_stats.sent_bytes += size as u64;
path_stats.sent_plpmtud_probes += is_mtud_probe as u64;
}

let packet = SentPacket {
path_generation: conn.paths.get_mut(&path_id).unwrap().data.generation(),
largest_acked: sent.largest_acked,
Expand Down
55 changes: 53 additions & 2 deletions noq-proto/src/connection/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,17 +232,46 @@ pub struct PathStats {
pub congestion_events: u64,
/// Spurious congestion events on the connection.
pub spurious_congestion_events: u64,
/// The number of QUIC packets sent on this path.
///
/// This counts all packets that are tracked for acknowledgement, including MTUD probes
/// and other probes. It does *not* count off-path packets (e.g. off-path path challenges,
/// off-path path responses, or NAT traversal probes) which are sent via a different code
/// path and are not tracked for acknowledgement.
///
/// More specific counters such as [`Self::sent_plpmtud_probes`] allow breaking this number
/// down further. To get the number of non-probe packets sent, subtract
/// [`Self::sent_plpmtud_probes`] from this value.
///
/// This counts individual QUIC packets, which may differ from [`UdpStats::datagrams`] when
/// packets are coalesced into a single UDP datagram.
pub sent_packets: u64,
/// The total number of QUIC bytes sent on this path (sum of all sent packet sizes).
///
/// This counts only the QUIC packet payload bytes, not UDP/IP header bytes.
/// It does not count bytes for ACK-only (non-ack-eliciting, non-padded) packets, in an
/// effort to stay consistent with [`Self::lost_bytes`].
///
/// If you're interested in the full amount of bytes transmitted, consider looking
/// at [`ConnectionStats::udp_tx`].
pub sent_bytes: u64,
/// The number of packets lost on this path.
///
/// This counts all packets declared lost, including MTUD probes. More specific counters
/// such as [`Self::lost_plpmtud_probes`] allow breaking this number down further.
pub lost_packets: u64,
/// The number of bytes lost on this path.
///
/// This does not count bytes for ACK-only (non-ack-eliciting, non-padded) packets.
pub lost_bytes: u64,
/// The number of PLPMTUD probe packets sent on this path.
///
/// These are also counted by [`UdpStats::datagrams`].
/// These are also counted by [`Self::sent_packets`] and [`Self::sent_bytes`].
/// They are also counted by [`UdpStats::datagrams`].
pub sent_plpmtud_probes: u64,
/// The number of PLPMTUD probe packets lost on this path.
///
/// These are not included in [`Self::lost_packets`] and [`Self::lost_bytes`].
/// These are also counted by [`Self::lost_packets`] and [`Self::lost_bytes`].
pub lost_plpmtud_probes: u64,
/// The number of times a black hole was detected in the path.
pub black_holes_detected: u64,
Expand All @@ -265,9 +294,21 @@ pub struct ConnectionStats {
pub frame_tx: FrameStats,
/// Statistics about frames received on the connection.
pub frame_rx: FrameStats,
/// The number of QUIC packets sent on the connection (sum across all paths).
///
/// See also [`PathStats::sent_packets`].
pub sent_packets: u64,
/// The total number of QUIC bytes sent on the connection (sum across all paths).
///
/// See also [`PathStats::sent_bytes`].
pub sent_bytes: u64,
Comment thread
matheus23 marked this conversation as resolved.
/// The number of packets lost on the connection.
///
/// See also [`PathStats::lost_packets`].
pub lost_packets: u64,
/// The number of bytes lost on the connection.
///
/// See also [`PathStats::lost_bytes`].
pub lost_bytes: u64,

/// Number of [`super::Transmit`] produced by this connection.
Expand All @@ -290,6 +331,8 @@ impl std::ops::Add<PathStats> for ConnectionStats {
cwnd: _,
congestion_events: _,
spurious_congestion_events: _,
sent_packets,
sent_bytes,
lost_packets,
lost_bytes,
sent_plpmtud_probes: _,
Expand All @@ -302,6 +345,8 @@ impl std::ops::Add<PathStats> for ConnectionStats {
udp_rx: self.udp_rx + udp_rx,
frame_tx: self.frame_tx + frame_tx,
frame_rx: self.frame_rx + frame_rx,
sent_packets: self.sent_packets + sent_packets,
sent_bytes: self.sent_bytes + sent_bytes,
lost_packets: self.lost_packets + lost_packets,
lost_bytes: self.lost_bytes + lost_bytes,
#[cfg(test)]
Expand All @@ -323,6 +368,8 @@ impl std::ops::AddAssign<PathStats> for ConnectionStats {
cwnd: _,
congestion_events: _,
spurious_congestion_events: _,
sent_packets: path_sent_packets,
sent_bytes: path_sent_bytes,
lost_packets: path_lost_packets,
lost_bytes: path_lost_bytes,
sent_plpmtud_probes: _,
Expand All @@ -335,6 +382,8 @@ impl std::ops::AddAssign<PathStats> for ConnectionStats {
udp_rx,
frame_tx,
frame_rx,
sent_packets,
sent_bytes,
lost_packets,
lost_bytes,
#[cfg(test)]
Expand All @@ -344,6 +393,8 @@ impl std::ops::AddAssign<PathStats> for ConnectionStats {
*udp_rx += path_udp_rx;
*frame_tx += path_frame_tx;
*frame_rx += path_frame_rx;
*sent_packets += path_sent_packets;
*sent_bytes += path_sent_bytes;
*lost_packets += path_lost_packets;
*lost_bytes += path_lost_bytes;
}
Expand Down
24 changes: 24 additions & 0 deletions noq-proto/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5111,3 +5111,27 @@ fn regression_initial_coalescing_large_cid() {
pair.time += Duration::from_secs(5);
pair.drive_client(); // this used to try to build a packet without enough datagram space
}

/// Snapshot test to prevent accidentally skipping code-paths related to `sent_packets` stats.
#[test]
fn sent_packets_stats_snapshot_test() {
let _guard = subscribe();
let mut pair = Pair::default();
let (client_ch, server_ch) = pair.connect();
pair.drive();

let client_stats = pair.client_conn_mut(client_ch).stats();
let server_stats = pair.server_conn_mut(server_ch).stats();
assert_eq!(client_stats.sent_packets, 13);
assert_eq!(server_stats.sent_packets, 10);

let s = pair.client_streams(client_ch).open(Dir::Uni).unwrap();
const MSG: &[u8] = b"Hello, World!";
pair.client_send(client_ch, s).write(MSG).unwrap();
pair.drive();

let client_stats2 = pair.client_conn_mut(client_ch).stats();
let server_stats2 = pair.server_conn_mut(server_ch).stats();
assert_eq!(client_stats2.sent_packets, 14);
assert_eq!(server_stats2.sent_packets, 11);
}
Loading