Skip to content

Commit 5f99c36

Browse files
erbr-otcarlescufi
erbr-ot
authored and
carlescufi
committed
Bluetooth: controller: fix procedure collision handling
If an instant based remote procedure 'overtakes' a local ditto the local procedure will be 'completed' by remote rejection but collision flag would not get set ensuring that a new local conflicting procedure cannot be started before the remote is completed. This can thus lead to invalid local initiation. Added unittest to cover case Fix by ensuring collision flag is set also in the above mentioned scenario. Signed-off-by: Erik Brockhoff <[email protected]>
1 parent 3ffcd75 commit 5f99c36

File tree

2 files changed

+272
-0
lines changed

2 files changed

+272
-0
lines changed

‎subsys/bluetooth/controller/ll_sw/ull_llcp_remote.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,12 @@ static void rr_st_idle(struct ll_conn *conn, uint8_t evt, void *param)
621621
ctx_local->rx_opcode = PDU_DATA_LLCTRL_TYPE_UNUSED;
622622
}
623623

624+
/*
625+
* Block/'hold back' future incompatible local procedures
626+
* in case we run a procedure with instant
627+
*/
628+
rr_set_collision(conn, with_instant);
629+
624630
/* Run remote procedure */
625631
rr_act_run(conn);
626632
rr_set_state(conn, RR_STATE_ACTIVE);

‎tests/bluetooth/controller/ctrl_conn_update/src/main.c

Lines changed: 266 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4194,6 +4194,272 @@ ZTEST(periph_rem, test_conn_update_periph_rem_collision)
41944194
zassert_equal(llcp_ctx_buffers_free(), test_ctx_buffers_cnt(),
41954195
"Free CTX buffers %d", llcp_ctx_buffers_free());
41964196
}
4197+
4198+
/*
4199+
* (A)
4200+
* Central-initiated Connection Parameters Request procedure.
4201+
* Central requests change in LE connection parameters, peripheral’s Host accepts.
4202+
*
4203+
* and
4204+
*
4205+
* (B)
4206+
* Peripheral-initiated Connection Parameters Request procedure.
4207+
* Peripheral requests change in LE connection parameters, central’s Host accepts.
4208+
*
4209+
* NOTE:
4210+
* Peripheral-initiated Connection Parameters Request procedure is paused.
4211+
* Central-initiated Connection Parameters Request procedure is finished.
4212+
* Peripheral-initiated Connection Parameters Request procedure is resumed.
4213+
*
4214+
* +-----+ +-------+ +-----+
4215+
* | UT | | LL_P | | LT |
4216+
* +-----+ +-------+ +-----+
4217+
* | | |
4218+
* | LE Connection Update | |
4219+
* |-------------------------->| | (B)
4220+
* | | LL_CONNECTION_PARAM_REQ |
4221+
* | |<--------------------------| (A)
4222+
* | | LL_CONNECTION_PARAM_REQ |
4223+
* | |-------------------------->| (B)
4224+
* | | |
4225+
* | | |
4226+
* | | |
4227+
* | LE Remote Connection | |
4228+
* | Parameter Request | |
4229+
* |<--------------------------| | (A)
4230+
* | LE Remote Connection | |
4231+
* | Parameter Request | |
4232+
* | Reply | |
4233+
* |-------------------------->| | (A)
4234+
* | | LL_REJECT_EXT_IND |
4235+
* | |<--------------------------| (B)
4236+
* | | |
4237+
* | LE Connection Update | |
4238+
* | Complete (collision) | |
4239+
* |<--------------------------| | (B)
4240+
* | | LL_CONNECTION_PARAM_RSP |
4241+
* | |-------------------------->| (A)
4242+
* | | |
4243+
* | LE Connection Update | |
4244+
* |-------------------------->| | (B)
4245+
* | | |
4246+
* | <------------------------> |
4247+
* | < LOCAL PROCEDURE PAUSED > |
4248+
* | <------------------------> |
4249+
* | | |
4250+
* | | LL_CONNECTION_UPDATE_IND |
4251+
* | |<--------------------------| (A)
4252+
* | | |
4253+
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4254+
* | | |
4255+
* | LE Connection Update | |
4256+
* | Complete | |
4257+
* |<--------------------------| | (A)
4258+
* | | |
4259+
* | <-------------------------> |
4260+
* | < LOCAL PROCEDURE RESUMED > |
4261+
* | <-------------------------> |
4262+
* | | |
4263+
* | | LL_CONNECTION_PARAM_REQ |
4264+
* | |-------------------------->| (B)
4265+
* | | |
4266+
* | | LL_CONNECTION_UPDATE_IND |
4267+
* | |<--------------------------| (B)
4268+
* | | |
4269+
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4270+
* | | |
4271+
* | LE Connection Update | |
4272+
* | Complete | |
4273+
* |<--------------------------| | (B)
4274+
* | | |
4275+
*/
4276+
ZTEST(periph_rem, test_conn_update_periph_rem_late_collision)
4277+
{
4278+
uint8_t err;
4279+
struct node_tx *tx;
4280+
struct node_rx_pdu *ntf;
4281+
uint16_t instant;
4282+
struct pdu_data_llctrl_reject_ext_ind reject_ext_ind = {
4283+
.reject_opcode = PDU_DATA_LLCTRL_TYPE_CONN_PARAM_REQ,
4284+
.error_code = BT_HCI_ERR_LL_PROC_COLLISION
4285+
};
4286+
struct node_rx_pu cu1 = { .status = BT_HCI_ERR_LL_PROC_COLLISION };
4287+
struct node_rx_pu cu = { .status = BT_HCI_ERR_SUCCESS };
4288+
4289+
/* Role */
4290+
test_set_role(&conn, BT_HCI_ROLE_PERIPHERAL);
4291+
4292+
/* Connect */
4293+
ull_cp_state_set(&conn, ULL_CP_CONNECTED);
4294+
4295+
/*******************/
4296+
4297+
/* (B) Initiate a Connection Parameter Request Procedure */
4298+
err = ull_cp_conn_update(&conn, req_B->interval_min, req_B->interval_max, req_B->latency,
4299+
req_B->timeout, NULL);
4300+
zassert_equal(err, BT_HCI_ERR_SUCCESS);
4301+
4302+
/* Prepare */
4303+
event_prepare(&conn);
4304+
4305+
/*******************/
4306+
4307+
/* (A) Rx */
4308+
lt_tx(LL_CONNECTION_PARAM_REQ, &conn, &conn_param_req);
4309+
4310+
/* Done */
4311+
event_done(&conn);
4312+
4313+
/*******************/
4314+
4315+
/* Prepare */
4316+
event_prepare(&conn);
4317+
4318+
/* (B) Tx Queue should have one LL Control PDU */
4319+
req_B->reference_conn_event_count = event_counter(&conn) - 1;
4320+
lt_rx(LL_CONNECTION_PARAM_REQ, &conn, &tx, req_B);
4321+
lt_rx_q_is_empty(&conn);
4322+
4323+
/* Done */
4324+
event_done(&conn);
4325+
4326+
/*******************/
4327+
/* (A) There should be one host notification */
4328+
ut_rx_pdu(LL_CONNECTION_PARAM_REQ, &ntf, &conn_param_req);
4329+
ut_rx_q_is_empty();
4330+
4331+
/* Release Ntf */
4332+
release_ntf(ntf);
4333+
4334+
/*******************/
4335+
/* Rx */
4336+
lt_tx(LL_REJECT_EXT_IND, &conn, &reject_ext_ind);
4337+
4338+
/* (A) */
4339+
ull_cp_conn_param_req_reply(&conn);
4340+
4341+
/*******************/
4342+
4343+
/* Prepare */
4344+
event_prepare(&conn);
4345+
conn_param_rsp.reference_conn_event_count = conn_param_req.reference_conn_event_count;
4346+
4347+
/* (A) Tx Queue should have one LL Control PDU */
4348+
lt_rx(LL_CONNECTION_PARAM_RSP, &conn, &tx, &conn_param_rsp);
4349+
lt_rx_q_is_empty(&conn);
4350+
4351+
/* Done */
4352+
event_done(&conn);
4353+
4354+
/* (A) There should be one host notification */
4355+
ut_rx_node(NODE_CONN_UPDATE, &ntf, &cu1);
4356+
ut_rx_q_is_empty();
4357+
4358+
/* Release Ntf */
4359+
release_ntf(ntf);
4360+
4361+
/* (B) Initiate a Connection Parameter Request Procedure */
4362+
err = ull_cp_conn_update(&conn, req_B->interval_min, req_B->interval_max, req_B->latency,
4363+
req_B->timeout, NULL);
4364+
4365+
/* Prepare */
4366+
event_prepare(&conn);
4367+
/* Done */
4368+
event_done(&conn);
4369+
4370+
4371+
/* (A) Rx */
4372+
conn_update_ind.instant = event_counter(&conn) + 6U;
4373+
instant = conn_update_ind.instant;
4374+
lt_tx(LL_CONNECTION_UPDATE_IND, &conn, &conn_update_ind);
4375+
/* Prepare */
4376+
event_prepare(&conn);
4377+
4378+
/* Done */
4379+
event_done(&conn);
4380+
4381+
/* Release Tx */
4382+
ull_cp_release_tx(&conn, tx);
4383+
4384+
/* */
4385+
while (!is_instant_reached(&conn, instant)) {
4386+
/* Prepare */
4387+
event_prepare(&conn);
4388+
4389+
/* (A) Tx Queue should NOT have a LL Control PDU */
4390+
lt_rx_q_is_empty(&conn);
4391+
4392+
/* Done */
4393+
event_done(&conn);
4394+
4395+
/* (A) There should NOT be a host notification */
4396+
ut_rx_q_is_empty();
4397+
}
4398+
4399+
/* Prepare */
4400+
event_prepare(&conn);
4401+
4402+
/* (B) Tx Queue should have one LL Control PDU */
4403+
req_B->reference_conn_event_count = event_counter(&conn) - 1;
4404+
lt_rx(LL_CONNECTION_PARAM_REQ, &conn, &tx, req_B);
4405+
lt_rx_q_is_empty(&conn);
4406+
4407+
/* Done */
4408+
event_done(&conn);
4409+
4410+
/* (A) There should be one host notification */
4411+
ut_rx_node(NODE_CONN_UPDATE, &ntf, &cu);
4412+
ut_rx_q_is_empty();
4413+
4414+
/* Release Ntf */
4415+
release_ntf(ntf);
4416+
4417+
/* Prepare */
4418+
event_prepare(&conn);
4419+
4420+
/* (B) Tx Queue should NOT have a LL Control PDU */
4421+
lt_rx_q_is_empty(&conn);
4422+
4423+
/* (B) Rx */
4424+
cu_ind_B->instant = instant = event_counter(&conn) + 6;
4425+
lt_tx(LL_CONNECTION_UPDATE_IND, &conn, cu_ind_B);
4426+
4427+
/* Done */
4428+
event_done(&conn);
4429+
4430+
/* */
4431+
while (!is_instant_reached(&conn, instant)) {
4432+
/* Prepare */
4433+
event_prepare(&conn);
4434+
4435+
/* (B) Tx Queue should NOT have a LL Control PDU */
4436+
lt_rx_q_is_empty(&conn);
4437+
4438+
/* Done */
4439+
event_done(&conn);
4440+
4441+
/* (B) There should NOT be a host notification */
4442+
ut_rx_q_is_empty();
4443+
}
4444+
4445+
/* Prepare */
4446+
event_prepare(&conn);
4447+
4448+
/* (B) Tx Queue should NOT have a LL Control PDU */
4449+
lt_rx_q_is_empty(&conn);
4450+
4451+
/* Done */
4452+
event_done(&conn);
4453+
4454+
/* (B) There should be one host notification */
4455+
ut_rx_node(NODE_CONN_UPDATE, &ntf, &cu);
4456+
ut_rx_q_is_empty();
4457+
4458+
/* Release Ntf */
4459+
release_ntf(ntf);
4460+
zassert_equal(llcp_ctx_buffers_free(), test_ctx_buffers_cnt(),
4461+
"Free CTX buffers %d", llcp_ctx_buffers_free());
4462+
}
41974463
#else /* CONFIG_BT_CTLR_CONN_PARAM_REQ */
41984464

41994465
/*

0 commit comments

Comments
 (0)