Bluetooth: controller: split: Fix DLE duplicate requests #23707
Conversation
Simplify the Data Length Update Procedure state check when processing incoming LENGTH_REQ/RSP PDUs. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Fix implementation to handle back-to-back and duplicate LENGTH_REQ PDU reception. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
cvinayak
added
backport v2.2-branch
bug
| @@ -4501,26 +4501,20 @@ static inline int length_req_rsp_recv(struct ll_conn *conn, memq_link_t *link, | |||
| if (/* Local idle, and Peer request then complete the Peer procedure | |||
| * with response. | |||
| */ | |||
| ((conn->llcp_length.req == conn->llcp_length.ack) && | |||
| (pdu_rx->llctrl.opcode == PDU_DATA_LLCTRL_TYPE_LENGTH_REQ)) || | |||
| ((conn->llcp_length.req == conn->llcp_length.ack) && tx) || | |||
There was a problem hiding this comment.
The criteria is that opcode equals PDU_DATA_LLCTRL_TYPE_LENGTH_REQ, not that tx!=NULL
Even though tx!=NULL when the opcode is correct, this is not immediately obvious, you need to scroll back in the code.
IMHO the original was easier to follow.
Are there significant reductions in code-size and/or execution time?
There was a problem hiding this comment.
Compare and branch on zero take less instructions and CPU cycles, in compare to load two values compare it and branch set of instructions.
Before simplification:
Memory region Used Size Region Size %age Used
FLASH: 251093 B 1 MB 23.95%
SRAM: 33652 B 256 KB 12.84%
IDT_LIST: 136 B 2 KB 6.64%
After simplification:
Memory region Used Size Region Size %age Used
FLASH: 251029 B 1 MB 23.94%
SRAM: 33652 B 256 KB 12.84%
IDT_LIST: 136 B 2 KB 6.64%
| PDU_DATA_LLCTRL_TYPE_LENGTH_RSP) || | ||
| (pdu_rx->llctrl.opcode == | ||
| PDU_DATA_LLCTRL_TYPE_LENGTH_REQ)))))) { | ||
| (conn->llcp_length.state == LLCP_LENGTH_STATE_RSP_WAIT)))) { |
There was a problem hiding this comment.
Can you explain why the test for PDU_DATA_LLCTRL_TYPE_LENGTH_RSP not is required?
I think the comment must be updated as well
There was a problem hiding this comment.
The function enters either for PDU_DATA_LLCTRL_TYPE_LENGTH_RSP or PDU_DATA_LLCTRL_TYPE_LENGTH_REQ, and it is redundant to check for the same (||) here.
Fix implementation to handle back-to-back and duplicate LENGTH_REQ PDU reception. Relates to zephyrproject-rtos#23707. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Fix implementation to handle back-to-back and duplicate LENGTH_REQ PDU reception. Relates to #23707. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Fix implementation to handle back-to-back and duplicate LENGTH_REQ PDU reception. Relates to zephyrproject-rtos#23707. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
|
@cvinayak thank you~~ |
|
@JiaDuo Your tester should send two LL_LENGTH_REQ PDU, one after the other in the same connection interval while not receiving the LL_LENGTH_RSP (nack the reception). This is an invalid behavior testing. |
|
Thank you very much! |
|
@cvinayak Can you give me more suggestions? |
Fix implementation to handle back-to-back and duplicate
LENGTH_REQ PDU reception.
Signed-off-by: Vinayak Kariappa Chettimada vich@nordicsemi.no