mirror of
https://github.com/n0-computer/noq.git
synced 2026-09-21 18:53:27 +00:00
Fix dropping oversized unblocks datagram senders
When dropping oversized datagrams after an MTU change, notify callers of `send_datagram_wait` that are waiting for space in the send buffer. Tracking issue: https://github.com/quinn-rs/quinn/issues/2456
This commit is contained in:
committed by
Benjamin Saunders
parent
ab9da17120
commit
c09ff21012
@@ -142,9 +142,12 @@ impl DatagramState {
|
||||
|
||||
/// Discard outgoing datagrams with a payload larger than `max_payload` bytes
|
||||
///
|
||||
/// Returns whether any datagrams were dropped.
|
||||
///
|
||||
/// Used to ensure that reductions in MTU don't get us stuck in a state where we have a datagram
|
||||
/// queued but can't send it.
|
||||
pub(super) fn drop_oversized(&mut self, max_payload: usize) {
|
||||
pub(super) fn drop_oversized(&mut self, max_payload: usize) -> bool {
|
||||
let mut dropped_any = false;
|
||||
self.outgoing.retain(|datagram| {
|
||||
let result = datagram.data.len() < max_payload;
|
||||
if !result {
|
||||
@@ -154,9 +157,11 @@ impl DatagramState {
|
||||
max_payload
|
||||
);
|
||||
self.outgoing_total -= datagram.data.len();
|
||||
dropped_any = true;
|
||||
}
|
||||
result
|
||||
});
|
||||
dropped_any
|
||||
}
|
||||
|
||||
/// Attempt to write a datagram frame into `buf`, consuming it from `self.outgoing`
|
||||
|
||||
@@ -1821,7 +1821,12 @@ impl Connection {
|
||||
.congestion
|
||||
.on_mtu_update(self.path.mtud.current_mtu());
|
||||
if let Some(max_datagram_size) = self.datagrams().max_size() {
|
||||
self.datagrams.drop_oversized(max_datagram_size);
|
||||
if self.datagrams.drop_oversized(max_datagram_size)
|
||||
&& self.datagrams.send_blocked
|
||||
{
|
||||
self.datagrams.send_blocked = false;
|
||||
self.events.push_back(Event::DatagramsUnblocked);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -3379,6 +3379,87 @@ fn voluntary_ack_with_large_datagrams() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Verify that dropping oversized datagrams will trigger a DatagramsUnblocked event.
|
||||
#[test]
|
||||
fn oversized_datagrams_trigger_unblock() {
|
||||
let _guard = subscribe();
|
||||
let mut pair = Pair::default();
|
||||
// Start the connection with a large MTU.
|
||||
const INITIAL_MTU: usize = 1300;
|
||||
pair.mtu = INITIAL_MTU;
|
||||
|
||||
let mut client_config = client_config();
|
||||
let mut transport_config = TransportConfig::default();
|
||||
let send_buffer_size = transport_config.datagram_send_buffer_size;
|
||||
transport_config.initial_mtu(INITIAL_MTU as u16);
|
||||
client_config.transport_config(transport_config.into());
|
||||
|
||||
let (client_ch, _) = pair.connect_with(client_config);
|
||||
|
||||
// Send datagrams until the send buffer is full.
|
||||
let max_size = pair.client_datagrams(client_ch).max_size().unwrap();
|
||||
let data = vec![0; max_size];
|
||||
loop {
|
||||
match pair
|
||||
.client_datagrams(client_ch)
|
||||
.send(data.clone().into(), false)
|
||||
{
|
||||
Ok(_) => {}
|
||||
Err(SendDatagramError::Blocked(_)) => {
|
||||
break;
|
||||
}
|
||||
Err(e) => panic!("unexpected error: {e}"),
|
||||
}
|
||||
}
|
||||
// Set the MTU to a smaller value so the queued datagrams cannot be sent.
|
||||
pair.mtu = 1200;
|
||||
|
||||
// Drive the pair until black hole detection kicks in and the path MTU is adjusted.
|
||||
while pair.step() {
|
||||
let err = loop {
|
||||
if let Err(e) = pair
|
||||
.client_datagrams(client_ch)
|
||||
.send(data.clone().into(), false)
|
||||
{
|
||||
break e;
|
||||
}
|
||||
};
|
||||
match err {
|
||||
SendDatagramError::Blocked(_) => {
|
||||
// continue with the next step but drain the DatagramsUnblocked events
|
||||
// emitted datagrams were sent out.
|
||||
while let Some(event) = pair.client_conn_mut(client_ch).poll() {
|
||||
tracing::info!("ignoring connection event: {event:?}");
|
||||
}
|
||||
}
|
||||
SendDatagramError::TooLarge => {
|
||||
// mtu adjusted, break the loop
|
||||
break;
|
||||
}
|
||||
_ => panic!("unexpected error: {err}"),
|
||||
}
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
pair.client_conn_mut(client_ch)
|
||||
.stats()
|
||||
.path
|
||||
.black_holes_detected,
|
||||
1,
|
||||
"expected a black hole to have been detected",
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
pair.client_datagrams(client_ch).send_buffer_space(),
|
||||
send_buffer_size,
|
||||
"expected the send buffer to be empty after too large datagrams were dropped",
|
||||
);
|
||||
match pair.client_conn_mut(client_ch).poll() {
|
||||
Some(Event::DatagramsUnblocked) => {}
|
||||
_ => panic!("expected DatagramsUnblocked event"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reject_short_idcid() {
|
||||
let _guard = subscribe();
|
||||
|
||||
Reference in New Issue
Block a user