Skip to content

Commit

Permalink
Match GSO segment size to the first datagram, not the MTU
Browse files Browse the repository at this point in the history
Reduces the amount of padding required when sending homogeneously
sized non-fragmentable data like application datagrams.
  • Loading branch information
Ralith committed Apr 27, 2024
1 parent 6ca2853 commit e7c47c4
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 10 deletions.
34 changes: 24 additions & 10 deletions quinn-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ impl Connection {
// Position in `buf` of the first byte of the current UDP datagram. When coalescing QUIC
// packets, this can be earlier than the start of the current QUIC packet.
let mut datagram_start = 0;
let mut segment_size = self.path.current_mtu();

// Send PATH_CHALLENGE for a previous path if necessary
if let Some(ref mut prev_path) = self.prev_path {
Expand Down Expand Up @@ -559,9 +560,9 @@ impl Connection {
&& self.peer_supports_ack_frequency();
}

// Reserving capacity can provide more capacity than we asked for.
// However we are not allowed to write more than MTU size. Therefore
// the maximum capacity is tracked separately.
// Reserving capacity can provide more capacity than we asked for. However, we are not
// allowed to write more than `segment_size`. Therefore the maximum capacity is tracked
// separately.
let mut buf_capacity = 0;

let mut coalesce = true;
Expand Down Expand Up @@ -663,17 +664,24 @@ impl Connection {

// Finish current packet
if let Some(mut builder) = builder_storage.take() {
// Pad the packet to make it suitable for sending with GSO
// which will always send the maximum PDU.
builder.pad_to(self.path.current_mtu());
if num_datagrams > 1 {
// Pad the current packet to GSO segment size so it can be included in the
// GSO batch.
builder.pad_to(segment_size);
}

builder.finish_and_track(now, self, sent_frames.take(), buf);

debug_assert_eq!(buf.len(), buf_capacity, "Packet must be padded");
if num_datagrams == 1 {
// Set the segment size for this GSO batch to the size of the first UDP
// datagram in the batch. Larger data that cannot be fragmented
// (e.g. application datagrams) will be included in a future batch.
segment_size = buf.len() as u16;
}
}

// Allocate space for another datagram
buf_capacity += self.path.current_mtu() as usize;
buf_capacity += segment_size as usize;
if buf.capacity() < buf_capacity {
// We reserve the maximum space for sending `max_datagrams` upfront
// to avoid any reallocations if more datagrams have to be appended later on.
Expand All @@ -683,12 +691,18 @@ impl Connection {
// (e.g. purely containing ACKs), modern memory allocators
// (e.g. mimalloc and jemalloc) will pool certain allocation sizes
// and therefore this is still rather efficient.
buf.reserve(max_datagrams * self.path.current_mtu() as usize);
buf.reserve(max_datagrams * segment_size as usize);
}
num_datagrams += 1;
coalesce = true;
pad_datagram = false;
datagram_start = buf.len();

debug_assert_eq!(
datagram_start % usize::from(segment_size),
0,
"datagrams in a GSO batch must be aligned to the segment size"
);
} else {
// We can append/coalesce the next packet into the current
// datagram.
Expand Down Expand Up @@ -946,7 +960,7 @@ impl Connection {
},
segment_size: match num_datagrams {
1 => None,
_ => Some(self.path.current_mtu() as usize),
_ => Some(segment_size as usize),
},
src_ip: self.local_ip,
})
Expand Down
2 changes: 2 additions & 0 deletions quinn-proto/src/connection/packet_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ impl PacketBuilder {
}
}

/// Append the minimum amount of padding such that, after encryption, the packet will occupy at
/// least `min_size` bytes
pub(super) fn pad_to(&mut self, min_size: u16) {
let prev = self.min_size;
self.min_size = self.datagram_start + (min_size as usize) - self.tag_len;
Expand Down

0 comments on commit e7c47c4

Please sign in to comment.