fix(s2n-quic-dc): clamp send quantum to max syscall size (#2549)

Author: camshaft

dc/s2n-quic-dc/src/msg/segment.rs

@@ -5,19 +5,75 @@ use crate::stream::TransportFeatures;
 use arrayvec::ArrayVec;
 use core::ops::Deref;
 use s2n_quic_core::{ensure, inet::ExplicitCongestionNotification};
+use s2n_quic_platform::features;
 use std::io::IoSlice;
 
-/// The maximum number of segments in sendmsg calls
-///
-/// From <https://elixir.bootlin.com/linux/v6.8.7/source/include/uapi/linux/uio.h#L27>
-/// > #define UIO_FASTIOV 8
-pub const MAX_COUNT: usize = if cfg!(target_os = "linux") { 8 } else { 1 };
+/// The maximum size of a IP+UDP header
+const IPV4_HEADER_LEN: u16 = 20;
+const IPV6_HEADER_LEN: u16 = 40;
+const UDP_HEADER_LEN: u16 = 8;
+
+const fn min_u16(a: u16, b: u16) -> u16 {
+    if a < b {
+        a
+    } else {
+        b
+    }
+}
 
-/// The maximum payload allowed in sendmsg calls using UDP
+/// The maximum number of segments in sendmsg calls
 ///
-/// From <https://github.com/torvalds/linux/blob/8cd26fd90c1ad7acdcfb9f69ca99d13aa7b24561/net/ipv4/ip_output.c#L987-L995>
-/// > Linux enforces a u16::MAX - IP_HEADER_LEN - UDP_HEADER_LEN
-pub const MAX_TOTAL: u16 = u16::MAX - 50;
+/// From <https://elixir.bootlin.com/linux/v6.8.7/source/include/uapi/linux/uio.h#L28>
+/// > #define UIO_MAXIOV  1024
+pub const MAX_COUNT: usize = if features::gso::IS_SUPPORTED {
+    // base the max segments on the max datagram size for the default ethernet mtu
+    let max_datagram_size = 1500 - min_u16(IPV4_HEADER_LEN, IPV6_HEADER_LEN) - UDP_HEADER_LEN;
+
+    (MAX_TOTAL / max_datagram_size) as _
+} else {
+    // only a single segment can be sent per syscall
+    1
+};
+
+/// The maximum payload allowed in sendmsg calls using IPv4+UDP
+const MAX_TOTAL_IPV4: u16 = if cfg!(target_os = "linux") {
+    // From <https://github.com/torvalds/linux/blob/8cd26fd90c1ad7acdcfb9f69ca99d13aa7b24561/net/ipv4/ip_output.c#L987-L995>
+    // > Linux enforces a u16::MAX - IP_HEADER_LEN - UDP_HEADER_LEN
+    u16::MAX - IPV4_HEADER_LEN - UDP_HEADER_LEN
+} else {
+    9001 - IPV4_HEADER_LEN - UDP_HEADER_LEN
+};
+
+/// The maximum payload allowed in sendmsg calls using IPv6+UDP
+const MAX_TOTAL_IPV6: u16 = if cfg!(target_os = "linux") {
+    // IPv6 doesn't include the IP header size in the calculation
+    u16::MAX - UDP_HEADER_LEN
+} else {
+    9001 - IPV6_HEADER_LEN - UDP_HEADER_LEN
+};
+
+/// The minimum payload size between the IPv4 and IPv6 sizes
+pub const MAX_TOTAL: u16 = min_u16(MAX_TOTAL_IPV4, MAX_TOTAL_IPV6);
+
+#[test]
+fn max_total_test() {
+    let tests = [("127.0.0.1:0", MAX_TOTAL_IPV4), ("[::1]:0", MAX_TOTAL_IPV6)];
+
+    for (addr, total) in tests {
+        let socket = std::net::UdpSocket::bind(addr).unwrap();
+        let addr = socket.local_addr().unwrap();
+
+        let mut buffer = vec![0u8; total as usize + 1];
+
+        // This behavior may not be consistent across kernel versions so the check is disabled by default
+        let _ = socket.send_to(&buffer, addr);
+
+        buffer.pop().unwrap();
+        socket
+            .send_to(&buffer, addr)
+            .expect("send should succeed when limited to MAX_TOTAL");
+    }
+}
 
 type Segments<'a> = ArrayVec<IoSlice<'a>, MAX_COUNT>;
 

dc/s2n-quic-dc/src/stream/environment/tokio.rs

@@ -65,7 +65,11 @@ where
     #[inline]
     pub fn build(self) -> io::Result<Environment<Sub>> {
         let clock = self.clock.unwrap_or_default();
-        let gso = self.gso.unwrap_or_default();
+        let gso = self.gso.unwrap_or_else(|| {
+            // rather than clamping it to the max burst size, let the CCA be the only
+            // component that controls send quantums
+            features::gso::MAX_SEGMENTS.into()
+        });
         let socket_options = self.socket_options.unwrap_or_default();
 
         let thread_count = self.threads.unwrap_or_else(|| {

dc/s2n-quic-dc/src/stream/send/flow.rs

@@ -5,7 +5,6 @@ use s2n_quic_core::varint::VarInt;
 
 pub mod blocking;
 pub mod non_blocking;
-pub use crate::msg::segment::MAX_TOTAL;
 
 /// Flow credits acquired by an application request
 #[derive(Debug)]
@@ -35,7 +34,7 @@ impl Request {
     /// Clamps the request with the given number of credits
     #[inline]
     pub fn clamp(&mut self, credits: u64) {
-        let len = self.len.min(credits.min(MAX_TOTAL as _) as usize);
+        let len = self.len.min(credits as usize);
 
         // if we didn't acquire the entire len, then clear the `is_fin` flag
         if self.len != len {

dc/s2n-quic-dc/src/stream/send/path.rs

@@ -1,11 +1,14 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
+use crate::msg::segment::MAX_TOTAL;
 use core::{
     fmt,
     sync::atomic::{AtomicU64, Ordering},
 };
-use s2n_quic_core::{inet::ExplicitCongestionNotification, varint::VarInt};
+use s2n_quic_core::{
+    inet::ExplicitCongestionNotification, path::MINIMUM_MAX_DATAGRAM_SIZE, varint::VarInt,
+};
 
 /// Contains the current state of a transmission path
 pub struct State {
@@ -95,6 +98,15 @@ impl State {
         data |= ecn as u8 as u64;
         data <<= 8;
 
+        let max_datagram_size = max_datagram_size.max(MINIMUM_MAX_DATAGRAM_SIZE);
+
+        // clamp the number of segments so it's under the max payload we can write in
+        // a single syscall
+        let max_segments = MAX_TOTAL / max_datagram_size;
+        let send_quantum = send_quantum.min(max_segments as u8);
+        // we need this to be at least 1
+        let send_quantum = send_quantum.max(1);
+
         data |= send_quantum as u64;
         data <<= 16;
 
@@ -149,6 +161,14 @@ mod tests {
                 let actual =
                     State::decode_info(State::encode_info(ecn, send_quantum, max_datagram_size));
 
+                // this needs to be at least MINIMUM_MAX_DATAGRAM_SIZE
+                let max_datagram_size = max_datagram_size.max(MINIMUM_MAX_DATAGRAM_SIZE);
+
+                // this needs to be at least 1 but less than what is allowed in a single syscall
+                let send_quantum = send_quantum
+                    .min((MAX_TOTAL / max_datagram_size) as u8)
+                    .max(1);
+
                 assert_eq!((ecn, send_quantum, max_datagram_size), actual);
             })
     }