Skip to content

Commit

Permalink
address more comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@KershawChang
KershawChang committed Oct 2, 2024
1 parent bf22de3 commit b5b887d
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 26 deletions.
2 changes: 1 addition & 1 deletion neqo-transport/src/connection/idle.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl IdleTimeout {
self.start(now) + max(self.timeout / 2, pto)
}

pub fn maybe_keep_alive_timeout(&self, now: Instant, pto: Duration) -> Option<Instant> {
pub fn next_keep_alive(&self, now: Instant, pto: Duration) -> Option<Instant> {
if self.keep_alive_outstanding {
return None;
}
Expand Down
5 changes: 2 additions & 3 deletions neqo-transport/src/connection/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,7 @@ impl Connection {
return timeout.duration_since(now);
}

let mut delays = SmallVec::<[_; 6]>::new();
let mut delays = SmallVec::<[_; 7]>::new();
if let Some(ack_time) = self.acks.ack_time(now) {
qtrace!([self], "Delayed ACK timer {:?}", ack_time);
delays.push(ack_time);
Expand All @@ -1069,8 +1069,7 @@ impl Connection {
delays.push(idle_time);

if self.streams.need_keep_alive() {
if let Some(keep_alive_time) = self.idle_timeout.maybe_keep_alive_timeout(now, pto)
{
if let Some(keep_alive_time) = self.idle_timeout.next_keep_alive(now, pto) {
qtrace!([self], "Keep alive timer {:?}", keep_alive_time);
delays.push(keep_alive_time);
}
Expand Down
48 changes: 26 additions & 22 deletions neqo-transport/src/connection/tests/idle.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ fn default_timeout() -> Duration {
ConnectionParameters::default().get_idle_timeout()
}

fn keep_alive_timeout() -> Duration {
default_timeout() / 2
}

fn test_idle_timeout(client: &mut Connection, server: &mut Connection, timeout: Duration) {
assert!(timeout > Duration::from_secs(1));
connect_force_idle(client, server);
Expand Down Expand Up @@ -414,10 +418,10 @@ fn keep_alive_initiator() {

// Marking the stream for keep-alive changes the idle timeout.
server.stream_keep_alive(stream, true).unwrap();
assert_idle(&mut server, now, default_timeout() / 2);
assert_idle(&mut server, now, keep_alive_timeout());

// Wait that long and the server should send a PING frame.
now += default_timeout() / 2;
now += keep_alive_timeout();
let pings_before = server.stats().frame_tx.ping;
let ping = server.process_output(now).dgram();
assert!(ping.is_some());
Expand All @@ -428,9 +432,9 @@ fn keep_alive_initiator() {
let out = server.process(out.as_ref(), now).dgram();
assert!(client.process(out.as_ref(), now).dgram().is_none());

// Check that there will be next keep-alive ping after default_timeout() / 2.
assert_idle(&mut server, now, default_timeout() / 2);
now += default_timeout() / 2;
// Check that there will be next keep-alive ping after keep_alive_timeout().
assert_idle(&mut server, now, keep_alive_timeout());
now += keep_alive_timeout();
let pings_before2 = server.stats().frame_tx.ping;
let ping = server.process_output(now).dgram();
assert!(ping.is_some());
Expand All @@ -447,10 +451,10 @@ fn keep_alive_lost() {
let mut now = now();

server.stream_keep_alive(stream, true).unwrap();
assert_idle(&mut server, now, default_timeout() / 2);
assert_idle(&mut server, now, keep_alive_timeout());

// Wait that long and the server should send a PING frame.
now += default_timeout() / 2;
now += keep_alive_timeout();
let pings_before = server.stats().frame_tx.ping;
let ping = server.process_output(now).dgram();
assert!(ping.is_some());
Expand All @@ -476,7 +480,7 @@ fn keep_alive_lost() {
// return some small timeout for the recovry although it does not have
// any outstanding data. Therefore we call it after AT_LEAST_PTO.
now += AT_LEAST_PTO;
assert_idle(&mut server, now, default_timeout() / 2 - AT_LEAST_PTO);
assert_idle(&mut server, now, keep_alive_timeout() - AT_LEAST_PTO);
}

/// The other peer can also keep it alive.
Expand All @@ -489,10 +493,10 @@ fn keep_alive_responder() {
let mut now = now();

client.stream_keep_alive(stream, true).unwrap();
assert_idle(&mut client, now, default_timeout() / 2);
assert_idle(&mut client, now, keep_alive_timeout());

// Wait that long and the client should send a PING frame.
now += default_timeout() / 2;
now += keep_alive_timeout();
eprintln!("after wait");
let pings_before = client.stats().frame_tx.ping;
let ping = client.process_output(now).dgram();
Expand All @@ -509,7 +513,7 @@ fn keep_alive_unmark() {
let stream = create_stream_idle(&mut client, &mut server);

client.stream_keep_alive(stream, true).unwrap();
assert_idle(&mut client, now(), default_timeout() / 2);
assert_idle(&mut client, now(), keep_alive_timeout());

client.stream_keep_alive(stream, false).unwrap();
assert_idle(&mut client, now(), default_timeout());
Expand Down Expand Up @@ -539,11 +543,11 @@ fn keep_alive_close() {
let stream = create_stream_idle(&mut client, &mut server);

client.stream_keep_alive(stream, true).unwrap();
assert_idle(&mut client, now(), default_timeout() / 2);
assert_idle(&mut client, now(), keep_alive_timeout());

client.stream_close_send(stream).unwrap();
transfer_force_idle(&mut client, &mut server);
assert_idle(&mut client, now(), default_timeout() / 2);
assert_idle(&mut client, now(), keep_alive_timeout());

server.stream_close_send(stream).unwrap();
transfer_force_idle(&mut server, &mut client);
Expand All @@ -560,19 +564,19 @@ fn keep_alive_reset() {
let stream = create_stream_idle(&mut client, &mut server);

client.stream_keep_alive(stream, true).unwrap();
assert_idle(&mut client, now(), default_timeout() / 2);
assert_idle(&mut client, now(), keep_alive_timeout());

client.stream_close_send(stream).unwrap();
transfer_force_idle(&mut client, &mut server);
assert_idle(&mut client, now(), default_timeout() / 2);
assert_idle(&mut client, now(), keep_alive_timeout());

server.stream_reset_send(stream, 0).unwrap();
transfer_force_idle(&mut server, &mut client);
assert_idle(&mut client, now(), default_timeout());

// The client will fade away from here.
let t = now() + (default_timeout() / 2);
assert_eq!(client.process_output(t).callback(), default_timeout() / 2);
let t = now() + keep_alive_timeout();
assert_eq!(client.process_output(t).callback(), keep_alive_timeout());
let t = now() + default_timeout();
assert_eq!(client.process_output(t), Output::None);
}
Expand All @@ -586,7 +590,7 @@ fn keep_alive_stop_sending() {
let stream = create_stream_idle(&mut client, &mut server);

client.stream_keep_alive(stream, true).unwrap();
assert_idle(&mut client, now(), default_timeout() / 2);
assert_idle(&mut client, now(), keep_alive_timeout());

client.stream_close_send(stream).unwrap();
client.stream_stop_sending(stream, 0).unwrap();
Expand All @@ -610,14 +614,14 @@ fn keep_alive_multiple_stop() {
let stream = create_stream_idle(&mut client, &mut server);

client.stream_keep_alive(stream, true).unwrap();
assert_idle(&mut client, now(), default_timeout() / 2);
assert_idle(&mut client, now(), keep_alive_timeout());

let other = client.stream_create(StreamType::BiDi).unwrap();
client.stream_keep_alive(other, true).unwrap();
assert_idle(&mut client, now(), default_timeout() / 2);
assert_idle(&mut client, now(), keep_alive_timeout());

client.stream_keep_alive(stream, false).unwrap();
assert_idle(&mut client, now(), default_timeout() / 2);
assert_idle(&mut client, now(), keep_alive_timeout());

client.stream_keep_alive(other, false).unwrap();
assert_idle(&mut client, now(), default_timeout());
Expand All @@ -640,7 +644,7 @@ fn keep_alive_large_rtt() {
endpoint.stream_keep_alive(stream, true).unwrap();
let delay = endpoint.process_output(now).callback();
qtrace!([endpoint], "new delay {:?}", delay);
assert!(delay > default_timeout() / 2);
assert!(delay > keep_alive_timeout());
assert!(delay > rtt);
}
}
Expand Down

0 comments on commit b5b887d

Please sign in to comment.