Merge pull request #77 from embassy-rs/acl-error-handling

Improve ACL processing error handling
embassy-rs · Aug 12, 2024 · 5e5eac8 · 5e5eac8
2 parents 9a99351 + 9a3818e
commit 5e5eac8
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 11 deletions.
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
@@ -34,7 +34,7 @@ jobs:
           RUST_LOG: trace
         run: |
           cd host
-          cargo test --test '*' -- --nocapture
+          cargo test --features log --test '*' -- --nocapture
       - name: Update failed status
         if: failure()
         env:

diff --git a/host/src/channel_manager.rs b/host/src/channel_manager.rs
@@ -289,6 +289,9 @@ impl<'d, const RXQ: usize> ChannelManager<'d, RXQ> {
                 match storage.state {
                     ChannelState::Connected if header.channel == storage.cid => {
                         if storage.flow_control.available() == 0 {
+                            // NOTE: This will trigger closing of the link, which might be a bit
+                            // too strict. But it should be controllable via the credits given,
+                            // which the remote should respect.
                             trace!("[l2cap][cid = {}] no credits available", header.channel);
                             return Err(Error::OutOfMemory);
                         }

diff --git a/host/src/host.rs b/host/src/host.rs
@@ -814,7 +814,7 @@ where
                     && !(&[L2CAP_CID_LE_U_SIGNAL, L2CAP_CID_ATT].contains(&header.channel))
                 {
                     warn!("[host] unsupported l2cap channel id {}", header.channel);
-                    return Ok(());
+                    return Err(Error::NotSupported);
                 }
 
                 // Avoids using the packet buffer for signalling packets
@@ -899,10 +899,11 @@ where
                 Ok(_) => {}
                 Err(e) => {
                     warn!("Error dispatching l2cap packet to channel: {:?}", e);
+                    return Err(e);
                 }
             },
             _ => {
-                unimplemented!()
+                return Err(Error::NotSupported);
             }
         }
         Ok(())
@@ -1054,14 +1055,33 @@ where
                     Ok(ControllerToHostPacket::Acl(acl)) => match self.handle_acl(acl) {
                         Ok(_) => {}
                         Err(e) => {
-                            trace!("Error processing ACL packet: {:?}", e);
-                            if let Error::OutOfMemory = e {
-                                warn!("[host] out of memory error on handle {:?}, disconnecting", acl.handle());
-                                self.connections.request_handle_disconnect(
-                                    acl.handle(),
-                                    DisconnectReason::RemoteDeviceTerminatedConnLowResources,
-                                );
-                            }
+                            // We disconnect on errors to ensure we don't leave the other end thinking
+                            // everything is ok.
+                            let reason = match e {
+                                Error::OutOfMemory => {
+                                    // Disconnect link due to low resources.
+                                    warn!("[host] out of memory error when processing ACL packet");
+                                    DisconnectReason::RemoteDeviceTerminatedConnLowResources
+                                }
+                                Error::Disconnected => {
+                                    // Already disconnected, request a disconnect to ensure we don't have
+                                    // any lingering connections, should be a noop
+                                    warn!("[host] already disconnected when processing ACL packet");
+                                    DisconnectReason::RemoteUserTerminatedConn
+                                }
+                                Error::NotSupported => {
+                                    // Attempt to use a feature not supported
+                                    warn!("[host] attempted to use unsupported feature when processing ACL packet");
+                                    DisconnectReason::UnsupportedRemoteFeature
+                                }
+                                e => {
+                                    // Otherwise blame the user.
+                                    warn!("[host] encountered error processing ACL packet: {:?}", e);
+                                    DisconnectReason::RemoteUserTerminatedConn
+                                }
+                            };
+                            warn!("[host] disconnecting handle {:?} after error", acl.handle());
+                            self.connections.request_handle_disconnect(acl.handle(), reason);
                             let mut m = self.metrics.borrow_mut();
                             m.rx_errors = m.rx_errors.wrapping_add(1);
                         }