Skip to content

Commit

Permalink
pw_bluetooth_proxy: Remove non-allocator version of L2capCoc create
Browse files Browse the repository at this point in the history 
All downstream users now provide a multibuf allocator.

Removes code paths for non-multibuf rx and fixes some missing checks in
the multibuf code path. Two now obsolete tests are removed.

Change-Id: I542078c71719e6b027be06c8f152fc9a97542b28
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/262513
Docs-Not-Needed: Ben Lawson <benlawson@google.com>
Commit-Queue: Austin Foxley <afoxley@google.com>
Reviewed-by: Ben Lawson <benlawson@google.com>
Lint: Lint 🤖 <android-build-ayeaye@system.gserviceaccount.com>
  • Loading branch information
@afoxley
afoxley authored and CQ Bot Account committed Jan 24, 2025
1 parent 14ec5af commit 149e8c7
Show file tree
Hide file tree
Showing 8 changed files with 223 additions and 512 deletions.
242 changes: 89 additions & 153 deletions pw_bluetooth_proxy/l2cap_coc.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ L2capCoc::L2capCoc(L2capCoc&& other)
rx_mps_(other.rx_mps_),
tx_mtu_(other.tx_mtu_),
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_)) {
receive_fn_(std::move(other.receive_fn_)) {
{
std::lock_guard lock(tx_mutex_);
std::lock_guard other_lock(other.tx_mutex_);
Expand Down Expand Up @@ -85,11 +84,8 @@ pw::Result<L2capCoc> L2capCoc::Create(
uint16_t connection_handle,
CocConfig rx_config,
CocConfig tx_config,
Function<void(pw::span<uint8_t> payload)>&& payload_from_controller_fn,
Function<void(L2capChannelEvent event)>&& event_fn,
Function<void(multibuf::MultiBuf&& payload)>&& receive_fn_multibuf) {
PW_CHECK(!receive_fn_multibuf || !payload_from_controller_fn);

Function<void(multibuf::MultiBuf&& payload)>&& receive_fn) {
if (!AreValidParameters(/*connection_handle=*/connection_handle,
/*local_cid=*/rx_config.cid,
/*remote_cid=*/tx_config.cid)) {
Expand All @@ -112,9 +108,8 @@ pw::Result<L2capCoc> L2capCoc::Create(
/*connection_handle=*/connection_handle,
/*rx_config=*/rx_config,
/*tx_config=*/tx_config,
/*payload_from_controller_fn=*/std::move(payload_from_controller_fn),
/*event_fn=*/std::move(event_fn),
/*receive_fn_multibuf=*/std::move(receive_fn_multibuf));
/*receive_fn=*/std::move(receive_fn));
}

pw::Status L2capCoc::ReplenishRxCredits(uint16_t additional_rx_credits) {
Expand Down Expand Up @@ -149,8 +144,41 @@ pw::Status L2capCoc::SendAdditionalRxCredits(uint16_t additional_rx_credits) {
return status;
}

void L2capCoc::ProcessPduFromControllerMultibuf(span<uint8_t> kframe) {
bool L2capCoc::DoHandlePduFromController(pw::span<uint8_t> kframe) {
if (state() != State::kRunning) {
PW_LOG_ERROR(
"btproxy: L2capCoc::HandlePduFromController on non-running "
"channel. local_cid: %u, remote_cid: %u, state: %u",
local_cid(),
remote_cid(),
cpp23::to_underlying(state()));
StopAndSendEvent(L2capChannelEvent::kRxWhileStopped);
return true;
}

std::lock_guard lock(rx_mutex_);
rx_remaining_credits_--;

uint16_t rx_credits_used = rx_total_credits_ - rx_remaining_credits_;
if (rx_credits_used >=
std::ceil(rx_total_credits_ * kRxCreditReplenishThreshold)) {
Status status = ReplenishRxCredits(rx_credits_used);
if (status.IsUnavailable()) {
PW_LOG_INFO(
"Unable to send %hu rx credits to remote (it has %hu credits "
"remaining). Will try on next PDU receive.",
rx_credits_used,
rx_total_credits_);
} else if (status.IsFailedPrecondition()) {
PW_LOG_WARN(
"Unable to send rx credits to remote, perhaps the connection has "
"been closed?");
} else {
PW_CHECK(status.ok());
rx_remaining_credits_ += rx_credits_used;
}
}

ConstByteSpan kframe_payload;
if (rx_sdu_bytes_remaining_ > 0) {
// Received PDU that is part of current SDU being assembled.
Expand All @@ -159,6 +187,19 @@ void L2capCoc::ProcessPduFromControllerMultibuf(span<uint8_t> kframe) {
// Lower layers should not (and cannot) invoke this callback on a packet
// with an incomplete basic L2CAP header.
PW_CHECK_OK(subsequent_kframe_view);

// Core Spec v6.0 Vol 3, Part A, 3.4.3: "If the payload size of any K-frame
// exceeds the receiver's MPS, the receiver shall disconnect the channel."
uint16_t payload_size = subsequent_kframe_view->payload_size().Read();
if (payload_size > rx_mps_) {
PW_LOG_ERROR(
"(CID %#x) Rx K-frame payload exceeds MPU. So stopping channel & "
"reporting it needs to be closed.",
local_cid());
StopAndSendEvent(L2capChannelEvent::kRxInvalid);
return true;
}

kframe_payload =
as_bytes(span(subsequent_kframe_view->payload().BackingStorage().data(),
subsequent_kframe_view->payload_size().Read()));
Expand All @@ -172,10 +213,34 @@ void L2capCoc::ProcessPduFromControllerMultibuf(span<uint8_t> kframe) {
"channel and reporting it needs to be closed.",
local_cid());
StopAndSendEvent(L2capChannelEvent::kRxInvalid);
return;
return true;
}

rx_sdu_bytes_remaining_ = first_kframe_view->sdu_length().Read();

// Core Spec v6.0 Vol 3, Part A, 3.4.3: "If the SDU length field value
// exceeds the receiver's MTU, the receiver shall disconnect the channel."
if (rx_sdu_bytes_remaining_ > rx_mtu_) {
PW_LOG_ERROR(
"(CID %#x) Rx K-frame SDU exceeds MTU. So stopping channel & "
"reporting it needs to be closed.",
local_cid());
StopAndSendEvent(L2capChannelEvent::kRxInvalid);
return true;
}

// Core Spec v6.0 Vol 3, Part A, 3.4.3: "If the payload size of any K-frame
// exceeds the receiver's MPS, the receiver shall disconnect the channel."
uint16_t payload_size = first_kframe_view->payload_size().Read();
if (payload_size > rx_mps_) {
PW_LOG_ERROR(
"(CID %#x) Rx K-frame payload exceeds MPU. So stopping channel & "
"reporting it needs to be closed.",
local_cid());
StopAndSendEvent(L2capChannelEvent::kRxInvalid);
return true;
}

rx_sdu_ =
rx_multibuf_allocator_.AllocateContiguous(rx_sdu_bytes_remaining_);
if (!rx_sdu_) {
Expand All @@ -184,7 +249,7 @@ void L2capCoc::ProcessPduFromControllerMultibuf(span<uint8_t> kframe) {
"and reporting it needs to be closed.",
local_cid());
StopAndSendEvent(L2capChannelEvent::kRxOutOfMemory);
return;
return true;
}

kframe_payload =
Expand All @@ -204,7 +269,7 @@ void L2capCoc::ProcessPduFromControllerMultibuf(span<uint8_t> kframe) {
"length. So stopping channel and reporting it needs to be closed.",
local_cid());
StopAndSendEvent(L2capChannelEvent::kRxInvalid);
return;
return true;
}
PW_CHECK_OK(status);

Expand All @@ -213,139 +278,13 @@ void L2capCoc::ProcessPduFromControllerMultibuf(span<uint8_t> kframe) {

if (rx_sdu_bytes_remaining_ == 0) {
// We have a full SDU, so invoke client callback.
receive_fn_multibuf_(std::move(*rx_sdu_));
if (receive_fn_) {
receive_fn_(std::move(*rx_sdu_));
}
rx_sdu_ = std::nullopt;
rx_sdu_offset_ = 0;
}
}

bool L2capCoc::DoHandlePduFromController(pw::span<uint8_t> kframe) {
if (state() != State::kRunning) {
PW_LOG_ERROR(
"btproxy: L2capCoc::HandlePduFromController on non-running "
"channel. local_cid: %u, remote_cid: %u, state: %u",
local_cid(),
remote_cid(),
cpp23::to_underlying(state()));
StopAndSendEvent(L2capChannelEvent::kRxWhileStopped);
return true;
}

{
std::lock_guard lock(rx_mutex_);
rx_remaining_credits_--;

uint16_t rx_credits_used = rx_total_credits_ - rx_remaining_credits_;
if (rx_credits_used >=
std::ceil(rx_total_credits_ * kRxCreditReplenishThreshold)) {
Status status = ReplenishRxCredits(rx_credits_used);
if (status.IsUnavailable()) {
PW_LOG_INFO(
"Unable to send %hu rx credits to remote (it has %hu credits "
"remaining). Will try on next PDU receive.",
rx_credits_used,
rx_total_credits_);
} else if (status.IsFailedPrecondition()) {
PW_LOG_WARN(
"Unable to send rx credits to remote, perhaps the connection has "
"been closed?");
} else {
PW_CHECK(status.ok());
rx_remaining_credits_ += rx_credits_used;
}
}
}

// TODO: https://pwbug.dev/369849508 - Make this the only path once clients
// move to MultiBuf Write() API.
if (receive_fn_multibuf_) {
ProcessPduFromControllerMultibuf(kframe);
return true;
}

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) {
Result<emboss::SubsequentKFrameView> subsequent_kframe_view =
MakeEmbossView<emboss::SubsequentKFrameView>(kframe);
if (!subsequent_kframe_view.ok()) {
PW_LOG_ERROR(
"(CID %#x) Buffer is too small for subsequent L2CAP K-frame. So "
"will drop.",
local_cid());
return true;
}
PW_LOG_INFO("(CID %#x) Dropping PDU that is part of current segmented SDU.",
local_cid());
if (subsequent_kframe_view->payload_size().Read() >
remaining_sdu_bytes_to_ignore_) {
// Core Spec v6.0 Vol 3, Part A, 3.4.3: "If the sum of the payload sizes
// for the K-frames exceeds the specified SDU length, the receiver shall
// disconnect the channel."
PW_LOG_ERROR(
"(CID %#x) Sum of K-frame payload sizes exceeds the specified SDU "
"length. So stopping channel & reporting it needs to be closed.",
local_cid());
StopAndSendEvent(L2capChannelEvent::kRxInvalid);
} else {
remaining_sdu_bytes_to_ignore_ -=
subsequent_kframe_view->payload_size().Read();
}
return true;
}

Result<emboss::FirstKFrameView> kframe_view =
MakeEmbossView<emboss::FirstKFrameView>(kframe);
if (!kframe_view.ok()) {
PW_LOG_ERROR(
"(CID %#x) Buffer is too small for L2CAP K-frame. So stopping channel "
"& reporting it needs to be closed.",
local_cid());
StopAndSendEvent(L2capChannelEvent::kRxInvalid);
return true;
}
uint16_t sdu_length = kframe_view->sdu_length().Read();
uint16_t payload_size = kframe_view->payload_size().Read();

// Core Spec v6.0 Vol 3, Part A, 3.4.3: "If the SDU length field value exceeds
// the receiver's MTU, the receiver shall disconnect the channel."
if (sdu_length > rx_mtu_) {
PW_LOG_ERROR(
"(CID %#x) Rx K-frame SDU exceeds MTU. So stopping channel & "
"reporting it needs to be closed.",
local_cid());
StopAndSendEvent(L2capChannelEvent::kRxInvalid);
return true;
}

// TODO: https://pwbug.dev/360932103 - Support SDU de-segmentation.
// We don't support SDU de-segmentation yet. If we see a SDU size larger than
// the current PDU size, we ignore that first PDU and all remaining PDUs for
// that SDU (which we track via remaining bytes expected for the SDU).
if (sdu_length > payload_size) {
PW_LOG_ERROR(
"(CID %#x) Encountered segmented L2CAP SDU (which is not yet "
"supported). So will drop all PDUs in SDU.",
local_cid());
remaining_sdu_bytes_to_ignore_ = sdu_length - payload_size;
return true;
}

// Core Spec v6.0 Vol 3, Part A, 3.4.3: "If the payload size of any K-frame
// exceeds the receiver's MPS, the receiver shall disconnect the channel."
if (payload_size > rx_mps_) {
PW_LOG_ERROR(
"(CID %#x) Rx K-frame payload exceeds MPU. So stopping channel & "
"reporting it needs to be closed.",
local_cid());
StopAndSendEvent(L2capChannelEvent::kRxInvalid);
return true;
}

SendPayloadFromControllerToClient(pw::span(
const_cast<uint8_t*>(kframe_view->payload().BackingStorage().data()),
kframe_view->payload_size().Read()));
return true;
}

Expand All @@ -354,16 +293,14 @@ bool L2capCoc::HandlePduFromHost(pw::span<uint8_t>) {
return false;
}

L2capCoc::L2capCoc(
pw::multibuf::MultiBufAllocator& rx_multibuf_allocator,
L2capChannelManager& l2cap_channel_manager,
L2capSignalingChannel* signaling_channel,
uint16_t connection_handle,
CocConfig rx_config,
CocConfig tx_config,
Function<void(pw::span<uint8_t> payload)>&& payload_from_controller_fn,
Function<void(L2capChannelEvent event)>&& event_fn,
Function<void(multibuf::MultiBuf&& payload)>&& receive_fn_multibuf)
L2capCoc::L2capCoc(pw::multibuf::MultiBufAllocator& rx_multibuf_allocator,
L2capChannelManager& l2cap_channel_manager,
L2capSignalingChannel* signaling_channel,
uint16_t connection_handle,
CocConfig rx_config,
CocConfig tx_config,
Function<void(L2capChannelEvent event)>&& event_fn,
Function<void(multibuf::MultiBuf&& payload)>&& receive_fn)
: L2capChannel(
/*l2cap_channel_manager=*/l2cap_channel_manager,
/*connection_handle=*/connection_handle,
Expand All @@ -379,8 +316,7 @@ L2capCoc::L2capCoc(
rx_mps_(rx_config.mps),
tx_mtu_(tx_config.mtu),
tx_mps_(tx_config.mps),
payload_from_controller_fn_(std::move(payload_from_controller_fn)),
receive_fn_multibuf_(std::move(receive_fn_multibuf)),
receive_fn_(std::move(receive_fn)),
rx_remaining_credits_(rx_config.credits),
rx_total_credits_(rx_config.credits),
tx_credits_(tx_config.credits) {
Expand Down
Loading

0 comments on commit 149e8c7

Please sign in to comment.