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

Bluetooth: controller: split: Fix central assert on multiple reconnections #22005

Merged

Conversation

cvinayak
Copy link
Contributor

This reverts commit 7417e6e.
This reverts commit d3e3f8d.

Updated the lll_conn_flush interface to pass the connection
handle while the LLL connection context stored handle has
been invalidated.

Fixes #22003.

Signed-off-by: Vinayak Kariappa Chettimada [email protected]

This reverts commit 7417e6e.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
…handle

This reverts commit d3e3f8d.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Updated the lll_conn_flush interface to pass the connection
handle while the LLL connection context stored handle has
been invalidated.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
@alexandru-porosanu-nxp
Copy link
Collaborator

I've tested on VEGABoard and the previous behavior (the assertion) doesn't happen. I guess that Nordic is fixed as well.

Copy link
Collaborator

@wopu-ot wopu-ot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I to not think that the PR solves the issue the original PR tried to solve: the mayfly_enqueue in ull_conn.c:1640 may be chained instead of being called inline, which causes the LLL to see the already invalidated handle. As far as I can tell, ll_conn_handle_get(conn) in line 1742 could have the same problem and some other mechanism is needed to ensure that lll_conn_flush sees the handle intact.

@George-Stefan
Copy link
Collaborator

George-Stefan commented Jan 17, 2020

ll_conn_handle_get(conn) returns the correct handle even if lll->handle is invalid.
In lll_conn_flush(), use the new handle param, not lll->handle

@wopu-ot
Copy link
Collaborator

wopu-ot commented Jan 17, 2020

ll_conn_handle_get(conn) returns the correct handle even if lll->handle is invalid.
In lll_conn_flush(), use the new handle param, not lll->handle

I stand corrected. I will need some time to think through the interaction with our LLL.

@bu5hm4nn
Copy link

bu5hm4nn commented Jan 23, 2020

👍 This pull request fixes crashes we were having after a few minutes of continuous transfer when running HCI_UART on nRF52810 with BlueZ 5.52 and connecting with mutliple devices especially Android devices.

@mtpr-ot
Copy link
Collaborator

mtpr-ot commented Jan 27, 2020

This PR also fixes race condition where slave ull_conn_done is invoked after flush. This would give an assert in ticker_update_conn_op_cb, as it attempted to update the ticker drift after ticker node had been stopped. I strongly recommend to merge this PR - @wopu-ot, any objections?

@mtpr-ot mtpr-ot self-requested a review January 27, 2020 12:31
@aescolar aescolar merged commit c793c0e into zephyrproject-rtos:master Jan 27, 2020
@cvinayak cvinayak deleted the github_ctrl_buffer_leak_fix branch March 1, 2021 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'central' failure on nrf52_pca10040
9 participants