pw_bluetooth_proxy: Separate l2cap coc mutex into tx and rx

Change-Id: I397efaae525d093cbdc3303db2861519aef2ecd1
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/256095
Docs-Not-Needed: Ali Saeed <[email protected]>
Pigweed-Auto-Submit: David Rees <[email protected]>
Commit-Queue: David Rees <[email protected]>
Docs-Not-Needed: David Rees <[email protected]>
Reviewed-by: Ali Saeed <[email protected]>
Presubmit-Verified: CQ Bot Account <[email protected]>
Lint: Lint 🤖 <[email protected]>

pw_bluetooth_proxy/l2cap_coc.cc

@@ -40,13 +40,19 @@ L2capCoc::L2capCoc(L2capCoc&& other)
       tx_mps_(other.tx_mps_),
       payload_from_controller_fn_(std::move(other.payload_from_controller_fn_)),
       receive_fn_multibuf_(std::move(other.receive_fn_multibuf_)) {
-  std::lock_guard lock(mutex_);
-  std::lock_guard other_lock(other.mutex_);
-  tx_credits_ = other.tx_credits_;
-  remaining_sdu_bytes_to_ignore_ = other.remaining_sdu_bytes_to_ignore_;
-  rx_sdu_ = std::move(other.rx_sdu_);
-  rx_sdu_offset_ = other.rx_sdu_offset_;
-  rx_sdu_bytes_remaining_ = other.rx_sdu_bytes_remaining_;
+  {
+    std::lock_guard lock(tx_mutex_);
+    std::lock_guard other_lock(other.tx_mutex_);
+    tx_credits_ = other.tx_credits_;
+  }
+  {
+    std::lock_guard lock(rx_mutex_);
+    std::lock_guard other_lock(other.rx_mutex_);
+    remaining_sdu_bytes_to_ignore_ = other.remaining_sdu_bytes_to_ignore_;
+    rx_sdu_ = std::move(other.rx_sdu_);
+    rx_sdu_offset_ = other.rx_sdu_offset_;
+    rx_sdu_bytes_remaining_ = other.rx_sdu_bytes_remaining_;
+  }
 }
 
 pw::Status L2capCoc::Write(pw::span<const uint8_t> payload) {
@@ -178,7 +184,7 @@ pw::Status L2capCoc::SendAdditionalRxCredits(uint16_t additional_rx_credits) {
 }
 
 void L2capCoc::ProcessPduFromControllerMultibuf(span<uint8_t> kframe) {
-  std::lock_guard lock(mutex_);
+  std::lock_guard lock(rx_mutex_);
   ConstByteSpan kframe_payload;
   if (rx_sdu_bytes_remaining_ > 0) {
     // Received PDU that is part of current SDU being assembled.
@@ -261,7 +267,7 @@ bool L2capCoc::HandlePduFromController(pw::span<uint8_t> kframe) {
     return true;
   }
 
-  std::lock_guard lock(mutex_);
+  std::lock_guard lock(rx_mutex_);
   // If `remaining_sdu_bytes_to_ignore_` is nonzero, we are in state where we
   // are dropping continuing PDUs in a segmented SDU.
   if (remaining_sdu_bytes_to_ignore_ > 0) {
@@ -348,7 +354,7 @@ bool L2capCoc::HandlePduFromController(pw::span<uint8_t> kframe) {
   return true;
 }
 
-bool L2capCoc::HandlePduFromHost(pw::span<uint8_t>) PW_LOCKS_EXCLUDED(mutex_) {
+bool L2capCoc::HandlePduFromHost(pw::span<uint8_t>) {
   // Always forward data from host to controller
   return false;
 }
@@ -386,7 +392,7 @@ std::optional<H4PacketWithH4> L2capCoc::DequeuePacket() {
     return std::nullopt;
   }
 
-  std::lock_guard lock(mutex_);
+  std::lock_guard lock(tx_mutex_);
   if (tx_credits_ == 0) {
     return std::nullopt;
   }
@@ -408,7 +414,7 @@ void L2capCoc::AddCredits(uint16_t credits) {
 
   bool credits_previously_zero;
   {
-    std::lock_guard lock(mutex_);
+    std::lock_guard lock(tx_mutex_);
 
     // Core Spec v6.0 Vol 3, Part A, 10.1: "The device receiving the credit
     // packet shall disconnect the L2CAP channel if the credit count exceeds

pw_bluetooth_proxy/public/pw_bluetooth_proxy/l2cap_coc.h

@@ -14,6 +14,8 @@
 
 #pragma once
 
+#include <optional>
+
 #include "pw_allocator/allocator.h"
 #include "pw_bluetooth_proxy/internal/l2cap_channel.h"
 #include "pw_bluetooth_proxy/internal/l2cap_signaling_channel.h"
@@ -125,13 +127,12 @@ class L2capCoc : public L2capChannel {
   // `SendPayloadFromControllerToClient` with the information payload contained
   // in `kframe`.
   bool HandlePduFromController(pw::span<uint8_t> kframe) override
-      PW_LOCKS_EXCLUDED(mutex_);
+      PW_LOCKS_EXCLUDED(rx_mutex_);
 
-  bool HandlePduFromHost(pw::span<uint8_t> kframe) override
-      PW_LOCKS_EXCLUDED(mutex_);
+  bool HandlePduFromHost(pw::span<uint8_t> kframe) override;
 
-  // Increment `send_credits_` by `credits`.
-  void AddCredits(uint16_t credits) PW_LOCKS_EXCLUDED(mutex_);
+  // Increment tx credits by `credits`.
+  void AddCredits(uint16_t credits) PW_LOCKS_EXCLUDED(tx_mutex_);
 
  private:
   explicit L2capCoc(
@@ -147,7 +148,7 @@ class L2capCoc : public L2capChannel {
 
   // Override: Dequeue a packet only if a credit is able to be subtracted.
   std::optional<H4PacketWithH4> DequeuePacket() override
-      PW_LOCKS_EXCLUDED(mutex_);
+      PW_LOCKS_EXCLUDED(tx_mutex_);
 
   bool SendPayloadFromControllerToClient(pw::span<uint8_t> payload) override {
     if (payload_from_controller_fn_) {
@@ -157,23 +158,28 @@ class L2capCoc : public L2capChannel {
   }
 
   void ProcessPduFromControllerMultibuf(span<uint8_t> kframe)
-      PW_LOCKS_EXCLUDED(mutex_);
+      PW_LOCKS_EXCLUDED(rx_mutex_);
 
   multibuf::MultiBufAllocator& rx_multibuf_allocator_;
   L2capSignalingChannel* signaling_channel_;
+
   uint16_t rx_mtu_;
   uint16_t rx_mps_;
   uint16_t tx_mtu_;
   uint16_t tx_mps_;
+
   Function<void(pw::span<uint8_t> payload)> payload_from_controller_fn_;
   Function<void(multibuf::MultiBuf&& payload)> receive_fn_multibuf_;
 
-  sync::Mutex mutex_;
-  uint16_t tx_credits_ PW_GUARDED_BY(mutex_);
-  uint16_t remaining_sdu_bytes_to_ignore_ PW_GUARDED_BY(mutex_) = 0;
-  std::optional<multibuf::MultiBuf> rx_sdu_ PW_GUARDED_BY(mutex_);
-  uint16_t rx_sdu_offset_ PW_GUARDED_BY(mutex_) = 0;
-  uint16_t rx_sdu_bytes_remaining_ PW_GUARDED_BY(mutex_) = 0;
+  sync::Mutex rx_mutex_;
+  uint16_t remaining_sdu_bytes_to_ignore_ PW_GUARDED_BY(rx_mutex_) = 0;
+  std::optional<multibuf::MultiBuf> rx_sdu_ PW_GUARDED_BY(rx_mutex_) =
+      std::nullopt;
+  uint16_t rx_sdu_offset_ PW_GUARDED_BY(rx_mutex_) = 0;
+  uint16_t rx_sdu_bytes_remaining_ PW_GUARDED_BY(rx_mutex_) = 0;
+
+  sync::Mutex tx_mutex_;
+  uint16_t tx_credits_ PW_GUARDED_BY(tx_mutex_);
 };
 
 }  // namespace pw::bluetooth::proxy