Merge #748

748: tcp: do not count window updates as duplicate acks. r=Dirbaio a=Dirbaio

rfc 2581:

> The TCP sender SHOULD use the "fast retransmit" algorithm to detect
> and repair loss, based on incoming duplicate ACKs.  The fast
> retransmit algorithm uses the arrival of 3 duplicate ACKs (4
> ***identical*** ACKs without the arrival of any other intervening packets)
> as an indication that a segment has been lost.  After receiving 3
> duplicate ACKs, TCP performs a retransmission of what appears to be
> the missing segment, without waiting for the retransmission timer to
> expire.

This means they have to have the same seq, ack and win, and therefore they must not be window updates.

Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>

src/socket/tcp.rs

@@ -1746,7 +1746,9 @@ impl<'a> Socket<'a> {
             TcpControl::Syn => 0,
             _ => self.remote_win_scale.unwrap_or(0),
         };
-        self.remote_win_len = (repr.window_len as usize) << (scale as usize);
+        let new_remote_win_len = (repr.window_len as usize) << (scale as usize);
+        let is_window_update = new_remote_win_len != self.remote_win_len;
+        self.remote_win_len = new_remote_win_len;
 
         if ack_len > 0 {
             // Dequeue acknowledged octets.
@@ -1778,7 +1780,8 @@ impl<'a> Socket<'a> {
                 Some(last_rx_ack)
                     if repr.payload.is_empty()
                         && last_rx_ack == ack_number
-                        && ack_number < self.remote_last_seq =>
+                        && ack_number < self.remote_last_seq
+                        && !is_window_update =>
                 {
                     // Increment duplicate ACK count
                     self.local_rx_dup_acks = self.local_rx_dup_acks.saturating_add(1);
@@ -5531,6 +5534,66 @@ mod test {
         );
     }
 
+    #[test]
+    fn test_fast_retransmit_duplicate_detection_with_window_update() {
+        let mut s = socket_established();
+
+        s.send_slice(b"abc").unwrap(); // This is lost
+        recv!(s, time 1000, Ok(TcpRepr {
+            seq_number: LOCAL_SEQ + 1,
+            ack_number: Some(REMOTE_SEQ + 1),
+            payload:    &b"abc"[..],
+            ..RECV_TEMPL
+        }));
+
+        // Normal ACK of previously received segment
+        send!(
+            s,
+            TcpRepr {
+                seq_number: REMOTE_SEQ + 1,
+                ack_number: Some(LOCAL_SEQ + 1),
+                ..SEND_TEMPL
+            }
+        );
+        // First duplicate
+        send!(
+            s,
+            TcpRepr {
+                seq_number: REMOTE_SEQ + 1,
+                ack_number: Some(LOCAL_SEQ + 1),
+                ..SEND_TEMPL
+            }
+        );
+        // Second duplicate
+        send!(
+            s,
+            TcpRepr {
+                seq_number: REMOTE_SEQ + 1,
+                ack_number: Some(LOCAL_SEQ + 1),
+                ..SEND_TEMPL
+            }
+        );
+
+        assert_eq!(s.local_rx_dup_acks, 2, "duplicate ACK counter is not set");
+
+        // This packet has a window update, hence should not be detected
+        // as a duplicate ACK and should reset the duplicate ACK count
+        send!(
+            s,
+            TcpRepr {
+                seq_number: REMOTE_SEQ + 1,
+                ack_number: Some(LOCAL_SEQ + 1),
+                window_len: 400,
+                ..SEND_TEMPL
+            }
+        );
+
+        assert_eq!(
+            s.local_rx_dup_acks, 0,
+            "duplicate ACK counter is not reset when receiving a window update"
+        );
+    }
+
     #[test]
     fn test_fast_retransmit_duplicate_detection() {
         let mut s = socket_established();