Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(transport): update ack frequency draft from 00 to 10 #2374

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
19 changes: 17 additions & 2 deletions neqo-transport/src/ackrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,14 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Management of the peer's ack rate.
//! Local node (i.e. data sender and ack receiver) managing the remote node's
//! (i.e. data receiver and ack sender) ack rate.
//!
//! Implementation of QUIC Acknowledgment Frequency draft 10.
//!
//! <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html>
//!
//! See [`crate::tracking::RecvdPackets`] for data receiver side.

use std::{cmp::max, time::Duration};

Expand Down Expand Up @@ -47,7 +54,14 @@ impl AckRate {
seqno,
u64::try_from(self.packets + 1).unwrap(),
u64::try_from(self.delay.as_micros()).unwrap(),
0,
// > If no ACK_FREQUENCY frames have been received, the data receiver
// > immediately acknowledges any subsequent packets that are received
// > out-of-order, as specified in Section 13.2 of [QUIC-TRANSPORT],
// > corresponding to a default value of 1. A value of 0 indicates
// > out-of-order packets do not elicit an immediate ACK.
//
// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-4>
1,
])
}

Expand Down Expand Up @@ -134,6 +148,7 @@ impl FlexibleAckRate {
}
}

// TODO: Rename to PeerAckFrequency
#[derive(Debug, Clone)]
pub enum PeerAckDelay {
Fixed(Duration),
Expand Down
27 changes: 24 additions & 3 deletions neqo-transport/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2896,6 +2896,13 @@ impl Connection {
self.stats.borrow_mut().frame_rx.ping += 1;
self.crypto.resend_unacked(space);
// Send an ACK immediately if we might not otherwise do so.
//
// TODO: Ackording to ack frequency draft, only
// IMMEDIATE_ACK_FRAME will send immediate ack, not ping frame.
// Maybe do so anyways? Or only if peer did not advertise
// support via transport parameter?
//
// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-5>
self.acks.immediate_ack(space, now);
}
Frame::Ack {
Expand Down Expand Up @@ -3029,17 +3036,31 @@ impl Connection {
}
Frame::AckFrequency {
seqno,
tolerance,
ack_eliciting_threshold: tolerance,
delay,
ignore_order,
reordering_threshold,
} => {
self.stats.borrow_mut().frame_rx.ack_frequency += 1;
// > The value of this field is in microseconds, unlike the
// > max_ack_delay transport parameter, which is in milliseconds.
//
// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-4>
let delay = Duration::from_micros(delay);
if delay < GRANULARITY {
// > A value smaller than the min_ack_delay advertised by
// > the peer is also invalid. Receipt of an invalid value
// > MUST be treated as a connection error of type
// > PROTOCOL_VIOLATION.
//
// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-4>
return Err(Error::ProtocolViolation);
}
self.acks
.ack_freq(seqno, tolerance - 1, delay, ignore_order);
.ack_freq(seqno, tolerance - 1, delay, reordering_threshold);
}
Frame::ImmediateAck {} => {
self.stats.borrow_mut().frame_rx.immediate_ack += 1;
self.acks.immediate_ack(space, now);
}
Frame::Datagram { data, .. } => {
self.stats.borrow_mut().frame_rx.datagram += 1;
Expand Down
54 changes: 36 additions & 18 deletions neqo-transport/src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,14 @@ pub const FRAME_TYPE_PATH_RESPONSE: FrameType = 0x1b;
pub const FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT: FrameType = 0x1c;
pub const FRAME_TYPE_CONNECTION_CLOSE_APPLICATION: FrameType = 0x1d;
pub const FRAME_TYPE_HANDSHAKE_DONE: FrameType = 0x1e;
// draft-ietf-quic-ack-delay
/// > A data sender signals the conditions under which it wants to receive ACK
/// > frames using an ACK_FREQUENCY frame
///
/// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-4>
pub const FRAME_TYPE_ACK_FREQUENCY: FrameType = 0xaf;
// TODO: Use
/// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-4>
pub const FRAME_IMMEDIATE_ACK: FrameType = 0x1f;
// draft-ietf-quic-datagram
pub const FRAME_TYPE_DATAGRAM: FrameType = 0x30;
pub const FRAME_TYPE_DATAGRAM_WITH_LEN: FrameType = 0x31;
Expand Down Expand Up @@ -189,18 +195,30 @@ pub enum Frame<'a> {
reason_phrase: String,
},
HandshakeDone,
/// > A data sender signals the conditions under which it wants to receive
/// > ACK frames using an ACK_FREQUENCY frame
///
/// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-4>
AckFrequency {
/// The current ACK frequency sequence number.
/// > the sequence number assigned to the ACK_FREQUENCY frame by the
/// > sender so receivers ignore obsolete frames.
seqno: u64,
/// The number of contiguous packets that can be received without
/// acknowledging immediately.
tolerance: u64,
/// The time to delay after receiving the first packet that is
/// not immediately acknowledged.
/// > the maximum number of ack-eliciting packets the recipient of this
/// > frame receives before sending an acknowledgment.
ack_eliciting_threshold: u64,
/// > the value to which the data sender requests the data receiver
/// > update its max_ack_delay
delay: u64,
/// Ignore reordering when deciding to immediately acknowledge.
ignore_order: bool,
/// > the maximum packet reordering before eliciting an immediate ACK, as
/// > specified in Section 6.2. If no ACK_FREQUENCY frames have been
/// > received, the data receiver immediately acknowledges any subsequent
/// > packets that are received out-of-order, as specified in Section 13.2
/// > of [QUIC-TRANSPORT], corresponding to a default value of 1. A value
/// > of 0 indicates out-of-order packets do not elicit an immediate ACK.
reordering_threshold: u64,
},
/// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-5>
ImmediateAck,
Datagram {
data: &'a [u8],
fill: bool,
Expand Down Expand Up @@ -255,6 +273,7 @@ impl<'a> Frame<'a> {
}
Self::HandshakeDone => FRAME_TYPE_HANDSHAKE_DONE,
Self::AckFrequency { .. } => FRAME_TYPE_ACK_FREQUENCY,
Self::ImmediateAck {} => FRAME_IMMEDIATE_ACK,
Self::Datagram { fill, .. } => {
if *fill {
FRAME_TYPE_DATAGRAM
Expand Down Expand Up @@ -622,25 +641,24 @@ impl<'a> Frame<'a> {
})
}
FRAME_TYPE_HANDSHAKE_DONE => Ok(Self::HandshakeDone),
// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-4>
FRAME_TYPE_ACK_FREQUENCY => {
let seqno = dv(dec)?;
let tolerance = dv(dec)?;
if tolerance == 0 {
return Err(Error::FrameEncodingError);
}
let delay = dv(dec)?;
let ignore_order = match d(dec.decode_uint::<u8>())? {
0 => false,
1 => true,
_ => return Err(Error::FrameEncodingError),
};
let reordering_threshold = dv(dec)?;
Ok(Self::AckFrequency {
seqno,
tolerance,
ack_eliciting_threshold: tolerance,
delay,
ignore_order,
reordering_threshold,
})
}
// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-5>
FRAME_IMMEDIATE_ACK => Ok(Self::ImmediateAck {}),
FRAME_TYPE_DATAGRAM | FRAME_TYPE_DATAGRAM_WITH_LEN => {
let fill = (t & DATAGRAM_FRAME_BIT_LEN) == 0;
let data = if fill {
Expand Down Expand Up @@ -981,9 +999,9 @@ mod tests {
fn ack_frequency() {
let f = Frame::AckFrequency {
seqno: 10,
tolerance: 5,
ack_eliciting_threshold: 5,
delay: 2000,
ignore_order: true,
reordering_threshold: 1,
};
just_dec(&f, "40af0a0547d001");
}
Expand Down
2 changes: 1 addition & 1 deletion neqo-transport/src/qlog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ impl From<Frame<'_>> for QuicFrame {
trigger_frame_type: Some(frame_type),
},
Frame::HandshakeDone => Self::HandshakeDone,
Frame::AckFrequency { .. } => Self::Unknown {
Frame::AckFrequency { .. } | Frame::ImmediateAck { .. } => Self::Unknown {
frame_type_value: None,
raw_frame_type: frame.get_type(),
raw: None,
Expand Down
7 changes: 6 additions & 1 deletion neqo-transport/src/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ pub struct FrameStats {
pub new_token: usize,

pub ack_frequency: usize,
pub immediate_ack: usize,
pub datagram: usize,
}

Expand Down Expand Up @@ -92,7 +93,11 @@ impl Debug for FrameStats {
self.path_challenge,
self.path_response,
)?;
writeln!(f, " ack_frequency {}", self.ack_frequency)
writeln!(
f,
" ack_frequency {}, immediate_ack {}",
self.ack_frequency, self.immediate_ack
)
}
}

Expand Down
12 changes: 11 additions & 1 deletion neqo-transport/src/tparams.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,17 @@ pub const INITIAL_SOURCE_CONNECTION_ID: TransportParameterId = 0x0f;
pub const RETRY_SOURCE_CONNECTION_ID: TransportParameterId = 0x10;
pub const VERSION_INFORMATION: TransportParameterId = 0x11;
pub const GREASE_QUIC_BIT: TransportParameterId = 0x2ab2;
pub const MIN_ACK_DELAY: TransportParameterId = 0xff02_de1a;
/// QUIC Ack Frequency draft 10
///
/// > A variable-length integer representing the minimum amount of time, in
/// > microseconds, that the endpoint sending this value is willing to delay an
/// > acknowledgment. This limit could be based on the data receiver's clock or
/// > timer granularity. min_ack_delay is used by the data sender to avoid
/// > requesting too small a value in the Requested Max Ack Delay field of the
/// > ACK_FREQUENCY frame.
///
/// <https://www.ietf.org/archive/id/draft-ietf-quic-ack-frequency-10.html#section-3>
pub const MIN_ACK_DELAY: TransportParameterId = 0xff04_de1b;
pub const MAX_DATAGRAM_FRAME_SIZE: TransportParameterId = 0x0020;

#[derive(Clone, Debug)]
Expand Down
Loading
Loading