Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit aa17a7c0 authored by Kyunglyul Hyun's avatar Kyunglyul Hyun
Browse files

Drop pending fragments for disconnected connection

This change addresses an issue where the
round_robin_scheduler becomes stuck in
an invalid state under heavy traffic conditions.

The problem occurs when a large packet is fragmented,
and these fragments await data buffer flushing but
the connection gets disconnected.

In this scenario, fragments_to_send_ never becomes empty,
preventing further packet processing due to
the absence of HCI_Number_Of_Completed_Packets events.

This is resolved by explictly dropping packet fragments
upon disconnection and restart round robin.

A test has been added to verify the fix.

Bug: 376379859
Bug: 336107032
Test: atest RoundRobinSchedulerTest
Flag: com.android.bluetooth.flags.drop_acl_fragment_on_disconnect

Change-Id: Ic3099aa25cb71e69f1e43bb6581a3fb682565af9
parent a1f6bd5d
Loading
Loading
Loading
Loading
+6 −0
Original line number Diff line number Diff line
@@ -43,6 +43,12 @@ public:

  [[nodiscard]] size_t size() const { return next_to_dequeue_.size(); }

  // Swaps the contents with the given instance.
  void swap(MultiPriorityQueue& that) {
    queues_.swap(that.queues_);
    next_to_dequeue_.swap(that.next_to_dequeue_);
  }

  // Push the item with specified priority
  void push(const T& t, int priority = 0) {
    queues_[priority].push(t);
+61 −12
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@
#include "hci/acl_manager/round_robin_scheduler.h"

#include <bluetooth/log.h>
#include <com_android_bluetooth_flags.h>

#include <memory>
#include <utility>
@@ -63,13 +64,24 @@ void RoundRobinScheduler::Register(ConnectionType connection_type, uint16_t hand
void RoundRobinScheduler::Unregister(uint16_t handle) {
  log::assert_that(acl_queue_handlers_.count(handle) == 1,
                   "assert failed: acl_queue_handlers_.count(handle) == 1");
  auto acl_queue_handler = acl_queue_handlers_.find(handle)->second;

  if (com::android::bluetooth::flags::drop_acl_fragment_on_disconnect()) {
    // Drop the pending fragments and recalculate number_of_sent_packets_
    drop_packet_fragments(handle);
  }

  auto& acl_queue_handler = acl_queue_handlers_.find(handle)->second;
  log::info("unregistering acl_queue handle={}, sent_packets={}", handle,
            acl_queue_handler.number_of_sent_packets_);

  bool credits_reclaimed_from_zero = acl_queue_handler.number_of_sent_packets_ > 0;

  // Reclaim outstanding packets
  if (acl_queue_handler.connection_type_ == ConnectionType::CLASSIC) {
    credits_reclaimed_from_zero &= (acl_packet_credits_ == 0);
    acl_packet_credits_ += acl_queue_handler.number_of_sent_packets_;
  } else {
    credits_reclaimed_from_zero &= (le_acl_packet_credits_ == 0);
    le_acl_packet_credits_ += acl_queue_handler.number_of_sent_packets_;
  }
  acl_queue_handler.number_of_sent_packets_ = 0;
@@ -80,6 +92,12 @@ void RoundRobinScheduler::Unregister(uint16_t handle) {
  }
  acl_queue_handlers_.erase(handle);
  starting_point_ = acl_queue_handlers_.begin();

  // Restart sending packets if we got acl credits
  if (com::android::bluetooth::flags::drop_acl_fragment_on_disconnect() &&
      credits_reclaimed_from_zero) {
    start_round_robin();
  }
}

void RoundRobinScheduler::SetLinkPriority(uint16_t handle, bool high_priority) {
@@ -101,7 +119,7 @@ void RoundRobinScheduler::start_round_robin() {
    return;
  }
  if (!fragments_to_send_.empty()) {
    auto connection_type = fragments_to_send_.front().first;
    auto connection_type = fragments_to_send_.front().connection_type_;
    bool classic_buffer_full =
            acl_packet_credits_ == 0 && connection_type == ConnectionType::CLASSIC;
    bool le_buffer_full = le_acl_packet_credits_ == 0 && connection_type == ConnectionType::LE;
@@ -167,17 +185,17 @@ void RoundRobinScheduler::buffer_packet(uint16_t acl_handle) {

  int acl_priority = acl_queue_handler->second.high_priority_ ? 1 : 0;
  if (packet->size() <= mtu) {
    fragments_to_send_.push(
            std::make_pair(connection_type, AclBuilder::Create(handle, packet_boundary_flag,
                                                               broadcast_flag, std::move(packet))),
    fragments_to_send_.push(packet_fragment{connection_type, handle, acl_priority,
                                            AclBuilder::Create(handle, packet_boundary_flag,
                                                               broadcast_flag, std::move(packet))},
                            acl_priority);
  } else {
    auto fragments = AclFragmenter(mtu, std::move(packet)).GetFragments();
    for (size_t i = 0; i < fragments.size(); i++) {
      fragments_to_send_.push(
              std::make_pair(connection_type,
              packet_fragment{connection_type, handle, acl_priority,
                              AclBuilder::Create(handle, packet_boundary_flag, broadcast_flag,
                                                std::move(fragments[i]))),
                                                 std::move(fragments[i]))},
              acl_priority);
      packet_boundary_flag = PacketBoundaryFlag::CONTINUING_FRAGMENT;
    }
@@ -189,6 +207,36 @@ void RoundRobinScheduler::buffer_packet(uint16_t acl_handle) {
  send_next_fragment();
}

// Drops packet fragments associated with the given handle.
void RoundRobinScheduler::drop_packet_fragments(uint16_t acl_handle) {
  if (fragments_to_send_.empty()) {
    return;
  }
  auto acl_queue_handler = acl_queue_handlers_.find(acl_handle);

  decltype(fragments_to_send_) new_fragments_to_send;
  while (!fragments_to_send_.empty()) {
    auto& fragment = fragments_to_send_.front();

    if (fragment.handle_ == acl_handle) {
      // This fragment is not sent to the controller.
      acl_queue_handler->second.number_of_sent_packets_--;
    } else {
      new_fragments_to_send.push(packet_fragment{fragment.connection_type_, fragment.handle_,
                                                 fragment.priority_, std::move(fragment.packet_)},
                                 fragment.priority_);
    }
    fragments_to_send_.pop();
  }

  if (new_fragments_to_send.empty()) {
    if (enqueue_registered_.exchange(false)) {
      hci_queue_end_->UnregisterEnqueue();
    }
  }
  fragments_to_send_.swap(new_fragments_to_send);
}

void RoundRobinScheduler::unregister_all_connections() {
  for (auto acl_queue_handler = acl_queue_handlers_.begin();
       acl_queue_handler != acl_queue_handlers_.end();
@@ -210,7 +258,8 @@ void RoundRobinScheduler::send_next_fragment() {

// Invoked from some external Queue Reactable context 1
std::unique_ptr<AclBuilder> RoundRobinScheduler::handle_enqueue_next_fragment() {
  ConnectionType connection_type = fragments_to_send_.front().first;
  ConnectionType connection_type = fragments_to_send_.front().connection_type_;

  if (connection_type == ConnectionType::CLASSIC) {
    log::assert_that(acl_packet_credits_ > 0, "assert failed: acl_packet_credits_ > 0");
    acl_packet_credits_ -= 1;
@@ -219,7 +268,7 @@ std::unique_ptr<AclBuilder> RoundRobinScheduler::handle_enqueue_next_fragment()
    le_acl_packet_credits_ -= 1;
  }

  auto raw_pointer = fragments_to_send_.front().second.release();
  auto raw_pointer = fragments_to_send_.front().packet_.release();
  fragments_to_send_.pop();
  if (fragments_to_send_.empty()) {
    if (enqueue_registered_.exchange(false)) {
@@ -228,7 +277,7 @@ std::unique_ptr<AclBuilder> RoundRobinScheduler::handle_enqueue_next_fragment()
    handler_->Post(
            common::BindOnce(&RoundRobinScheduler::start_round_robin, common::Unretained(this)));
  } else {
    ConnectionType next_connection_type = fragments_to_send_.front().first;
    ConnectionType next_connection_type = fragments_to_send_.front().connection_type_;
    bool classic_buffer_full =
            next_connection_type == ConnectionType::CLASSIC && acl_packet_credits_ == 0;
    bool le_buffer_full = next_connection_type == ConnectionType::LE && le_acl_packet_credits_ == 0;
+9 −2
Original line number Diff line number Diff line
@@ -46,6 +46,13 @@ public:
    bool high_priority_ = false;           // For A2dp use
  };

  struct packet_fragment {
    ConnectionType connection_type_;
    uint16_t handle_;
    int priority_;
    std::unique_ptr<AclBuilder> packet_;
  };

  void Register(ConnectionType connection_type, uint16_t handle,
                std::shared_ptr<acl_manager::AclConnection::Queue> queue);
  void Unregister(uint16_t handle);
@@ -56,6 +63,7 @@ public:
private:
  void start_round_robin();
  void buffer_packet(uint16_t acl_handle);
  void drop_packet_fragments(uint16_t acl_handle);
  void unregister_all_connections();
  void send_next_fragment();
  std::unique_ptr<AclBuilder> handle_enqueue_next_fragment();
@@ -64,8 +72,7 @@ private:
  os::Handler* handler_ = nullptr;
  Controller* controller_ = nullptr;
  std::map<uint16_t, acl_queue_handler> acl_queue_handlers_;
  common::MultiPriorityQueue<std::pair<ConnectionType, std::unique_ptr<AclBuilder>>, 2>
          fragments_to_send_;
  common::MultiPriorityQueue<packet_fragment, 2> fragments_to_send_;
  uint16_t max_acl_packet_credits_ = 0;
  uint16_t acl_packet_credits_ = 0;
  uint16_t le_max_acl_packet_credits_ = 0;
+54 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@

#include "hci/acl_manager/round_robin_scheduler.h"

#include <com_android_bluetooth_flags.h>
#include <gtest/gtest.h>

#include "common/bidi_queue.h"
@@ -420,6 +421,59 @@ TEST_F(RoundRobinSchedulerTest, receive_le_credit_when_next_fragment_is_classic)
  round_robin_scheduler_->Unregister(le_handle);
}

TEST_F(RoundRobinSchedulerTest, unregister_reclaim_credits) {
  com::android::bluetooth::flags::provider_->drop_acl_fragment_on_disconnect(true);

  uint16_t handle = 0x01;
  auto connection_queue = std::make_shared<AclConnection::Queue>(20);
  auto new_connection_queue = std::make_shared<AclConnection::Queue>(20);

  round_robin_scheduler_->Register(RoundRobinScheduler::ConnectionType::CLASSIC, handle,
                                   connection_queue);

  ASSERT_NO_FATAL_FAILURE(SetPacketFuture(controller_->max_acl_packet_credits_));

  AclConnection::QueueUpEnd* queue_up_end = connection_queue->GetUpEnd();

  std::vector<uint8_t> huge_packet(2000);
  std::vector<uint8_t> packet = {0x01, 0x02, 0x03};
  std::vector<uint8_t> new_packet = {0x04, 0x05, 0x06};

  // Make acl_packet_credits_ = 0 and remain 1 acl fragment in fragments_to_send_
  for (uint16_t i = 0; i < controller_->max_acl_packet_credits_ - 1; i++) {
    EnqueueAclUpEnd(queue_up_end, packet);
  }
  EnqueueAclUpEnd(queue_up_end, huge_packet);

  packet_future_->wait();
  ASSERT_EQ(round_robin_scheduler_->GetCredits(), 0);

  // Credits should be reclaimed
  round_robin_scheduler_->Unregister(handle);
  ASSERT_EQ(round_robin_scheduler_->GetCredits(), controller_->max_acl_packet_credits_);

  while (!sent_acl_packets_.empty()) {
    sent_acl_packets_.pop();
  }

  round_robin_scheduler_->Register(RoundRobinScheduler::ConnectionType::CLASSIC, handle,
                                   new_connection_queue);
  ASSERT_NO_FATAL_FAILURE(SetPacketFuture(controller_->max_acl_packet_credits_));

  AclConnection::QueueUpEnd* new_queue_up_end = new_connection_queue->GetUpEnd();
  for (uint16_t i = 0; i < controller_->max_acl_packet_credits_; i++) {
    EnqueueAclUpEnd(new_queue_up_end, new_packet);
  }

  packet_future_->wait();

  // Pending fragments shouldn't be sent
  for (uint16_t i = 0; i < controller_->max_acl_packet_credits_; i++) {
    VerifyPacket(handle, new_packet);
  }
  round_robin_scheduler_->Unregister(handle);
}

}  // namespace
}  // namespace acl_manager
}  // namespace hci