Browse Source

Dispatch a TCP ACK every time window increases at all.

Commit 21282fdb was not completely sufficient because e.g. receiving
one octet and then blocking meant that an ACK with window length
of one is then sent, and this isn't motivating the other TCP stacks
all that much.

In any case, preemptively notifying the peer of window size increase
can only be beneficial, modulo network congestion.
whitequark 8 years ago
parent
commit
f76b4be42d
1 changed files with 20 additions and 7 deletions
  1. 20 7
      src/socket/tcp.rs

+ 20 - 7
src/socket/tcp.rs

@@ -273,6 +273,8 @@ pub struct TcpSocket<'a> {
     /// The last acknowledgement number sent.
     /// The last acknowledgement number sent.
     /// I.e. in an idle socket, remote_seq_no+rx_buffer.len().
     /// I.e. in an idle socket, remote_seq_no+rx_buffer.len().
     remote_last_ack: Option<TcpSeqNumber>,
     remote_last_ack: Option<TcpSeqNumber>,
+    /// The last window length sent.
+    remote_last_win: u16,
     /// The speculative remote window size.
     /// The speculative remote window size.
     /// I.e. the actual remote window size minus the count of in-flight octets.
     /// I.e. the actual remote window size minus the count of in-flight octets.
     remote_win_len:  usize,
     remote_win_len:  usize,
@@ -305,6 +307,7 @@ impl<'a> TcpSocket<'a> {
             remote_seq_no:   TcpSeqNumber::default(),
             remote_seq_no:   TcpSeqNumber::default(),
             remote_last_seq: TcpSeqNumber::default(),
             remote_last_seq: TcpSeqNumber::default(),
             remote_last_ack: None,
             remote_last_ack: None,
+            remote_last_win: 0,
             remote_win_len:  0,
             remote_win_len:  0,
             remote_mss:      DEFAULT_MSS,
             remote_mss:      DEFAULT_MSS,
         })
         })
@@ -351,6 +354,7 @@ impl<'a> TcpSocket<'a> {
         self.remote_seq_no   = TcpSeqNumber::default();
         self.remote_seq_no   = TcpSeqNumber::default();
         self.remote_last_seq = TcpSeqNumber::default();
         self.remote_last_seq = TcpSeqNumber::default();
         self.remote_last_ack = None;
         self.remote_last_ack = None;
+        self.remote_last_win = 0;
         self.remote_win_len  = 0;
         self.remote_win_len  = 0;
         self.remote_mss      = DEFAULT_MSS;
         self.remote_mss      = DEFAULT_MSS;
         self.timer.reset();
         self.timer.reset();
@@ -624,12 +628,6 @@ impl<'a> TcpSocket<'a> {
         // another (stale) SYN.
         // another (stale) SYN.
         if !self.may_recv() { return Err(Error::Illegal) }
         if !self.may_recv() { return Err(Error::Illegal) }
 
 
-        // If we advertised a zero window, make sure to send an ACK so that the peer
-        // can resume sending data.
-        if self.rx_buffer.window() == 0 {
-            self.remote_last_ack = None;
-        }
-
         #[cfg(any(test, feature = "verbose"))]
         #[cfg(any(test, feature = "verbose"))]
         let old_length = self.rx_buffer.len();
         let old_length = self.rx_buffer.len();
         let buffer = self.rx_buffer.dequeue(size);
         let buffer = self.rx_buffer.dequeue(size);
@@ -1083,6 +1081,7 @@ impl<'a> TcpSocket<'a> {
 
 
             // Send an acknowledgement.
             // Send an acknowledgement.
             self.remote_last_ack = Some(self.remote_seq_no + self.rx_buffer.len());
             self.remote_last_ack = Some(self.remote_seq_no + self.rx_buffer.len());
+            self.remote_last_win = self.rx_buffer.window() as u16;
             Ok(Some(self.ack_reply(ip_repr, &repr)))
             Ok(Some(self.ack_reply(ip_repr, &repr)))
         } else {
         } else {
             // No data to acknowledge; the logic to acknowledge SYN and FIN flags
             // No data to acknowledge; the logic to acknowledge SYN and FIN flags
@@ -1193,6 +1192,8 @@ impl<'a> TcpSocket<'a> {
             // If we have data to transmit and it fits into partner's window, do it.
             // If we have data to transmit and it fits into partner's window, do it.
         } else if self.ack_to_transmit() && repr.ack_number.is_some() {
         } else if self.ack_to_transmit() && repr.ack_number.is_some() {
             // If we have data to acknowledge, do it.
             // If we have data to acknowledge, do it.
+        } else if repr.window_len > self.remote_last_win {
+            // If we have window length increase to advertise, do it.
         } else if self.timer.should_retransmit(timestamp).is_some() {
         } else if self.timer.should_retransmit(timestamp).is_some() {
             // If we have packets to retransmit, do it.
             // If we have packets to retransmit, do it.
         } else if repr.control == TcpControl::Rst {
         } else if repr.control == TcpControl::Rst {
@@ -1258,6 +1259,7 @@ impl<'a> TcpSocket<'a> {
         // We've sent a packet successfully, so we can update the internal state now.
         // We've sent a packet successfully, so we can update the internal state now.
         self.remote_last_seq = repr.seq_number + repr.segment_len();
         self.remote_last_seq = repr.seq_number + repr.segment_len();
         self.remote_last_ack = repr.ack_number;
         self.remote_last_ack = repr.ack_number;
+        self.remote_last_win = repr.window_len;
 
 
         if !self.seq_to_transmit(repr.control) && repr.segment_len() > 0 {
         if !self.seq_to_transmit(repr.control) && repr.segment_len() > 0 {
             // If we've transmitted all data could (and there was something at all,
             // If we've transmitted all data could (and there was something at all,
@@ -1454,6 +1456,7 @@ mod test {
             assert_eq!(s1.remote_seq_no,    s2.remote_seq_no,   "remote_seq_no");
             assert_eq!(s1.remote_seq_no,    s2.remote_seq_no,   "remote_seq_no");
             assert_eq!(s1.remote_last_seq,  s2.remote_last_seq, "remote_last_seq");
             assert_eq!(s1.remote_last_seq,  s2.remote_last_seq, "remote_last_seq");
             assert_eq!(s1.remote_last_ack,  s2.remote_last_ack, "remote_last_ack");
             assert_eq!(s1.remote_last_ack,  s2.remote_last_ack, "remote_last_ack");
+            assert_eq!(s1.remote_last_win,  s2.remote_last_win, "remote_last_win");
             assert_eq!(s1.remote_win_len,   s2.remote_win_len,  "remote_win_len");
             assert_eq!(s1.remote_win_len,   s2.remote_win_len,  "remote_win_len");
             assert_eq!(s1.timer,            s2.timer,           "timer");
             assert_eq!(s1.timer,            s2.timer,           "timer");
         })
         })
@@ -1658,6 +1661,7 @@ mod test {
         assert_eq!(s.state, State::CloseWait);
         assert_eq!(s.state, State::CloseWait);
         sanity!(s, TcpSocket {
         sanity!(s, TcpSocket {
             remote_last_ack: Some(REMOTE_SEQ + 1 + 6 + 1),
             remote_last_ack: Some(REMOTE_SEQ + 1 + 6 + 1),
+            remote_last_win: 58,
             ..socket_close_wait()
             ..socket_close_wait()
         });
         });
     }
     }
@@ -1853,6 +1857,7 @@ mod test {
         s.local_seq_no    = LOCAL_SEQ + 1;
         s.local_seq_no    = LOCAL_SEQ + 1;
         s.remote_last_seq = LOCAL_SEQ + 1;
         s.remote_last_seq = LOCAL_SEQ + 1;
         s.remote_last_ack = Some(REMOTE_SEQ + 1);
         s.remote_last_ack = Some(REMOTE_SEQ + 1);
+        s.remote_last_win = 64;
         s
         s
     }
     }
 
 
@@ -2985,7 +2990,15 @@ mod test {
             ..RECV_TEMPL
             ..RECV_TEMPL
         })));
         })));
         recv!(s, time 0, Err(Error::Exhausted));
         recv!(s, time 0, Err(Error::Exhausted));
-        assert_eq!(s.recv(6), Ok(&b"abcdef"[..]));
+        assert_eq!(s.recv(3), Ok(&b"abc"[..]));
+        recv!(s, time 0, Ok(TcpRepr {
+            seq_number: LOCAL_SEQ + 1,
+            ack_number: Some(REMOTE_SEQ + 1 + 6),
+            window_len: 3,
+            ..RECV_TEMPL
+        }));
+        recv!(s, time 0, Err(Error::Exhausted));
+        assert_eq!(s.recv(3), Ok(&b"def"[..]));
         recv!(s, time 0, Ok(TcpRepr {
         recv!(s, time 0, Ok(TcpRepr {
             seq_number: LOCAL_SEQ + 1,
             seq_number: LOCAL_SEQ + 1,
             ack_number: Some(REMOTE_SEQ + 1 + 6),
             ack_number: Some(REMOTE_SEQ + 1 + 6),