From 32f6597fa83a3de6673d4d28c6aa776349d0d108 Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Fri, 29 Apr 2022 07:59:31 +0200 Subject: [PATCH 01/10] Bluetooth: Controller: llcp: add missing handling LL_UNKNOWN in CTE llcp There was missing handling of LL_UNKNOWN_RSP in CTE request control procedure.In case there is a pending CTE request and peer responses with LL_UNKNOWN_RSP then Host should be notified with HCI_LE_CTE_- Request_Failed event. The pending CTE request procedure should be completed. Signed-off-by: Piotr Pryga --- subsys/bluetooth/controller/hci/hci.c | 6 ++- .../controller/ll_sw/ull_conn_types.h | 10 +++++ subsys/bluetooth/controller/ll_sw/ull_llcp.c | 13 +++++++ .../controller/ll_sw/ull_llcp_common.c | 37 ++++++++++++------- .../controller/ll_sw/ull_llcp_features.h | 13 +++++-- 5 files changed, 61 insertions(+), 18 deletions(-) diff --git a/subsys/bluetooth/controller/hci/hci.c b/subsys/bluetooth/controller/hci/hci.c index 935b827d32f3d..db1a26e5b867c 100644 --- a/subsys/bluetooth/controller/hci/hci.c +++ b/subsys/bluetooth/controller/hci/hci.c @@ -7908,7 +7908,11 @@ static void le_unknown_rsp(struct pdu_data *pdu_data, uint16_t handle, le_remote_feat_complete(BT_HCI_ERR_UNSUPP_REMOTE_FEATURE, NULL, handle, buf); break; - +#if defined(CONFIG_BT_CTLR_DF_CONN_CTE_REQ) + case PDU_DATA_LLCTRL_TYPE_CTE_REQ: + le_df_cte_req_failed(BT_HCI_ERR_UNSUPP_REMOTE_FEATURE, handle, buf); + break; +#endif /* CONFIG_BT_CTLR_DF_CONN_CTE_REQ */ default: BT_WARN("type: 0x%02x", pdu_data->llctrl.unknown_rsp.type); break; diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn_types.h b/subsys/bluetooth/controller/ll_sw/ull_conn_types.h index 54c7bb36b146e..85cbafc34de85 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn_types.h +++ b/subsys/bluetooth/controller/ll_sw/ull_conn_types.h @@ -404,7 +404,17 @@ struct llcp_struct { struct { uint8_t sent; uint8_t valid; + /* + * Stores features supported by peer device. The content of the member may be + * verified when feature exchange procedure has completed, valid member is set to 1. + */ uint64_t features_peer; + /* + * Stores features common for two connected devices. Before feature exchange + * procedure is completed, the member stores information about all features + * supported by local device. After completion of the procedure, the feature set + * may be limited to features that are common. + */ uint64_t features_used; } fex; diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp.c b/subsys/bluetooth/controller/ll_sw/ull_llcp.c index e08f4fb20c45e..1fea6bf9ea232 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp.c @@ -971,6 +971,19 @@ uint8_t ull_cp_cte_req(struct ll_conn *conn, uint8_t min_cte_len, uint8_t cte_ty { struct proc_ctx *ctx; + /* If Controller gained, awareness: + * - by Feature Exchange control procedure that peer device does not support CTE response, + * - by reception LL_UNKNOWN_RSP with unknown type LL_CTE_REQ that peer device does not + * recognize CTE request, + * then response to Host that CTE request enable command is not possible due to unsupported + * remote feature. + */ + if ((conn->llcp.fex.valid && + (!(conn->llcp.fex.features_peer & BIT64(BT_LE_FEAT_BIT_CONN_CTE_RESP)))) || + (!conn->llcp.fex.valid && !feature_cte_req(conn))) { + return BT_HCI_ERR_UNSUPP_REMOTE_FEATURE; + } + /* The request may be started by periodic CTE request procedure, so it skips earlier * verification of PHY. In case the PHY has changed to CODE the request should be stopped. */ diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c index b26a0ced761a8..e77ed00243de3 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c @@ -219,6 +219,9 @@ static void lp_comm_ntf_cte_req(struct ll_conn *conn, struct proc_ctx *ctx, stru llcp_ntf_encode_cte_req(pdu); } break; + case PDU_DATA_LLCTRL_TYPE_UNKNOWN_RSP: + llcp_ntf_encode_unknown_rsp(ctx, pdu); + break; case PDU_DATA_LLCTRL_TYPE_REJECT_EXT_IND: llcp_ntf_encode_reject_ext_ind(ctx, pdu); break; @@ -228,6 +231,17 @@ static void lp_comm_ntf_cte_req(struct ll_conn *conn, struct proc_ctx *ctx, stru } } +static void lp_comm_ntf_cte_req_tx(struct ll_conn *conn, struct proc_ctx *ctx) +{ + if (llcp_ntf_alloc_is_available()) { + lp_comm_ntf(conn, ctx); + ull_cp_cte_req_set_disable(conn); + ctx->state = LP_COMMON_STATE_IDLE; + } else { + ctx->state = LP_COMMON_STATE_WAIT_NTF; + } +} + static void lp_comm_complete_cte_req(struct ll_conn *conn, struct proc_ctx *ctx) { if (conn->llcp.cte_req.is_enabled) { @@ -238,22 +252,19 @@ static void lp_comm_complete_cte_req(struct ll_conn *conn, struct proc_ctx *ctx) conn->llcp.cte_req.req_interval; } ctx->state = LP_COMMON_STATE_IDLE; - } else if (llcp_ntf_alloc_is_available()) { - lp_comm_ntf(conn, ctx); - ull_cp_cte_req_set_disable(conn); - ctx->state = LP_COMMON_STATE_IDLE; } else { - ctx->state = LP_COMMON_STATE_WAIT_NTF; + lp_comm_ntf_cte_req_tx(conn, ctx); } } else if (ctx->response_opcode == PDU_DATA_LLCTRL_TYPE_REJECT_EXT_IND && - ctx->reject_ext_ind.reject_opcode == PDU_DATA_LLCTRL_TYPE_CTE_REQ) { - if (llcp_ntf_alloc_is_available()) { - lp_comm_ntf(conn, ctx); - ull_cp_cte_req_set_disable(conn); - ctx->state = LP_COMMON_STATE_IDLE; - } else { - ctx->state = LP_COMMON_STATE_WAIT_NTF; - } + ctx->reject_ext_ind.reject_opcode == PDU_DATA_LLCTRL_TYPE_CTE_REQ) { + lp_comm_ntf_cte_req_tx(conn, ctx); + } else if (ctx->response_opcode == PDU_DATA_LLCTRL_TYPE_UNKNOWN_RSP && + ctx->unknown_response.type == PDU_DATA_LLCTRL_TYPE_CTE_REQ) { + /* CTE response is unsupported in peer, so disable locally for this + * connection + */ + feature_unmask_features(conn, LL_FEAT_BIT_CONNECTION_CTE_REQ); + lp_comm_ntf_cte_req_tx(conn, ctx); } else if (ctx->response_opcode == PDU_DATA_LLCTRL_TYPE_UNUSED) { /* This path is related with handling disable the CTE REQ when PHY * has been changed to CODED PHY. BT 5.3 Core Vol 4 Part E 7.8.85 diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_features.h b/subsys/bluetooth/controller/ll_sw/ull_llcp_features.h index a715ada5df42d..1f876412d42e3 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_features.h +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_features.h @@ -117,6 +117,15 @@ static inline bool feature_phy_coded(struct ll_conn *conn) #endif } +static inline bool feature_cte_req(struct ll_conn *conn) +{ +#if defined(CONFIG_BT_CTLR_DF) && defined(CONFIG_BT_CTLR_DF_CONN_CTE_REQ) + return conn->llcp.fex.features_used & LL_FEAT_BIT_CONNECTION_CTE_REQ; +#else + return 0; +#endif +} + /* * for asymmetric features we can check either if we support it * or if the peer supports it @@ -148,10 +157,6 @@ static inline bool feature_peer_smi_tx(struct ll_conn *conn) * per_adv * pwr_class1 * min_chann - * CTE_req - * CTE_rsp - * CTE_tx - * CTE_rx * ant_sw_CTE_tx * ant_sw_CTE_rx * tone_ext From bee58e213aeb1b34bc13312092f21acb2c898e5e Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Fri, 29 Apr 2022 08:50:23 +0200 Subject: [PATCH 02/10] Bluetooth: Controller: df: Fix CTE reqest proc enable if feat supported The CTE request procedure should be enabled only if a peer supports CTE response feature. That information can be obtained by feature exchange procedure. If there were no feature exchange then CTE request feature may be disabled if a peer responses with LL_UNKNOWN_RSP for a CTE request. The implementation of ll_df_set_conn_cte_req_enable was checking if CTE response feature is supported only when there was feature exchange. There was missing possibility to stop CTE request if a peer responded with LL_UNKNOWN_RSP for an earlier CTE request. The commit changes the implementation of ll_df_set_conn_cte_req_enable. The CTE response feature check is moved to ull_cp_cte_req function, because it belongs more to control procedure than to function that handles Host request to start the procedure. Second change is related with use of conn->llcp.fex.features_used. It stores information about features supported by peer. It does not depend on execution of the feature exchange control procedure. By the way, there were removed else statement in ll_df_set_conn_cte_- req_enable because it was not needed. Signed-off-by: Piotr Pryga --- subsys/bluetooth/controller/ll_sw/ull_df.c | 64 ++++++++++------------ 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_df.c b/subsys/bluetooth/controller/ll_sw/ull_df.c index dc2cddd3ecb9d..24af5cf624ffa 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_df.c +++ b/subsys/bluetooth/controller/ll_sw/ull_df.c @@ -1166,51 +1166,43 @@ uint8_t ll_df_set_conn_cte_req_enable(uint16_t handle, uint8_t enable, ull_cp_cte_req_set_disable(conn); return BT_HCI_ERR_SUCCESS; - } else { - if (!conn->lll.df_rx_cfg.is_initialized) { - return BT_HCI_ERR_CMD_DISALLOWED; - } + } - if (conn->llcp.cte_req.is_enabled) { - return BT_HCI_ERR_CMD_DISALLOWED; - } + if (!conn->lll.df_rx_cfg.is_initialized) { + return BT_HCI_ERR_CMD_DISALLOWED; + } + + if (conn->llcp.cte_req.is_enabled) { + return BT_HCI_ERR_CMD_DISALLOWED; + } #if defined(CONFIG_BT_CTLR_PHY) - /* CTE request may be enabled only in case the receiver PHY is not CODED */ - if (conn->lll.phy_rx == PHY_CODED) { - return BT_HCI_ERR_CMD_DISALLOWED; - } + /* CTE request may be enabled only in case the receiver PHY is not CODED */ + if (conn->lll.phy_rx == PHY_CODED) { + return BT_HCI_ERR_CMD_DISALLOWED; + } #endif /* CONFIG_BT_CTLR_PHY */ - if (cte_request_interval != 0 && cte_request_interval < conn->lll.latency) { - return BT_HCI_ERR_CMD_DISALLOWED; - } - - if (requested_cte_length < BT_HCI_LE_CTE_LEN_MIN || - requested_cte_length > BT_HCI_LE_CTE_LEN_MAX) { - return BT_HCI_ERR_UNSUPP_FEATURE_PARAM_VAL; - } - - if (requested_cte_type != BT_HCI_LE_AOA_CTE && - requested_cte_type != BT_HCI_LE_AOD_CTE_1US && - requested_cte_type != BT_HCI_LE_AOD_CTE_2US) { - return BT_HCI_ERR_UNSUPP_FEATURE_PARAM_VAL; - } + if (cte_request_interval != 0 && cte_request_interval < conn->lll.latency) { + return BT_HCI_ERR_CMD_DISALLOWED; + } - /* If controller is aware of features supported by peer device then check - * whether required features are enabled. - */ - if (conn->llcp.fex.valid && - (!(conn->llcp.fex.features_peer & BIT64(BT_LE_FEAT_BIT_CONN_CTE_RESP)))) { - return BT_HCI_ERR_UNSUPP_REMOTE_FEATURE; - } + if (requested_cte_length < BT_HCI_LE_CTE_LEN_MIN || + requested_cte_length > BT_HCI_LE_CTE_LEN_MAX) { + return BT_HCI_ERR_UNSUPP_FEATURE_PARAM_VAL; + } - conn->llcp.cte_req.is_enabled = 1U; - conn->llcp.cte_req.req_interval = cte_request_interval; - conn->llcp.cte_req.cte_type = requested_cte_type; - conn->llcp.cte_req.min_cte_len = requested_cte_length; + if (requested_cte_type != BT_HCI_LE_AOA_CTE && + requested_cte_type != BT_HCI_LE_AOD_CTE_1US && + requested_cte_type != BT_HCI_LE_AOD_CTE_2US) { + return BT_HCI_ERR_UNSUPP_FEATURE_PARAM_VAL; } + conn->llcp.cte_req.is_enabled = 1U; + conn->llcp.cte_req.req_interval = cte_request_interval; + conn->llcp.cte_req.cte_type = requested_cte_type; + conn->llcp.cte_req.min_cte_len = requested_cte_length; + return ull_cp_cte_req(conn, requested_cte_length, requested_cte_type); } #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_REQ */ From a5212f07c088a8567e38c77a1ee5548dc916cf71 Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Fri, 29 Apr 2022 14:36:38 +0200 Subject: [PATCH 03/10] tests: Bluetooth: cte_req: Add tests for handling LL_UNKNOWN_RSP Add test that verify correct behavior of the CTE request procedure in case of reception of LL_UNKNOWN_RSP PDU from peer device. Signed-off-by: Piotr Pryga --- .../controller/ctrl_cte_req/src/main.c | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/tests/bluetooth/controller/ctrl_cte_req/src/main.c b/tests/bluetooth/controller/ctrl_cte_req/src/main.c index 7306cbc570a13..3a11c4b9c3102 100644 --- a/tests/bluetooth/controller/ctrl_cte_req/src/main.c +++ b/tests/bluetooth/controller/ctrl_cte_req/src/main.c @@ -734,6 +734,96 @@ void test_cte_req_reject_inv_ll_param_peripheral_remote(void) "Free CTX buffers %d", ctx_buffers_free()); } +/* +-----+ +-------+ +-----+ + * | UT | | LL_A | | LT | + * +-----+ +-------+ +-----+ + * | | | + * | Start initiation | | + * | CTE Request Proc. | | + * |--------------------------->| | + * | | | + * | | LL_LE_CTE_REQ | + * | |------------------------------->| + * | | | + * | | LL_UNKNOWN_RSP | + * | |<-------------------------------| + * | | | + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * | | | + * | LE CTE Request Failed | | + * |<---------------------------| | + * | | | + * | | | + */ +void test_cte_req_ll_unknown_rsp_local(uint8_t role) +{ + uint8_t err; + struct node_tx *tx; + + struct pdu_data_llctrl_cte_req local_cte_req = { + .cte_type_req = BT_HCI_LE_AOD_CTE_1US, + .min_cte_len_req = BT_HCI_LE_CTE_LEN_MIN, + }; + + struct pdu_data_llctrl_unknown_rsp unknown_rsp = { .type = PDU_DATA_LLCTRL_TYPE_CTE_REQ }; + struct node_rx_pdu *ntf; + + /* Role */ + test_set_role(&conn, role); + + /* Connect */ + ull_cp_state_set(&conn, ULL_CP_CONNECTED); + + /* Initiate an CTE Request Procedure */ + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); + zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); + + /* Prepare */ + event_prepare(&conn); + + /* Tx Queue should have one LL Control PDU */ + lt_rx(LL_CTE_REQ, &conn, &tx, &local_cte_req); + lt_rx_q_is_empty(&conn); + + /* Rx */ + lt_tx(LL_UNKNOWN_RSP, &conn, &unknown_rsp); + + /* Done */ + event_done(&conn); + + /* Receive notification of reception of unknown response. The notification is changed to + * HCI_LE_CTE_Request_Failed before send to host by HCI. This is why it is verified if CTE + * request state machine sends LL_UNKNOWN_RSP towards host. + */ + ut_rx_pdu(LL_UNKNOWN_RSP, &ntf, &unknown_rsp); + + /* The RX queue should be empty now */ + ut_rx_q_is_empty(); + + /* Release Ntf */ + ull_cp_release_ntf(ntf); + + /* Release tx node */ + ull_cp_release_tx(&conn, tx); + + zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt(), "Free CTX buffers %d", + ctx_buffers_free()); + + /* Verify that CTE response feature is marked as not supported by peer device */ + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); + zassert_equal(err, BT_HCI_ERR_UNSUPP_REMOTE_FEATURE, NULL); +} + +void test_cte_req_ll_unknown_rsp_central_local(void) +{ + test_cte_req_ll_unknown_rsp_local(BT_HCI_ROLE_CENTRAL); +} + +void test_cte_req_ll_unknown_rsp_peripheral_local(void) +{ + test_cte_req_ll_unknown_rsp_local(BT_HCI_ROLE_PERIPHERAL); +} + /* Tests related with PHY update procedure and CTE request procedure "collision" */ #define PREFER_S2_CODING 0U @@ -1509,6 +1599,10 @@ void test_main(void) setup, unit_test_noop), ztest_unit_test_setup_teardown(test_cte_req_reject_inv_ll_param_peripheral_remote, setup, unit_test_noop), + ztest_unit_test_setup_teardown(test_cte_req_ll_unknown_rsp_central_local, setup, + unit_test_noop), + ztest_unit_test_setup_teardown(test_cte_req_ll_unknown_rsp_peripheral_local, setup, + unit_test_noop), ztest_unit_test_setup_teardown( test_central_local_cte_req_wait_for_phy_update_complete_and_disable, setup, unit_test_noop), From b6a680fb6ac52620c3ce6bf6067f855ddae304f1 Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Fri, 29 Apr 2022 14:42:47 +0200 Subject: [PATCH 04/10] tests: Bluetooth: cte_req: Enable not used test case There was a test case that was never executed. It was not added to a test suite in test_main. The commit adds the test case. Signed-off-by: Piotr Pryga --- tests/bluetooth/controller/ctrl_cte_req/src/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bluetooth/controller/ctrl_cte_req/src/main.c b/tests/bluetooth/controller/ctrl_cte_req/src/main.c index 3a11c4b9c3102..2b1b14cf3d285 100644 --- a/tests/bluetooth/controller/ctrl_cte_req/src/main.c +++ b/tests/bluetooth/controller/ctrl_cte_req/src/main.c @@ -1610,7 +1610,7 @@ void test_main(void) test_central_local_cte_req_wait_for_phy_update_complete_and_disable, setup, unit_test_noop), ztest_unit_test_setup_teardown( - test_peripheral_local_cte_req_wait_for_phy_update_complete, setup, + test_central_local_cte_req_wait_for_phy_update_complete, setup, unit_test_noop), ztest_unit_test_setup_teardown( test_central_local_phy_update_wait_for_cte_req_complete, setup, From d8134f77cd924f5d2597c4779ce9fa96ebe09cb6 Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Fri, 29 Apr 2022 14:44:39 +0200 Subject: [PATCH 05/10] tests: Bluetooth: Add missing NTF release There were couple of test cases where Host notification object was not released. The commit adds missing release calls. Also commnets related with check if RX queue is empty were changed to describe what is verified then. Signed-off-by: Piotr Pryga --- .../controller/ctrl_cte_req/src/main.c | 35 +++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/tests/bluetooth/controller/ctrl_cte_req/src/main.c b/tests/bluetooth/controller/ctrl_cte_req/src/main.c index 2b1b14cf3d285..83e218e76895f 100644 --- a/tests/bluetooth/controller/ctrl_cte_req/src/main.c +++ b/tests/bluetooth/controller/ctrl_cte_req/src/main.c @@ -110,9 +110,12 @@ void test_cte_req_central_local(void) /* Receive notification of sampled CTE response */ ut_rx_pdu(LL_CTE_RSP, &ntf, &remote_cte_rsp); - /* There should not be a host notifications */ + /* The RX queue should be empty now */ ut_rx_q_is_empty(); + /* Release Ntf */ + ull_cp_release_ntf(ntf); + /* Release tx node */ ull_cp_release_tx(&conn, tx); @@ -288,12 +291,15 @@ void test_cte_req_peripheral_local(void) /* Receive notification of sampled CTE response */ ut_rx_pdu(LL_CTE_RSP, &ntf, &remote_cte_rsp); + /* The RX queue should be empty now */ + ut_rx_q_is_empty(); + + /* Release Ntf */ + ull_cp_release_ntf(ntf); + /* Release tx node */ ull_cp_release_tx(&conn, tx); - /* There should not be a host notifications */ - ut_rx_q_is_empty(); - zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt(), "Free CTX buffers %d", ctx_buffers_free()); } @@ -504,9 +510,12 @@ void test_cte_req_rejected_inv_ll_param_central_local(void) /* Receive notification of sampled CTE response */ ut_rx_pdu(LL_REJECT_EXT_IND, &ntf, &remote_reject_ext_ind); - /* There should not be a host notifications */ + /* The RX queue should be empty now */ ut_rx_q_is_empty(); + /* Release Ntf */ + ull_cp_release_ntf(ntf); + /* Release tx node */ ull_cp_release_tx(&conn, tx); @@ -580,12 +589,15 @@ void test_cte_req_rejected_inv_ll_param_peripheral_local(void) /* Receive notification of sampled CTE response */ ut_rx_pdu(LL_REJECT_EXT_IND, &ntf, &remote_reject_ext_ind); + /* The RX queue should be empty now */ + ut_rx_q_is_empty(); + + /* Release Ntf */ + ull_cp_release_ntf(ntf); + /* Release tx node */ ull_cp_release_tx(&conn, tx); - /* There should not be a host notifications */ - ut_rx_q_is_empty(); - zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt(), "Free CTX buffers %d", ctx_buffers_free()); } @@ -934,9 +946,12 @@ static void run_local_cte_req(struct pdu_data_llctrl_cte_req *cte_req) /* Receive notification of sampled CTE response */ ut_rx_pdu(LL_CTE_RSP, &ntf, &remote_cte_rsp); - /* There should not be a host notifications */ + /* The RX queue should be empty now */ ut_rx_q_is_empty(); + /* Release Ntf */ + ull_cp_release_ntf(ntf); + /* Release tx node */ ull_cp_release_tx(&conn, tx); } @@ -1014,7 +1029,7 @@ void check_phy_update_and_cte_req_complete(bool is_local, struct pdu_data_llctrl ull_cp_release_tx(&conn, tx); } - /* There should not be a host notifications */ + /* The RX queue should be empty now */ ut_rx_q_is_empty(); check_current_phy_state(&conn, phy_req->tx_phys, PREFER_S2_CODING, phy_req->tx_phys); From f4ab463c190860bef585ebdd29baea45c2da4146 Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Fri, 29 Apr 2022 14:47:41 +0200 Subject: [PATCH 06/10] Bluetooth: Controller: llcp: Add handling of unexpe CTRL PDU by CTE req There was missing code responsible for handling of unexpected response for CTE request. The commit adds code that will terminate connection in case a peer device reposnes with unexpected control PDU that is not a remote procedure reques. Signed-off-by: Piotr Pryga --- subsys/bluetooth/controller/ll_sw/ull_llcp_common.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c index e77ed00243de3..934f8dd769ec0 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c @@ -274,7 +274,9 @@ static void lp_comm_complete_cte_req(struct ll_conn *conn, struct proc_ctx *ctx) ull_cp_cte_req_set_disable(conn); ctx->state = LP_COMMON_STATE_IDLE; } else { - /* Illegal response opcode */ + /* Illegal response opcode, internally changes state to + * LP_COMMON_STATE_IDLE + */ lp_comm_terminate_invalid_pdu(conn, ctx); } } else { From 1eb751b68b0a5e24b10ce795f042f3f5a8c40f6b Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Wed, 4 May 2022 07:33:00 +0200 Subject: [PATCH 07/10] tests: Bluetooth: llcp: cte_req: remove invalid rsp test There is a test that verifies a behavior of CTE request control procedure if receives LL_UNKNOWN_RSP. The expected behavior is to terminate connection if that is response handling is not implemented by a command. Other unexpected responses are treated as new remote control procedures. The CTE request has implemented handling of LL_UNKNOWN_RSP, hence this particular test is no longer valid. There is still open question how to handle other unexpected reposne PDUs while waiting for LL_CTE_RSP. That is not defined by BT 5.3. Core specification and not decided by community. The test is removed to because it fails and blocks CI. Signed-off-by: Piotr Pryga --- .../controller/ctrl_cte_req/src/main.c | 108 ------------------ 1 file changed, 108 deletions(-) diff --git a/tests/bluetooth/controller/ctrl_cte_req/src/main.c b/tests/bluetooth/controller/ctrl_cte_req/src/main.c index 83e218e76895f..3d15ff37883c7 100644 --- a/tests/bluetooth/controller/ctrl_cte_req/src/main.c +++ b/tests/bluetooth/controller/ctrl_cte_req/src/main.c @@ -123,112 +123,6 @@ void test_cte_req_central_local(void) "Free CTX buffers %d", ctx_buffers_free()); } -/* Tests of invalid rsp execution of CTE Request Procedure */ - -/* +-----+ +-------+ +-----+ - * | UT | | LL_A | | LT | - * +-----+ +-------+ +-----+ - * | | | - * | Start initiation | | - * | CTE Request Proc. | | - * |--------------------------->| | - * | | | - * | | LL_LE_CTE_REQ | - * | |------------------>| - * | | | - * | | LL__RSP | - * | |<------------------| - * | | | - * ~~~~~~~~~~~~~~~~~ TERMINATE CONNECTION ~~~~~~~~~~~~~~ - * | | | - * | | | - * | | | - */ -void test_cte_req_central_local_invalid_rsp(void) -{ - uint8_t err; - struct node_tx *tx; - struct pdu_data_llctrl_unknown_rsp unknown_rsp = { - .type = PDU_DATA_LLCTRL_TYPE_CTE_REQ - }; - struct pdu_data_llctrl_reject_ind reject_ind = { - .error_code = BT_HCI_ERR_LL_PROC_COLLISION - }; - struct pdu_data_llctrl_cte_req local_cte_req = { - .cte_type_req = BT_HCI_LE_AOA_CTE, - .min_cte_len_req = BT_HCI_LE_CTE_LEN_MIN, - }; - - /* Role */ - test_set_role(&conn, BT_HCI_ROLE_PERIPHERAL); - - /* Connect */ - ull_cp_state_set(&conn, ULL_CP_CONNECTED); - - /* Initiate an CTE Request Procedure */ - err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); - zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); - - /* Prepare */ - event_prepare(&conn); - - /* Tx Queue should have one LL Control PDU */ - lt_rx(LL_CTE_REQ, &conn, &tx, &local_cte_req); - lt_rx_q_is_empty(&conn); - - /* Rx */ - lt_tx(LL_UNKNOWN_RSP, &conn, &unknown_rsp); - - /* Done */ - event_done(&conn); - - /* There should not be a host notifications */ - ut_rx_q_is_empty(); - - /* Release tx node */ - ull_cp_release_tx(&conn, tx); - - /* Termination 'triggered' */ - zassert_equal(conn.llcp_terminate.reason_final, BT_HCI_ERR_LMP_PDU_NOT_ALLOWED, - "Terminate reason %d", conn.llcp_terminate.reason_final); - - /* Clear termination flag for subsequent test cycle */ - conn.llcp_terminate.reason_final = 0; - - zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt(), - "Free CTX buffers %d", ctx_buffers_free()); - - /* Initiate another CTE Request Procedure */ - err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); - zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); - - /* Prepare */ - event_prepare(&conn); - - /* Tx Queue should have one LL Control PDU */ - lt_rx(LL_CTE_REQ, &conn, &tx, &local_cte_req); - lt_rx_q_is_empty(&conn); - - /* Rx */ - lt_tx(LL_REJECT_IND, &conn, &reject_ind); - - /* Done */ - event_done(&conn); - - /* There should not be a host notifications */ - ut_rx_q_is_empty(); - - /* Release tx node */ - ull_cp_release_tx(&conn, tx); - - /* Termination 'triggered' */ - zassert_equal(conn.llcp_terminate.reason_final, BT_HCI_ERR_LMP_PDU_NOT_ALLOWED, - "Terminate reason %d", conn.llcp_terminate.reason_final); - - zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt(), - "Free CTX buffers %d", ctx_buffers_free()); -} - /* +-----+ +-------+ +-----+ * | UT | | LL_A | | LT | * +-----+ +-------+ +-----+ @@ -1599,8 +1493,6 @@ void test_main(void) ztest_test_suite( cte_req, ztest_unit_test_setup_teardown(test_cte_req_central_local, setup, unit_test_noop), - ztest_unit_test_setup_teardown(test_cte_req_central_local_invalid_rsp, setup, - unit_test_noop), ztest_unit_test_setup_teardown(test_cte_req_peripheral_local, setup, unit_test_noop), ztest_unit_test_setup_teardown(test_cte_req_central_remote, setup, unit_test_noop), From 40f2d73a897aed28fbdf073697d0294a499a4cba Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Mon, 16 May 2022 11:53:51 +0200 Subject: [PATCH 08/10] tests: Bluetooth: df: Fix wrong max antenna switch pattern len config Due to change in Kconfig CONFIG_BT_CTLR_DF_MAX_ANT_SW_PATTERN_LEN the Kconfigs in tests were wrong and tests were not building. The commit sets the max antenna switch pattern length to actual maximum value. Signed-off-by: Piotr Pryga --- tests/bluetooth/controller/mock_ctrl/include/kconfig.h | 2 +- tests/bluetooth/df/connection_cte_req/prj.conf | 2 +- tests/bluetooth/df/connection_cte_tx_params/prj.conf | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/bluetooth/controller/mock_ctrl/include/kconfig.h b/tests/bluetooth/controller/mock_ctrl/include/kconfig.h index a5a097ea2b8b9..8d3b76f58e69e 100644 --- a/tests/bluetooth/controller/mock_ctrl/include/kconfig.h +++ b/tests/bluetooth/controller/mock_ctrl/include/kconfig.h @@ -158,7 +158,7 @@ #endif #ifndef CONFIG_BT_CTLR_DF_MAX_ANT_SW_PATTERN_LEN -#define CONFIG_BT_CTLR_DF_MAX_ANT_SW_PATTERN_LEN 39 +#define CONFIG_BT_CTLR_DF_MAX_ANT_SW_PATTERN_LEN 38 #endif /* Kconfig Cheats */ diff --git a/tests/bluetooth/df/connection_cte_req/prj.conf b/tests/bluetooth/df/connection_cte_req/prj.conf index 6280ed8e69939..55ba125739027 100644 --- a/tests/bluetooth/df/connection_cte_req/prj.conf +++ b/tests/bluetooth/df/connection_cte_req/prj.conf @@ -28,4 +28,4 @@ CONFIG_BT_DF_CONNECTION_CTE_RSP=y CONFIG_BT_DF_CONNECTION_CTE_REQ=y # Set antenna switch pattern to max allowed value -CONFIG_BT_CTLR_DF_MAX_ANT_SW_PATTERN_LEN=39 +CONFIG_BT_CTLR_DF_MAX_ANT_SW_PATTERN_LEN=38 diff --git a/tests/bluetooth/df/connection_cte_tx_params/prj.conf b/tests/bluetooth/df/connection_cte_tx_params/prj.conf index 6519aeea950c5..c6cca863d455d 100644 --- a/tests/bluetooth/df/connection_cte_tx_params/prj.conf +++ b/tests/bluetooth/df/connection_cte_tx_params/prj.conf @@ -17,4 +17,4 @@ CONFIG_BT_DF=y CONFIG_BT_CTLR_DF=y # set antenna switch pattern to max allowed value -CONFIG_BT_CTLR_DF_MAX_ANT_SW_PATTERN_LEN=39 +CONFIG_BT_CTLR_DF_MAX_ANT_SW_PATTERN_LEN=38 From 3339e0280fd785f907d6f942122e79432857c938 Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Mon, 16 May 2022 11:58:27 +0200 Subject: [PATCH 09/10] tests: Bluetooth: df: Add missing peer supported feature information There was missing information about supported CTE response feature by peer device. That caused a test_set_conn_cte_req_enable to fail. The test expected that peer device supprots CTE response. The missing information is added in a common code responsible for connection object preparation. Signed-off-by: Piotr Pryga --- tests/bluetooth/df/common/src/bt_conn_common.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/bluetooth/df/common/src/bt_conn_common.c b/tests/bluetooth/df/common/src/bt_conn_common.c index f9eeed32fb1f3..43b86e736d520 100644 --- a/tests/bluetooth/df/common/src/bt_conn_common.c +++ b/tests/bluetooth/df/common/src/bt_conn_common.c @@ -14,6 +14,7 @@ #include +#include #include #include #include @@ -43,6 +44,8 @@ uint16_t ut_bt_create_connection(void) conn->llcp.cte_req.is_enabled = 0U; #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_REQ */ + conn->llcp.fex.features_used |= BIT(BT_LE_FEAT_BIT_CONN_CTE_RESP); + return conn->lll.handle; } From b33b05dc3539bfb3baca1ef45d0cd7fc133b5132 Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Mon, 16 May 2022 12:02:32 +0200 Subject: [PATCH 10/10] tests: Bluetooth: df: Add missing conn handle assignment There was missing assignment to a global variable that stores connection handle. The value was always the same, hence it test were not failing. The value was passed over from setup state done for other tests. This change makes sure there is always correct handle stored in global variable. Signed-off-by: Piotr Pryga --- tests/bluetooth/df/connection_cte_req/src/test_cte_req_enable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bluetooth/df/connection_cte_req/src/test_cte_req_enable.c b/tests/bluetooth/df/connection_cte_req/src/test_cte_req_enable.c index 6cd6d782374de..abf92e3a70ae1 100644 --- a/tests/bluetooth/df/connection_cte_req/src/test_cte_req_enable.c +++ b/tests/bluetooth/df/connection_cte_req/src/test_cte_req_enable.c @@ -194,7 +194,7 @@ static void cte_rx_param_setup(void) cte_req_params_set(); - ut_bt_create_connection(); + g_conn_handle = ut_bt_create_connection(); ut_bt_set_periph_latency(g_conn_handle, CONN_PERIPH_LATENCY); send_set_conn_cte_rx_params(g_conn_handle, &cte_rx_params, true);