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

Commit a10d3117 authored by Jakub Pawlowski's avatar Jakub Pawlowski Committed by Jakub Pawłowski
Browse files

connection_manager: one method for direct connect establishment 

Currently in stack we have two methods for establishing direct conenct:
* create_le_connection
* direct_connect_add

create_le_connection is making direct call to GD to schedule
direct connection. It relies on timer in GD to stop the attempt after 30
seconds. It doesn't track which app requested direct connect, so it
can't properly cancel direct connect if request was made from multiple
clients, and just one cancels it.

direct_connect_add keeps track of clients that made a request, but it
works only if it's the only function used to establish direct connect.
If create_le_connection, or GD is called directly, the counting breaks.

The logic for scheduling/canceling direct connect using
direct_connect_add was already tested in unit tests, but when
create_le_connection is in use through the stack, those tests don't
reflect the stack behavior.

In create_le_connection, some extra address checks and lookups were
added to prevent stack from attempting direct connect to targetted
announcements. These are copied to direct_connect_add.

Test: atest net_test_conn_multiplexing
Bug: 372202918
Flag: EXEMPT, covered under unittests
Change-Id: I76b2a4d5012fe473aa01a3df89e7b4794689a1d6
parent 8ccbfbd7

system/stack/connection_manager/connection_manager.cc

+32 −32
Original line number Diff line number Diff line
@@ -455,57 +455,59 @@ static void find_in_device_record(const RawAddress& bd_addr, tBLE_BD_ADDR* addre 
  return;

}



bool create_le_connection(uint8_t /* id */, const RawAddress& bd_addr, tBLE_ADDR_TYPE addr_type) {

bool direct_connect_add(uint8_t app_id, const RawAddress& address, tBLE_ADDR_TYPE addr_type) {

  tBLE_BD_ADDR address_with_type{

          .type = addr_type,

          .bda = bd_addr,

          .bda = address,

  };



  find_in_device_record(bd_addr, &address_with_type);



  log::debug("Creating le direct connection to:{} type:{} (initial type: {})", address_with_type,

             AddressTypeText(address_with_type.type), AddressTypeText(addr_type));

  find_in_device_record(address, &address_with_type);



  if (address_with_type.type == BLE_ADDR_ANONYMOUS) {

    log::warn(

            "Creating le direct connection to:{}, address type 'anonymous' is "

            "invalid",

            address_with_type);

    log::warn("Can't use anonymous address for connection: {}", address_with_type);

    return false;

  }



  bluetooth::shim::ACL_AcceptLeConnectionFrom(address_with_type,

                                              /* is_direct */ true);

  return true;

}

  log::debug("app_id=0x{:x}, address={} (initial type: {})", static_cast<int>(app_id),

             address_with_type, AddressTypeText(addr_type));



/** Add a device to the direct connection list. Returns true if device

 * added to the list, false otherwise */

bool direct_connect_add(uint8_t app_id, const RawAddress& address) {

  log::debug("app_id={}, address={}", static_cast<int>(app_id), address);

  bool in_acceptlist = false;

  auto it = bgconn_dev.find(address);

  if (it != bgconn_dev.end()) {

    const tAPPS_CONNECTING& info = it->second;

    // app already trying to connect to this particular device

    if (it->second.doing_direct_conn.count(app_id)) {

      log::info("direct connect attempt from app_id=0x{:x} already in progress", app_id);

    if (info.doing_direct_conn.count(app_id)) {

      log::info("attempt from app_id=0x{:x} to {} already in progress", app_id, address_with_type);

      return false;

    }



    // This is to match existing GD connection manager behavior - if multiple apps try direct

    // connect at same time, only 1st request is fully processed

    if (!info.doing_direct_conn.empty()) {

      log::info("app_id=0x{:x}: attempt from other app already in progress, will merge {}", app_id,

                address_with_type);

      return true;

    }



    // are we already in the acceptlist ?

    if (it->second.is_in_accept_list) {

      log::warn("Background connection attempt already in progress app_id={:x}", app_id);

    if (info.is_in_accept_list) {

      log::warn("background connect attempt already in progress app_id=0x{:x} {}", app_id,

                address_with_type);

      in_acceptlist = true;

    }

  }



  if (!in_acceptlist) {

    if (!bluetooth::shim::ACL_AcceptLeConnectionFrom(BTM_Sec_GetAddressWithType(address), true)) {

    if (!bluetooth::shim::ACL_AcceptLeConnectionFrom(address_with_type, true /* is_direct */)) {

      // if we can't add to acceptlist, turn parameters back to slow.

      log::warn("Unable to add le device to acceptlist");

      log::warn("unable to add le device to acceptlist {}", address_with_type);

      return false;

    }

    bgconn_dev[address].is_in_accept_list = true;

  } else {

    // if already in accept list, we should just bump parameters up for direct

    // connection. There is no API for that yet, so use API that's adding to accept list.

    bluetooth::shim::ACL_AcceptLeConnectionFrom(address_with_type, true /* is_direct */);

  }



  // Setup a timer
@@ -514,7 +516,6 @@ bool direct_connect_add(uint8_t app_id, const RawAddress& address) { 
                    base::BindOnce(&wl_direct_connect_timeout_cb, app_id, address));



  bgconn_dev[address].doing_direct_conn.emplace(app_id, unique_alarm_ptr(timeout, &alarm_free));



  return true;

}
@@ -526,13 +527,13 @@ bool direct_connect_remove(uint8_t app_id, const RawAddress& address, bool conne 
  log::debug("app_id={}, address={}", static_cast<int>(app_id), address);

  auto it = bgconn_dev.find(address);

  if (it == bgconn_dev.end()) {

    log::warn("Unable to find background connection to remove peer:{}", address);

    log::warn("unable to find entry to remove: {}", address);

    return false;

  }



  auto app_it = it->second.doing_direct_conn.find(app_id);

  if (app_it == it->second.doing_direct_conn.end()) {

    log::warn("Unable to find direct connection to remove peer:{}", address);

    log::warn("unable to find direct connection to remove: {}", address);

    return false;

  }
@@ -544,13 +545,12 @@ bool direct_connect_remove(uint8_t app_id, const RawAddress& address, bool conne 


  if (is_anyone_interested_to_use_accept_list(it)) {

    if (connection_timeout) {

      /* In such case we need to add device back to allow list because,

       * when connection timeout out, the lower layer removes device from

       * the allow list.

      /* In such case we need to add device back to allow list because, when connection timeout

       * out, the lower layer removes device from the allow list.

       */

      if (!bluetooth::shim::ACL_AcceptLeConnectionFrom(BTM_Sec_GetAddressWithType(address),

                                                       false)) {

        log::warn("Failed to re-add device {} to accept list after connection timeout", address);

                                                       false /* is_direct */)) {

        log::warn("Failed to re-add {} to accept list after connection timeout", address);

      }

    }

    return true;

system/stack/connection_manager/connection_manager.h

+22 −10
Original line number Diff line number Diff line
@@ -37,10 +37,22 @@ namespace connection_manager { 


using tAPP_ID = uint8_t;



/* for background connection */

/* Mark device as using targeted announcements.

 *

 * @return true if device added to the list, false otherwise */

bool background_connect_targeted_announcement_add(tAPP_ID app_id, const RawAddress& address);



/* Add a background connect request.

 *

 * @return true if device added to the list, false otherwise */

bool background_connect_add(tAPP_ID app_id, const RawAddress& address);



/* Remove a background connection request.

 *

 * @return true if the request is removed, false otherwise.

 */

bool background_connect_remove(tAPP_ID app_id, const RawAddress& address);



bool remove_unconditional(const RawAddress& address);



void reset(bool after_reset);
@@ -50,15 +62,15 @@ void on_connection_complete(const RawAddress& address); 


std::set<tAPP_ID> get_apps_connecting_to(const RawAddress& remote_bda);



/* create_le_connection is adding device directly to AclManager, and relying on it's "direct

 * connect" implementation.

 * direct_connect_add method is doing multiplexing of apps request, and

 * sending the request to AclManager, but it lacks some extra checks and lookups. Currently these

 * methods are exclusive, if you try to use both you will get some bad behavior. These should be

 * merged into one. */

bool create_le_connection(uint8_t /* id */, const RawAddress& bd_addr,

/* Add a direct connect request.

 *

 * @return true if device added to the list, false otherwise */

bool direct_connect_add(tAPP_ID app_id, const RawAddress& address,

                        tBLE_ADDR_TYPE addr_type = BLE_ADDR_PUBLIC);

bool direct_connect_add(tAPP_ID app_id, const RawAddress& address);

/* Remove a direct connection request.

 *

 * @return true if the request is removed, false otherwise.

 */

bool direct_connect_remove(tAPP_ID app_id, const RawAddress& address,

                           bool connection_timeout = false);

system/stack/fuzzers/l2cap_fuzzer.cc

+2 −2
Original line number Diff line number Diff line
@@ -97,7 +97,7 @@ void SnoopLogger::SetL2capChannelOpen(uint16_t, uint16_t, uint16_t, uint16_t, bo 
}  // namespace bluetooth



namespace connection_manager {

bool create_le_connection(uint8_t /* id */, const RawAddress& /* bd_addr */,

bool direct_connect_add(uint8_t /* id */, const RawAddress& /* bd_addr */,

                        tBLE_ADDR_TYPE /* addr_type */) {

  return true;

}

system/stack/gatt/gatt_api.cc

+1 −1
Original line number Diff line number Diff line
@@ -1494,7 +1494,7 @@ bool GATT_Connect(tGATT_IF gatt_if, const RawAddress& bd_addr, tBLE_ADDR_TYPE ad 
        log::warn("{} already added to gatt_if {} direct conn list", bd_addr, gatt_if);

      }



      ret = connection_manager::create_le_connection(gatt_if, bd_addr, addr_type);

      ret = connection_manager::direct_connect_add(gatt_if, bd_addr, addr_type);

    }



  } else {

system/stack/gatt/gatt_main.cc

+1 −1
Original line number Diff line number Diff line
@@ -235,7 +235,7 @@ static bool gatt_connect(const RawAddress& rem_bda, tBLE_ADDR_TYPE addr_type, tG 
  }



  p_tcb->att_lcid = L2CAP_ATT_CID;

  return connection_manager::create_le_connection(gatt_if, rem_bda, addr_type);

  return connection_manager::direct_connect_add(gatt_if, rem_bda, addr_type);

}



/*******************************************************************************