smoltcp-rs · Sep 22, 2017 · Sep 22, 2017
Showing with 21 additions and 24 deletions.

+21 −24 src/socket/tcp.rs
diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs
@@ -1,5 +1,6 @@
 // Heads up! Before working on this file you should read, at least, RFC 793 and
-// the parts of RFC 1122 that discuss TCP.
+// the parts of RFC 1122 that discuss TCP. Consult RFC 7414 when implementing
+// a new feature.
 
 use core::{cmp, fmt};
 
@@ -1093,6 +1094,8 @@ impl<'a> TcpSocket<'a> {
         let payload_len = repr.payload.len();
         if payload_len == 0 { return Ok(None) }
 
+        let assembler_was_empty = self.assembler.is_empty();
+
         // Try adding payload octets to the assembler.
         match self.assembler.add(payload_offset, payload_len) {
             Ok(()) => {
@@ -1120,19 +1123,24 @@ impl<'a> TcpSocket<'a> {
             self.rx_buffer.enqueue_unallocated(contig_len);
         }
 
-        if self.assembler.is_empty() {
-            Ok(None)
-        } else {
-            // If the assembler isn't empty, some segments at the start of our window got lost.
-            // Send a reply acknowledging the data we already have; RFC 793 does not specify
-            // the behavior triggerd by such a reply, but RFC 1122 section 4.2.2.21 states that
-            // most congestion control algorithms implement what's called a "fast retransmit",
-            // where a threshold amount of duplicate ACKs triggers retransmission without
-            // the need to wait for a timeout to expire.
+        if !self.assembler.is_empty() {
+            // Print the ranges recorded in the assembler.
             net_trace!("[{}]{}:{}: assembler: {}",
                        self.debug_id, self.local_endpoint, self.remote_endpoint,
                        self.assembler);
+        }
+
+        // Per RFC 5681, we should send an immediate ACK when either:
+        //  1) an out-of-order segment is received, or
+        //  2) a segment arrives that fills in all or part of a gap in sequence space.
+        if !self.assembler.is_empty() || !assembler_was_empty {
+            // Note that we change the transmitter state here.
+            // This is fine because smoltcp assumes that it can always transmit zero or one
+            // packets for every packet it receives.
+            self.remote_last_ack = Some(self.remote_seq_no + self.rx_buffer.len());
             Ok(Some(self.ack_reply(ip_repr, &repr)))
+        } else {
+            Ok(None)
         }
     }
 
@@ -1318,17 +1326,7 @@ impl<'a> TcpSocket<'a> {
         }
 
         if repr.control == TcpControl::Syn {
-            // Compute the maximum segment size, deriving it from from the underlying
-            // maximum transmission unit and the IP and TCP header sizes.
-            //
-            // Note that what we actually *want* is for the other party to limit
-            // the total length of the TCP segment, but what we *get* is limiting
-            // the amount of data in the TCP segment. As a result, if they interpret
-            // the requirement naively and send us a TCP packet with both some options
-            // and an MSS-sized payload, that packet's last few bytes will get split
-            // into a tiny fragment.
-            //
-            // TCP is not a well-designed protocol.
+            // See RFC 6691 for an explanation of this calculation.
             let mut max_segment_size = limits.max_transmission_unit;
             max_segment_size -= ip_repr.buffer_len();
             max_segment_size -= repr.header_len();
@@ -3281,13 +3279,12 @@ mod test {
             ack_number: Some(LOCAL_SEQ + 1),
             payload:    &b"abcdef"[..],
             ..SEND_TEMPL
-        });
-        recv!(s, [TcpRepr {
+        }, Ok(Some(TcpRepr {
             seq_number: LOCAL_SEQ + 1,
             ack_number: Some(REMOTE_SEQ + 1 + 6),
             window_len: 58,
             ..RECV_TEMPL
-        }]);
+        })));
         assert_eq!(s.recv(10), Ok(&b"abcdef"[..]));
     }