feat(client): track and report specific negotiation restrictions
Replace basic validation with a detailed negotiation process that records specific parameter changes. This allows the client to provide better feedback when a server restricts requested session parameters like duration or DSCP.
This commit is contained in:
7 files changed
+409
-87
No files matched your search
@@ -60,6 +60,13 @@ fn normal_open_uses_negotiated_dscp_not_requested_dscp() {
|
||||
|
||||
let negotiated = assert_open_started(client.open().unwrap());
|
||||
assert_eq!(negotiated.params.dscp, 0);
|
||||
assert_eq!(
|
||||
negotiated.restrictions,
|
||||
vec![crate::NegotiationRestriction::DscpChanged {
|
||||
requested: 46,
|
||||
negotiated: 0,
|
||||
}]
|
||||
);
|
||||
assert_eq!(
|
||||
socket_traffic_class(&client.socket, client.remote).unwrap(),
|
||||
0
|
||||
|
||||
@@ -1,10 +1,29 @@
|
||||
use super::*;
|
||||
use crate::NegotiationRestriction;
|
||||
|
||||
fn assert_negotiates(
|
||||
requested: &Params,
|
||||
returned: &Params,
|
||||
policy: NegotiationPolicy,
|
||||
) -> NegotiatedParams {
|
||||
negotiate_params(requested, returned.clone(), policy)
|
||||
.unwrap_or_else(|err| panic!("expected negotiation success, got {err:?}"))
|
||||
}
|
||||
|
||||
fn rejection_reason(requested: &Params, returned: &Params, policy: NegotiationPolicy) -> String {
|
||||
match negotiate_params(requested, returned.clone(), policy) {
|
||||
Err(ClientError::NegotiationRejected { reason }) => reason,
|
||||
other => panic!("expected negotiation rejection, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_negotiation_accepts_identical_params() {
|
||||
let config = ClientConfig::default();
|
||||
let params = params_from_config(&config).unwrap();
|
||||
assert!(validate_negotiated_params(¶ms, ¶ms, NegotiationPolicy::Strict).is_ok());
|
||||
let negotiated = assert_negotiates(¶ms, ¶ms, NegotiationPolicy::Strict);
|
||||
assert_eq!(negotiated.params, params);
|
||||
assert!(negotiated.restrictions.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -13,20 +32,38 @@ fn strict_negotiation_rejects_changed_params() {
|
||||
let requested = params_from_config(&config).unwrap();
|
||||
let mut returned = requested.clone();
|
||||
returned.dscp = 1;
|
||||
assert!(matches!(
|
||||
validate_negotiated_params(&requested, &returned, NegotiationPolicy::Strict),
|
||||
Err(ClientError::NegotiationRejected { .. })
|
||||
));
|
||||
assert_eq!(
|
||||
rejection_reason(&requested, &returned, NegotiationPolicy::Strict),
|
||||
NegotiationRestriction::DscpChanged {
|
||||
requested: requested.dscp,
|
||||
negotiated: returned.dscp,
|
||||
}
|
||||
.message()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loose_negotiation_accepts_server_restricted_params() {
|
||||
let config = ClientConfig::default();
|
||||
let requested = params_from_config(&config).unwrap();
|
||||
let mut requested = params_from_config(&config).unwrap();
|
||||
requested.length = 128;
|
||||
let mut returned = requested.clone();
|
||||
returned.duration_ns /= 2;
|
||||
returned.length = 0;
|
||||
assert!(validate_negotiated_params(&requested, &returned, NegotiationPolicy::Loose).is_ok());
|
||||
let negotiated = assert_negotiates(&requested, &returned, NegotiationPolicy::Loose);
|
||||
assert_eq!(
|
||||
negotiated.restrictions,
|
||||
vec![
|
||||
NegotiationRestriction::DurationReduced {
|
||||
requested_ns: requested.duration_ns,
|
||||
negotiated_ns: returned.duration_ns,
|
||||
},
|
||||
NegotiationRestriction::LengthReduced {
|
||||
requested: requested.length,
|
||||
negotiated: returned.length,
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -37,11 +74,10 @@ fn loose_negotiation_rejects_non_positive_returned_interval() {
|
||||
for interval_ns in [0, -1] {
|
||||
let mut returned = requested.clone();
|
||||
returned.interval_ns = interval_ns;
|
||||
assert!(matches!(
|
||||
validate_negotiated_params(&requested, &returned, NegotiationPolicy::Loose),
|
||||
Err(ClientError::NegotiationRejected { reason })
|
||||
if reason == "interval must be positive"
|
||||
));
|
||||
assert_eq!(
|
||||
rejection_reason(&requested, &returned, NegotiationPolicy::Loose),
|
||||
"interval must be positive"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -51,21 +87,28 @@ fn loose_negotiation_accepts_documented_duration_combinations() {
|
||||
let requested_finite = params_from_config(&finite_config).unwrap();
|
||||
|
||||
let returned_same_finite = requested_finite.clone();
|
||||
assert!(validate_negotiated_params(
|
||||
assert!(assert_negotiates(
|
||||
&requested_finite,
|
||||
&returned_same_finite,
|
||||
NegotiationPolicy::Loose
|
||||
)
|
||||
.is_ok());
|
||||
.restrictions
|
||||
.is_empty());
|
||||
|
||||
let mut returned_shorter_finite = requested_finite.clone();
|
||||
returned_shorter_finite.duration_ns /= 2;
|
||||
assert!(validate_negotiated_params(
|
||||
&requested_finite,
|
||||
&returned_shorter_finite,
|
||||
NegotiationPolicy::Loose
|
||||
)
|
||||
.is_ok());
|
||||
assert_eq!(
|
||||
assert_negotiates(
|
||||
&requested_finite,
|
||||
&returned_shorter_finite,
|
||||
NegotiationPolicy::Loose
|
||||
)
|
||||
.restrictions,
|
||||
vec![NegotiationRestriction::DurationReduced {
|
||||
requested_ns: requested_finite.duration_ns,
|
||||
negotiated_ns: returned_shorter_finite.duration_ns,
|
||||
}]
|
||||
);
|
||||
|
||||
let continuous_config = ClientConfig {
|
||||
duration: None,
|
||||
@@ -74,21 +117,28 @@ fn loose_negotiation_accepts_documented_duration_combinations() {
|
||||
let requested_continuous = params_from_config(&continuous_config).unwrap();
|
||||
|
||||
let returned_continuous = requested_continuous.clone();
|
||||
assert!(validate_negotiated_params(
|
||||
assert!(assert_negotiates(
|
||||
&requested_continuous,
|
||||
&returned_continuous,
|
||||
NegotiationPolicy::Loose
|
||||
)
|
||||
.is_ok());
|
||||
.restrictions
|
||||
.is_empty());
|
||||
|
||||
let mut returned_finite = requested_continuous.clone();
|
||||
returned_finite.duration_ns = 1_000_000_000;
|
||||
assert!(validate_negotiated_params(
|
||||
&requested_continuous,
|
||||
&returned_finite,
|
||||
NegotiationPolicy::Loose
|
||||
)
|
||||
.is_ok());
|
||||
assert_eq!(
|
||||
assert_negotiates(
|
||||
&requested_continuous,
|
||||
&returned_finite,
|
||||
NegotiationPolicy::Loose
|
||||
)
|
||||
.restrictions,
|
||||
vec![NegotiationRestriction::DurationReduced {
|
||||
requested_ns: requested_continuous.duration_ns,
|
||||
negotiated_ns: returned_finite.duration_ns,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -98,11 +148,10 @@ fn loose_negotiation_rejects_finite_request_returned_continuous() {
|
||||
let mut returned = requested.clone();
|
||||
returned.duration_ns = 0;
|
||||
|
||||
assert!(matches!(
|
||||
validate_negotiated_params(&requested, &returned, NegotiationPolicy::Loose),
|
||||
Err(ClientError::NegotiationRejected { reason })
|
||||
if reason == "server returned continuous duration for finite request"
|
||||
));
|
||||
assert_eq!(
|
||||
rejection_reason(&requested, &returned, NegotiationPolicy::Loose),
|
||||
"server returned continuous duration for finite request"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -111,10 +160,10 @@ fn loose_negotiation_rejects_negative_returned_length() {
|
||||
let requested = params_from_config(&config).unwrap();
|
||||
let mut returned = requested.clone();
|
||||
returned.length = -1;
|
||||
assert!(matches!(
|
||||
validate_negotiated_params(&requested, &returned, NegotiationPolicy::Loose),
|
||||
Err(ClientError::NegotiationRejected { reason }) if reason == "length must be non-negative"
|
||||
));
|
||||
assert_eq!(
|
||||
rejection_reason(&requested, &returned, NegotiationPolicy::Loose),
|
||||
"length must be non-negative"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -123,10 +172,10 @@ fn strict_negotiation_rejects_negative_returned_length() {
|
||||
let requested = params_from_config(&config).unwrap();
|
||||
let mut returned = requested.clone();
|
||||
returned.length = -1;
|
||||
assert!(matches!(
|
||||
validate_negotiated_params(&requested, &returned, NegotiationPolicy::Strict),
|
||||
Err(ClientError::NegotiationRejected { reason }) if reason == "length must be non-negative"
|
||||
));
|
||||
assert_eq!(
|
||||
rejection_reason(&requested, &returned, NegotiationPolicy::Strict),
|
||||
"length must be non-negative"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -137,10 +186,9 @@ fn loose_negotiation_rejects_runtime_invalid_returned_dscp() {
|
||||
for dscp in [-1, 64] {
|
||||
let mut returned = requested.clone();
|
||||
returned.dscp = dscp;
|
||||
assert!(matches!(
|
||||
validate_negotiated_params(&requested, &returned, NegotiationPolicy::Loose),
|
||||
Err(ClientError::NegotiationRejected { reason })
|
||||
if reason == "dscp must be in range 0..=63"
|
||||
));
|
||||
assert_eq!(
|
||||
rejection_reason(&requested, &returned, NegotiationPolicy::Loose),
|
||||
"dscp must be in range 0..=63"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -79,13 +79,21 @@ fn no_test_loose_negotiation_accepts_restricted_params() {
|
||||
let mut config = default_test_config(SocketAddr::from(([127, 0, 0, 1], 1)));
|
||||
config.run_mode = RunMode::NoTest;
|
||||
config.negotiation_policy = NegotiationPolicy::Loose;
|
||||
let mut params = params_from_config(&config).unwrap();
|
||||
let requested = params_from_config(&config).unwrap();
|
||||
let mut params = requested.clone();
|
||||
params.duration_ns /= 2;
|
||||
let server = no_test_server(params.clone(), 0);
|
||||
config.server_addr = server.addr.to_string();
|
||||
let mut client = Client::connect(config).unwrap();
|
||||
let negotiated = assert_no_test_completed(client.open().unwrap());
|
||||
assert_eq!(negotiated.params, params);
|
||||
assert_eq!(
|
||||
negotiated.restrictions,
|
||||
vec![crate::NegotiationRestriction::DurationReduced {
|
||||
requested_ns: requested.duration_ns,
|
||||
negotiated_ns: params.duration_ns,
|
||||
}]
|
||||
);
|
||||
server.join();
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user