From 1f8110687d09f09b504082317fb1405e27b922d8 Mon Sep 17 00:00:00 2001 From: david-sawatzke Date: Tue, 14 Sep 2021 18:08:04 +0200 Subject: [PATCH 1/2] Fix typos in tcp docs --- src/socket/tcp.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index 270918f..21fdb39 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -883,7 +883,7 @@ impl<'a> TcpSocket<'a> { } /// Check whether the transmit half of the full-duplex connection is open - /// (see [may_send](#method.may_send), and the transmit buffer is not full. + /// (see [may_send](#method.may_send)), and the transmit buffer is not full. #[inline] pub fn can_send(&self) -> bool { if !self.may_send() { @@ -906,7 +906,7 @@ impl<'a> TcpSocket<'a> { } /// Check whether the receive half of the full-duplex connection buffer is open - /// (see [may_recv](#method.may_recv), and the receive buffer is not empty. + /// (see [may_recv](#method.may_recv)), and the receive buffer is not empty. #[inline] pub fn can_recv(&self) -> bool { if !self.may_recv() { From ddfabb42f0837a48fb82a0e34c53c12b4475aa66 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 15 Sep 2021 04:01:43 +0200 Subject: [PATCH 2/2] tcp: fix delayed ack causing ack not to be sent after 3 packets. --- src/socket/tcp.rs | 124 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 101 insertions(+), 23 deletions(-) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index 270918f..8600d54 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -300,6 +300,13 @@ impl Timer { } } +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +enum AckDelayTimer { + Idle, + Waiting(Instant), + Immediate, +} + /// A Transmission Control Protocol socket. /// /// A TCP socket may passively listen for connections or actively connect to another endpoint. @@ -375,7 +382,7 @@ pub struct TcpSocket<'a> { ack_delay: Option, /// Delayed ack timer. If set, packets containing exclusively /// ACK or window updates (ie, no data) won't be sent until expiry. - ack_delay_until: Option, + ack_delay_timer: AckDelayTimer, /// Nagle's Algorithm enabled. nagle: bool, @@ -437,7 +444,7 @@ impl<'a> TcpSocket<'a> { local_rx_last_seq: None, local_rx_dup_acks: 0, ack_delay: Some(ACK_DELAY_DEFAULT), - ack_delay_until: None, + ack_delay_timer: AckDelayTimer::Idle, nagle: true, #[cfg(feature = "async")] @@ -660,7 +667,7 @@ impl<'a> TcpSocket<'a> { self.remote_mss = DEFAULT_MSS; self.remote_last_ts = None; self.ack_delay = Some(ACK_DELAY_DEFAULT); - self.ack_delay_until = None; + self.ack_delay_timer = AckDelayTimer::Idle; self.nagle = true; #[cfg(feature = "async")] @@ -1889,8 +1896,8 @@ impl<'a> TcpSocket<'a> { // Handle delayed acks if let Some(ack_delay) = self.ack_delay { if self.ack_to_transmit() || self.window_to_update() { - self.ack_delay_until = match self.ack_delay_until { - None => { + self.ack_delay_timer = match self.ack_delay_timer { + AckDelayTimer::Idle => { net_trace!( "{}:{}:{}: starting delayed ack timer", self.meta.handle, @@ -1898,19 +1905,28 @@ impl<'a> TcpSocket<'a> { self.remote_endpoint ); - Some(cx.now + ack_delay) + AckDelayTimer::Waiting(cx.now + ack_delay) } // RFC1122 says "in a stream of full-sized segments there SHOULD be an ACK // for at least every second segment". // For now, we send an ACK every second received packet, full-sized or not. - Some(_) => { + AckDelayTimer::Waiting(_) => { net_trace!( "{}:{}:{}: delayed ack timer already started, forcing expiry", self.meta.handle, self.local_endpoint, self.remote_endpoint ); - None + AckDelayTimer::Immediate + } + AckDelayTimer::Immediate => { + net_trace!( + "{}:{}:{}: delayed ack timer already force-expired", + self.meta.handle, + self.local_endpoint, + self.remote_endpoint + ); + AckDelayTimer::Immediate } }; } @@ -1998,9 +2014,10 @@ impl<'a> TcpSocket<'a> { } fn delayed_ack_expired(&self, timestamp: Instant) -> bool { - match self.ack_delay_until { - None => true, - Some(t) => t <= timestamp, + match self.ack_delay_timer { + AckDelayTimer::Idle => true, + AckDelayTimer::Waiting(t) => t <= timestamp, + AckDelayTimer::Immediate => true, } } @@ -2309,16 +2326,26 @@ impl<'a> TcpSocket<'a> { self.timer.rewind_keep_alive(cx.now, self.keep_alive); // Reset delayed-ack timer - if self.ack_delay_until.is_some() { - net_trace!( - "{}:{}:{}: stop delayed ack timer", - self.meta.handle, - self.local_endpoint, - self.remote_endpoint - ); - - self.ack_delay_until = None; + match self.ack_delay_timer { + AckDelayTimer::Idle => {} + AckDelayTimer::Waiting(_) => { + net_trace!( + "{}:{}:{}: stop delayed ack timer", + self.meta.handle, + self.local_endpoint, + self.remote_endpoint + ) + } + AckDelayTimer::Immediate => { + net_trace!( + "{}:{}:{}: stop delayed ack timer (was force-expired)", + self.meta.handle, + self.local_endpoint, + self.remote_endpoint + ) + } } + self.ack_delay_timer = AckDelayTimer::Idle; // Leave the rest of the state intact if sending a keep-alive packet, since those // carry a fake segment. @@ -2369,10 +2396,12 @@ impl<'a> TcpSocket<'a> { PollAt::Now } else { let want_ack = self.ack_to_transmit() || self.window_to_update(); - let delayed_ack_poll_at = match (want_ack, self.ack_delay_until) { + + let delayed_ack_poll_at = match (want_ack, self.ack_delay_timer) { (false, _) => PollAt::Ingress, - (true, None) => PollAt::Now, - (true, Some(t)) => PollAt::Time(t), + (true, AckDelayTimer::Idle) => PollAt::Now, + (true, AckDelayTimer::Waiting(t)) => PollAt::Time(t), + (true, AckDelayTimer::Immediate) => PollAt::Now, }; let timeout_poll_at = match (self.remote_last_ts, self.timeout) { @@ -6544,6 +6573,55 @@ mod test { ); } + #[test] + fn test_delayed_ack_three_packets() { + let mut s = socket_established(); + s.set_ack_delay(Some(ACK_DELAY_DEFAULT)); + send!( + s, + TcpRepr { + seq_number: REMOTE_SEQ + 1, + ack_number: Some(LOCAL_SEQ + 1), + payload: &b"abc"[..], + ..SEND_TEMPL + } + ); + + // No ACK is immediately sent. + recv!(s, Err(Error::Exhausted)); + + send!( + s, + TcpRepr { + seq_number: REMOTE_SEQ + 1 + 3, + ack_number: Some(LOCAL_SEQ + 1), + payload: &b"def"[..], + ..SEND_TEMPL + } + ); + + send!( + s, + TcpRepr { + seq_number: REMOTE_SEQ + 1 + 6, + ack_number: Some(LOCAL_SEQ + 1), + payload: &b"ghi"[..], + ..SEND_TEMPL + } + ); + + // Every 2nd (or more) packet, ACK is sent without delay. + recv!( + s, + Ok(TcpRepr { + seq_number: LOCAL_SEQ + 1, + ack_number: Some(REMOTE_SEQ + 1 + 9), + window_len: 55, + ..RECV_TEMPL + }) + ); + } + // =========================================================================================// // Tests for Nagle's Algorithm // =========================================================================================//