From d40eaa2bc721717fa67652cd762823593188ec93 Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Tue, 24 May 2022 17:41:09 +0200 Subject: [PATCH 1/2] Bluetooth: Controller: Fix DLE HCI params parsing LE Write Suggested Default Data Length and LE Set Data Length commands are suggestions from host and should be validated only as per HCI specification regarding internal setting of LLCP. LLCP is allowed to use other values if needed. Signed-off-by: Szymon Janc --- subsys/bluetooth/controller/ll_sw/ull_conn.c | 48 +++++++++++++++++-- .../controller/mock_ctrl/include/kconfig.h | 2 +- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn.c b/subsys/bluetooth/controller/ll_sw/ull_conn.c index a81087cd86192..553dedd4b45ca 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn.c +++ b/subsys/bluetooth/controller/ll_sw/ull_conn.c @@ -684,14 +684,30 @@ uint8_t ll_version_ind_send(uint16_t handle) } #if defined(CONFIG_BT_CTLR_DATA_LENGTH) +static bool ll_len_validate(uint16_t tx_octets, uint16_t tx_time) +{ + /* validate if within HCI allowed range */ + if (!IN_RANGE(tx_octets, PDU_DC_PAYLOAD_SIZE_MIN, + PDU_DC_PAYLOAD_SIZE_MAX)) { + return false; + } + + /* validate if within HCI allowed range */ + if (!IN_RANGE(tx_time, PDU_DC_PAYLOAD_TIME_MIN, + PDU_DC_PAYLOAD_TIME_MAX_CODED)) { + return false; + } + + return true; +} + uint32_t ll_length_req_send(uint16_t handle, uint16_t tx_octets, uint16_t tx_time) { struct ll_conn *conn; if (IS_ENABLED(CONFIG_BT_CTLR_PARAM_CHECK) && - ((tx_octets > LL_LENGTH_OCTETS_TX_MAX) || - (tx_time > PDU_DC_PAYLOAD_TIME_MAX_CODED))) { + !ll_len_validate(tx_octets, tx_time)) { return BT_HCI_ERR_INVALID_PARAM; } @@ -778,7 +794,10 @@ void ll_length_default_get(uint16_t *max_tx_octets, uint16_t *max_tx_time) uint32_t ll_length_default_set(uint16_t max_tx_octets, uint16_t max_tx_time) { - /* TODO: parameter check (for BT 5.0 compliance) */ + if (IS_ENABLED(CONFIG_BT_CTLR_PARAM_CHECK) && + !ll_len_validate(max_tx_octets, max_tx_time)) { + return BT_HCI_ERR_INVALID_PARAM; + } default_tx_octets = max_tx_octets; default_tx_time = max_tx_time; @@ -8146,8 +8165,31 @@ uint8_t ull_dle_update_eff(struct ll_conn *conn) return dle_changed; } +static void ull_len_data_length_trim(uint16_t *tx_octets, uint16_t *tx_time) +{ +#if defined(CONFIG_BT_CTLR_PHY_CODED) + uint16_t tx_time_max = + PDU_DC_MAX_US(LL_LENGTH_OCTETS_TX_MAX, PHY_CODED); +#else /* !CONFIG_BT_CTLR_PHY_CODED */ + uint16_t tx_time_max = + PDU_DC_MAX_US(LL_LENGTH_OCTETS_TX_MAX, PHY_1M); +#endif /* !CONFIG_BT_CTLR_PHY_CODED */ + + /* trim to supported values */ + if (*tx_octets > LL_LENGTH_OCTETS_TX_MAX) { + *tx_octets = LL_LENGTH_OCTETS_TX_MAX; + } + + if (*tx_time > tx_time_max) { + *tx_time = tx_time_max; + } +} + void ull_dle_local_tx_update(struct ll_conn *conn, uint16_t tx_octets, uint16_t tx_time) { + /* Trim to supported values */ + ull_len_data_length_trim(&tx_octets, &tx_time); + conn->lll.dle.default_tx_octets = tx_octets; #if defined(CONFIG_BT_CTLR_PHY) diff --git a/tests/bluetooth/controller/mock_ctrl/include/kconfig.h b/tests/bluetooth/controller/mock_ctrl/include/kconfig.h index ee3878f8ef1ec..403df1bc1e28c 100644 --- a/tests/bluetooth/controller/mock_ctrl/include/kconfig.h +++ b/tests/bluetooth/controller/mock_ctrl/include/kconfig.h @@ -176,6 +176,6 @@ #define CONFIG_BT_CTLR_SUBVERSION_NUMBER 0x5678 #define CONFIG_BT_CTLR_ASSERT_HANDLER y #define CONFIG_BT_BUF_ACL_TX_COUNT 7 -#define CONFIG_BT_BUF_ACL_TX_SIZE 27 +#define CONFIG_BT_BUF_ACL_TX_SIZE 251 #define CONFIG_BT_CTLR_RX_BUFFERS 7 #define CONFIG_NET_BUF_USER_DATA_SIZE 8 From 395363641b0523b53becdd32bd95e22b4d756f92 Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Thu, 1 Sep 2022 09:58:58 +0200 Subject: [PATCH 2/2] test: Bluetooth: EDTT: Disable invalid tests Some tests assume that if IUT accepted HCI_LE_Set_Data_Length command than HCI_LE_Data_Length_Change event will always be sent. This is not the case anymore as HCI_LE_Data_Length_Change is sent only if effective parameters changed. Those tests should be re-enabled when EDDT implementation is updated. Signed-off-by: Szymon Janc --- .../tests_scripts/ll.set1.llcp.test_list | 4 ++-- .../edtt_ble_test_app/tests_scripts/ll.set1.test_list | 4 ++-- .../tests_scripts/ll.set2.llcp.test_list | 8 ++++---- .../edtt_ble_test_app/tests_scripts/ll.set2.test_list | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set1.llcp.test_list b/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set1.llcp.test_list index eae770b51eaa8..fb6f04bb2d296 100644 --- a/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set1.llcp.test_list +++ b/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set1.llcp.test_list @@ -44,9 +44,9 @@ LL/CON/CEN/BV-35-C LL/CON/CEN/BV-41-C LL/CON/CEN/BV-43-C LL/CON/CEN/BV-73-C -LL/CON/CEN/BV-74-C +#LL/CON/CEN/BV-74-C # Needs testcase implementation update #LL/CON/CEN/BV-76-C # Needs testcase implementation update -LL/CON/CEN/BV-77-C +#LL/CON/CEN/BV-77-C # Needs testcase implementation update #LL/CON/PER/BI-08-C # This test implementation is not valid, and will fail with refactored LLCP LL/CON/PER/BV-04-C LL/CON/PER/BV-05-C diff --git a/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set1.test_list b/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set1.test_list index ae5564aeb0c9a..009d69d57b807 100644 --- a/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set1.test_list +++ b/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set1.test_list @@ -45,10 +45,10 @@ LL/CON/CEN/BV-41-C LL/CON/CEN/BV-43-C ## With updated EDTT, this now fails due to error in legacy DLE handling. Fixed in refactored LLCP #LL/CON/CEN/BV-73-C -LL/CON/CEN/BV-74-C +#LL/CON/CEN/BV-74-C # Needs testcase implementation update ## With updated EDTT, this now fails due to error in legacy DLE handling. Fixed in refactored LLCP #LL/CON/CEN/BV-76-C -LL/CON/CEN/BV-77-C +#LL/CON/CEN/BV-77-C # Needs testcase implementation update LL/CON/PER/BI-08-C LL/CON/PER/BV-04-C LL/CON/PER/BV-05-C diff --git a/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set2.llcp.test_list b/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set2.llcp.test_list index 5013b729140f6..795f5b92b710b 100644 --- a/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set2.llcp.test_list +++ b/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set2.llcp.test_list @@ -6,10 +6,10 @@ LL/CON/PER/BV-33-C LL/CON/PER/BV-34-C LL/CON/PER/BV-40-C LL/CON/PER/BV-42-C -LL/CON/PER/BV-77-C -LL/CON/PER/BV-78-C -LL/CON/PER/BV-80-C -LL/CON/PER/BV-81-C +#LL/CON/PER/BV-77-C # Needs testcase implementation update +#LL/CON/PER/BV-78-C # Needs testcase implementation update +#LL/CON/PER/BV-80-C # Needs testcase implementation update +#LL/CON/PER/BV-81-C # Needs testcase implementation update LL/DDI/ADV/BV-01-C LL/DDI/ADV/BV-02-C LL/DDI/ADV/BV-03-C diff --git a/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set2.test_list b/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set2.test_list index 5013b729140f6..795f5b92b710b 100644 --- a/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set2.test_list +++ b/tests/bluetooth/bsim_bt/edtt_ble_test_app/tests_scripts/ll.set2.test_list @@ -6,10 +6,10 @@ LL/CON/PER/BV-33-C LL/CON/PER/BV-34-C LL/CON/PER/BV-40-C LL/CON/PER/BV-42-C -LL/CON/PER/BV-77-C -LL/CON/PER/BV-78-C -LL/CON/PER/BV-80-C -LL/CON/PER/BV-81-C +#LL/CON/PER/BV-77-C # Needs testcase implementation update +#LL/CON/PER/BV-78-C # Needs testcase implementation update +#LL/CON/PER/BV-80-C # Needs testcase implementation update +#LL/CON/PER/BV-81-C # Needs testcase implementation update LL/DDI/ADV/BV-01-C LL/DDI/ADV/BV-02-C LL/DDI/ADV/BV-03-C