Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backport v3.5-branch] Bluetooth: Controller: Few control procedure and compiler re-ordering fixes #74112

Conversation

cvinayak
Copy link
Contributor

@cvinayak cvinayak commented Jun 11, 2024

Add unit tests to cover explicit LLCP error code check and
cover the same in the Controller implementation.

A Host shall consider any error code that it does not
explicitly understand equivalent to the error code
Unspecified Error (0x1F).

Refactor reused function in BT_CTLR_LE_ENC feature.

Fix missing validation of Connection Update Ind PDU. Ignore
invalid connection update parameters and force a silent
local connection termination.

Only perform retention if not already done.
Ensure 'sched' is performed on phy ntf even if dle is not.

Ensure that in LLCP reference to node_rx is cleared when
retention is NOT used, to avoid corruption of node_rx later
re-allocated

In case a CPR is intiated but rejected due to CPR active on
other connection, rx nodes are leaked due to retained node not
being properly released.

In ull_disable, it is imperative that the callback is set up before a
second reference counter check, otherwise it may happen that an LLL done
event has already passed when the disable callback and semaphore is
assigned.

This causes the HCI thread to wait until timeout and assert after
ull_ticker_stop_with_mark.

For certain compilers, due to compiler optimizations, it can be seen
from the assembler code that the callback is assigned after the second
reference counter check.

By adding memory barriers, the code correctly reorders code to the
expected sequence.

Fixes #75981.

@cvinayak cvinayak marked this pull request as ready for review June 12, 2024 03:52
@cvinayak cvinayak added the Backport Backport PR and backport failure issues label Jun 12, 2024
@fabiobaltieri
Copy link
Member

@cvinayak do you have a tracking bug for this? big PR for a backport btw, is it all reallly needed?

mtpr-ot and others added 8 commits July 17, 2024 06:36
In ull_disable, it is imperative that the callback is set up before a
second reference counter check, otherwise it may happen that an LLL done
event has already passed when the disable callback and semaphore is
assigned.

This causes the HCI thread to wait until timeout and assert after
ull_ticker_stop_with_mark.

For certain compilers, due to compiler optimizations, it can be seen
from the assembler code that the callback is assigned after the second
reference counter check.

By adding memory barriers, the code correctly reorders code to the
expected sequence.

Signed-off-by: Morten Priess <mtpr@oticon.com>
(cherry picked from commit 7f82b6a)
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
In case a CPR is intiated but rejected due to CPR active on
other connection, rx nodes are leaked due to retained node not
being properly released.

Signed-off-by: Erik Brockhoff <erbr@oticon.com>
(cherry picked from commit edef1b7)
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Ensure that in LLCP reference to node_rx is cleared when
retention is NOT used, to avoid corruption of node_rx later
re-allocated

Signed-off-by: Erik Brockhoff <erbr@oticon.com>
(cherry picked from commit 806a4fc)
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Only perform retention if not already done.
Ensure 'sched' is performed on phy ntf even if dle is not.

Signed-off-by: Erik Brockhoff <erbr@oticon.com>
(cherry picked from commit 9d8059b)
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Fix missing validation of Connection Update Ind PDU. Ignore
invalid connection update parameters and force a silent
local connection termination.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
(cherry picked from commit 4b6d3f1)
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Refactor reused function in BT_CTLR_LE_ENC feature.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
(cherry picked from commit fe205a5)
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
A Host shall consider any error code that it does not
explicitly understand equivalent to the error code
Unspecified Error (0x1F).

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
(cherry picked from commit 78466c8)
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Add unit tests to cover explicit LLCP error code check and
cover the same in the Controller implementation.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
(cherry picked from commit d6f2bc9)
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
@cvinayak cvinayak force-pushed the github_v3_5_branch_bt_ctlr_fix branch from 4dc4d72 to 59510f9 Compare July 17, 2024 04:37
@cvinayak cvinayak added this to the v3.5.1 milestone Jul 18, 2024
@jhedberg jhedberg merged commit ac9c9d2 into zephyrproject-rtos:v3.5-branch Jul 22, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Controller area: Bluetooth Backport Backport PR and backport failure issues
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants