Radically simplify and optimize TCP packet dispatch.

This commit completely reworks packet dispatch in TCP sockets, and brings significant improvements to processing as well. In particular: * Challenge ACKs now do not reset retransmit timer; instead, TcpSocket::process directly returns a TcpRepr without altering any internal state at all. * Retransmit and close (aka TIME-WAIT) timers are unified and restructured into a enum that actually matches semantics of the timers. * If a packet cannot be emitted, no internal state is changed. * The dispatch of RST packets in case of connection abort is brought in line with dispatch of all other packets. * Packet dispatch now follows a series of steps with clean separation of concerns, like packet processing: 1. If we should retransmit, update state to assume that all in-flight packets are lost. 2. Prepare the packet that would be sent next, considering the in-flight packets, if any. 3. Check if the packet contains anything new, or it's the same as the one already in flight. If it is, bail. 4. Finalize and try to actually transmit the packet. If we can't do that, bail. 5. Update the internal state to reflect that the packet we've just sent is in flight.
smoltcp-rs · Aug 25, 2017 · 72359cd · 72359cd
1 parent bf916e6
commit 72359cd
Showing 5 changed files with 397 additions and 359 deletions.
diff --git a/src/iface/arp_cache.rs b/src/iface/arp_cache.rs
@@ -101,7 +101,7 @@ impl<'a> Cache for SliceCache<'a> {
         if let None = self.find(protocol_addr) {
             let lru_index = self.lru();
 
-            if net_trace_enabled!() {
+            if net_log_enabled!(trace) {
                 let (old_protocol_addr, old_hardware_addr, _counter) = self.storage[lru_index];
                 if !old_protocol_addr.is_unspecified() {
                     net_trace!("evicting {} => {}", old_protocol_addr, old_hardware_addr);

diff --git a/src/iface/ethernet.rs b/src/iface/ethernet.rs
@@ -27,7 +27,7 @@ enum Response<'a> {
     Nop,
     Arp(ArpRepr),
     Icmpv4(Ipv4Repr, Icmpv4Repr<'a>),
-    Tcp(IpRepr, TcpRepr<'a>)
+    Tcp((IpRepr, TcpRepr<'a>))
 }
 
 impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
@@ -314,7 +314,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
                 <Socket as AsSocket<TcpSocket>>::try_as_socket) {
             match tcp_socket.process(timestamp, &ip_repr, ip_payload) {
                 // The packet is valid and handled by socket.
-                Ok(()) => return Ok(Response::Nop),
+                Ok(reply) => return Ok(reply.map_or(Response::Nop, Response::Tcp)),
                 // The packet isn't addressed to the socket.
                 // Send RST only if no other socket accepts the packet.
                 Err(Error::Rejected) => continue,
@@ -330,8 +330,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
             // Never reply to a TCP RST packet with another TCP RST packet.
             Ok(Response::Nop)
         } else {
-            let (ip_reply_repr, tcp_reply_repr) = TcpSocket::rst_reply(&ip_repr, &tcp_repr);
-            Ok(Response::Tcp(ip_reply_repr, tcp_reply_repr))
+            Ok(Response::Tcp(TcpSocket::rst_reply(&ip_repr, &tcp_repr)))
         }
     }
 
@@ -393,7 +392,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
                     icmpv4_repr.emit(&mut Icmpv4Packet::new(payload));
                 })
             }
-            Response::Tcp(ip_repr, tcp_repr) => {
+            Response::Tcp((ip_repr, tcp_repr)) => {
                 emit_packet!(Ip, ip_repr, |payload| {
                     tcp_repr.emit(&mut TcpPacket::new(payload),
                                   &ip_repr.src_addr(), &ip_repr.dst_addr());

diff --git a/src/macros.rs b/src/macros.rs
@@ -1,3 +1,15 @@
+#[cfg(feature = "log")]
+macro_rules! net_log_enabled {
+    (trace) => (log_enabled!($crate::log::LogLevel::Trace));
+    (debug) => (log_enabled!($crate::log::LogLevel::Debug));
+}
+
+#[cfg(not(feature = "log"))]
+macro_rules! net_log_enabled {
+    (trace) => (false);
+    (debug) => (false);
+}
+
 macro_rules! net_trace {
     ($($arg:expr),*) => {
         #[cfg(feature = "log")]
@@ -7,16 +19,6 @@ macro_rules! net_trace {
     }
 }
 
-macro_rules! net_trace_enabled {
-    () => ({
-        #[cfg(feature = "log")]
-        fn enabled() -> bool { log_enabled!($crate::log::LogLevel::Trace) }
-        #[cfg(not(feature = "log"))]
-        fn enabled() -> bool { false }
-        enabled()
-    })
-}
-
 macro_rules! net_debug {
     ($($arg:expr),*) => {
         #[cfg(feature = "log")]

diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs
diff --git a/src/wire/tcp.rs b/src/wire/tcp.rs
@@ -599,7 +599,7 @@ impl<'a> TcpOption<'a> {
     }
 }
 
-/// The control flags of a Transmission Control Protocol packet.
+/// The possible control flags of a Transmission Control Protocol packet.
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub enum Control {
     None,
@@ -608,6 +608,16 @@ pub enum Control {
     Rst
 }
 
+impl Control {
+    /// Return the length of a control flag, in terms of sequence space.
+    pub fn len(self) -> usize {
+        match self {
+            Control::Syn | Control::Fin  => 1,
+            Control::Rst | Control::None => 0
+        }
+    }
+}
+
 /// A high-level representation of a Transmission Control Protocol packet.
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub struct Repr<'a> {
@@ -728,12 +738,7 @@ impl<'a> Repr<'a> {
 
     /// Return the length of the segment, in terms of sequence space.
     pub fn segment_len(&self) -> usize {
-        let mut length = self.payload.len();
-        match self.control {
-            Control::Syn | Control::Fin => length += 1,
-            _ => ()
-        }
-        length
+        self.payload.len() + self.control.len()
     }
 }