Account for probe size limits before coalescing

When a datagram is a loss probe, it must be at most 1200 bytes. This
limit must be accounted for before we judge whether there's enough
space to coalesce another packet.
This commit is contained in:
Benjamin Saunders
2024-11-16 13:17:24 -08:00
parent 2cd3b14216
commit 83e74fe3ed
2 changed files with 24 additions and 15 deletions
+22 -4
View File
@@ -32,7 +32,7 @@ use crate::{
token::ResetToken,
transport_parameters::TransportParameters,
Dir, Duration, EndpointConfig, Frame, Instant, Side, StreamId, Transmit, TransportError,
TransportErrorCode, VarInt, MAX_CID_SIZE, MAX_STREAM_COUNT, MIN_INITIAL_SIZE,
TransportErrorCode, VarInt, INITIAL_MTU, MAX_CID_SIZE, MAX_STREAM_COUNT, MIN_INITIAL_SIZE,
TIMER_GRANULARITY,
};
@@ -692,13 +692,21 @@ impl Connection {
// waste large amounts of bandwidth. The exact threshold is a bit arbitrary
// and might benefit from further tuning, though there's no universally
// optimal value.
//
// Additionally, if this datagram is a loss probe and `segment_size` is
// larger than `INITIAL_MTU`, then padding it to `segment_size` to continue
// the GSO batch would risk failure to recover from a reduction in path
// MTU. Loss probes are the only packets for which we might grow
// `buf_capacity` by less than `segment_size`.
const MAX_PADDING: usize = 16;
let packet_len_unpadded = cmp::max(builder.min_size, buf.len())
- datagram_start
+ builder.tag_len;
if packet_len_unpadded + MAX_PADDING < segment_size {
if packet_len_unpadded + MAX_PADDING < segment_size
|| datagram_start + segment_size > buf_capacity
{
trace!(
"GSO truncated by demand for {} padding bytes",
"GSO truncated by demand for {} padding bytes or loss probe",
segment_size - packet_len_unpadded
);
builder_storage = Some(builder);
@@ -741,7 +749,17 @@ impl Connection {
}
// Allocate space for another datagram
buf_capacity += segment_size;
let next_datagram_size_limit = match self.spaces[space_id].loss_probes {
0 => segment_size,
_ => {
self.spaces[space_id].loss_probes -= 1;
// Clamp the datagram to at most the minimum MTU to ensure that loss probes
// can get through and enable recovery even if the path MTU has shrank
// unexpectedly.
usize::from(INITIAL_MTU)
}
};
buf_capacity += next_datagram_size_limit;
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.
+2 -11
View File
@@ -1,5 +1,3 @@
use std::cmp;
use bytes::Bytes;
use rand::Rng;
use tracing::{trace, trace_span};
@@ -8,7 +6,7 @@ use super::{spaces::SentPacket, Connection, SentFrames};
use crate::{
frame::{self, Close},
packet::{Header, InitialHeader, LongType, PacketNumber, PartialEncode, SpaceId, FIXED_BIT},
ConnectionId, Instant, TransportError, TransportErrorCode, INITIAL_MTU,
ConnectionId, Instant, TransportError, TransportErrorCode,
};
pub(super) struct PacketBuilder {
@@ -38,7 +36,7 @@ impl PacketBuilder {
space_id: SpaceId,
dst_cid: ConnectionId,
buffer: &mut Vec<u8>,
mut buffer_capacity: usize,
buffer_capacity: usize,
datagram_start: usize,
ack_eliciting: bool,
conn: &mut Connection,
@@ -79,13 +77,6 @@ impl PacketBuilder {
}
let space = &mut conn.spaces[space_id];
if space.loss_probes != 0 {
space.loss_probes -= 1;
// Clamp the packet size to at most the minimum MTU to ensure that loss probes can get
// through and enable recovery even if the path MTU has shrank unexpectedly.
buffer_capacity = cmp::min(buffer_capacity, datagram_start + usize::from(INITIAL_MTU));
}
let exact_number = match space_id {
SpaceId::Data => conn.packet_number_filter.allocate(&mut conn.rng, space),
_ => space.get_tx_number(),