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
48 changes: 48 additions & 0 deletions quiche/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,10 @@ pub struct Stats {
path_challenge_rx_count: u64,
bytes_in_flight_duration_msec: u64,
tx_buffered_inconsistent: bool,
handshake_duration_ms: i64,
key_update_count: u64,
peer_max_idle_timeout_ms: i64,
connection_duration_ms: u64,
}

pub struct TransportParams {
Expand Down Expand Up @@ -1396,6 +1400,50 @@ pub extern "C" fn quiche_conn_stats(conn: &Connection, out: &mut Stats) {
stats.bytes_in_flight_duration.as_millis() as u64;
out.tx_buffered_inconsistent =
stats.tx_buffered_state != TxBufferTrackingState::Ok;
out.handshake_duration_ms = stats
.handshake_duration
.map_or(-1, |d| d.as_millis() as i64);
out.key_update_count = stats.key_update_count;
out.peer_max_idle_timeout_ms = stats
.peer_max_idle_timeout
.map_or(-1, |d| d.as_millis() as i64);
out.connection_duration_ms = stats.connection_duration.as_millis() as u64;
}

/// Returns the handshake duration in milliseconds, or -1 if the
/// handshake has not yet completed.
#[no_mangle]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the extra ffi functions below needed despite the data being available via quiche_conn_stats above?

pub extern "C" fn quiche_conn_stats_handshake_duration_ms(
conn: &Connection,
) -> i64 {
conn.stats()
.handshake_duration
.map_or(-1, |d| d.as_millis() as i64)
}

/// Returns the number of 1-RTT key updates.
#[no_mangle]
pub extern "C" fn quiche_conn_stats_key_update_count(conn: &Connection) -> u64 {
conn.stats().key_update_count
}

/// Returns the peer's max idle timeout in milliseconds, or -1 if
/// not available.
#[no_mangle]
pub extern "C" fn quiche_conn_stats_peer_max_idle_timeout_ms(
conn: &Connection,
) -> i64 {
conn.stats()
.peer_max_idle_timeout
.map_or(-1, |d| d.as_millis() as i64)
}

/// Returns the connection duration in milliseconds.
#[no_mangle]
pub extern "C" fn quiche_conn_stats_connection_duration_ms(
conn: &Connection,
) -> u64 {
conn.stats().connection_duration.as_millis() as u64
}

#[no_mangle]
Expand Down
62 changes: 62 additions & 0 deletions quiche/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,15 @@ where
/// Key phase bit used for outgoing protected packets.
key_phase: bool,

/// The time the connection was created.
created_at: Instant,

/// The time the handshake was completed.
handshake_completed_at: Option<Instant>,

/// The number of 1-RTT key updates.
key_update_count: u64,

/// Whether an ack-eliciting packet has been sent since last receiving a
/// packet.
ack_eliciting_sent: bool,
Expand Down Expand Up @@ -2160,6 +2169,12 @@ impl<F: BufFactory> Connection<F> {

key_phase: false,

created_at: Instant::now(),

handshake_completed_at: None,

key_update_count: 0,

ack_eliciting_sent: false,

closed: false,
Expand Down Expand Up @@ -3322,6 +3337,8 @@ impl<F: BufFactory> Connection<F> {

trace!("{} key update verified", self.trace_id);

self.key_update_count += 1;

let _ = self.crypto_ctx[epoch].crypto_seal.replace(seal_next);

let open_prev = self.crypto_ctx[epoch]
Expand Down Expand Up @@ -7638,6 +7655,20 @@ impl<F: BufFactory> Connection<F> {
path_challenge_rx_count: self.path_challenge_rx_count,
bytes_in_flight_duration: self.bytes_in_flight_duration(),
tx_buffered_state: self.tx_buffered_state,
handshake_duration: self
.handshake_completed_at
.map(|completed| completed.duration_since(self.created_at)),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the connection store the handshake_duration instead of handshake_completed_at?

key_update_count: self.key_update_count,
peer_max_idle_timeout: if self.parsed_peer_transport_params &&
self.peer_transport_params.max_idle_timeout > 0
{
Some(Duration::from_millis(
self.peer_transport_params.max_idle_timeout,
))
} else {
None
},
connection_duration: self.created_at.elapsed(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have considered providing the creation time since that's also an important piece of info when logging info on a connection. The caller can compute duration on it's own based on that.

I could see there being some challenges with logging wanting to use SystemTime instead of Monotonic time.

}
}

Expand Down Expand Up @@ -7889,6 +7920,10 @@ impl<F: BufFactory> Connection<F> {

self.handshake_completed = self.handshake.is_completed();

if self.handshake_completed && self.handshake_completed_at.is_none() {
self.handshake_completed_at = Some(Instant::now());
}

self.alpn = self.handshake.alpn_protocol().to_vec();

let raw_params = self.handshake.quic_transport_params();
Expand Down Expand Up @@ -9242,6 +9277,19 @@ pub struct Stats {

/// Health state of the connection's tx_buffered.
pub tx_buffered_state: TxBufferTrackingState,

/// The time from connection creation to handshake completion.
/// `None` if the handshake has not yet completed.
pub handshake_duration: Option<Duration>,

/// The number of 1-RTT key updates.
pub key_update_count: u64,

/// The peer's max idle timeout from transport parameters.
pub peer_max_idle_timeout: Option<Duration>,

/// Wall-clock time since connection creation.
pub connection_duration: Duration,
}

impl std::fmt::Debug for Stats {
Expand All @@ -9259,6 +9307,20 @@ impl std::fmt::Debug for Stats {
self.sent_bytes, self.recv_bytes, self.lost_bytes,
)?;

write!(
f,
" key_update_count={} connection_duration={:?}",
self.key_update_count, self.connection_duration,
)?;

if let Some(hs) = self.handshake_duration {
write!(f, " handshake_duration={:?}", hs)?;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add key_update_count ?


if let Some(idle) = self.peer_max_idle_timeout {
write!(f, " peer_max_idle_timeout={:?}", idle)?;
}

Ok(())
}
}
Expand Down
50 changes: 50 additions & 0 deletions quiche/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12486,3 +12486,53 @@ fn connect_custom_client_dcid_too_short() {
);
assert_eq!(client.err().unwrap(), Error::InvalidDcidInitialization);
}

#[rstest]
fn connection_timing_stats_before_handshake(
#[values("cubic", "bbr2_gcongestion")] cc_algorithm_name: &str,
) {
let pipe = test_utils::Pipe::new(cc_algorithm_name).unwrap();

let client_stats = pipe.client.stats();
let server_stats = pipe.server.stats();

// Before handshake, handshake_duration should be None.
assert!(client_stats.handshake_duration.is_none());
assert!(server_stats.handshake_duration.is_none());

// connection_duration should be positive after creation.
assert!(client_stats.connection_duration > Duration::ZERO);
assert!(server_stats.connection_duration > Duration::ZERO);

// key_update_count should start at 0.
assert_eq!(client_stats.key_update_count, 0);
assert_eq!(server_stats.key_update_count, 0);
}

#[rstest]
fn connection_timing_stats_after_handshake(
#[values("cubic", "bbr2_gcongestion")] cc_algorithm_name: &str,
) {
let mut pipe = test_utils::Pipe::new(cc_algorithm_name).unwrap();
assert_eq!(pipe.handshake(), Ok(()));

let client_stats = pipe.client.stats();
let server_stats = pipe.server.stats();

// After handshake, handshake_duration should be Some.
assert!(client_stats.handshake_duration.is_some());
assert!(server_stats.handshake_duration.is_some());

// connection_duration should be positive.
assert!(client_stats.connection_duration > Duration::ZERO);
assert!(server_stats.connection_duration > Duration::ZERO);

// key_update_count should still be 0 after handshake.
assert_eq!(client_stats.key_update_count, 0);
assert_eq!(server_stats.key_update_count, 0);

// peer_max_idle_timeout should reflect transport params.
// The default test config sets max_idle_timeout to 180000ms.
assert!(client_stats.peer_max_idle_timeout.is_some());
assert!(server_stats.peer_max_idle_timeout.is_some());
}