Skip to content

Commit

Permalink
Bluetooth: controller: Fix Data Length Update
Browse files Browse the repository at this point in the history 
Before a LL_FEATURE_REQ is sent, verify that the Data Length Update
procedure is still supported as it could have been unmasked as the result
of a previously completed Feature Exchange procedure.

Add unit test to verify the case of enqueuing both a
Feature Exchange procedure and Data Length Update procedure, where the
Feature Exchange procedure would result in the unmasking of the Data
Length Update procedure.

Signed-off-by: Thomas Ebert Hansen <[email protected]>
  • Loading branch information
@thoh-ot @henrikbrixandersen
thoh-ot authored and henrikbrixandersen committed Feb 9, 2024
1 parent 8c41297 commit b15452b
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 1 deletion.
10 changes: 10 additions & 0 deletions subsys/bluetooth/controller/ll_sw/ull_llcp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,16 @@ uint8_t ull_cp_data_length_update(struct ll_conn *conn, uint16_t max_tx_octets,
{
struct proc_ctx *ctx;

if (!feature_dle(conn)) {
/* Data Length Update procedure not supported */

/* Returning BT_HCI_ERR_SUCCESS here might seem counter-intuitive,
* but nothing in the specification seems to suggests
* BT_HCI_ERR_UNSUPP_REMOTE_FEATURE.
*/
return BT_HCI_ERR_SUCCESS;
}

ctx = llcp_create_local_procedure(PROC_DATA_LENGTH_UPDATE);

if (!ctx) {
Expand Down
6 changes: 5 additions & 1 deletion subsys/bluetooth/controller/ll_sw/ull_llcp_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ static void lp_comm_send_req(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t
#endif /* CONFIG_BT_CTLR_CENTRAL_ISO || CONFIG_BT_CTLR_PERIPHERAL_ISO */
#if defined(CONFIG_BT_CTLR_DATA_LENGTH)
case PROC_DATA_LENGTH_UPDATE:
if (!ull_cp_remote_dle_pending(conn)) {
if (feature_dle(conn) && !ull_cp_remote_dle_pending(conn)) {
if (llcp_lr_ispaused(conn) || !llcp_tx_alloc_peek(conn, ctx)) {
ctx->state = LP_COMMON_STATE_WAIT_TX;
} else {
Expand All @@ -662,6 +662,10 @@ static void lp_comm_send_req(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t
/* REQ was received from peer and RSP not yet sent
* lets piggy-back on RSP instead af sending REQ
* thus we can complete local req
*
* OR
*
* Data Length Update procedure no longer supported
*/
llcp_lr_complete(conn);
ctx->state = LP_COMMON_STATE_IDLE;
Expand Down
108 changes: 108 additions & 0 deletions tests/bluetooth/controller/ctrl_data_length_update/src/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,114 @@ ZTEST(dle_central, test_data_length_update_central_loc_unknown_rsp)
"Free CTX buffers %d", llcp_ctx_buffers_free());
}

/*
* Locally triggered Data Length Update procedure
*
*
* Start a Feature Exchange procedure and Data Length Update procedure.
*
* The Feature Exchange procedure completes, removing Data Length Update
* procedure support.
*
* Expect that the already enqueued Data Length Update procedure completes
* without doing anything.
*
* +-----+ +-------+ +-----+
* | UT | | LL_A | | LT |
* +-----+ +-------+ +-----+
* | | |
* | Start | |
* | Feature Exchange Proc. | |
* |--------------------------->| |
* | | |
* | Start | |
* | Data Length Update Proc. | |
* |--------------------------->| |
* | | |
* | | LL_FEATURE_REQ |
* | |----------------------------->|
* | | |
* | | LL_FEATURE_RSP |
* | |<-----------------------------|
* | | |
* ~~~~~~~~~~~~~~~~~~~~~~~ Unmask DLE support ~~~~~~~~~~~~~~~~~~~~
* | | |
* | Feature Exchange Proc. | |
* | Complete | |
* |<---------------------------| |
* | | |
*/
ZTEST(dle_central, test_data_length_update_central_loc_unsupported)
{
uint8_t err;
struct node_tx *tx;
struct node_rx_pdu *ntf;

struct pdu_data_llctrl_feature_req local_feature_req;
struct pdu_data_llctrl_feature_rsp remote_feature_rsp;
struct pdu_data_llctrl_feature_rsp exp_remote_feature_rsp;

sys_put_le64(DEFAULT_FEATURE, local_feature_req.features);
sys_put_le64(0ULL, remote_feature_rsp.features);
sys_put_le64(0ULL, exp_remote_feature_rsp.features);


test_set_role(&conn, BT_HCI_ROLE_CENTRAL);
/* Connect */
ull_cp_state_set(&conn, ULL_CP_CONNECTED);
/* Init DLE data */
ull_conn_default_tx_octets_set(251);
ull_conn_default_tx_time_set(2120);
ull_dle_init(&conn, PHY_1M);

/* Confirm DLE is indicated as supported */
zassert_equal(feature_dle(&conn), true, "DLE Feature masked out");

/* Initiate a Feature Exchange Procedure */
err = ull_cp_feature_exchange(&conn, 1U);
zassert_equal(err, BT_HCI_ERR_SUCCESS);

/* Initiate a Data Length Update Procedure */
err = ull_cp_data_length_update(&conn, 211, 1800);
zassert_equal(err, BT_HCI_ERR_SUCCESS);

event_prepare(&conn);
/* Tx Queue should have one LL Control PDU */
lt_rx(LL_FEATURE_REQ, &conn, &tx, &local_feature_req);
lt_rx_q_is_empty(&conn);

/* Rx */
lt_tx(LL_FEATURE_RSP, &conn, &remote_feature_rsp);

event_done(&conn);
/* There should be one host notification */

ut_rx_pdu(LL_FEATURE_RSP, &ntf, &exp_remote_feature_rsp);

ut_rx_q_is_empty();

ull_cp_release_tx(&conn, tx);
release_ntf(ntf);

/* Confirm DLE is no longer indicated as supported */
zassert_equal(feature_dle(&conn), false, "DLE Feature not masked out");

/* Prepare another event for enqueued Data Length Update procedure */
event_prepare(&conn);
/* Tx Queue should have no LL Control PDU */
lt_rx_q_is_empty(&conn);
event_done(&conn);

/* Confirm DLE is no longer indicated as supported */
zassert_equal(feature_dle(&conn), false, "DLE Feature not masked out");

/* There should not be a host notifications */
ut_rx_q_is_empty();

zassert_equal(llcp_ctx_buffers_free(), test_ctx_buffers_cnt(),
"Free CTX buffers %d", llcp_ctx_buffers_free());
}

/*
* Locally triggered Data Length Update procedure
*
Expand Down

0 comments on commit b15452b

Please sign in to comment.