From 64352622841021f56032de307f123d7306256ccc Mon Sep 17 00:00:00 2001 From: Jonathan Rico Date: Wed, 24 Apr 2024 13:33:18 +0200 Subject: [PATCH] Bluetooth: L2CAP: Handle REJECT_RSP for ECRED We can (and do) open multiple channels with a single L2CAP command. If the remote doesn't support dynamic channels at all, then it sends back a REJECT_RSP. We only destroyed the first channel that matched the command PDU identifier. Fix that and remove all channels that match. Also add a test that verifies the patch. Signed-off-by: Jonathan Rico --- subsys/bluetooth/host/l2cap.c | 8 +- tests/bsim/bluetooth/host/l2cap/compile.sh | 2 + .../host/l2cap/ecred/dut/CMakeLists.txt | 23 ++++ .../bluetooth/host/l2cap/ecred/dut/Kconfig | 18 +++ .../bluetooth/host/l2cap/ecred/dut/prj.conf | 29 +++++ .../bluetooth/host/l2cap/ecred/dut/src/dut.c | 117 ++++++++++++++++++ .../bluetooth/host/l2cap/ecred/dut/src/main.c | 45 +++++++ .../host/l2cap/ecred/peer/CMakeLists.txt | 23 ++++ .../bluetooth/host/l2cap/ecred/peer/prj.conf | 29 +++++ .../host/l2cap/ecred/peer/src/main.c | 45 +++++++ .../host/l2cap/ecred/peer/src/peer.c | 38 ++++++ .../host/l2cap/ecred/test_scripts/_compile.sh | 14 +++ .../host/l2cap/ecred/test_scripts/run.sh | 26 ++++ 13 files changed, 411 insertions(+), 6 deletions(-) create mode 100644 tests/bsim/bluetooth/host/l2cap/ecred/dut/CMakeLists.txt create mode 100644 tests/bsim/bluetooth/host/l2cap/ecred/dut/Kconfig create mode 100644 tests/bsim/bluetooth/host/l2cap/ecred/dut/prj.conf create mode 100644 tests/bsim/bluetooth/host/l2cap/ecred/dut/src/dut.c create mode 100644 tests/bsim/bluetooth/host/l2cap/ecred/dut/src/main.c create mode 100644 tests/bsim/bluetooth/host/l2cap/ecred/peer/CMakeLists.txt create mode 100644 tests/bsim/bluetooth/host/l2cap/ecred/peer/prj.conf create mode 100644 tests/bsim/bluetooth/host/l2cap/ecred/peer/src/main.c create mode 100644 tests/bsim/bluetooth/host/l2cap/ecred/peer/src/peer.c create mode 100755 tests/bsim/bluetooth/host/l2cap/ecred/test_scripts/_compile.sh create mode 100755 tests/bsim/bluetooth/host/l2cap/ecred/test_scripts/run.sh diff --git a/subsys/bluetooth/host/l2cap.c b/subsys/bluetooth/host/l2cap.c index 87d0080cda..c18e7c922e 100644 --- a/subsys/bluetooth/host/l2cap.c +++ b/subsys/bluetooth/host/l2cap.c @@ -2101,13 +2101,9 @@ static void reject_cmd(struct bt_l2cap *l2cap, uint8_t ident, struct bt_conn *conn = l2cap->chan.chan.conn; struct bt_l2cap_le_chan *chan; - /* Check if there is a outstanding channel */ - chan = l2cap_remove_ident(conn, ident); - if (!chan) { - return; + while ((chan = l2cap_remove_ident(conn, ident))) { + bt_l2cap_chan_del(&chan->chan); } - - bt_l2cap_chan_del(&chan->chan); } #endif /* CONFIG_BT_L2CAP_DYNAMIC_CHANNEL */ diff --git a/tests/bsim/bluetooth/host/l2cap/compile.sh b/tests/bsim/bluetooth/host/l2cap/compile.sh index 413928f4a2..b8f5201dd2 100755 --- a/tests/bsim/bluetooth/host/l2cap/compile.sh +++ b/tests/bsim/bluetooth/host/l2cap/compile.sh @@ -17,6 +17,8 @@ app=tests/bsim/bluetooth/host/l2cap/stress conf_file=prj_nofrag.conf compile app=tests/bsim/bluetooth/host/l2cap/stress conf_file=prj_syswq.conf compile app=tests/bsim/bluetooth/host/l2cap/split/dut compile app=tests/bsim/bluetooth/host/l2cap/split/tester compile +app=tests/bsim/bluetooth/host/l2cap/ecred/dut compile +app=tests/bsim/bluetooth/host/l2cap/ecred/peer compile app=tests/bsim/bluetooth/host/l2cap/credits compile app=tests/bsim/bluetooth/host/l2cap/credits conf_file=prj_ecred.conf compile app=tests/bsim/bluetooth/host/l2cap/credits_seg_recv compile diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/dut/CMakeLists.txt b/tests/bsim/bluetooth/host/l2cap/ecred/dut/CMakeLists.txt new file mode 100644 index 0000000000..200ffba68c --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/dut/CMakeLists.txt @@ -0,0 +1,23 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) + +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) + +project(ecred_dut) + +add_subdirectory(${ZEPHYR_BASE}/tests/bluetooth/common/testlib testlib) +target_link_libraries(app PRIVATE testlib) + +add_subdirectory(${ZEPHYR_BASE}/tests/bsim/babblekit babblekit) +target_link_libraries(app PRIVATE babblekit) + +zephyr_include_directories( + ${BSIM_COMPONENTS_PATH}/libUtilv1/src/ + ${BSIM_COMPONENTS_PATH}/libPhyComv1/src/ +) + +target_sources(app PRIVATE + src/main.c + src/dut.c +) diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/dut/Kconfig b/tests/bsim/bluetooth/host/l2cap/ecred/dut/Kconfig new file mode 100644 index 0000000000..d1289e2c12 --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/dut/Kconfig @@ -0,0 +1,18 @@ +# Kconfig options for the test +# +# Only used as single point for log level configuration. +# Can be extended with any new kconfig, really. +# +# Copyright (c) 2024 Nordic Semiconductor ASA +# SPDX-License-Identifier: Apache-2.0 + +menu "Test configuration" + +module = APP +module-str = app + +source "subsys/logging/Kconfig.template.log_config" + +endmenu + +source "Kconfig.zephyr" diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/dut/prj.conf b/tests/bsim/bluetooth/host/l2cap/ecred/dut/prj.conf new file mode 100644 index 0000000000..ba574d9f20 --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/dut/prj.conf @@ -0,0 +1,29 @@ +CONFIG_BT=y +CONFIG_BT_DEVICE_NAME="ecred" +CONFIG_BT_CENTRAL=y + +CONFIG_BT_SMP=y # Next config depends on it +CONFIG_BT_L2CAP_DYNAMIC_CHANNEL=y +CONFIG_BT_L2CAP_ECRED=y + +# Disable auto-initiated procedures so they don't +# mess with the test's execution. +CONFIG_BT_AUTO_PHY_UPDATE=n +CONFIG_BT_AUTO_DATA_LEN_UPDATE=n +CONFIG_BT_GAP_AUTO_UPDATE_CONN_PARAMS=n + +# Dependency of testlib/adv and testlib/scan. +CONFIG_BT_EXT_ADV=y + +CONFIG_ASSERT=y +CONFIG_LOG=y +CONFIG_THREAD_NAME=y +CONFIG_LOG_THREAD_ID_PREFIX=y + +CONFIG_ARCH_POSIX_TRAP_ON_FATAL=y + +# CONFIG_APP_LOG_LEVEL_DBG=y +# CONFIG_BT_L2CAP_LOG_LEVEL_DBG=y + +# Don't print the bt init stuff +CONFIG_BT_HCI_CORE_LOG_LEVEL_WRN=y diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/dut/src/dut.c b/tests/bsim/bluetooth/host/l2cap/ecred/dut/src/dut.c new file mode 100644 index 0000000000..0a822efc4b --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/dut/src/dut.c @@ -0,0 +1,117 @@ +/* + * Copyright (c) 2024 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include +#include + +#include "testlib/conn.h" +#include "testlib/scan.h" + +#include "babblekit/flags.h" +#include "babblekit/testcase.h" + +LOG_MODULE_REGISTER(dut, CONFIG_APP_LOG_LEVEL); + +static atomic_t disconnected_channels; + +static struct bt_l2cap_le_chan chans[4]; + +void sent_cb(struct bt_l2cap_chan *chan) +{ + LOG_DBG("%p", chan); +} + +int recv_cb(struct bt_l2cap_chan *chan, struct net_buf *buf) +{ + LOG_DBG("%p", chan); + + return 0; +} + +void l2cap_chan_connected_cb(struct bt_l2cap_chan *l2cap_chan) +{ + TEST_ASSERT("This shouldn't happen"); +} + +void l2cap_chan_disconnected_cb(struct bt_l2cap_chan *chan) +{ + atomic_inc(&disconnected_channels); +} + +static struct bt_l2cap_chan_ops ops = { + .connected = l2cap_chan_connected_cb, + .disconnected = l2cap_chan_disconnected_cb, + .recv = recv_cb, + .sent = sent_cb, +}; + +void entrypoint_dut(void) +{ + /* Test purpose: + * + * Verify that a peer that doesn't support ECRED channels doesn't result + * in us keeping half-open channels. + * + * Two devices: + * - `dut`: tries to establish 4 ecred chans + * - `peer`: rejects the request + * + * Initial conditions: + * - Both devices are connected + * + * Procedure: + * - [dut] request to establish 4 ecred channels + * - [peer] reject command as unknown + * - [dut] get `disconnected` called on all 4 channels + * + * [verdict] + * - each channel gets the `disconnected` callback called + */ + int err; + bt_addr_le_t peer = {}; + struct bt_conn *conn = NULL; + + TEST_START("dut"); + + /* Initialize Bluetooth */ + err = bt_enable(NULL); + TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)", err); + LOG_DBG("Bluetooth initialized"); + + err = bt_testlib_scan_find_name(&peer, "ecred_peer"); + TEST_ASSERT(!err, "Failed to start scan (err %d)", err); + + /* Create a connection using that address */ + err = bt_testlib_connect(&peer, &conn); + TEST_ASSERT(!err, "Failed to initiate connection (err %d)", err); + + LOG_DBG("Connected"); + + LOG_INF("Send ECRED connection request"); + struct bt_l2cap_chan *chan_list[5] = {0}; + + for (int i = 0; i < 4; i++) { + /* Register the callbacks */ + chans[i].chan.ops = &ops; + + /* Add the channel to the connection request list */ + chan_list[i] = &chans[i].chan; + } + + /* The PSM doesn't matter, as the peer doesn't support the command */ + err = bt_l2cap_ecred_chan_connect(conn, chan_list, 0x0080); + TEST_ASSERT(!err, "Error connecting l2cap channels (err %d)\n", err); + + LOG_INF("Wait until peer rejects the channel establishment request"); + while (atomic_get(&disconnected_channels) < 4) { + k_sleep(K_MSEC(10)); + } + + TEST_PASS_AND_EXIT("dut"); +} diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/dut/src/main.c b/tests/bsim/bluetooth/host/l2cap/ecred/dut/src/main.c new file mode 100644 index 0000000000..300b77e778 --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/dut/src/main.c @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2024 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +#include "bs_tracing.h" +#include "bstests.h" +#include "babblekit/testcase.h" + +extern void entrypoint_dut(void); +extern enum bst_result_t bst_result; + + +static void test_end_cb(void) +{ + if (bst_result != Passed) { + TEST_PRINT("Test has not passed."); + } +} + +static const struct bst_test_instance entrypoints[] = { + { + .test_id = "dut", + .test_delete_f = test_end_cb, + .test_main_f = entrypoint_dut, + }, + BSTEST_END_MARKER, +}; + +static struct bst_test_list *install(struct bst_test_list *tests) +{ + return bst_add_tests(tests, entrypoints); +}; + +bst_test_install_t test_installers[] = {install, NULL}; + +int main(void) +{ + bst_main(); + + return 0; +} diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/peer/CMakeLists.txt b/tests/bsim/bluetooth/host/l2cap/ecred/peer/CMakeLists.txt new file mode 100644 index 0000000000..bfc8237ea6 --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/peer/CMakeLists.txt @@ -0,0 +1,23 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) + +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) + +project(ecred_peer) + +add_subdirectory(${ZEPHYR_BASE}/tests/bluetooth/common/testlib testlib) +target_link_libraries(app PRIVATE testlib) + +add_subdirectory(${ZEPHYR_BASE}/tests/bsim/babblekit babblekit) +target_link_libraries(app PRIVATE babblekit) + +zephyr_include_directories( + ${BSIM_COMPONENTS_PATH}/libUtilv1/src/ + ${BSIM_COMPONENTS_PATH}/libPhyComv1/src/ +) + +target_sources(app PRIVATE + src/main.c + src/peer.c +) diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/peer/prj.conf b/tests/bsim/bluetooth/host/l2cap/ecred/peer/prj.conf new file mode 100644 index 0000000000..e7941a800a --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/peer/prj.conf @@ -0,0 +1,29 @@ +CONFIG_BT=y +CONFIG_BT_DEVICE_NAME="ecred_peer" +CONFIG_BT_PERIPHERAL=y + +CONFIG_BT_SMP=y +CONFIG_BT_L2CAP_DYNAMIC_CHANNEL=y +CONFIG_BT_L2CAP_ECRED=n + +# Disable auto-initiated procedures so they don't +# mess with the test's execution. +CONFIG_BT_AUTO_PHY_UPDATE=n +CONFIG_BT_AUTO_DATA_LEN_UPDATE=n +CONFIG_BT_GAP_AUTO_UPDATE_CONN_PARAMS=n + +# Dependency of testlib/adv and testlib/scan. +CONFIG_BT_EXT_ADV=y + +CONFIG_ASSERT=y +CONFIG_LOG=y +CONFIG_THREAD_NAME=y +CONFIG_LOG_THREAD_ID_PREFIX=y + +CONFIG_ARCH_POSIX_TRAP_ON_FATAL=y + +# CONFIG_APP_LOG_LEVEL_DBG=y +# CONFIG_BT_L2CAP_LOG_LEVEL_DBG=y + +# Don't print the bt init stuff +CONFIG_BT_HCI_CORE_LOG_LEVEL_WRN=y diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/peer/src/main.c b/tests/bsim/bluetooth/host/l2cap/ecred/peer/src/main.c new file mode 100644 index 0000000000..4ba6fc1efe --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/peer/src/main.c @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2024 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +#include "bs_tracing.h" +#include "bstests.h" +#include "babblekit/testcase.h" + +extern void entrypoint_peer(void); +extern enum bst_result_t bst_result; + + +static void test_end_cb(void) +{ + if (bst_result != Passed) { + TEST_PRINT("Test has not passed."); + } +} + +static const struct bst_test_instance entrypoints[] = { + { + .test_id = "peer", + .test_delete_f = test_end_cb, + .test_main_f = entrypoint_peer, + }, + BSTEST_END_MARKER, +}; + +static struct bst_test_list *install(struct bst_test_list *tests) +{ + return bst_add_tests(tests, entrypoints); +}; + +bst_test_install_t test_installers[] = {install, NULL}; + +int main(void) +{ + bst_main(); + + return 0; +} diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/peer/src/peer.c b/tests/bsim/bluetooth/host/l2cap/ecred/peer/src/peer.c new file mode 100644 index 0000000000..026ff5a5b6 --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/peer/src/peer.c @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2024 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include +#include +#include + +#include "testlib/adv.h" +#include "testlib/att_read.h" +#include "testlib/att_write.h" +#include "testlib/conn.h" + +#include "babblekit/flags.h" +#include "babblekit/testcase.h" + +void entrypoint_peer(void) +{ + int err; + struct bt_conn *conn; + + /* Mark test as in progress. */ + TEST_START("peer"); + + /* Initialize Bluetooth */ + err = bt_enable(NULL); + TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)", err); + + err = bt_testlib_adv_conn(&conn, BT_ID_DEFAULT, bt_get_name()); + TEST_ASSERT(!err, "Failed to start connectable advertising (err %d)", err); + + TEST_PASS("peer"); +} diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/test_scripts/_compile.sh b/tests/bsim/bluetooth/host/l2cap/ecred/test_scripts/_compile.sh new file mode 100755 index 0000000000..44b965da81 --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/test_scripts/_compile.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# Copyright 2023 Nordic Semiconductor ASA +# SPDX-License-Identifier: Apache-2.0 +set -eu +: "${ZEPHYR_BASE:?ZEPHYR_BASE must be defined}" + +INCR_BUILD=1 + +source ${ZEPHYR_BASE}/tests/bsim/compile.source + +app="$(guess_test_relpath)/dut" compile +app="$(guess_test_relpath)/peer" compile + +wait_for_background_jobs diff --git a/tests/bsim/bluetooth/host/l2cap/ecred/test_scripts/run.sh b/tests/bsim/bluetooth/host/l2cap/ecred/test_scripts/run.sh new file mode 100755 index 0000000000..43ac0f88e6 --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/ecred/test_scripts/run.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +# Copyright (c) 2024 Nordic Semiconductor +# SPDX-License-Identifier: Apache-2.0 + +set -eu + +source ${ZEPHYR_BASE}/tests/bsim/sh_common.source + +test_name="$(guess_test_long_name)" + +simulation_id=${test_name} +verbosity_level=2 + +SIM_LEN_US=$((1 * 1000 * 1000)) + +dut_exe="${BSIM_OUT_PATH}/bin/bs_${BOARD_TS}_${test_name}_dut_prj_conf" +peer_exe="${BSIM_OUT_PATH}/bin/bs_${BOARD_TS}_${test_name}_peer_prj_conf" + +cd ${BSIM_OUT_PATH}/bin + +Execute "${dut_exe}" -v=${verbosity_level} -s=${simulation_id} -d=0 -rs=420 -testid=dut +Execute "${peer_exe}" -v=${verbosity_level} -s=${simulation_id} -d=1 -rs=69 -testid=peer + +Execute ./bs_2G4_phy_v1 -v=${verbosity_level} -s=${simulation_id} -D=2 -sim_length=${SIM_LEN_US} $@ + +wait_for_background_jobs