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

Implement acknowledgement frequency #1553

Merged
merged 4 commits into from
Jul 24, 2023
Merged
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
91 changes: 91 additions & 0 deletions quinn-proto/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub struct TransportConfig {
pub(crate) initial_mtu: u16,
pub(crate) min_mtu: u16,
pub(crate) mtu_discovery_config: Option<MtuDiscoveryConfig>,
pub(crate) ack_frequency_config: Option<AckFrequencyConfig>,

pub(crate) persistent_congestion_threshold: u32,
pub(crate) keep_alive_interval: Option<Duration>,
Expand Down Expand Up @@ -218,6 +219,17 @@ impl TransportConfig {
self
}

/// Specifies the ACK frequency config (see [`AckFrequencyConfig`] for details)
///
/// The provided configuration will be ignored if the peer does not support the acknowledgement
/// frequency QUIC extension.
///
/// Defaults to `None`, which disables the ACK frequency feature.
pub fn ack_frequency_config(&mut self, value: Option<AckFrequencyConfig>) -> &mut Self {
self.ack_frequency_config = value;
self
}

/// Number of consecutive PTOs after which network is considered to be experiencing persistent congestion.
pub fn persistent_congestion_threshold(&mut self, value: u32) -> &mut Self {
self.persistent_congestion_threshold = value;
Expand Down Expand Up @@ -316,6 +328,7 @@ impl Default for TransportConfig {
initial_mtu: INITIAL_MTU,
min_mtu: INITIAL_MTU,
mtu_discovery_config: Some(MtuDiscoveryConfig::default()),
ack_frequency_config: None,

persistent_congestion_threshold: 3,
keep_alive_interval: None,
Expand Down Expand Up @@ -365,6 +378,84 @@ impl fmt::Debug for TransportConfig {
}
}

/// Parameters for controlling the peer's acknowledgement frequency
///
/// The parameters provided in this config will be sent to the peer at the beginning of the
/// connection, so it can take them into account when sending acknowledgements (see each parameter's
/// description for details on how it influences acknowledgement frequency).
///
/// Quinn's implementation follows the fourth draft of the
/// [QUIC Acknowledgement Frequency extension](https://datatracker.ietf.org/doc/html/draft-ietf-quic-ack-frequency-04).
Ralith marked this conversation as resolved.
Show resolved Hide resolved
/// The defaults produce behavior slightly different than the behavior without this extension,
/// because they change the way reordered packets are handled (see
/// [`AckFrequencyConfig::reordering_threshold`] for details).
#[derive(Clone, Debug)]
pub struct AckFrequencyConfig {
pub(crate) ack_eliciting_threshold: VarInt,
pub(crate) max_ack_delay: Option<Duration>,
pub(crate) reordering_threshold: VarInt,
}

impl AckFrequencyConfig {
/// The ack-eliciting threshold we will request the peer to use
///
/// This threshold represents the number of ack-eliciting packets an endpoint may receive
/// without immediately sending an ACK.
///
/// The remote peer should send at least one ACK frame when more than this number of
/// ack-eliciting packets have been received. A value of 0 results in a receiver immediately
/// acknowledging every ack-eliciting packet.
///
/// Defaults to 1, which sends ACK frames for every other ack-eliciting packet.
pub fn ack_eliciting_threshold(&mut self, value: VarInt) -> &mut Self {
self.ack_eliciting_threshold = value;
self
}

/// The `max_ack_delay` we will request the peer to use
///
/// This parameter represents the maximum amount of time that an endpoint waits before sending
/// an ACK when the ack-eliciting threshold hasn't been reached.
///
/// The provided `max_ack_delay` will be clamped to be at least the peer's `min_ack_delay`
/// transport parameter.
///
/// Defaults to `None`, in which case the peer's original `max_ack_delay` will be used, as
/// obtained from its transport parameters.
pub fn max_ack_delay(&mut self, value: Option<Duration>) -> &mut Self {
self.max_ack_delay = value;
self
}

/// The reordering threshold we will request the peer to use
///
/// This threshold represents the amount of out-of-order packets that will trigger an endpoint
/// to send an ACK, without waiting for `ack_eliciting_threshold` to be exceeded or for
/// `max_ack_delay` to be elapsed.
///
/// A value of 0 indicates out-of-order packets do not elicit an immediate ACK. A value of 1
/// immediately acknowledges any packets that are received out of order (this is also the
/// behavior when the extension is disabled).
///
/// It is recommended to set this value to [`TransportConfig::packet_threshold`] minus one.
/// Since the default value for [`TransportConfig::packet_threshold`] is 3, this value defaults
/// to 2.
pub fn reordering_threshold(&mut self, value: VarInt) -> &mut Self {
self.reordering_threshold = value;
self
}
}

impl Default for AckFrequencyConfig {
fn default() -> Self {
Self {
ack_eliciting_threshold: VarInt(1),
max_ack_delay: None,
reordering_threshold: VarInt(2),
}
}
}

/// Parameters governing MTU discovery.
///
/// # The why of MTU discovery
Expand Down
124 changes: 124 additions & 0 deletions quinn-proto/src/connection/ack_frequency.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
use crate::connection::spaces::PendingAcks;
use crate::frame::AckFrequency;
use crate::{AckFrequencyConfig, TransportError, VarInt, TIMER_GRANULARITY};
use std::time::Duration;

/// State associated to ACK frequency
pub(super) struct AckFrequencyState {
//
// Sending ACK_FREQUENCY frames
//
in_flight_ack_frequency_frame: Option<(u64, Duration)>,
next_outgoing_sequence_number: VarInt,
pub(super) peer_max_ack_delay: Duration,

//
// Receiving ACK_FREQUENCY frames
//
last_ack_frequency_frame: Option<u64>,
pub(super) max_ack_delay: Duration,
}

impl AckFrequencyState {
pub(super) fn new(default_max_ack_delay: Duration) -> Self {
Self {
in_flight_ack_frequency_frame: None,
next_outgoing_sequence_number: VarInt(0),
peer_max_ack_delay: default_max_ack_delay,

last_ack_frequency_frame: None,
max_ack_delay: default_max_ack_delay,
}
}

/// Returns the `max_ack_delay` that should be requested of the peer when sending an
/// ACK_FREQUENCY frame
pub(super) fn candidate_max_ack_delay(&self, config: &AckFrequencyConfig) -> Duration {
// Use the peer's max_ack_delay if no custom max_ack_delay was provided in the config
config.max_ack_delay.unwrap_or(self.peer_max_ack_delay)
}

/// Returns the `max_ack_delay` for the purposes of calculating the PTO
///
/// This `max_ack_delay` is defined as the maximum of the peer's current `max_ack_delay` and all
/// in-flight `max_ack_delay`s (i.e. proposed values that haven't been acknowledged yet, but
/// might be already in use by the peer).
pub(super) fn max_ack_delay_for_pto(&self) -> Duration {
// Note: we have at most one in-flight ACK_FREQUENCY frame
if let Some((_, max_ack_delay)) = self.in_flight_ack_frequency_frame {
self.peer_max_ack_delay.max(max_ack_delay)
} else {
self.peer_max_ack_delay
}
}

/// Returns the next sequence number for an ACK_FREQUENCY frame
pub(super) fn next_sequence_number(&mut self) -> VarInt {
assert!(self.next_outgoing_sequence_number <= VarInt::MAX);

let seq = self.next_outgoing_sequence_number;
self.next_outgoing_sequence_number.0 += 1;
seq
}

/// Returns true if we should send an ACK_FREQUENCY frame
pub(super) fn should_send_ack_frequency(&self) -> bool {
// Currently, we only allow sending a single ACK_FREQUENCY frame. There is no need to send
// more, because none of the sent values needs to be updated in the course of the connection
self.next_outgoing_sequence_number.0 == 0
}

/// Notifies the [`AckFrequencyState`] that a packet containing an ACK_FREQUENCY frame was sent
pub(super) fn ack_frequency_sent(&mut self, pn: u64, requested_max_ack_delay: Duration) {
self.in_flight_ack_frequency_frame = Some((pn, requested_max_ack_delay));
}

/// Notifies the [`AckFrequencyState`] that a packet has been ACKed
pub(super) fn on_acked(&mut self, pn: u64) {
match self.in_flight_ack_frequency_frame {
Some((number, requested_max_ack_delay)) if number == pn => {
self.in_flight_ack_frequency_frame = None;
self.peer_max_ack_delay = requested_max_ack_delay;
}
_ => {}
}
}

/// Notifies the [`AckFrequencyState`] that an ACK_FREQUENCY frame was received
///
/// Updates the endpoint's params according to the payload of the ACK_FREQUENCY frame, or
/// returns an error in case the requested `max_ack_delay` is invalid.
///
/// Returns `true` if the frame was processed and `false` if it was ignored because of being
/// stale.
pub(super) fn ack_frequency_received(
&mut self,
frame: &AckFrequency,
pending_acks: &mut PendingAcks,
) -> Result<bool, TransportError> {
if self
.last_ack_frequency_frame
.map_or(false, |highest_sequence_nr| {
frame.sequence.into_inner() <= highest_sequence_nr
})
{
return Ok(false);
}

self.last_ack_frequency_frame = Some(frame.sequence.into_inner());

// Update max_ack_delay
let max_ack_delay = Duration::from_micros(frame.request_max_ack_delay.into_inner());
if max_ack_delay < TIMER_GRANULARITY {
return Err(TransportError::PROTOCOL_VIOLATION(
"Requested Max Ack Delay in ACK_FREQUENCY frame is less than min_ack_delay",
));
}
self.max_ack_delay = max_ack_delay;

// Update the rest of the params
pending_acks.set_ack_frequency_params(frame);

Ok(true)
}
}
Loading