fix(client): align DSCP wire semantics

This commit is contained in:
Nils Andreas Svee committed 2026-08-11 20:29:57 +07:00
1 parent 347af39d74
commit 3d5a8fae14
18 files changed
+249 -140

No files matched your search

+33 -25
View File
@@ -30,7 +30,7 @@ use crate::{
PreparedProbe, SessionMachine, TimeoutBatch, MAX_OPEN_PACKET_SIZE,
},
socket::{connect_tokio_udp_socket, resolve_remote_tokio, validate_open_timeouts},
socket_options::{apply_dscp_to_tokio_socket, clear_dscp_on_tokio_socket},
socket_options::{apply_traffic_class_to_tokio_socket, clear_dscp_on_tokio_socket},
timing::ClientTimestamp,
};
@@ -49,7 +49,7 @@ struct PreparedAsyncOpen {
machine: PreparedOpenAcceptance,
schedule: Option<ProbeSchedule>,
recv_buffer_len: Option<usize>,
negotiated_dscp: Option<u8>,
negotiated_traffic_class: Option<u8>,
}
#[derive(Debug)]
@@ -219,7 +219,7 @@ pub struct AsyncClient {
schedule: Option<ProbeSchedule>,
remote: SocketAddr,
recv_buffer: Vec<u8>,
applied_dscp: Option<u8>,
applied_traffic_class: Option<u8>,
prepared_open: Option<PreparedOpenRequest>,
prepared_probe: Option<PreparedProbe>,
#[cfg(test)]
@@ -247,7 +247,7 @@ impl AsyncClient {
schedule: None,
remote,
recv_buffer: vec![0_u8; recv_buffer_size(false, None)?],
applied_dscp: None,
applied_traffic_class: None,
prepared_open: Some(prepared_open),
prepared_probe: None,
#[cfg(test)]
@@ -618,7 +618,7 @@ impl AsyncClient {
self.schedule = None;
self.prepared_probe = None;
if self.clear_peer_close_dscp().is_ok() {
self.applied_dscp = None;
self.applied_traffic_class = None;
}
}
return Poll::Ready(Ok(events));
@@ -641,7 +641,7 @@ impl AsyncClient {
source,
}));
}
let previous_dscp = self.applied_dscp;
let previous_traffic_class = self.applied_traffic_class;
let expected_bytes = prepared.bytes.len();
#[cfg(test)]
@@ -661,7 +661,7 @@ impl AsyncClient {
let mut rollback = DscpRollback::armed(
&self.socket,
self.remote,
previous_dscp,
previous_traffic_class,
#[cfg(test)]
&self.test_hooks.fail_dscp_restore,
);
@@ -698,7 +698,7 @@ impl AsyncClient {
rollback.disarm();
self.schedule = None;
self.prepared_probe = None;
self.applied_dscp = None;
self.applied_traffic_class = None;
if let Err(error) = validate_datagram_length(expected_bytes, bytes) {
return Poll::Ready(Err(error));
@@ -811,7 +811,7 @@ impl AsyncClient {
machine,
schedule: None,
recv_buffer_len: None,
negotiated_dscp: None,
negotiated_traffic_class: None,
});
};
let schedule = match ProbeSchedule::new(opened_at.mono, negotiated) {
@@ -826,12 +826,12 @@ impl AsyncClient {
return Err(Box::new(PreparedAsyncOpenFailure { primary, machine }));
}
};
let negotiated_dscp = match u8::try_from(negotiated.params.dscp) {
Ok(dscp) => dscp,
let negotiated_traffic_class = match u8::try_from(negotiated.params.dscp) {
Ok(traffic_class) => traffic_class,
Err(_) => {
return Err(Box::new(PreparedAsyncOpenFailure {
primary: ClientError::InvalidConfig {
reason: "negotiated dscp must be in range 0..=63".to_owned(),
reason: "negotiated dscp must be in range 0..=255".to_owned(),
},
machine,
}));
@@ -855,12 +855,16 @@ impl AsyncClient {
source: io::Error::other("injected negotiated DSCP failure"),
})
} else {
apply_dscp_to_tokio_socket(&self.socket, self.remote, negotiated_dscp)
apply_traffic_class_to_tokio_socket(&self.socket, self.remote, negotiated_traffic_class)
};
#[cfg(not(test))]
let dscp_result = apply_dscp_to_tokio_socket(&self.socket, self.remote, negotiated_dscp);
let dscp_result = apply_traffic_class_to_tokio_socket(
&self.socket,
self.remote,
negotiated_traffic_class,
);
if let Err(primary) = dscp_result {
self.restore_dscp_best_effort(self.applied_dscp);
self.restore_dscp_best_effort(self.applied_traffic_class);
return Err(Box::new(PreparedAsyncOpenFailure { primary, machine }));
}
@@ -868,7 +872,7 @@ impl AsyncClient {
machine,
schedule: Some(schedule),
recv_buffer_len: Some(recv_buffer_len),
negotiated_dscp: Some(negotiated_dscp),
negotiated_traffic_class: Some(negotiated_traffic_class),
})
}
@@ -878,7 +882,7 @@ impl AsyncClient {
}
let outcome = self.machine.commit_open(prepared.machine);
self.schedule = prepared.schedule;
self.applied_dscp = prepared.negotiated_dscp;
self.applied_traffic_class = prepared.negotiated_traffic_class;
self.prepared_open = None;
outcome
}
@@ -950,9 +954,11 @@ impl AsyncClient {
clear_dscp_on_tokio_socket(&self.socket, self.remote)
}
fn restore_dscp_best_effort(&self, previous_dscp: Option<u8>) {
let _ = match previous_dscp {
Some(dscp) => apply_dscp_to_tokio_socket(&self.socket, self.remote, dscp),
fn restore_dscp_best_effort(&self, previous_traffic_class: Option<u8>) {
let _ = match previous_traffic_class {
Some(traffic_class) => {
apply_traffic_class_to_tokio_socket(&self.socket, self.remote, traffic_class)
}
None => clear_dscp_on_tokio_socket(&self.socket, self.remote),
};
}
@@ -961,7 +967,7 @@ impl AsyncClient {
struct DscpRollback<'a> {
socket: &'a tokio::net::UdpSocket,
remote: SocketAddr,
previous_dscp: Option<u8>,
previous_traffic_class: Option<u8>,
armed: bool,
#[cfg(test)]
fail_restore: &'a Cell<bool>,
@@ -971,13 +977,13 @@ impl<'a> DscpRollback<'a> {
fn armed(
socket: &'a tokio::net::UdpSocket,
remote: SocketAddr,
previous_dscp: Option<u8>,
previous_traffic_class: Option<u8>,
#[cfg(test)] fail_restore: &'a Cell<bool>,
) -> Self {
Self {
socket,
remote,
previous_dscp,
previous_traffic_class,
armed: true,
#[cfg(test)]
fail_restore,
@@ -996,8 +1002,10 @@ impl<'a> DscpRollback<'a> {
source: io::Error::other("injected DSCP restore failure"),
});
}
match self.previous_dscp {
Some(dscp) => apply_dscp_to_tokio_socket(self.socket, self.remote, dscp)?,
match self.previous_traffic_class {
Some(traffic_class) => {
apply_traffic_class_to_tokio_socket(self.socket, self.remote, traffic_class)?
}
None => clear_dscp_on_tokio_socket(self.socket, self.remote)?,
}
self.armed = false;
+16 -12
View File
@@ -493,7 +493,7 @@ fn opening_cancellation_before_submission_is_transactional() {
assert!(client.machine.prepare_open_request().is_ok());
assert!(client.prepared_open.is_some());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
});
assert!(server.finish().is_empty());
}
@@ -527,7 +527,7 @@ fn opening_cancellation_after_submission_can_retry() {
assert!(client.machine.prepare_open_request().is_ok());
assert!(client.prepared_open.is_some());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
assert!(matches!(
client.open().await.unwrap(),
OpenOutcome::Started { .. }
@@ -555,7 +555,7 @@ fn adapter_failure_sends_preencoded_cleanup_without_committing_open() {
));
assert!(client.machine.prepare_open_request().is_ok());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
});
assert_eq!(server.finish().len(), 2);
}
@@ -598,7 +598,7 @@ fn no_test_open_has_no_schedule_or_dscp_state() {
));
assert!(client.prepared_open.is_none());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
assert!(client.is_run_complete());
assert!(matches!(
client.close().await,
@@ -879,7 +879,7 @@ fn peer_close_dscp_failure_preserves_events_and_logical_ownership() {
assert!(client.is_run_complete());
assert!(client.schedule.is_none());
assert!(client.prepared_probe.is_none());
assert_eq!(client.applied_dscp, Some(46));
assert_eq!(client.applied_traffic_class, Some(184));
assert_eq!(client.next_send_deadline(), None);
assert_eq!(socket_dscp(&client), 46);
});
@@ -936,7 +936,7 @@ fn close_cancellation_and_would_block_restore_dscp_before_suspend() {
assert!(client.machine.is_open());
assert!(client.schedule.is_some());
assert_eq!(client.next_send_deadline(), deadline);
assert_eq!(client.applied_dscp, Some(46));
assert_eq!(client.applied_traffic_class, Some(184));
assert_eq!(client.test_hooks.send_attempts.get(), attempts);
assert_eq!(socket_dscp(&client), 46);
@@ -947,7 +947,7 @@ fn close_cancellation_and_would_block_restore_dscp_before_suspend() {
drop(close);
assert!(client.machine.is_open());
assert!(client.schedule.is_some());
assert_eq!(client.applied_dscp, Some(46));
assert_eq!(client.applied_traffic_class, Some(184));
assert_eq!(socket_dscp(&client), 46);
assert!(matches!(
@@ -955,7 +955,7 @@ fn close_cancellation_and_would_block_restore_dscp_before_suspend() {
[ClientEvent::SessionClosed { .. }]
));
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
});
assert_eq!(server.finish().len(), 2);
}
@@ -972,7 +972,7 @@ fn close_errors_preserve_open_state_and_primary_send_error() {
assert!(client.machine.is_open());
assert!(client.schedule.is_some());
assert_eq!(client.next_send_deadline(), deadline);
assert_eq!(client.applied_dscp, Some(46));
assert_eq!(client.applied_traffic_class, Some(184));
assert_eq!(socket_dscp(&client), 46);
client.test_hooks.fail_dscp_restore.set(true);
@@ -1001,7 +1001,7 @@ fn close_would_block_restore_failure_stops_without_committing() {
));
assert!(client.machine.is_open());
assert!(client.schedule.is_some());
assert_eq!(client.applied_dscp, Some(46));
assert_eq!(client.applied_traffic_class, Some(184));
});
assert_eq!(server.finish().len(), 1);
}
@@ -1027,7 +1027,7 @@ fn successful_short_close_commits_before_length_error() {
assert!(!client.machine.is_open());
assert!(client.schedule.is_none());
assert!(client.prepared_probe.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
assert!(matches!(
client.machine.prepare_close(),
Err(ClientError::AlreadyClosed)
@@ -1085,7 +1085,11 @@ fn blocking_and_async_hmac_dscp_lifecycle_are_semantically_equivalent() {
open_negotiated(&blocking_open),
open_negotiated(&async_open)
);
assert_eq!(open_negotiated(&async_open).params.dscp, 46);
assert_eq!(
open_negotiated(&async_open).params.dscp,
184,
"negotiated Params::dscp is the raw wire byte for codepoint 46"
);
assert_matching_event_shape(&blocking_sent[0], &async_sent[0]);
assert_matching_event_shape(&blocking_reply[0], &async_reply[0]);
assert_matching_event_shape(&blocking_close[0], &async_close[0]);
+33 -29
View File
@@ -20,7 +20,7 @@ use crate::{
SessionMachine, MAX_OPEN_PACKET_SIZE,
},
socket::{connect_udp_socket, resolve_remote, validate_open_timeouts},
socket_options::{apply_dscp_to_socket, clear_dscp_on_socket},
socket_options::{apply_traffic_class_to_socket, clear_dscp_on_socket},
timing::ClientTimestamp,
};
@@ -42,8 +42,8 @@ struct PreparedClientOpen {
machine: PreparedOpenAcceptance,
schedule: Option<ProbeSchedule>,
recv_buffer_len: Option<usize>,
negotiated_dscp: Option<u8>,
previous_dscp: Option<u8>,
negotiated_traffic_class: Option<u8>,
previous_traffic_class: Option<u8>,
post_open_recv_timeout: Option<Duration>,
}
@@ -103,7 +103,7 @@ pub struct Client {
socket: UdpSocket,
remote: SocketAddr,
recv_buffer: Vec<u8>,
applied_dscp: Option<u8>,
applied_traffic_class: Option<u8>,
#[cfg(test)]
test_hooks: ClientTestHooks,
#[cfg(test)]
@@ -132,7 +132,7 @@ impl Client {
socket,
remote,
recv_buffer: vec![0_u8; recv_buffer_size(false, None)?],
applied_dscp: None,
applied_traffic_class: None,
#[cfg(test)]
test_hooks: ClientTestHooks::default(),
#[cfg(test)]
@@ -192,14 +192,14 @@ impl Client {
operation: "close event result",
source,
})?;
let previous_dscp = self.applied_dscp;
let previous_traffic_class = self.applied_traffic_class;
let expected_bytes = prepared.bytes.len();
self.clear_close_dscp()?;
let bytes = match self.send_close_datagram(prepared.bytes) {
Ok(bytes) => bytes,
Err(err) => {
self.restore_dscp_best_effort(previous_dscp);
self.restore_dscp_best_effort(previous_traffic_class);
return Err(ClientError::Socket(err));
}
};
@@ -213,7 +213,7 @@ impl Client {
.unwrap_or_else(ClientTimestamp::now);
let event = self.runtime.commit_local_close(prepared.commit, sent_at);
self.schedule = None;
self.applied_dscp = None;
self.applied_traffic_class = None;
validate_datagram_length(expected_bytes, bytes)?;
events.push(event);
@@ -275,7 +275,7 @@ impl Client {
if self.runtime.is_peer_closed() {
self.schedule = None;
if self.clear_close_dscp().is_ok() {
self.applied_dscp = None;
self.applied_traffic_class = None;
}
}
Ok(events)
@@ -521,8 +521,8 @@ impl Client {
machine,
schedule: None,
recv_buffer_len: None,
negotiated_dscp: None,
previous_dscp: self.applied_dscp,
negotiated_traffic_class: None,
previous_traffic_class: self.applied_traffic_class,
post_open_recv_timeout: self.runtime.config().socket_config.recv_timeout,
});
};
@@ -540,12 +540,12 @@ impl Client {
.recv_buffer_len_override
.get()
.unwrap_or(recv_buffer_len);
let negotiated_dscp = match u8::try_from(negotiated.params.dscp) {
Ok(dscp) => dscp,
let negotiated_traffic_class = match u8::try_from(negotiated.params.dscp) {
Ok(traffic_class) => traffic_class,
Err(_) => {
return Err(Box::new(PreparedClientOpenFailure {
primary: ClientError::InvalidConfig {
reason: "negotiated dscp must be in range 0..=63".to_owned(),
reason: "negotiated dscp must be in range 0..=255".to_owned(),
},
machine,
}));
@@ -556,8 +556,8 @@ impl Client {
machine,
schedule: Some(schedule),
recv_buffer_len: Some(recv_buffer_len),
negotiated_dscp: Some(negotiated_dscp),
previous_dscp: self.applied_dscp,
negotiated_traffic_class: Some(negotiated_traffic_class),
previous_traffic_class: self.applied_traffic_class,
post_open_recv_timeout: self.runtime.config().socket_config.recv_timeout,
})
}
@@ -570,12 +570,14 @@ impl Client {
machine,
schedule,
recv_buffer_len,
negotiated_dscp,
previous_dscp,
negotiated_traffic_class,
previous_traffic_class,
post_open_recv_timeout,
} = prepared;
if let (Some(recv_buffer_len), Some(negotiated_dscp)) = (recv_buffer_len, negotiated_dscp) {
if let (Some(recv_buffer_len), Some(negotiated_traffic_class)) =
(recv_buffer_len, negotiated_traffic_class)
{
let previous_len = self.recv_buffer.len();
let additional = recv_buffer_len.saturating_sub(previous_len);
if let Err(source) = self.recv_buffer.try_reserve(additional) {
@@ -591,23 +593,23 @@ impl Client {
self.test_hooks
.prepared_active_session_before_dscp
.set(machine.has_prepared_active_session());
if let Err(primary) = self.apply_open_dscp(negotiated_dscp) {
if let Err(primary) = self.apply_open_dscp(negotiated_traffic_class) {
self.recv_buffer.truncate(previous_len);
self.restore_dscp_best_effort(previous_dscp);
self.restore_dscp_best_effort(previous_traffic_class);
self.send_cleanup_close_best_effort(machine.cleanup_close_packet());
return Err(primary);
}
if let Err(source) = self.restore_open_read_timeout(post_open_recv_timeout) {
let primary = ClientError::ReadTimeoutRestore { source };
self.recv_buffer.truncate(previous_len);
self.restore_dscp_best_effort(previous_dscp);
self.restore_dscp_best_effort(previous_traffic_class);
self.send_cleanup_close_best_effort(machine.cleanup_close_packet());
return Err(primary);
}
let outcome = self.runtime.commit_open(machine);
self.schedule = schedule;
self.applied_dscp = Some(negotiated_dscp);
self.applied_traffic_class = Some(negotiated_traffic_class);
Ok(outcome)
} else {
debug_assert!(schedule.is_none());
@@ -616,18 +618,20 @@ impl Client {
}
let outcome = self.runtime.commit_open(machine);
self.schedule = None;
self.applied_dscp = None;
self.applied_traffic_class = None;
Ok(outcome)
}
}
fn restore_dscp_best_effort(&self, previous_dscp: Option<u8>) {
fn restore_dscp_best_effort(&self, previous_traffic_class: Option<u8>) {
#[cfg(test)]
if self.test_hooks.fail_dscp_restore.replace(false) {
return;
}
let _ = match previous_dscp {
Some(dscp) => apply_dscp_to_socket(&self.socket, self.remote, dscp),
let _ = match previous_traffic_class {
Some(traffic_class) => {
apply_traffic_class_to_socket(&self.socket, self.remote, traffic_class)
}
None => clear_dscp_on_socket(&self.socket, self.remote),
};
}
@@ -654,7 +658,7 @@ impl Client {
}
}
fn apply_open_dscp(&self, dscp: u8) -> Result<(), ClientError> {
fn apply_open_dscp(&self, traffic_class: u8) -> Result<(), ClientError> {
#[cfg(test)]
if self.test_hooks.fail_open_dscp.replace(false) {
return Err(ClientError::SocketOption {
@@ -663,7 +667,7 @@ impl Client {
source: io::Error::other("injected negotiated DSCP failure"),
});
}
apply_dscp_to_socket(&self.socket, self.remote, dscp)
apply_traffic_class_to_socket(&self.socket, self.remote, traffic_class)
}
fn restore_open_read_timeout(&self, timeout: Option<Duration>) -> io::Result<()> {
+6 -6
View File
@@ -45,7 +45,7 @@ fn close_sends_one_close_packet_with_negotiated_token() {
));
assert!(!client.runtime.is_open());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
let packets: Vec<_> = server.rx.iter().take(2).collect();
let close = &packets[1];
assert_eq!(close[3], flags::FLAG_CLOSE);
@@ -82,7 +82,7 @@ fn close_event_reservation_failure_precedes_dscp_clear_and_send() {
assert!(client.test_hooks.fail_close_dscp_clear.get());
assert!(client.runtime.is_open());
assert!(client.schedule.is_some());
assert_eq!(client.applied_dscp, Some(0));
assert_eq!(client.applied_traffic_class, Some(0));
client.test_hooks.fail_close_dscp_clear.set(false);
client.close().unwrap();
@@ -114,7 +114,7 @@ fn close_dscp_clear_failure_precedes_send_and_preserves_open_state() {
assert_eq!(client.test_hooks.close_send_attempts.get(), 0);
assert!(client.runtime.is_open());
assert!(client.schedule.is_some());
assert_eq!(client.applied_dscp, Some(0));
assert_eq!(client.applied_traffic_class, Some(0));
client.close().unwrap();
assert_eq!(client.test_hooks.close_send_attempts.get(), 1);
@@ -147,7 +147,7 @@ fn close_send_failure_leaves_machine_and_schedule_open_for_retry() {
assert_eq!(client.test_hooks.close_sent_at.get(), Some(sent_at));
assert!(client.runtime.is_open());
assert!(client.schedule.is_some());
assert_eq!(client.applied_dscp, Some(0));
assert_eq!(client.applied_traffic_class, Some(0));
assert_eq!(client.test_hooks.close_send_attempts.get(), 1);
assert!(matches!(
@@ -196,7 +196,7 @@ fn short_successful_close_commits_before_length_mismatch() {
));
assert!(!client.runtime.is_open());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
assert_eq!(client.test_hooks.close_sent_at.get(), None);
assert!(matches!(client.close(), Err(ClientError::AlreadyClosed)));
server.join();
@@ -351,7 +351,7 @@ fn peer_close_dscp_cleanup_failure_preserves_events_and_closed_state() {
));
assert!(client.is_peer_closed());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, Some(0));
assert_eq!(client.applied_traffic_class, Some(0));
assert_eq!(client.test_hooks.close_send_attempts.get(), 0);
let open = server.rx.recv_timeout(Duration::from_millis(100)).unwrap();
let echo = server.rx.recv_timeout(Duration::from_millis(100)).unwrap();
@@ -42,7 +42,10 @@ fn params_from_config_maps_compatibility_fields() {
assert_eq!(params.received_stats, ReceivedStats::Window);
assert_eq!(params.stamp_at, StampAt::Midpoint);
assert_eq!(params.clock, Clock::Wall);
assert_eq!(params.dscp, 46, "config DSCP codepoint must not be shifted");
assert_eq!(
params.dscp, 184,
"config DSCP codepoint 46 must be shifted into raw wire byte 184"
);
assert_eq!(
params.server_fill.as_ref().map(|fill| fill.value.as_str()),
Some("rand")
@@ -67,7 +70,11 @@ fn params_from_config_accepts_boundary_values() {
dscp: 63,
..ClientConfig::default()
};
assert_eq!(params_from_config(&config).unwrap().dscp, 63);
assert_eq!(
params_from_config(&config).unwrap().dscp,
252,
"boundary codepoint 63 must be shifted into raw wire byte 252"
);
}
#[test]
+14 -10
View File
@@ -9,9 +9,11 @@ use crate::socket_options::socket_traffic_class;
target_os = "illumos",
target_os = "haiku",
)))]
fn normal_open_applies_negotiated_dscp_after_open_and_close_clears_it() {
fn normal_open_applies_negotiated_traffic_class_after_open_and_close_clears_it() {
let mut params = default_params();
params.dscp = 46;
// Raw wire TOS/Traffic Class byte for DSCP codepoint 46 (EF): a
// compliant server echoes back the raw byte it received, unchanged.
params.dscp = 184;
let server = start_fake_server(move |socket, tx| {
let (_, peer) = recv_request(&socket, &tx);
let reply = open_reply(FLAG_OPEN | FLAG_REPLY, TOKEN, &params, None);
@@ -48,7 +50,7 @@ fn normal_open_applies_negotiated_dscp_after_open_and_close_clears_it() {
target_os = "illumos",
target_os = "haiku",
)))]
fn normal_open_uses_negotiated_dscp_not_requested_dscp() {
fn normal_open_uses_negotiated_traffic_class_not_requested_dscp() {
let mut returned = default_params();
returned.dscp = 0;
let server = open_success_server(returned);
@@ -82,9 +84,10 @@ fn normal_open_uses_negotiated_dscp_not_requested_dscp() {
target_os = "illumos",
target_os = "haiku",
)))]
fn failed_close_send_restores_negotiated_dscp_and_keeps_session_open() {
fn failed_close_send_restores_negotiated_traffic_class_and_keeps_session_open() {
let mut params = default_params();
params.dscp = 46;
// Raw wire TOS/Traffic Class byte for DSCP codepoint 46 (EF).
params.dscp = 184;
let server = start_fake_server(move |socket, tx| {
let (_, peer) = recv_request(&socket, &tx);
socket
@@ -106,7 +109,7 @@ fn failed_close_send_restores_negotiated_dscp_and_keeps_session_open() {
assert!(matches!(error, ClientError::Socket(_)));
assert!(client.runtime.is_open());
assert!(client.schedule.is_some());
assert_eq!(client.applied_dscp, Some(46));
assert_eq!(client.applied_traffic_class, Some(184));
assert_eq!(
socket_traffic_class(&client.socket, client.remote).unwrap() & 0xfc,
184
@@ -139,7 +142,7 @@ fn failed_close_keeps_send_error_primary_when_dscp_restoration_also_fails() {
assert!(error.to_string().contains("injected close send failure"));
assert!(client.runtime.is_open());
assert!(client.schedule.is_some());
assert_eq!(client.applied_dscp, Some(0));
assert_eq!(client.applied_traffic_class, Some(0));
client.close().unwrap();
server.join();
}
@@ -152,9 +155,10 @@ fn failed_close_keeps_send_error_primary_when_dscp_restoration_also_fails() {
target_os = "illumos",
target_os = "haiku",
)))]
fn authenticated_peer_close_clears_schedule_and_negotiated_dscp() {
fn authenticated_peer_close_clears_schedule_and_negotiated_traffic_class() {
let mut params = default_params();
params.dscp = 46;
// Raw wire TOS/Traffic Class byte for DSCP codepoint 46 (EF).
params.dscp = 184;
let server = start_fake_server(move |socket, tx| {
let (_, peer) = recv_request(&socket, &tx);
socket
@@ -195,7 +199,7 @@ fn authenticated_peer_close_clears_schedule_and_negotiated_dscp() {
]
));
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
assert_eq!(
socket_traffic_class(&client.socket, client.remote).unwrap(),
0
@@ -33,13 +33,17 @@ fn strict_negotiation_rejects_changed_params() {
..ClientConfig::default()
};
let requested = params_from_config(&config).unwrap();
assert_eq!(
requested.dscp, 184,
"codepoint 46 must become raw wire byte 184"
);
let mut returned = requested.clone();
returned.dscp = 0;
assert_eq!(
rejection_reason(&requested, &returned, NegotiationPolicy::Strict),
NegotiationRestriction::DscpChanged {
requested: requested.dscp,
negotiated: returned.dscp,
requested: 46,
negotiated: 0,
}
.message()
);
@@ -183,17 +187,34 @@ fn strict_negotiation_rejects_negative_returned_length() {
#[test]
fn loose_negotiation_rejects_runtime_invalid_returned_dscp() {
let config = ClientConfig::default();
let config = ClientConfig {
dscp: 46,
..ClientConfig::default()
};
let requested = params_from_config(&config).unwrap();
assert_eq!(
requested.dscp, 184,
"codepoint 46 must become raw wire byte 184"
);
for dscp in [-1, 64] {
for dscp in [-1, 256] {
let mut returned = requested.clone();
returned.dscp = dscp;
assert_eq!(
rejection_reason(&requested, &returned, NegotiationPolicy::Loose),
"dscp must be in range 0..=63"
"dscp must be in range 0..=255"
);
}
// A raw returned value of 184 must be accepted even though it exceeds the
// 0..=63 codepoint range; it is the correctly negotiated wire value for
// codepoint 46 and is not subject to the codepoint bound at all.
let returned = requested.clone();
assert!(
assert_negotiates(&requested, &returned, NegotiationPolicy::Loose)
.restrictions
.is_empty()
);
}
#[test]
@@ -13,7 +13,7 @@ fn no_test_success_validates_params() {
assert_eq!(negotiated.params, params);
assert!(client.next_send_deadline().is_none());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
server.join();
}
+9 -7
View File
@@ -10,7 +10,7 @@ fn successful_open_handshake() {
let negotiated = assert_open_started(client.open().unwrap());
assert_eq!(negotiated.params, params);
assert!(client.schedule.is_some());
assert_eq!(client.applied_dscp, Some(0));
assert_eq!(client.applied_traffic_class, Some(0));
server.join();
}
@@ -284,7 +284,7 @@ fn schedule_duration_overflow_cannot_partially_open() {
assert!(failure.machine.cleanup_close_packet().is_some());
assert!(client.runtime.prepare_open_request().is_ok());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
assert_eq!(client.recv_buffer.len(), previous_recv_len);
server.join();
}
@@ -346,7 +346,9 @@ fn cleanup_send_failure_does_not_replace_primary_open_error() {
#[test]
fn receive_buffer_reservation_failure_precedes_dscp_application() {
let mut returned = default_params();
returned.dscp = 46;
// Raw wire TOS/Traffic Class byte for DSCP codepoint 46 (EF), matching
// what the client below requests so negotiation succeeds unchanged.
returned.dscp = 184;
let server = start_fake_server(move |socket, tx| {
let (_, peer) = recv_request(&socket, &tx);
socket
@@ -376,7 +378,7 @@ fn receive_buffer_reservation_failure_precedes_dscp_application() {
assert!(client.test_hooks.fail_open_dscp.get());
assert!(client.runtime.prepare_open_request().is_ok());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
server.join();
}
@@ -406,7 +408,7 @@ fn dscp_application_failure_leaves_machine_connected() {
assert!(client.test_hooks.prepared_active_session_before_dscp.get());
assert!(client.runtime.prepare_open_request().is_ok());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
server.join();
}
@@ -432,7 +434,7 @@ fn read_timeout_restoration_failure_leaves_machine_connected() {
));
assert!(client.runtime.prepare_open_request().is_ok());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
server.join();
}
@@ -458,6 +460,6 @@ fn dscp_rollback_failure_keeps_timeout_restoration_primary() {
assert!(matches!(error, ClientError::ReadTimeoutRestore { .. }));
assert!(client.runtime.prepare_open_request().is_ok());
assert!(client.schedule.is_none());
assert_eq!(client.applied_dscp, None);
assert_eq!(client.applied_traffic_class, None);
server.join();
}
@@ -14,7 +14,7 @@ fn echo_reply_metadata_propagates_observed_dscp_with_ancillary() {
let (size, _) = socket.recv_from(&mut buf).unwrap();
tx.send(buf[..size].to_vec()).unwrap();
let seq = u32::from_le_bytes(buf[12..16].try_into().unwrap());
crate::socket_options::apply_dscp_to_socket(&socket, peer, 46).unwrap();
crate::socket_options::apply_traffic_class_to_socket(&socket, peer, 184).unwrap();
let reply_packet =
echo_reply_packet(TOKEN, seq, &params, &TimestampFields::default(), None);
socket.send_to(&reply_packet, peer).unwrap();
+4 -2
View File
@@ -12,8 +12,10 @@ use irtt_proto::{Clock, ReceivedStats, StampAt};
pub(crate) const DEFAULT_PORT: u16 = 2112;
/// Largest valid DSCP codepoint accepted by client configuration.
///
/// DSCP is a six-bit value in the range `0..=63`. The client maps it into the
/// socket traffic-class field when configuring packet priority.
/// DSCP is a six-bit value in the range `0..=63`. The client shifts it left
/// by two bits into the raw IP TOS / Traffic Class byte carried by
/// [`irtt_proto::Params::dscp`] and applied to the socket during the active
/// test phase.
pub const MAX_DSCP_CODEPOINT: u8 = 63;
/// Largest IPv4 TTL or IPv6 hop-limit value accepted by client configuration.
///
+3 -3
View File
@@ -124,7 +124,7 @@ mod tests {
use crate::{
event::PacketMeta,
receive::{configure_receive_metadata, recv_datagram},
socket_options::apply_dscp_to_socket,
socket_options::apply_traffic_class_to_socket,
timing::ClientTimestamp,
};
@@ -277,7 +277,7 @@ mod tests {
fn ipv4_traffic_class_metadata_is_observed_when_kernel_provides_it() {
let (sender, receiver) = connected_ipv4_loopback_pair();
configure_receive_metadata(&receiver, sender.local_addr().unwrap()).unwrap();
apply_dscp_to_socket(&sender, receiver.local_addr().unwrap(), 46).unwrap();
apply_traffic_class_to_socket(&sender, receiver.local_addr().unwrap(), 184).unwrap();
sender.send(b"dscp").unwrap();
let mut buf = [0_u8; 16];
@@ -299,7 +299,7 @@ mod tests {
return;
};
configure_receive_metadata(&receiver, sender.local_addr().unwrap()).unwrap();
apply_dscp_to_socket(&sender, receiver.local_addr().unwrap(), 46).unwrap();
apply_traffic_class_to_socket(&sender, receiver.local_addr().unwrap(), 184).unwrap();
sender.send(b"dscp").unwrap();
let mut buf = [0_u8; 16];
+2 -1
View File
@@ -18,6 +18,7 @@ use crate::{
metadata::ReceiveMeta,
probe::{CompletedSet, PendingMap, PendingProbe, TimedOutMap},
session::{negotiate_params, NegotiatedParams},
socket_options::dscp_codepoint_to_traffic_class,
timing::ClientTimestamp,
};
@@ -848,7 +849,7 @@ pub(crate) fn params_from_config(config: &ClientConfig) -> Result<Params, Client
received_stats: config.received_stats,
stamp_at: config.stamp_at,
clock: config.clock,
dscp: i64::from(config.dscp),
dscp: i64::from(dscp_codepoint_to_traffic_class(config.dscp)?),
server_fill: config.server_fill.clone().map(|value| ServerFill { value }),
})
}
+25 -11
View File
@@ -2,16 +2,18 @@ use std::fmt;
use irtt_proto::{Clock, Params, ReceivedStats, StampAt, PROTOCOL_VERSION};
use crate::{
config::{NegotiationPolicy, MAX_DSCP_CODEPOINT},
error::ClientError,
};
use crate::{config::NegotiationPolicy, error::ClientError};
/// Protocol parameters accepted for a session.
///
/// `params` contains the server-returned values that the client will use for
/// echo packets. `restrictions` records accepted differences from the request
/// when loose negotiation is enabled.
///
/// `params.dscp` is the raw IP TOS / Traffic Class byte, not a DSCP
/// codepoint: for a configured codepoint of 46 (EF), an unrestricted
/// negotiation leaves `params.dscp == 184`. [`NegotiationRestriction::DscpChanged`]
/// reports codepoints instead, for consistency with [`crate::ClientConfig::dscp`].
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct NegotiatedParams {
/// Server-returned protocol parameters used for the session.
@@ -57,6 +59,10 @@ pub enum NegotiationRestriction {
/// Returned clock source differs from the request.
ClockChanged { requested: Clock, negotiated: Clock },
/// Returned DSCP codepoint differs from the request.
///
/// These values are DSCP codepoints (`0..=63`) for human-facing
/// consistency with [`crate::ClientConfig::dscp`], not the raw wire
/// `Params::dscp` byte carried by [`NegotiatedParams::params`].
DscpChanged { requested: i64, negotiated: i64 },
/// Returned server payload fill behavior differs from the request.
ServerFillChanged,
@@ -268,8 +274,11 @@ pub(crate) fn negotiate_params(
policy,
&mut restrictions,
NegotiationRestriction::DscpChanged {
requested: requested.dscp,
negotiated: returned.dscp,
// `requested`/`negotiated` here are raw wire TOS/Traffic Class
// bytes; shift back to the codepoint the user actually
// configured for a human-facing restriction.
requested: requested.dscp >> 2,
negotiated: returned.dscp >> 2,
},
)?;
}
@@ -309,10 +318,14 @@ fn validate_duration_restriction(requested: i64, returned: i64) -> Result<(), Cl
Ok(())
}
/// Validates a server-returned `Params::dscp`, which is a raw IP TOS /
/// Traffic Class byte and therefore valid across the full `0..=255` wire
/// range, not the `0..=`[`MAX_DSCP_CODEPOINT`](crate::config::MAX_DSCP_CODEPOINT)
/// range of the public codepoint.
fn validate_dscp_restriction(returned: i64) -> Result<(), ClientError> {
if !(0..=i64::from(MAX_DSCP_CODEPOINT)).contains(&returned) {
if !(0..=255).contains(&returned) {
return Err(ClientError::NegotiationRejected {
reason: format!("dscp must be in range 0..={MAX_DSCP_CODEPOINT}"),
reason: "dscp must be in range 0..=255".to_owned(),
});
}
@@ -348,7 +361,8 @@ mod tests {
received_stats: ReceivedStats::Both,
stamp_at: StampAt::Both,
clock: Clock::Both,
dscp: 46,
// Raw wire TOS/Traffic Class byte for DSCP codepoint 46 (EF).
dscp: 184,
server_fill: Some(ServerFill {
value: "rand".to_owned(),
}),
@@ -402,8 +416,8 @@ mod tests {
assert_eq!(
rejection_reason(&requested, &returned, NegotiationPolicy::Strict),
NegotiationRestriction::DscpChanged {
requested: requested.dscp,
negotiated: returned.dscp,
requested: requested.dscp >> 2,
negotiated: returned.dscp >> 2,
}
.message()
);
+35 -19
View File
@@ -10,6 +10,13 @@ use crate::{
error::ClientError,
};
/// Converts a public DSCP codepoint (`0..=`[`MAX_DSCP_CODEPOINT`]) into the
/// raw IP TOS / Traffic Class byte it occupies the upper six bits of.
///
/// This is the single place `codepoint << 2` is spelled out; both the
/// config-to-wire conversion and any direct codepoint-based socket
/// application go through this function so the shift cannot be duplicated or
/// accidentally applied twice.
pub(crate) fn dscp_codepoint_to_traffic_class(dscp: u8) -> Result<u32, ClientError> {
if dscp > MAX_DSCP_CODEPOINT {
return Err(ClientError::InvalidConfig {
@@ -19,13 +26,19 @@ pub(crate) fn dscp_codepoint_to_traffic_class(dscp: u8) -> Result<u32, ClientErr
Ok(u32::from(dscp) << 2)
}
pub(crate) fn apply_dscp_to_socket(
/// Applies an already-raw IP TOS / Traffic Class byte to the socket directly,
/// without any codepoint shift.
pub(crate) fn apply_traffic_class_to_socket(
socket: &UdpSocket,
remote: SocketAddr,
dscp_codepoint: u8,
traffic_class: u8,
) -> Result<(), ClientError> {
let traffic_class = dscp_codepoint_to_traffic_class(dscp_codepoint)?;
set_socket_traffic_class(socket, remote, traffic_class, "set negotiated DSCP")
set_socket_traffic_class(
socket,
remote,
u32::from(traffic_class),
"set negotiated DSCP",
)
}
pub(crate) fn clear_dscp_on_socket(
@@ -35,14 +48,20 @@ pub(crate) fn clear_dscp_on_socket(
set_socket_traffic_class(socket, remote, 0, "clear DSCP before close")
}
/// Applies an already-raw IP TOS / Traffic Class byte to the Tokio socket
/// directly, without any codepoint shift.
#[cfg(feature = "tokio")]
pub(crate) fn apply_dscp_to_tokio_socket(
pub(crate) fn apply_traffic_class_to_tokio_socket(
socket: &tokio::net::UdpSocket,
remote: SocketAddr,
dscp_codepoint: u8,
traffic_class: u8,
) -> Result<(), ClientError> {
let traffic_class = dscp_codepoint_to_traffic_class(dscp_codepoint)?;
set_tokio_socket_traffic_class(socket, remote, traffic_class, "set negotiated DSCP")
set_tokio_socket_traffic_class(
socket,
remote,
u32::from(traffic_class),
"set negotiated DSCP",
)
}
#[cfg(feature = "tokio")]
@@ -454,10 +473,10 @@ mod tests {
let socket = UdpSocket::bind(SocketAddr::from(([127, 0, 0, 1], 0))).unwrap();
socket.connect(remote).unwrap();
apply_dscp_to_socket(&socket, remote, 46).unwrap();
let traffic_class = socket_traffic_class(&socket, remote).unwrap();
assert_eq!(traffic_class & 0xfc, 184);
assert_eq!(traffic_class & 0b11, 0);
// 184 is the raw wire TOS/Traffic Class byte for DSCP codepoint 46
// (EF); applying it directly must not shift it again.
apply_traffic_class_to_socket(&socket, remote, 184).unwrap();
assert_eq!(socket_traffic_class(&socket, remote).unwrap(), 184);
clear_dscp_on_socket(&socket, remote).unwrap();
assert_eq!(socket_traffic_class(&socket, remote).unwrap(), 0);
@@ -487,10 +506,8 @@ mod tests {
socket.set_nonblocking(true).unwrap();
let socket = tokio::net::UdpSocket::from_std(socket).unwrap();
apply_dscp_to_tokio_socket(&socket, remote, 46).unwrap();
let traffic_class = tokio_socket_traffic_class(&socket, remote).unwrap();
assert_eq!(traffic_class & 0xfc, 184);
assert_eq!(traffic_class & 0b11, 0);
apply_traffic_class_to_tokio_socket(&socket, remote, 184).unwrap();
assert_eq!(tokio_socket_traffic_class(&socket, remote).unwrap(), 184);
clear_dscp_on_tokio_socket(&socket, remote).unwrap();
assert_eq!(tokio_socket_traffic_class(&socket, remote).unwrap(), 0);
@@ -515,7 +532,7 @@ mod tests {
return;
};
apply_dscp_to_socket(&socket, remote, 46).unwrap();
apply_traffic_class_to_socket(&socket, remote, 184).unwrap();
let traffic_class = match socket_traffic_class(&socket, remote) {
Ok(traffic_class) => traffic_class,
Err(error) if is_unsupported_socket_readback(&error) => {
@@ -524,8 +541,7 @@ mod tests {
}
Err(error) => panic!("{error}"),
};
assert_eq!(traffic_class & 0xfc, 184);
assert_eq!(traffic_class & 0b11, 0);
assert_eq!(traffic_class, 184);
clear_dscp_on_socket(&socket, remote).unwrap();
assert_eq!(socket_traffic_class(&socket, remote).unwrap(), 0);