소스 검색

tcp: don't force-send data on retransmit.

Previous code had an `if` to force sending a packet when retransmitting.
When the remote window is zero this would cause an infinite loop of
sending empty packets, because the "retransmit" flag would never get cleared.

Remove the force-retransmit, and add an explicit check on `seq_to_transmit` for
pending SYNs because SYN retransmission relied on it.

Found with cargo-fuzz.
Dario Nieuwenhuis 3 년 전
부모
커밋
675c0a109f
1개의 변경된 파일59개의 추가작업 그리고 8개의 파일을 삭제
  1. 59 8
      src/socket/tcp.rs

+ 59 - 8
src/socket/tcp.rs

@@ -1994,6 +1994,11 @@ impl<'a> TcpSocket<'a> {
         // Have we sent data that hasn't been ACKed yet?
         let data_in_flight = self.remote_last_seq != self.local_seq_no;
 
+        // If we want to send a SYN and we haven't done so, do it!
+        if matches!(self.state, State::SynSent | State::SynReceived) && !data_in_flight {
+            return true;
+        }
+
         // max sequence number we can send.
         let max_send_seq =
             self.local_seq_no + core::cmp::min(self.remote_win_len, self.tx_buffer.len());
@@ -2097,7 +2102,19 @@ impl<'a> TcpSocket<'a> {
                     self.remote_endpoint,
                     retransmit_delta
                 );
+
+                // Rewind "last sequence number sent", as if we never
+                // had sent them. This will cause all data in the queue
+                // to be sent again.
                 self.remote_last_seq = self.local_seq_no;
+
+                // Clear the `should_retransmit` state. If we can't retransmit right
+                // now for whatever reason (like zero window), this avoids an
+                // infinite polling loop where `poll_at` returns `Now` but `dispatch`
+                // can't actually do anything.
+                self.timer.set_for_idle(cx.now, self.keep_alive);
+
+                // Inform RTTE, so that it can avoid bogus measurements.
                 self.rtte.on_retransmit();
             }
         }
@@ -2135,14 +2152,6 @@ impl<'a> TcpSocket<'a> {
                 self.local_endpoint,
                 self.remote_endpoint
             );
-        } else if self.timer.should_retransmit(cx.now).is_some() {
-            // If we have packets to retransmit, do it.
-            net_trace!(
-                "{}:{}:{}: retransmit timer expired",
-                self.meta.handle,
-                self.local_endpoint,
-                self.remote_endpoint
-            );
         } else if self.timer.should_keep_alive(cx.now) {
             // If we need to transmit a keep-alive packet, do it.
             net_trace!(
@@ -5506,6 +5515,48 @@ mod test {
         );
     }
 
+    #[test]
+    fn test_fast_retransmit_zero_window() {
+        let mut s = socket_established();
+
+        send!(s, time 1000, TcpRepr {
+            seq_number: REMOTE_SEQ + 1,
+            ack_number: Some(LOCAL_SEQ + 1),
+            ..SEND_TEMPL
+        });
+
+        s.send_slice(b"abc").unwrap();
+
+        recv!(s, time 0, Ok(TcpRepr {
+            seq_number: LOCAL_SEQ + 1,
+            ack_number: Some(REMOTE_SEQ + 1),
+            payload:    &b"abc"[..],
+            ..RECV_TEMPL
+        }));
+
+        // 3 dup acks
+        send!(s, time 1050, TcpRepr {
+            seq_number: REMOTE_SEQ + 1,
+            ack_number: Some(LOCAL_SEQ + 1),
+            ..SEND_TEMPL
+        });
+        send!(s, time 1050, TcpRepr {
+            seq_number: REMOTE_SEQ + 1,
+            ack_number: Some(LOCAL_SEQ + 1),
+            ..SEND_TEMPL
+        });
+        send!(s, time 1050, TcpRepr {
+            seq_number: REMOTE_SEQ + 1,
+            ack_number: Some(LOCAL_SEQ + 1),
+            window_len: 0, // boom
+            ..SEND_TEMPL
+        });
+
+        // even though we're in "fast retransmit", we shouldn't
+        // force-send anything because the remote's window is full.
+        recv!(s, Err(Error::Exhausted));
+    }
+
     // =========================================================================================//
     // Tests for window management.
     // =========================================================================================//