Move the TCP receive window clamping hack downwards in stack.

Otherwise, our response ACKs did not get the clamping treatment, and severe packet loss resulted. Also, explain why it's needed and how it works.
smoltcp-rs · Aug 30, 2017 · a137126 · a137126
1 parent 280ddde
commit a137126
Showing 2 changed files with 20 additions and 31 deletions.
diff --git a/src/iface/ethernet.rs b/src/iface/ethernet.rs
@@ -458,7 +458,26 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
                                   &ip_repr.src_addr(), &ip_repr.dst_addr());
                 })
             }
-            Packet::Tcp((ip_repr, tcp_repr)) => {
+            Packet::Tcp((ip_repr, mut tcp_repr)) => {
+                // This is a terrible hack to make TCP performance more acceptable on systems
+                // where the TCP buffers are significantly larger than network buffers,
+                // e.g. a 64 kB TCP receive buffer (and so, when empty, a 64k window)
+                // together with four 1500 B Ethernet receive buffers. If left untreated,
+                // this would result in our peer pushing our window and sever packet loss.
+                //
+                // I'm really not happy about this "solution" but I don't know what else to do.
+                let limits = self.device.limits();
+                if let Some(max_burst_size) = limits.max_burst_size {
+                    let mut max_segment_size = limits.max_transmission_unit;
+                    max_segment_size -= ip_repr.buffer_len();
+                    max_segment_size -= tcp_repr.header_len();
+
+                    let max_window_size = max_burst_size * max_segment_size;
+                    if tcp_repr.window_len as usize > max_window_size {
+                        tcp_repr.window_len = max_window_size as u16;
+                    }
+                }
+
                 self.dispatch_ip(timestamp, ip_repr, |ip_repr, payload| {
                     tcp_repr.emit(&mut TcpPacket::new(payload),
                                   &ip_repr.src_addr(), &ip_repr.dst_addr());

diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs
@@ -1241,13 +1241,6 @@ impl<'a> TcpSocket<'a> {
             repr.max_seg_size = Some(max_segment_size as u16);
         }
 
-        if let Some(max_burst_size) = limits.max_burst_size {
-            let max_window_size = max_burst_size * max_segment_size;
-            if repr.window_len as usize > max_window_size {
-                repr.window_len = max_window_size as u16;
-            }
-        }
-
         emit((ip_repr, repr))?;
 
         // We've sent a packet successfully, so we can update the internal state now.
@@ -2893,29 +2886,6 @@ mod test {
         }));
     }
 
-    #[test]
-    fn test_window_size_clamp() {
-        let mut s = socket_established();
-        s.rx_buffer = SocketBuffer::new(vec![0; 32767]);
-
-        let mut limits = DeviceLimits::default();
-        limits.max_transmission_unit = 1520;
-
-        limits.max_burst_size = None;
-        s.send_slice(b"abcdef").unwrap();
-        s.dispatch(0, &limits, |(_ip_repr, tcp_repr)| {
-            assert_eq!(tcp_repr.window_len, 32767);
-            Ok(())
-        }).unwrap();
-
-        limits.max_burst_size = Some(4);
-        s.send_slice(b"abcdef").unwrap();
-        s.dispatch(0, &limits, |(_ip_repr, tcp_repr)| {
-            assert_eq!(tcp_repr.window_len, 5920);
-            Ok(())
-        }).unwrap();
-    }
-
     // =========================================================================================//
     // Tests for flow control.
     // =========================================================================================//