Move the TCP receive window clamping hack downwards in stack.

whitequark · whitequark · commit a13712647931 · 2017-08-30T13:55:33.000Z
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.
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.
     // =========================================================================================//
