Bluetooth: Host: fix stack overflow and improve timing performance for ECC emulation #82217

valeriosetti · 2024-11-28T09:17:32Z

This PR fixes issues (1) and (3) reported here, i.e.

Without the increase in CONFIG_BT_LONG_WQ_STACK_SIZE all Bluetooth pairings on the nRF53 and nRF54 series will fail on target

There's nearly a 10x increase in pairing time (takes up to 10 seconds) which is not only really bad, but will also lead to timeouts on tests on hardware as well

and also potentially reduce issue (4), i.e.

There's a significant increase in memory as pointed out by TinyCrypt deprecation - library's usage removal part 2 (bluetooth) #79931 (comment)

valeriosetti · 2024-11-28T09:22:45Z

@sjanc if/when the CI is green, can you please launch AutoPTS?

cvinayak

nitpick... Nice if the commit title is "Bluetooth: Host: ...". Otherwise LGTM.

When BT_SEND_ECC_EMULATION and the platform uses Mbed TLS as PSA Crypto provider, we select the Cortex-M software optimized implementation of the secp256r1 curve algorithms. This is much faster than the standard support provided by Mbed TLS and it also reduces ROM footprint. Signed-off-by: Valerio Setti <[email protected]>

After the switch from TinyCrypt to PSA Crypto API as crypto backend, runtime crashes might happen on some platform due to BT_LONG_WQ's stack size not being large enough. This commit fixes this issue. Signed-off-by: Valerio Setti <[email protected]>

jhedberg

Looks fine to me, assuming that CI and auto-pts are green.

jhedberg · 2024-11-28T09:31:26Z

And assuming that the tests are green, I think it would be good to fast-track merging this. Not sure if it'd be ok to add the Hotfix label, since this isn't fixing direct upstream CI breakage, but it is definitely unblocking other kinds of testing and potentially fixing downstream CI issues.

This reverts commit 184c0f9. Reducing system heap memory to 1024 caused runtime failures on some platforms. This commit revert that change and return to the previous size. Signed-off-by: Valerio Setti <[email protected]>

alwa-nordic

Approved as a quick fix.

But there should be a followup. The Bluetooth subsystem should not have to know about all kinds of implementations of crypto and when it's possible to enable them.

Instead Bluetooth should just declare a dependency on a particular API, and the crypto subsystem should figure out how to optimize that request. Something like select WANT_CRYPTO_SECP256R1 maybe?

Thalley

Changes look good, but I'll need to run some on-target tests first

sjanc · 2024-11-28T09:51:09Z

#AutoPTS run zephyr nrf53 SM

codecoup-tester · 2024-11-28T09:52:13Z

Scheduled PR #82217 (comment), board: nrf53, estimated start time: 10:53:12, test case count: 71, estimated duration: 0

Test cases to be run

SM/CEN/PROT/BV-01-C
SM/CEN/JW/BV-05-C
SM/CEN/JW/BI-04-C
SM/CEN/JW/BI-01-C
SM/CEN/PKE/BV-01-C
SM/CEN/PKE/BV-04-C
SM/CEN/PKE/BI-01-C
SM/CEN/PKE/BI-02-C
SM/CEN/OOB/BV-01-C
SM/CEN/OOB/BV-03-C
SM/CEN/OOB/BV-09-C
SM/CEN/OOB/BI-01-C
SM/CEN/EKS/BV-01-C
SM/CEN/EKS/BI-01-C
SM/CEN/SIGN/BV-01-C
SM/CEN/SIGN/BV-03-C
SM/CEN/SIGN/BI-01-C
SM/CEN/KDU/BV-04-C
SM/CEN/KDU/BV-05-C
SM/CEN/KDU/BV-06-C
SM/CEN/KDU/BV-10-C
SM/CEN/KDU/BV-11-C
SM/CEN/KDU/BI-04-C
SM/CEN/PIS/BV-02-C
SM/CEN/PIS/BV-03-C
SM/CEN/SCJW/BV-01-C
SM/CEN/SCJW/BV-04-C
SM/CEN/SCJW/BI-01-C
SM/CEN/SCPK/BV-01-C
SM/CEN/SCPK/BV-04-C
SM/CEN/SCPK/BI-01-C
SM/CEN/SCPK/BI-02-C
SM/CEN/SCOB/BV-01-C
SM/CEN/SCOB/BV-04-C
SM/CEN/SCOB/BI-01-C
SM/CEN/SCOB/BI-04-C
SM/PER/PROT/BV-02-C
SM/PER/JW/BV-02-C
SM/PER/JW/BI-03-C
SM/PER/JW/BI-02-C
SM/PER/PKE/BV-02-C
SM/PER/PKE/BV-05-C
SM/PER/PKE/BI-03-C
SM/PER/OOB/BV-02-C
SM/PER/OOB/BV-04-C
SM/PER/OOB/BV-10-C
SM/PER/OOB/BI-02-C
SM/PER/EKS/BV-02-C
SM/PER/EKS/BI-02-C
SM/PER/KDU/BV-01-C
SM/PER/KDU/BV-02-C
SM/PER/KDU/BV-03-C
SM/PER/KDU/BV-07-C
SM/PER/KDU/BV-08-C
SM/PER/KDU/BV-09-C
SM/PER/KDU/BI-02-C
SM/PER/KDU/BI-03-C
SM/PER/KDU/BI-04-C
SM/PER/PIS/BV-01-C
SM/PER/PIS/BV-02-C
SM/PER/SCJW/BV-02-C
SM/PER/SCJW/BV-03-C
SM/PER/SCJW/BI-02-C
SM/PER/SCPK/BV-02-C
SM/PER/SCPK/BV-03-C
SM/PER/SCPK/BI-03-C
SM/PER/SCPK/BI-04-C
SM/PER/SCOB/BV-02-C
SM/PER/SCOB/BV-03-C
SM/PER/SCOB/BI-02-C
SM/PER/SCOB/BI-03-C

jhedberg · 2024-11-28T10:06:44Z

A hotfix for the (unrelated) CI failure is here: #82222

Thalley

Everything works out of the box and the performs is on par with TC

Great work

codecoup-tester · 2024-11-28T10:38:35Z

AutoPTS Bot results:

Failed tests (2)

SM SM/CEN/KDU/BI-04-C FAIL
SM SM/PER/KDU/BI-04-C FAIL

Successful tests (69)

SM SM/CEN/EKS/BI-01-C PASS
SM SM/CEN/EKS/BV-01-C PASS
SM SM/CEN/JW/BI-01-C PASS
SM SM/CEN/JW/BI-04-C PASS
SM SM/CEN/JW/BV-05-C PASS
SM SM/CEN/KDU/BV-04-C PASS
SM SM/CEN/KDU/BV-05-C PASS
SM SM/CEN/KDU/BV-06-C PASS
SM SM/CEN/KDU/BV-10-C PASS
SM SM/CEN/KDU/BV-11-C PASS
SM SM/CEN/OOB/BI-01-C PASS
SM SM/CEN/OOB/BV-01-C PASS
SM SM/CEN/OOB/BV-03-C PASS
SM SM/CEN/OOB/BV-09-C PASS
SM SM/CEN/PIS/BV-02-C PASS
SM SM/CEN/PIS/BV-03-C PASS
SM SM/CEN/PKE/BI-01-C PASS
SM SM/CEN/PKE/BI-02-C PASS
SM SM/CEN/PKE/BV-01-C PASS
SM SM/CEN/PKE/BV-04-C PASS
SM SM/CEN/PROT/BV-01-C PASS
SM SM/CEN/SCJW/BI-01-C PASS
SM SM/CEN/SCJW/BV-01-C PASS
SM SM/CEN/SCJW/BV-04-C PASS
SM SM/CEN/SCOB/BI-01-C PASS
SM SM/CEN/SCOB/BI-04-C PASS
SM SM/CEN/SCOB/BV-01-C PASS
SM SM/CEN/SCOB/BV-04-C PASS
SM SM/CEN/SCPK/BI-01-C PASS
SM SM/CEN/SCPK/BI-02-C PASS
SM SM/CEN/SCPK/BV-01-C PASS
SM SM/CEN/SCPK/BV-04-C PASS
SM SM/CEN/SIGN/BI-01-C PASS
SM SM/CEN/SIGN/BV-01-C PASS
SM SM/CEN/SIGN/BV-03-C PASS
SM SM/PER/EKS/BI-02-C PASS
SM SM/PER/EKS/BV-02-C PASS
SM SM/PER/JW/BI-02-C PASS
SM SM/PER/JW/BI-03-C PASS
SM SM/PER/JW/BV-02-C PASS
SM SM/PER/KDU/BI-02-C PASS
SM SM/PER/KDU/BI-03-C PASS
SM SM/PER/KDU/BV-01-C PASS
SM SM/PER/KDU/BV-02-C PASS
SM SM/PER/KDU/BV-03-C PASS
SM SM/PER/KDU/BV-07-C PASS
SM SM/PER/KDU/BV-08-C PASS
SM SM/PER/KDU/BV-09-C PASS
SM SM/PER/OOB/BI-02-C PASS
SM SM/PER/OOB/BV-02-C PASS
SM SM/PER/OOB/BV-04-C PASS
SM SM/PER/OOB/BV-10-C PASS
SM SM/PER/PIS/BV-01-C PASS
SM SM/PER/PIS/BV-02-C PASS
SM SM/PER/PKE/BI-03-C PASS
SM SM/PER/PKE/BV-02-C PASS
SM SM/PER/PKE/BV-05-C PASS
SM SM/PER/PROT/BV-02-C PASS
SM SM/PER/SCJW/BI-02-C PASS
SM SM/PER/SCJW/BV-02-C PASS
SM SM/PER/SCJW/BV-03-C PASS
SM SM/PER/SCOB/BI-02-C PASS
SM SM/PER/SCOB/BI-03-C PASS
SM SM/PER/SCOB/BV-02-C PASS
SM SM/PER/SCOB/BV-03-C PASS
SM SM/PER/SCPK/BI-03-C PASS
SM SM/PER/SCPK/BI-04-C PASS
SM SM/PER/SCPK/BV-02-C PASS
SM SM/PER/SCPK/BV-03-C PASS

Thalley · 2024-11-28T10:44:31Z

AutoPTS Bot results:
Failed tests (2)

Successful tests (69)

These are expected results (the failing tests were also failing before the TC deprecation).

This commit comments out some test AC cases that are failing in bsim. These will be investigated in PR zephyrproject-rtos#82199. Signed-off-by: Valerio Setti <[email protected]>

valeriosetti · 2024-11-28T11:03:26Z

@fabiobaltieri here's a follow-up for the results of west build -p always -b nrf52dk/nrf52832 samples/bluetooth/peripheral_hids on different branches:

main

Memory region         Used Size  Region Size  %age Used
           FLASH:      203988 B       512 KB     38.91%
             RAM:       30904 B        64 KB     47.16%
        IDT_LIST:          0 GB        32 KB      0.00%

tinycrypt (as you reported in 79931)

Memory region         Used Size  Region Size  %age Used
           FLASH:      181116 B       512 KB     34.55%
             RAM:       29240 B        64 KB     44.62%
        IDT_LIST:          0 GB        32 KB      0.00%

this PR

Memory region         Used Size  Region Size  %age Used
           FLASH:      191408 B       512 KB     36.51%
             RAM:       30968 B        64 KB     47.25%
        IDT_LIST:          0 GB        32 KB      0.00%

The ROM increase is half of what it was with #79931.

Edit: if you also want to save RAM, you can set CONFIG_MBEDTLS_PSA_KEY_SLOT_COUNT to a lower value. The default 16 is quite large for basic BT application I think. With 4 for example you'll have

Memory region         Used Size  Region Size  %age Used
           FLASH:      191408 B       512 KB     36.51%
             RAM:       29752 B        64 KB     45.40%
        IDT_LIST:          0 GB        32 KB      0.00%

fabiobaltieri · 2024-11-28T11:39:10Z

@valeriosetti cool, thanks for the followup.

jhedberg · 2024-11-28T13:41:03Z

This is also fixing breakage on Silabs boards, as discovered here: #81702

jhedberg · 2024-11-28T14:03:39Z

Added the Hotfix label, since I think this falls under what's defined here: https://docs.zephyrproject.org/latest/project/dev_env_and_tools.html#hotfix

zephyrbot added the area: Bluetooth Host label Nov 28, 2024

valeriosetti requested a review from Thalley November 28, 2024 09:18

zephyrbot added the area: Bluetooth label Nov 28, 2024

valeriosetti requested a review from jhedberg November 28, 2024 09:18

zephyrbot requested review from alwa-nordic, hermabe, rugeGerritsen, sjanc and theob-pro November 28, 2024 09:18

zephyrbot assigned jhedberg and alwa-nordic Nov 28, 2024

valeriosetti requested review from PavelVPV and cvinayak November 28, 2024 09:18

cvinayak reviewed Nov 28, 2024

View reviewed changes

valeriosetti added 2 commits November 28, 2024 10:24

valeriosetti force-pushed the bt-improvements branch from e3126db to 6698bed Compare November 28, 2024 09:25

valeriosetti changed the title ~~bt: host: fix stack overflow and improve timing performance for ECC emulation~~ Bluetooth: jost: fix stack overflow and improve timing performance for ECC emulation Nov 28, 2024

valeriosetti changed the title ~~Bluetooth: jost: fix stack overflow and improve timing performance for ECC emulation~~ Bluetooth: Host: fix stack overflow and improve timing performance for ECC emulation Nov 28, 2024

valeriosetti requested a review from cvinayak November 28, 2024 09:26

jhedberg previously approved these changes Nov 28, 2024

View reviewed changes

alwa-nordic previously approved these changes Nov 28, 2024

View reviewed changes

valeriosetti dismissed stale reviews from alwa-nordic and jhedberg via 0b912ec November 28, 2024 09:38

zephyrbot added area: Bluetooth HCI Bluetooth HCI Driver area: Samples Samples labels Nov 28, 2024

zephyrbot requested a review from HoZHel November 28, 2024 09:39

zephyrbot requested review from kartben and nashif November 28, 2024 09:39

Thalley requested changes Nov 28, 2024

View reviewed changes

alwa-nordic previously approved these changes Nov 28, 2024

View reviewed changes

Thalley previously approved these changes Nov 28, 2024

View reviewed changes

tests: bsim: comment out some failing AC tests

43dd9c1

This commit comments out some test AC cases that are failing in bsim. These will be investigated in PR zephyrproject-rtos#82199. Signed-off-by: Valerio Setti <[email protected]>

valeriosetti dismissed stale reviews from Thalley and alwa-nordic via 43dd9c1 November 28, 2024 10:49

zephyrbot added platform: nRF BSIM Nordic Semiconductors, nRF BabbleSim area: Bluetooth Audio labels Nov 28, 2024

zephyrbot requested review from asbjornsabo, Casper-Bonde-Bose and MariuszSkamra November 28, 2024 10:50

valeriosetti mentioned this pull request Nov 28, 2024

Bluetooth: Controller: Fix reschedule of ticker with yield, re-enable bsim audio tests using ext_adv #82199

Open

jhedberg approved these changes Nov 28, 2024

View reviewed changes

kedMertens mentioned this pull request Nov 28, 2024

boards: silabs: add default uart-pipe options #81702

Merged

Thalley approved these changes Nov 28, 2024

View reviewed changes

jhedberg added the Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc. label Nov 28, 2024

fabiobaltieri approved these changes Nov 28, 2024

View reviewed changes

fabiobaltieri merged commit 1320b3d into zephyrproject-rtos:main Nov 28, 2024
32 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bluetooth: Host: fix stack overflow and improve timing performance for ECC emulation #82217

Bluetooth: Host: fix stack overflow and improve timing performance for ECC emulation #82217

valeriosetti commented Nov 28, 2024

valeriosetti commented Nov 28, 2024

cvinayak left a comment

jhedberg left a comment

jhedberg commented Nov 28, 2024

alwa-nordic left a comment

Thalley left a comment

sjanc commented Nov 28, 2024

codecoup-tester commented Nov 28, 2024

jhedberg commented Nov 28, 2024

Thalley left a comment

codecoup-tester commented Nov 28, 2024

Thalley commented Nov 28, 2024

valeriosetti commented Nov 28, 2024 •

edited

Loading

fabiobaltieri commented Nov 28, 2024

jhedberg commented Nov 28, 2024

jhedberg commented Nov 28, 2024

Bluetooth: Host: fix stack overflow and improve timing performance for ECC emulation #82217

Bluetooth: Host: fix stack overflow and improve timing performance for ECC emulation #82217

Conversation

valeriosetti commented Nov 28, 2024

valeriosetti commented Nov 28, 2024

cvinayak left a comment

Choose a reason for hiding this comment

jhedberg left a comment

Choose a reason for hiding this comment

jhedberg commented Nov 28, 2024

alwa-nordic left a comment

Choose a reason for hiding this comment

Thalley left a comment

Choose a reason for hiding this comment

sjanc commented Nov 28, 2024

codecoup-tester commented Nov 28, 2024

jhedberg commented Nov 28, 2024

Thalley left a comment

Choose a reason for hiding this comment

codecoup-tester commented Nov 28, 2024

Thalley commented Nov 28, 2024

valeriosetti commented Nov 28, 2024 • edited Loading

fabiobaltieri commented Nov 28, 2024

jhedberg commented Nov 28, 2024

jhedberg commented Nov 28, 2024

valeriosetti commented Nov 28, 2024 •

edited

Loading