Skip to content

Commit

Permalink
pw_bluetooth_proxy: Use LE_ACL_Data_Packet_Length
Browse files Browse the repository at this point in the history
Bug: 379339642, 360932103
Change-Id: I47c2ea40ef9e10111c9f2ac8cc00b6319950194b
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/254673
Commit-Queue: Ali Saeed <saeedali@google.com>
Reviewed-by: David Rees <drees@google.com>
Lint: Lint 🤖 <android-build-ayeaye@system.gserviceaccount.com>
Reviewed-by: Ben Lawson <benlawson@google.com>
Docs-Not-Needed: Ben Lawson <benlawson@google.com>
  • Loading branch information
acsaeed authored and CQ Bot Account committed Dec 19, 2024
1 parent 48dfcf5 commit 8269056
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 11 deletions.
11 changes: 11 additions & 0 deletions pw_bluetooth_proxy/acl_data_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,17 @@ void AclDataChannel::ProcessSpecificLEReadBufferSizeCommandCompleteEvent(
read_buffer_event.total_num_le_acl_data_packets().Write(host_max);
}

const uint16_t le_acl_data_packet_length =
read_buffer_event.le_acl_data_packet_length().Read();
// TODO: https://pwbug.dev/380316252 - Support shared buffers.
if (le_acl_data_packet_length == 0) {
PW_LOG_ERROR(
"Controller shares data buffers between BR/EDR and LE transport, which "
"is not yet supported. So channels on LE transport will not be "
"functional.");
}
l2cap_channel_manager_.set_le_acl_data_packet_length(
le_acl_data_packet_length);
// Send packets that may have queued before we acquired any LE ACL credits.
l2cap_channel_manager_.DrainChannelQueues();
}
Expand Down
11 changes: 10 additions & 1 deletion pw_bluetooth_proxy/gatt_notify_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,17 @@

namespace pw::bluetooth::proxy {
pw::Status GattNotifyChannel::Write(pw::span<const uint8_t> attribute_value) {
std::optional<uint16_t> max_l2cap_payload_size = MaxL2capPayloadSize();
if (!max_l2cap_payload_size) {
PW_LOG_ERROR("Tried to write before LE_Read_Buffer_Size processed.");
return Status::FailedPrecondition();
}
if (*max_l2cap_payload_size <= emboss::AttHandleValueNtf::MinSizeInBytes()) {
PW_LOG_ERROR("LE ACL data packet size limit does not support writing.");
return Status::FailedPrecondition();
}
const uint16_t max_attribute_size =
MaxL2capPayloadSize() - emboss::AttHandleValueNtf::MinSizeInBytes();
*max_l2cap_payload_size - emboss::AttHandleValueNtf::MinSizeInBytes();
if (attribute_value.size() > max_attribute_size) {
PW_LOG_ERROR("Attribute too large (%zu > %d). So will not process.",
attribute_value.size(),
Expand Down
17 changes: 13 additions & 4 deletions pw_bluetooth_proxy/l2cap_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,19 @@ pw::Result<H4PacketWithH4> L2capChannel::PopulateL2capPacket(
return h4_packet;
}

uint16_t L2capChannel::MaxL2capPayloadSize() const {
return l2cap_channel_manager_.GetH4BuffSize() - sizeof(emboss::H4PacketType) -
emboss::AclDataFrameHeader::IntrinsicSizeInBytes() -
emboss::BasicL2capHeader::IntrinsicSizeInBytes();
std::optional<uint16_t> L2capChannel::MaxL2capPayloadSize() const {
std::optional<uint16_t> le_acl_data_packet_length =
l2cap_channel_manager_.le_acl_data_packet_length();
if (!le_acl_data_packet_length) {
return std::nullopt;
}

uint16_t max_acl_data_size_based_on_h4_buffer =
l2cap_channel_manager_.GetH4BuffSize() - sizeof(emboss::H4PacketType) -
emboss::AclDataFrameHeader::IntrinsicSizeInBytes();
uint16_t max_acl_data_size = std::min(max_acl_data_size_based_on_h4_buffer,
*le_acl_data_packet_length);
return max_acl_data_size - emboss::BasicL2capHeader::IntrinsicSizeInBytes();
}

void L2capChannel::ReportPacketsMayBeReadyToSend() {
Expand Down
1 change: 1 addition & 0 deletions pw_bluetooth_proxy/proxy_host_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,7 @@ TEST_F(GattNotifyTest, ReturnsErrorIfAttributeTooLarge) {
std::move(send_to_controller_fn),
/*le_acl_credits_to_reserve=*/0,
/*br_edr_acl_credits_to_reserve=*/0);
PW_TEST_EXPECT_OK(SendLeReadBufferResponseFromController(proxy, 0));

// attribute_value 1 byte too large
std::array<uint8_t,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ class GattNotifyChannel : public L2capChannel {
/// UNAVAILABLE: If channel could not acquire the resources to queue the send
/// at this time (transient error).
/// INVALID_ARGUMENT: If `attribute_value` is too large.
/// FAILED_PRECONDITION: If channel is not `State::kRunning`.
/// FAILED_PRECONDITION: If channel is not `State::kRunning` or ACL data
/// channel has not yet been initialized (see logs).
/// @endrst
pw::Status Write(pw::span<const uint8_t> attribute_value);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,10 @@ class L2capChannel : public IntrusiveForwardList<L2capChannel>::Item {
pw::Result<H4PacketWithH4> PopulateTxL2capPacket(uint16_t data_length);

// Returns the maximum size supported for Tx L2CAP PDU payloads.
uint16_t MaxL2capPayloadSize() const;
//
// Returns std::nullopt if LE_ACL_Data_Packet_Length was not yet provided in
// an LE_Read_Buffer_Size command complete event.
std::optional<uint16_t> MaxL2capPayloadSize() const;

// Alert `L2capChannelManager` that queued packets may be ready to send.
// When calling this method, ensure no locks are held that are also acquired
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

#pragma once

#include <atomic>

#include "pw_bluetooth_proxy/internal/acl_data_channel.h"
#include "pw_bluetooth_proxy/internal/h4_storage.h"
#include "pw_bluetooth_proxy/internal/l2cap_channel.h"
Expand Down Expand Up @@ -83,6 +85,24 @@ class L2capChannelManager {
void HandleDisconnectionComplete(
const L2capStatusTracker::DisconnectParams& params);

// Core Spec v6.0 Vol 4, Part E, Section 7.8.2: "The LE_ACL_Data_Packet_Length
// parameter shall be used to determine the maximum size of the L2CAP PDU
// fragments that are contained in ACL data packets". A value of 0 means "No
// dedicated LE Buffer exists".
//
// Return std::nullopt if HCI_LE_Read_Buffer_Size command complete event has
// not yet been received.
//
// TODO: https://pwbug.dev/379339642 - Add tests to confirm this value caps
// the size of Tx L2capCoc segments when segmentation is implemented.
std::optional<uint16_t> le_acl_data_packet_length() const {
return le_acl_data_packet_length_;
}

void set_le_acl_data_packet_length(uint16_t le_acl_data_packet_length) {
le_acl_data_packet_length_ = le_acl_data_packet_length;
}

private:
// Circularly advance `it`, wrapping around to front if `it` reaches the end.
void Advance(IntrusiveForwardList<L2capChannel>::iterator& it)
Expand All @@ -99,6 +119,8 @@ class L2capChannelManager {
// Owns H4 packet buffers.
H4Storage h4_storage_;

std::atomic<std::optional<uint16_t>> le_acl_data_packet_length_;

// Enforce mutual exclusion of all operations on channels.
sync::Mutex channels_mutex_;

Expand Down
6 changes: 4 additions & 2 deletions pw_bluetooth_proxy/pw_bluetooth_proxy_private/test_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,10 @@ Result<EmbossT> CreateAndPopulateToHostEventView(H4PacketWithHci& h4_packet,

// Send an LE_Read_Buffer_Size (V2) CommandComplete event to `proxy` to request
// the reservation of a number of LE ACL send credits.
Status SendLeReadBufferResponseFromController(ProxyHost& proxy,
uint8_t num_credits_to_reserve);
Status SendLeReadBufferResponseFromController(
ProxyHost& proxy,
uint8_t num_credits_to_reserve,
uint16_t le_acl_data_packet_length = 251);

Status SendReadBufferResponseFromController(ProxyHost& proxy,
uint8_t num_credits_to_reserve);
Expand Down
7 changes: 5 additions & 2 deletions pw_bluetooth_proxy/test_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,10 @@ Result<KFrameWithStorage> SetupKFrame(uint16_t handle,

// Send an LE_Read_Buffer_Size (V2) CommandComplete event to `proxy` to request
// the reservation of a number of LE ACL send credits.
Status SendLeReadBufferResponseFromController(ProxyHost& proxy,
uint8_t num_credits_to_reserve) {
Status SendLeReadBufferResponseFromController(
ProxyHost& proxy,
uint8_t num_credits_to_reserve,
uint16_t le_acl_data_packet_length) {
std::array<
uint8_t,
emboss::LEReadBufferSizeV2CommandCompleteEventWriter::SizeInBytes()>
Expand All @@ -169,6 +171,7 @@ Status SendLeReadBufferResponseFromController(ProxyHost& proxy,
view.command_complete().command_opcode_enum().Write(
emboss::OpCode::LE_READ_BUFFER_SIZE_V2);
view.total_num_le_acl_data_packets().Write(num_credits_to_reserve);
view.le_acl_data_packet_length().Write(le_acl_data_packet_length);

proxy.HandleH4HciFromController(std::move(h4_packet));
return OkStatus();
Expand Down

0 comments on commit 8269056

Please sign in to comment.