gsadmin/noq
1
0
Fork 0
mirror of https://github.com/n0-computer/noq.git synced 2026-09-22 11:13:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

Move simple field access to accessor functions

Browse Source 
This helps encapsulation by ensuring that no outside users can mutate
these fields.  Reducing the amount of logic that needs to be reasoned
about.
This commit is contained in:
Floris Bruynooghe
2025-04-10 16:07:16 +02:00
parent 147e602329
commit bf3e3b2b0a
3 changed files with 85 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
quinn-proto/src/connection/mod.rs
+43 -41
View File
@@ -578,7 +578,7 @@ impl Connection {
.for_path(path_id)
.peek_tx_number();
let frame_space_1rtt = buf
.segment_size
.segment_size()
.saturating_sub(self.predict_1rtt_overhead(pn, path_id));
// Is there data or a close message to send in this space?
@@ -617,11 +617,11 @@ impl Connection {
// We are NOT coalescing (the default is we are, so this was turned off in an
// earlier iteration) OR there is not enough space for another *packet* in this
// datagram (buf_capacity - buf_end == unused space in datagram).
if !coalesce || buf.buf_capacity - buf_end < MIN_PACKET_SPACE + tag_len {
if !coalesce || buf.datagram_max_offset() - buf_end < MIN_PACKET_SPACE + tag_len {
// We need to send 1 more datagram and extend the buffer for that.
// Is 1 more datagram allowed?
if buf.num_datagrams >= buf.max_datagrams {
if buf.num_datagrams() >= buf.max_datagrams() {
// No more datagrams allowed
break;
}
@@ -632,10 +632,9 @@ impl Connection {
// for starting another datagram. If there is any anti-amplification
// budget left, we always allow a full MTU to be sent
// (see https://github.com/quinn-rs/quinn/issues/1082)
if self
.path_data(path_id)
.anti_amplification_blocked((buf.segment_size * buf.num_datagrams) as u64 + 1)
{
if self.path_data(path_id).anti_amplification_blocked(
(buf.segment_size() * buf.num_datagrams()) as u64 + 1,
) {
trace!("blocked by anti-amplification");
break;
}
@@ -645,13 +644,13 @@ impl Connection {
if ack_eliciting && self.spaces[space_id].for_path(path_id).loss_probes == 0 {
// Assume the current packet will get padded to fill the segment
let untracked_bytes = if let Some(builder) = &builder_storage {
buf.buf_capacity - builder.partial_encode.start
buf.datagram_max_offset() - builder.partial_encode.start
} else {
0
} as u64;
debug_assert!(untracked_bytes <= buf.segment_size as u64);
debug_assert!(untracked_bytes <= buf.segment_size() as u64);
let bytes_to_send = buf.segment_size as u64 + untracked_bytes;
let bytes_to_send = buf.segment_size() as u64 + untracked_bytes;
// TODO(@divma): move to method of path
if self.path_data(path_id).in_flight.bytes + bytes_to_send
>= self.path_data(path_id).congestion.window()
@@ -684,7 +683,7 @@ impl Connection {
builder.pad_to(MIN_INITIAL_SIZE);
}
if buf.num_datagrams > 1 {
if buf.num_datagrams() > 1 {
// If too many padding bytes would be required to continue the GSO batch
// after this packet, end the GSO batch here. Ensures that fixed-size frames
// with heterogeneous sizes (e.g. application datagrams) won't inadvertently
@@ -699,14 +698,15 @@ impl Connection {
// `buf_capacity` by less than `segment_size`.
const MAX_PADDING: usize = 16;
let packet_len_unpadded = cmp::max(builder.min_size, buf.len())
- buf.datagram_start
- buf.datagram_start_offset()
+ builder.tag_len;
if packet_len_unpadded + MAX_PADDING < buf.segment_size
|| buf.datagram_start + buf.segment_size > buf.buf_capacity
if packet_len_unpadded + MAX_PADDING < buf.segment_size()
|| buf.datagram_start_offset() + buf.segment_size()
> buf.datagram_max_offset()
{
trace!(
"GSO truncated by demand for {} padding bytes or loss probe",
buf.segment_size - packet_len_unpadded
buf.segment_size() - packet_len_unpadded
);
builder_storage = Some(builder);
break;
@@ -714,27 +714,29 @@ impl Connection {
// Pad the current datagram to GSO segment size so it can be included in the
// GSO batch.
builder.pad_to(buf.segment_size as u16);
builder.pad_to(buf.segment_size() as u16);
}
builder.finish_and_track(now, self, path_id, sent_frames.take(), buf.buf);
if buf.num_datagrams == 1 && space_id == SpaceId::Data {
// Now that we know the size of the first datagram, check whether
// the data we planned to send will fit in the next segment. If
// not, bails out and leave it for the next GSO batch. We can't
// easily compute the right segment size before the original call to
// `space_can_send`, because at that time we haven't determined
// whether we're going to coalesce with the first datagram or
// potentially pad it to `MIN_INITIAL_SIZE`.
if buf.num_datagrams() == 1 {
buf.clip_datagram_size();
if space_id == SpaceId::Data {
// Now that we know the size of the first datagram, check
// whether the data we planned to send will fit in the next
// segment. If not, bails out and leave it for the next GSO
// batch. We can't easily compute the right segment size before
// the original call to `space_can_send`, because at that time
// we haven't determined whether we're going to coalesce with
// the first datagram or potentially pad it to
// `MIN_INITIAL_SIZE`.
let frame_space_1rtt = buf
.segment_size
.saturating_sub(self.predict_1rtt_overhead(pn, path_id));
if self.space_can_send(space_id, frame_space_1rtt).is_empty() {
break;
let frame_space_1rtt = buf
.segment_size()
.saturating_sub(self.predict_1rtt_overhead(pn, path_id));
if self.space_can_send(space_id, frame_space_1rtt).is_empty() {
break;
}
}
}
}
@@ -749,7 +751,7 @@ impl Connection {
// unexpectedly.
buf.start_new_datagram_with_size(std::cmp::min(
usize::from(INITIAL_MTU),
buf.segment_size,
buf.segment_size(),
));
}
};
@@ -764,7 +766,7 @@ impl Connection {
}
}
debug_assert!(buf.buf_capacity - buf.len() >= MIN_PACKET_SPACE);
debug_assert!(buf.datagram_max_offset() - buf.len() >= MIN_PACKET_SPACE);
//
// From here on, we've determined that a packet will definitely be sent.
@@ -877,7 +879,7 @@ impl Connection {
// Send an off-path PATH_RESPONSE. Prioritized over on-path data to ensure that path
// validation can occur while the link is saturated.
if space_id == SpaceId::Data && buf.num_datagrams == 1 {
if space_id == SpaceId::Data && buf.num_datagrams() == 1 {
let remote = self.path_data(path_id).remote;
if let Some((token, remote)) = self.path_responses.pop_off_path(remote) {
// `unwrap` guaranteed to succeed because `builder_storage` was populated just
@@ -928,7 +930,7 @@ impl Connection {
!(sent.is_ack_only(&self.streams)
&& !can_send.acks
&& can_send.other
&& (buf.buf_capacity - builder.datagram_start)
&& (buf.datagram_max_offset() - builder.datagram_start)
== self.path_data(path_id).current_mtu() as usize
&& self.datagrams.outgoing.is_empty()),
"SendableFrames was {can_send:?}, but only ACKs have been written"
@@ -972,10 +974,10 @@ impl Connection {
.mtud
.poll_transmit(now, next_pn)?;
debug_assert_eq!(buf.num_datagrams, 0);
debug_assert_eq!(buf.num_datagrams(), 0);
buf.start_new_datagram_with_size(probe_size as usize);
debug_assert_eq!(buf.datagram_start, 0);
debug_assert_eq!(buf.datagram_start_offset(), 0);
// TODO(flub): I'm not particularly happy about this unwrap. But let's leave it
// for now until more stuff is settled. We probably should check earlier on
// in poll_transmit that we have a valid CID to use.
@@ -1018,13 +1020,13 @@ impl Connection {
trace!(
"sending {} bytes in {} datagrams",
buf.len(),
buf.num_datagrams
buf.num_datagrams()
);
self.path_data_mut(path_id).inc_total_sent(buf.len() as u64);
self.stats
.udp_tx
.on_sent(buf.num_datagrams as u64, buf.len());
.on_sent(buf.num_datagrams() as u64, buf.len());
Some(Transmit {
destination: self.path_data(path_id).remote,
@@ -1034,9 +1036,9 @@ impl Connection {
} else {
None
},
segment_size: match buf.num_datagrams {
segment_size: match buf.num_datagrams() {
1 => None,
_ => Some(buf.segment_size),
_ => Some(buf.segment_size()),
},
src_ip: self.local_ip,
})
@@ -1070,7 +1072,7 @@ impl Connection {
// sent once, immediately after migration, when the CID is known to be valid. Even
// if a post-migration packet caused the CID to be retired, it's fair to pretend
// this is sent first.
debug_assert_eq!(buf.datagram_start, 0);
debug_assert_eq!(buf.datagram_start_offset(), 0);
let mut builder =
PacketBuilder::new(now, SpaceId::Data, path_id, *prev_cid, buf, false, self)?;
trace!("validating previous path with PATH_CHALLENGE {:08x}", token);
quinn-proto/src/connection/packet_builder.rs
+2 -2
View File
@@ -150,11 +150,11 @@ impl PacketBuilder {
buffer.len() + (sample_size + 4).saturating_sub(number.len() + tag_len),
partial_encode.start + dst_cid.len() + 6,
);
let max_size = buffer.buf_capacity - tag_len;
let max_size = buffer.datagram_max_offset() - tag_len;
debug_assert!(max_size >= min_size);
Some(Self {
datagram_start: buffer.datagram_start,
datagram_start: buffer.datagram_start_offset(),
space: space_id,
path: path_id,
partial_encode,
quinn-proto/src/connection/transmit_buf.rs
+40 -4
View File
@@ -30,14 +30,14 @@ pub(super) struct TransmitBuf<'a> {
///
/// Note that when coalescing packets this might be before the start of the current
/// packet.
pub(super) datagram_start: usize,
datagram_start: usize,
/// The maximum offset allowed to be used for the current datagram in the buffer
///
/// The first and last datagram in a batch are allowed to be smaller then the maximum
/// size. All datagrams in between need to be exactly this size.
pub(super) buf_capacity: usize,
buf_capacity: usize,
/// The maximum number of datagrams allowed to write into [`TransmitBuf::buf`]
pub(super) max_datagrams: usize,
max_datagrams: usize,
/// The number of datagrams already (partially) written into the buffer
///
/// Incremented by a call to [`TransmitBuf::start_new_datagram`].
@@ -50,7 +50,7 @@ pub(super) struct TransmitBuf<'a> {
/// For the first datagram this is set to the maximum size a datagram is allowed to be:
/// the current path MTU. After the first datagram is finished this is reduced to the
/// size of the first datagram and can no longer change.
pub(super) segment_size: usize,
segment_size: usize,
}
impl<'a> TransmitBuf<'a> {
@@ -140,6 +140,42 @@ impl<'a> TransmitBuf<'a> {
self.buf_capacity = self.buf.len();
}
/// Returns the GSO segment size
///
/// This is also the maximum size datagrams are allowed to be. The first and last
/// datagram in a batch are allowed to be smaller however. After the first datagram the
/// segment size is clipped to the size of the first datagram.
pub(super) fn segment_size(&self) -> usize {
self.segment_size
}
/// Returns the number of datagrams written into the buffer
///
/// The last datagram is not necessarily finished yet.
pub(super) fn num_datagrams(&self) -> usize {
self.num_datagrams
}
/// Returns the maximum number of datagrams allowed to be written into the buffer
pub(super) fn max_datagrams(&self) -> usize {
self.max_datagrams
}
/// Returns the start offset of the current datagram in the buffer
///
/// In other words, this offset contains the first byte of the current datagram.
pub(super) fn datagram_start_offset(&self) -> usize {
self.datagram_start
}
/// Returns the maximum offset in the buffer allowed for the current datagram
///
/// The first and last datagram in a batch are allowed to be smaller then the maximum
/// size. All datagrams in between need to be exactly this size.
pub(super) fn datagram_max_offset(&self) -> usize {
self.buf_capacity
}
/// Returns `true` if the buffer did not have anything written into it
pub(super) fn is_empty(&self) -> bool {
self.len() == 0