Browse Source

Limiting the number of poll loops to prevent DoS events

Updating process loop to be defined by socket buffers when possible

Further changes after hardware testing

Fixing test

Adding CHANGELOG entry

Expanding comment after review

Update src/iface/interface/mod.rs

Co-authored-by: Catherine <whitequark@whitequark.org>

Removing redundant phrasing in comment

Updating verbiage to remove references to unsafe and safe
Ryan Summers 9 months ago
parent
commit
8c2cef1996
2 changed files with 64 additions and 69 deletions
  1. 2 1
      CHANGELOG.md
  2. 62 68
      src/iface/interface/mod.rs

+ 2 - 1
CHANGELOG.md

@@ -6,7 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 
 ## [Unreleased]
 ## [Unreleased]
 
 
-No unreleased changes yet.
+### Changed
+- iface: The `poll` function now only performs a single cycle of processing sockets ([#954](https://github.com/smoltcp-rs/smoltcp/pull/954))
 
 
 ## [0.11.0] - 2023-12-23
 ## [0.11.0] - 2023-12-23
 
 

+ 62 - 68
src/iface/interface/mod.rs

@@ -396,6 +396,13 @@ impl Interface {
     /// This function returns a boolean value indicating whether any packets were
     /// This function returns a boolean value indicating whether any packets were
     /// processed or emitted, and thus, whether the readiness of any socket might
     /// processed or emitted, and thus, whether the readiness of any socket might
     /// have changed.
     /// have changed.
+    ///
+    /// # Note
+    /// This function performs a bounded amount of work per call to avoid
+    /// starving other tasks of CPU time. If it returns true, there may still be
+    /// packets to be received or transmitted. Depending on system design,
+    /// calling this function in a loop may cause a denial of service if
+    /// packets cannot be processed faster than they arrive.
     pub fn poll<D>(
     pub fn poll<D>(
         &mut self,
         &mut self,
         timestamp: Instant,
         timestamp: Instant,
@@ -429,23 +436,12 @@ impl Interface {
             }
             }
         }
         }
 
 
-        let mut readiness_may_have_changed = false;
-
-        loop {
-            let mut did_something = false;
-            did_something |= self.socket_ingress(device, sockets);
-            did_something |= self.socket_egress(device, sockets);
-
-            #[cfg(feature = "proto-igmp")]
-            {
-                did_something |= self.igmp_egress(device);
-            }
+        let mut readiness_may_have_changed = self.socket_ingress(device, sockets);
+        readiness_may_have_changed |= self.socket_egress(device, sockets);
 
 
-            if did_something {
-                readiness_may_have_changed = true;
-            } else {
-                break;
-            }
+        #[cfg(feature = "proto-igmp")]
+        {
+            readiness_may_have_changed |= self.igmp_egress(device);
         }
         }
 
 
         readiness_may_have_changed
         readiness_may_have_changed
@@ -507,67 +503,65 @@ impl Interface {
     {
     {
         let mut processed_any = false;
         let mut processed_any = false;
 
 
-        while let Some((rx_token, tx_token)) = device.receive(self.inner.now) {
-            let rx_meta = rx_token.meta();
-            rx_token.consume(|frame| {
-                if frame.is_empty() {
-                    return;
-                }
+        let Some((rx_token, tx_token)) = device.receive(self.inner.now) else {
+            return processed_any;
+        };
 
 
-                match self.inner.caps.medium {
-                    #[cfg(feature = "medium-ethernet")]
-                    Medium::Ethernet => {
-                        if let Some(packet) = self.inner.process_ethernet(
-                            sockets,
-                            rx_meta,
-                            frame,
-                            &mut self.fragments,
-                        ) {
-                            if let Err(err) =
-                                self.inner.dispatch(tx_token, packet, &mut self.fragmenter)
-                            {
-                                net_debug!("Failed to send response: {:?}", err);
-                            }
+        let rx_meta = rx_token.meta();
+        rx_token.consume(|frame| {
+            if frame.is_empty() {
+                return;
+            }
+
+            match self.inner.caps.medium {
+                #[cfg(feature = "medium-ethernet")]
+                Medium::Ethernet => {
+                    if let Some(packet) =
+                        self.inner
+                            .process_ethernet(sockets, rx_meta, frame, &mut self.fragments)
+                    {
+                        if let Err(err) =
+                            self.inner.dispatch(tx_token, packet, &mut self.fragmenter)
+                        {
+                            net_debug!("Failed to send response: {:?}", err);
                         }
                         }
                     }
                     }
-                    #[cfg(feature = "medium-ip")]
-                    Medium::Ip => {
-                        if let Some(packet) =
-                            self.inner
-                                .process_ip(sockets, rx_meta, frame, &mut self.fragments)
-                        {
-                            if let Err(err) = self.inner.dispatch_ip(
-                                tx_token,
-                                PacketMeta::default(),
-                                packet,
-                                &mut self.fragmenter,
-                            ) {
-                                net_debug!("Failed to send response: {:?}", err);
-                            }
+                }
+                #[cfg(feature = "medium-ip")]
+                Medium::Ip => {
+                    if let Some(packet) =
+                        self.inner
+                            .process_ip(sockets, rx_meta, frame, &mut self.fragments)
+                    {
+                        if let Err(err) = self.inner.dispatch_ip(
+                            tx_token,
+                            PacketMeta::default(),
+                            packet,
+                            &mut self.fragmenter,
+                        ) {
+                            net_debug!("Failed to send response: {:?}", err);
                         }
                         }
                     }
                     }
-                    #[cfg(feature = "medium-ieee802154")]
-                    Medium::Ieee802154 => {
-                        if let Some(packet) = self.inner.process_ieee802154(
-                            sockets,
-                            rx_meta,
-                            frame,
-                            &mut self.fragments,
+                }
+                #[cfg(feature = "medium-ieee802154")]
+                Medium::Ieee802154 => {
+                    if let Some(packet) =
+                        self.inner
+                            .process_ieee802154(sockets, rx_meta, frame, &mut self.fragments)
+                    {
+                        if let Err(err) = self.inner.dispatch_ip(
+                            tx_token,
+                            PacketMeta::default(),
+                            packet,
+                            &mut self.fragmenter,
                         ) {
                         ) {
-                            if let Err(err) = self.inner.dispatch_ip(
-                                tx_token,
-                                PacketMeta::default(),
-                                packet,
-                                &mut self.fragmenter,
-                            ) {
-                                net_debug!("Failed to send response: {:?}", err);
-                            }
+                            net_debug!("Failed to send response: {:?}", err);
                         }
                         }
                     }
                     }
                 }
                 }
-                processed_any = true;
-            });
-        }
+            }
+            processed_any = true;
+        });
 
 
         processed_any
         processed_any
     }
     }