From ad46ed78d4896eb54564fe90872feb3a5752afeb Mon Sep 17 00:00:00 2001 From: Erik Brockhoff Date: Mon, 8 Apr 2024 14:03:00 +0200 Subject: [PATCH] bluetooth: controller: fix periph failure to disconnect on proc. collision If central initiates incompatible procedure after having replied (with _IND), peripheral fails to disconnect as spec'ed. Fix by correctly setting the INCOMPAT flag to reserved on IND receipt to enforce the disconnect. Signed-off-by: Erik Brockhoff --- .../controller/ll_sw/ull_llcp_conn_upd.c | 1 + .../bluetooth/controller/ll_sw/ull_llcp_phy.c | 1 + .../controller/ctrl_phy_update/src/main.c | 89 +++++++++++++++++++ 3 files changed, 91 insertions(+) diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c index e922901f67d06b..6152a303616ebe 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c @@ -585,6 +585,7 @@ static void lp_cu_st_wait_rx_conn_update_ind(struct ll_conn *conn, struct proc_c switch (evt) { case LP_CU_EVT_CONN_UPDATE_IND: llcp_pdu_decode_conn_update_ind(ctx, param); + llcp_rr_set_incompat(conn, INCOMPAT_RESERVED); /* Keep RX node to use for NTF */ llcp_rx_node_retain(ctx); ctx->state = LP_CU_STATE_WAIT_INSTANT; diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c index 61ee22506061ef..435d54db9ed33e 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c @@ -717,6 +717,7 @@ static void lp_pu_st_wait_rx_phy_update_ind(struct ll_conn *conn, struct proc_ct switch (evt) { case LP_PU_EVT_PHY_UPDATE_IND: LL_ASSERT(conn->lll.role == BT_HCI_ROLE_PERIPHERAL); + llcp_rr_set_incompat(conn, INCOMPAT_RESERVED); llcp_pdu_decode_phy_update_ind(ctx, (struct pdu_data *)param); const uint8_t end_procedure = pu_check_update_ind(conn, ctx); diff --git a/tests/bluetooth/controller/ctrl_phy_update/src/main.c b/tests/bluetooth/controller/ctrl_phy_update/src/main.c index 8101149bd1b6b4..5951b685932b95 100644 --- a/tests/bluetooth/controller/ctrl_phy_update/src/main.c +++ b/tests/bluetooth/controller/ctrl_phy_update/src/main.c @@ -585,6 +585,95 @@ ZTEST(phy_periph, test_phy_update_periph_loc) "Free CTX buffers %d", llcp_ctx_buffers_free()); } +ZTEST(phy_periph, test_phy_update_periph_loc_invalid_central) +{ + uint8_t err; + struct node_tx *tx; + struct node_rx_pdu *ntf; + struct pdu_data_llctrl_phy_req req = { .rx_phys = PHY_2M, .tx_phys = PHY_2M }; + uint16_t instant; + struct pdu_data_llctrl_conn_param_req conn_param_req = { }; + + struct node_rx_pu pu = { .status = BT_HCI_ERR_SUCCESS }; + + struct pdu_data_llctrl_phy_upd_ind phy_update_ind = { .c_to_p_phy = PHY_2M, + .p_to_c_phy = PHY_2M }; + + /* Role */ + test_set_role(&conn, BT_HCI_ROLE_PERIPHERAL); + + /* Connect */ + ull_cp_state_set(&conn, ULL_CP_CONNECTED); + + /* Initiate an PHY Update Procedure */ + err = ull_cp_phy_update(&conn, PHY_2M, PREFER_S8_CODING, PHY_2M, HOST_INITIATED); + zassert_equal(err, BT_HCI_ERR_SUCCESS); + + /* Prepare */ + event_prepare(&conn); + + /* Tx Queue should have one LL Control PDU */ + lt_rx(LL_PHY_REQ, &conn, &tx, &req); + lt_rx_q_is_empty(&conn); + + /* Check that data tx was paused */ + zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused"); + + /* TX Ack */ + event_tx_ack(&conn, tx); + + /* Check that data tx is no longer paused */ + zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused"); + + /* Done */ + event_done(&conn); + + /* Release Tx */ + ull_cp_release_tx(&conn, tx); + + /* Prepare */ + event_prepare(&conn); + + /* Tx Queue should NOT have a LL Control PDU */ + lt_rx_q_is_empty(&conn); + + /* Rx */ + phy_update_ind.instant = instant = event_counter(&conn) + 6; + lt_tx(LL_PHY_UPDATE_IND, &conn, &phy_update_ind); + + /* Done */ + event_done(&conn); + + /* Check that data tx is no longer paused */ + zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused"); + + /* One event ahead */ + /* Prepare */ + event_prepare(&conn); + + /* Tx Queue should NOT have a LL Control PDU */ + lt_rx_q_is_empty(&conn); + + /* Done */ + event_done(&conn); + + /* There should NOT be a host notification */ + ut_rx_q_is_empty(); + + /* 'Inject' invalid param request from central */ + /* Prepare */ + event_prepare(&conn); + /* Rx */ + lt_tx(LL_CONNECTION_PARAM_REQ, &conn, &conn_param_req); + + /* Done */ + event_done(&conn); + + /* Termination 'triggered' */ + zassert_equal(conn.llcp_terminate.reason_final, BT_HCI_ERR_DIFF_TRANS_COLLISION, + "Terminate reason %d", conn.llcp_terminate.reason_final); +} + ZTEST(phy_periph, test_phy_update_periph_rem) { struct node_tx *tx;