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: Fix multiple Extended Adv chain reception #79100

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cvinayak
Copy link
Contributor

@cvinayak cvinayak commented Sep 27, 2024

Fix assertion when enabling simultaneous multiple Extended Advertising chain reception that is enabled by increasing supported auxiliary scan contexts.

Supported auxiliary scan contexts can be increased using CONFIG_BT_CTLR_SCAN_AUX_SET value.

Fixes assertions like:

ASSERTION FAIL [lll_aux] @ WEST_TOPDIR/zephyr/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c

Fix assertion when enabling simultaneous multiple Extended
Advertising chain reception that is enabled by increasing
supported auxiliary scan contexts.

Supported auxiliary scan contexts can be increased using
CONFIG_BT_CTLR_SCAN_AUX_SET value.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
@cvinayak
Copy link
Contributor Author

cvinayak commented Sep 27, 2024

@Tronil @jakkra This fix ensures that ULL is independently able to track and receive multiple interleaved Extended Advertising chains. Association of a Scan Aux context with parent Scan context is only maintained when using LLL scheduling between reception of ADV_EXT_IND and a scheduled reception of AUX_ADV_IND. The association is continued as long as continued LLL scheduling is used through the reception of rest of AUX_CHAIN_IND PDUs; the association is dropped when switching to ULL scheduling.

@cvinayak cvinayak added this to the v4.0.0 milestone Sep 27, 2024
Comment on lines +1552 to +1555
/* Reset Scan context association with any Aux context as a new
* extended advertising chain is being setup for reception here.
*/
lll->lll_aux = NULL;
Copy link
Collaborator

Choose a reason for hiding this comment

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

if lll_aux is non-NULL before this, we don't need to do any cleanups or similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is just an association do that received PDUs are added to a linked list in the aux context. By setting it to NULL, a new aux context will get associated to which the new chain of PDUs will be added to the linked list.

Comment on lines +1278 to +1285
/* TODO: Would the reset of Scan context association with Aux
* context here interfere with LLL scheduling? May be LLL
* scheduling will gracefully stop further reception and release
* linked Rx buffers maintained in the Aux context?
*
* Is this code redundant/faulty here, now that Scan LLL and Aux
* ULL scheduling reset it anyway?
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Who are you asking? :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code has been here, and has not caused harm yet :-) .... the comment is here so that in case we faced the scenario that the question ask, then we know that this is the place to look for. CI is passing and a long duration manual test of running passive scanning with over 10 extended advertiser of ZLL and SDC application around seems stable for over couple hours.
Do not want to delete this code yet, unless i can get a failure due to this code. git blame will associate this comment with this commit and will help with thought process in case of future failure.

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.

4 participants