gsadmin/noq
1
0
Fork 0
mirror of https://github.com/n0-computer/noq.git synced 2026-09-25 04:35:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

Move tracking of outgoing ACKs into a dedicated PendingAcks struct

Browse Source 
This will make it easier to add additional logic for  ACK suppression.
This commit is contained in:
Matthias Einwag
2021-06-11 19:53:37 +00:00
committed by Dirkjan Ochtman
parent 9e9d1c09b1
commit f51a39f71d
2 changed files with 72 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
quinn-proto/src/connection/mod.rs
+12 -18
View File
@@ -725,13 +725,9 @@ where
);
pad_datagram |= sent.requires_padding;
// If we sent any acks, don't immediately resend them. Setting this even if ack_only is
// false needlessly prevents us from ACKing the next packet if it's ACK-only, but saves
// the need for subtler logic to avoid double-transmitting acks all the time.
// This reset needs to happen before we check whether more data
// is available in this space - because otherwise it would return
// `true` purely due to the ACKs.
self.spaces[space_id].permit_ack_only &= sent.acks.is_empty();
if !sent.acks.is_empty() {
self.spaces[space_id].pending_acks.acks_sent();
}
// Keep information about the packet around until it gets finalized
sent_frames = Some(sent);
@@ -1456,9 +1452,6 @@ where
}
let space = &mut self.spaces[space_id];
space.pending_acks.insert_one(packet);
if space.pending_acks.len() > MAX_ACK_BLOCKS {
space.pending_acks.pop_min();
}
if packet >= space.rx_packet {
space.rx_packet = packet;
// Update outgoing spin bit, inverting iff we're the client
@@ -2166,7 +2159,9 @@ where
match frame {
Frame::Ack(_) | Frame::Padding | Frame::Close(Close::Connection(_)) => {}
_ => {
self.spaces[packet.header.space()].permit_ack_only = true;
self.spaces[packet.header.space()]
.pending_acks
.ack_eliciting_frame_received();
}
}
// Process frames
@@ -2252,7 +2247,9 @@ where
match frame {
Frame::Ack(_) | Frame::Padding | Frame::Close(_) => {}
_ => {
self.spaces[SpaceId::Data].permit_ack_only = true;
self.spaces[SpaceId::Data]
.pending_acks
.ack_eliciting_frame_received();
}
}
// Check whether this could be a probing packet
@@ -2603,7 +2600,7 @@ where
// ACK
// 0-RTT packets must never carry acks (which would have to be of handshake packets)
if !space.pending_acks.is_empty() {
if !space.pending_acks.ranges().is_empty() {
debug_assert!(space.crypto.is_some(), "tried to send ACK in 0-RTT");
trace!("ACK");
let ecn = if self.receiving_ecn {
@@ -2611,8 +2608,8 @@ where
} else {
None
};
frame::Ack::encode(0, &space.pending_acks, ecn, buf);
sent.acks = space.pending_acks.clone();
sent.acks = space.pending_acks.ranges().clone();
frame::Ack::encode(0, &sent.acks, ecn, buf);
self.stats.frame_tx.acks += 1;
}
@@ -3121,9 +3118,6 @@ mod state {
}
}
/// Ensures we can always fit all our ACKs in a single minimum-MTU packet with room to spare
const MAX_ACK_BLOCKS: usize = 64;
struct PrevCrypto<K>
where
K: crypto::PacketKey,
quinn-proto/src/connection/spaces.rs
+60 -6
View File
@@ -26,9 +26,7 @@ where
/// Data to send
pub(crate) pending: Retransmits,
/// Packet numbers to acknowledge
pub(crate) pending_acks: ArrayRangeSet,
/// Set iff we have received a non-ack frame since the last ack-only packet we sent
pub(crate) permit_ack_only: bool,
pub(crate) pending_acks: PendingAcks,
/// The packet number of the next packet that will be sent, if any.
pub(crate) next_packet_number: u64,
@@ -79,8 +77,7 @@ where
rx_packet: 0,
pending: Retransmits::default(),
pending_acks: ArrayRangeSet::new(),
permit_ack_only: false,
pending_acks: PendingAcks::default(),
next_packet_number: 0,
largest_acked_packet: None,
@@ -149,7 +146,7 @@ where
}
pub(crate) fn can_send(&self) -> SendableFrames {
let acks = self.permit_ack_only && !self.pending_acks.is_empty();
let acks = self.pending_acks.can_send();
let other = !self.pending.is_empty() || self.ping_pending;
SendableFrames { acks, other }
@@ -436,6 +433,63 @@ impl SendableFrames {
}
}
#[derive(Debug, Default)]
pub(crate) struct PendingAcks {
permit_ack_only: bool,
ranges: ArrayRangeSet,
}
impl PendingAcks {
/// Whether any ACK frames can be sent
pub fn can_send(&self) -> bool {
self.permit_ack_only && !self.ranges.is_empty()
}
/// Should be called whenever an ACK eliciting frame was received
///
/// This requires sending new outgoing ACKs
pub fn ack_eliciting_frame_received(&mut self) {
self.permit_ack_only = true;
}
/// Should be called whenever ACKs have been sent
///
/// This will suppress sending further ACKs until additional ACK eliciting frames arrive
pub fn acks_sent(&mut self) {
// If we sent any acks, don't immediately resend them. Setting this even if ack_only is
// false needlessly prevents us from ACKing the next packet if it's ACK-only, but saves
// the need for subtler logic to avoid double-transmitting acks all the time.
// This reset needs to happen before we check whether more data
// is available in this space - because otherwise it would return
// `true` purely due to the ACKs
self.permit_ack_only = false;
}
/// Insert one packet that needs to be acknowledged
pub fn insert_one(&mut self, packet: u64) {
self.ranges.insert_one(packet);
if self.ranges.len() > MAX_ACK_BLOCKS {
self.ranges.pop_min();
}
}
/// Removes the given ACKs from the set of pending ACKs
pub fn subtract(&mut self, acks: &ArrayRangeSet) {
self.ranges.subtract(acks);
if self.ranges.is_empty() {
self.permit_ack_only = false;
}
}
/// Returns the set of currently pending ACK ranges
pub fn ranges(&self) -> &ArrayRangeSet {
&self.ranges
}
}
/// Ensures we can always fit all our ACKs in a single minimum-MTU packet with room to spare
const MAX_ACK_BLOCKS: usize = 64;
#[cfg(test)]
mod test {
use super::*;