diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn.c b/subsys/bluetooth/controller/ll_sw/ull_conn.c index c5005c6a816..ff19a9a2f38 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn.c +++ b/subsys/bluetooth/controller/ll_sw/ull_conn.c @@ -1479,32 +1479,33 @@ void ull_conn_done(struct node_rx_event_done *done) } #endif /* CONFIG_BT_CTLR_LE_ENC */ - /* Peripheral received terminate ind or + /* Legacy LLCP: + * Peripheral received terminate ind or * Central received ack for the transmitted terminate ind or * Central transmitted ack for the received terminate ind or * there has been MIC failure + * Refactored LLCP: + * reason_final is set exactly under the above conditions */ reason_final = conn->llcp_terminate.reason_final; if (reason_final && ( -#if defined(CONFIG_BT_PERIPHERAL) - lll->role || -#else /* CONFIG_BT_PERIPHERAL */ - 0 || -#endif /* CONFIG_BT_PERIPHERAL */ -#if defined(CONFIG_BT_CENTRAL) #if defined(CONFIG_BT_LL_SW_LLCP_LEGACY) - (((conn->llcp_terminate.req - - conn->llcp_terminate.ack) & 0xFF) == - TERM_ACKED) || - conn->central.terminate_ack || - (reason_final == BT_HCI_ERR_TERM_DUE_TO_MIC_FAIL) -#else /* CONFIG_BT_LL_SW_LLCP_LEGACY */ - 1 -#endif /* CONFIG_BT_LL_SW_LLCP_LEGACY */ -#else /* CONFIG_BT_CENTRAL */ - 1 +#if defined(CONFIG_BT_CENTRAL) + (((conn->llcp_terminate.req - + conn->llcp_terminate.ack) & 0xFF) == + TERM_ACKED) || + conn->central.terminate_ack || + (reason_final == BT_HCI_ERR_TERM_DUE_TO_MIC_FAIL) || #endif /* CONFIG_BT_CENTRAL */ - )) { +#if defined(CONFIG_BT_PERIPHERAL) + lll->role +#else /* CONFIG_BT_PERIPHERAL */ + false +#endif /* CONFIG_BT_PERIPHERAL */ +#else /* defined(CONFIG_BT_LL_SW_LLCP_LEGACY) */ + true +#endif /* !defined(CONFIG_BT_LL_SW_LLCP_LEGACY) */ + )) { conn_cleanup(conn, reason_final); return; diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c index c614e8ca025..7804c54fe85 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c @@ -77,6 +77,7 @@ enum { enum { RP_COMMON_STATE_IDLE, RP_COMMON_STATE_WAIT_RX, + RP_COMMON_STATE_POSTPONE_TERMINATE, RP_COMMON_STATE_WAIT_TX, RP_COMMON_STATE_WAIT_TX_ACK, RP_COMMON_STATE_WAIT_NTF, @@ -93,6 +94,7 @@ enum { RP_COMMON_EVT_REQUEST, }; + static void lp_comm_ntf(struct ll_conn *conn, struct proc_ctx *ctx); static void lp_comm_terminate_invalid_pdu(struct ll_conn *conn, struct proc_ctx *ctx); @@ -781,8 +783,17 @@ void llcp_lp_comm_init_proc(struct proc_ctx *ctx) void llcp_lp_comm_run(struct ll_conn *conn, struct proc_ctx *ctx, void *param) { lp_comm_execute_fsm(conn, ctx, LP_COMMON_EVT_RUN, param); + } +static void rp_comm_terminate(struct ll_conn *conn, struct proc_ctx *ctx) +{ + llcp_rr_complete(conn); + ctx->state = RP_COMMON_STATE_IDLE; + + /* Mark the connection for termination */ + conn->llcp_terminate.reason_final = ctx->data.term.error_code; +} /* * LLCP Remote Procedure Common FSM */ @@ -821,6 +832,8 @@ static void rp_comm_rx_decode(struct ll_conn *conn, struct proc_ctx *ctx, struct break; case PDU_DATA_LLCTRL_TYPE_TERMINATE_IND: llcp_pdu_decode_terminate_ind(ctx, pdu); + /* Make sure no data is tx'ed after RX of terminate ind */ + llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_TERMINATE); break; #if defined(CONFIG_BT_CTLR_DATA_LENGTH) case PDU_DATA_LLCTRL_TYPE_LENGTH_REQ: @@ -1051,12 +1064,19 @@ static void rp_comm_send_rsp(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t break; #endif /* CONFIG_BT_CTLR_MIN_USED_CHAN && CONFIG_BT_CENTRAL */ case PROC_TERMINATE: - /* No response */ - llcp_rr_complete(conn); - ctx->state = RP_COMMON_STATE_IDLE; - - /* Mark the connection for termination */ - conn->llcp_terminate.reason_final = ctx->data.term.error_code; +#if defined(CONFIG_BT_CENTRAL) + if (conn->lll.role == BT_HCI_ROLE_CENTRAL) { + /* No response, but postpone terminate until next event + * to ensure acking the reception of TERMINATE_IND + */ + ctx->state = RP_COMMON_STATE_POSTPONE_TERMINATE; + break; + } +#endif +#if defined(CONFIG_BT_PERIPHERAL) + /* Terminate right away */ + rp_comm_terminate(conn, ctx); +#endif break; #if defined(CONFIG_BT_CTLR_DATA_LENGTH) case PROC_DATA_LENGTH_UPDATE: @@ -1102,6 +1122,26 @@ static void rp_comm_st_wait_rx(struct ll_conn *conn, struct proc_ctx *ctx, uint8 } } +static void rp_comm_st_postpone_terminate(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t evt, + void *param) +{ + switch (evt) { + case RP_COMMON_EVT_RUN: + LL_ASSERT(ctx->proc == PROC_TERMINATE); + + /* Note: now we terminate, mimicking legacy LLCP behaviour + * A check should be added to ensure that the ack of the terminate_ind was + * indeed tx'ed and not scheduled out/postponed by LLL + */ + rp_comm_terminate(conn, ctx); + + break; + default: + /* Ignore other evts */ + break; + } +} + static void rp_comm_st_wait_tx(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t evt, void *param) { switch (evt) { @@ -1181,6 +1221,9 @@ static void rp_comm_execute_fsm(struct ll_conn *conn, struct proc_ctx *ctx, uint case RP_COMMON_STATE_WAIT_RX: rp_comm_st_wait_rx(conn, ctx, evt, param); break; + case RP_COMMON_STATE_POSTPONE_TERMINATE: + rp_comm_st_postpone_terminate(conn, ctx, evt, param); + break; case RP_COMMON_STATE_WAIT_TX: rp_comm_st_wait_tx(conn, ctx, evt, param); break; diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h b/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h index c52853d0ea9..00b35b913df 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h @@ -33,6 +33,7 @@ enum llcp_tx_q_pause_data_mask { LLCP_TX_QUEUE_PAUSE_DATA_ENCRYPTION = 0x01, LLCP_TX_QUEUE_PAUSE_DATA_PHY_UPDATE = 0x02, LLCP_TX_QUEUE_PAUSE_DATA_DATA_LENGTH = 0x04, + LLCP_TX_QUEUE_PAUSE_DATA_TERMINATE = 0x08, }; #if ((CONFIG_BT_CTLR_LLCP_COMMON_TX_CTRL_BUF_NUM <\ diff --git a/tests/bluetooth/controller/ctrl_terminate/src/main.c b/tests/bluetooth/controller/ctrl_terminate/src/main.c index c9dd4079899..2c295320c3d 100644 --- a/tests/bluetooth/controller/ctrl_terminate/src/main.c +++ b/tests/bluetooth/controller/ctrl_terminate/src/main.c @@ -66,9 +66,16 @@ static void test_terminate_rem(uint8_t role) /* Done */ event_done(&conn); + /* Prepare */ + event_prepare(&conn); + + /* Done */ + event_done(&conn); + /* There should be no host notification */ ut_rx_q_is_empty(); + zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt(), "Free CTX buffers %d", ctx_buffers_free()); }