tests: bsim: bluetooth: mesh: Remove TinyCrypt bsim tests #90208

PavelVPV · 2025-05-20T09:31:43Z

This PR removes TinyCrypt Bluetooth mesh bsim tests.

alxelax · 2025-05-20T12:29:37Z

If you want to test the low latency feature of Controller, please, implement bsim tests for Controller. Mesh is not correct place for testing this feature.

PavelVPV · 2025-05-21T05:53:38Z

If you want to test the low latency feature of Controller, please, implement bsim tests for Controller. Mesh is not correct place for testing this feature.

Could you show me where I added a new test for low latency feature in this PR?

alxelax · 2025-05-21T07:14:37Z

If you want to test the low latency feature of Controller, please, implement bsim tests for Controller. Mesh is not correct place for testing this feature.

Could you show me where I added a new test for low latency feature in this PR?

here: 3d813ab

all tests those are run with the low latency feature except transport and friendship are unnecessary

alxelax

Testing of the low latency feature should go to ZLL bsim tests.

PavelVPV · 2025-05-21T07:21:12Z

Testing of the low latency feature should go to ZLL bsim tests.

I disagree with this. This should be discussed separately. This PR is not about low latency feature at all.

alxelax · 2025-05-21T07:26:42Z

Testing of the low latency feature should go to ZLL bsim tests.

I disagree with this. This should be discussed separately. This PR is not about low latency feature at all.

This PR increases test scope on unnecessary manner. It should be done correctly. I will not approve such approach.

PavelVPV · 2025-05-21T07:32:51Z

Testing of the low latency feature should go to ZLL bsim tests.

I disagree with this. This should be discussed separately. This PR is not about low latency feature at all.

This PR increases test scope on unnecessary manner. It should be done correctly. I will not approve such approach.

It doesn't if you see the final diff except legacyt advertiser tests, same as yours.

PavelVPV · 2025-05-21T07:33:07Z

If you want to test the low latency feature of Controller, please, implement bsim tests for Controller. Mesh is not correct place for testing this feature.

Could you show me where I added a new test for low latency feature in this PR?

here: 3d813ab

all tests those are run with the low latency feature except transport and friendship are unnecessary

This is a preparation for switching to PSA.

alxelax · 2025-05-21T07:38:46Z

If you want to test the low latency feature of Controller, please, implement bsim tests for Controller. Mesh is not correct place for testing this feature.

Could you show me where I added a new test for low latency feature in this PR?

here: 3d813ab
all tests those are run with the low latency feature except transport and friendship are unnecessary

This is a preparation for switching to PSA.

It is why it should be done properly.

alxelax · 2025-05-21T07:47:28Z

I guess it is worth to split removing Tinycrypt and refactoring the low latency feature usage in different PRs to not block Tinycrypt deprecation procedure by not related discussion.

PavelVPV · 2025-05-21T08:12:51Z

I guess it is worth to split removing Tinycrypt and refactoring the low latency feature usage in different PRs to not block Tinycrypt deprecation procedure by not related discussion.

Yes, therefore I swithed low latency tests to PSA as well, so the scope doesn't change (except legacy advertiser tests w/o low lat feature).

alxelax · 2025-05-21T08:24:53Z

I guess it is worth to split removing Tinycrypt and refactoring the low latency feature usage in different PRs to not block Tinycrypt deprecation procedure by not related discussion.

Yes, therefore I swithed low latency tests to PSA as well, so the scope doesn't change (except legacy advertiser tests w/o low lat feature).

After adding legacy advertiser tests w/o low latency feature, all these tests with the low latency became obsolete and should be removed.
Mesh is not the right place to test difference in ZLL Controller behavior with and without its features.

omkar3141 · 2025-05-21T08:26:05Z

I guess it is worth to split removing Tinycrypt and refactoring the low latency feature usage in different PRs to not block Tinycrypt deprecation procedure by not related discussion.

Yes, therefore I swithed low latency tests to PSA as well, so the scope doesn't change (except legacy advertiser tests w/o low lat feature).

After adding legacy advertiser tests w/o low latency feature, all these tests with the low latency became obsolete and should be removed. Mesh is not the right place to test difference in ZLL Controller behavior with and without its features.

It is possible to simply create some light-weight advertiser test by calling host's advertiser APIs and test low latency using that.

PavelVPV · 2025-05-21T08:27:31Z

I guess it is worth to split removing Tinycrypt and refactoring the low latency feature usage in different PRs to not block Tinycrypt deprecation procedure by not related discussion.

Yes, therefore I swithed low latency tests to PSA as well, so the scope doesn't change (except legacy advertiser tests w/o low lat feature).

After adding legacy advertiser tests w/o low latency feature, all these tests with the low latency became obsolete and should be removed. Mesh is not the right place to test difference in ZLL Controller behavior with and without its features.

Different topic.

omkar3141 · 2025-05-21T08:28:00Z

tests/bsim/bluetooth/mesh/prj.conf

-# Enable tinycrypt as a crypto backend
-CONFIG_BT_MESH_USES_TINYCRYPT=y
+# Increase the number of key slots in PSA Crypto core
+CONFIG_MBEDTLS_PSA_KEY_SLOT_COUNT=64


Side note: We will have to write recommendation on how to compute this number. When people start changing Netkey/Appkey indices this number will change, and unfortunately Kconfig parser does not do mathematical operations.

Unfortunately, static key slot allocation is available only here. Our Zephyr fork doesn't support it at the moment since different mbedTLS backend. They are allocated from heap.

omkar3141

This looks ok to me. It will nice if we can take the low latency tests as a seperate discussion or PR, and let developers who want these tests to update them or refactor them to be not dependent on mesh.

alxelax · 2025-05-21T08:32:45Z

I guess it is worth to split removing Tinycrypt and refactoring the low latency feature usage in different PRs to not block Tinycrypt deprecation procedure by not related discussion.

Yes, therefore I swithed low latency tests to PSA as well, so the scope doesn't change (except legacy advertiser tests w/o low lat feature).

After adding legacy advertiser tests w/o low latency feature, all these tests with the low latency became obsolete and should be removed. Mesh is not the right place to test difference in ZLL Controller behavior with and without its features.

Different topic.

I already suggested moving these changes to different PR and continue discussion there. This is not correct to push these changes here motivating by Tinycrypt deprecation importance.

This is a preparation commit for switching all bsim tests to PSA as TinyCrypt will be removed soon. Signed-off-by: Pavel Vasilyev <pavel.vasilyev@nordicsemi.no>

PavelVPV · 2025-05-21T08:43:12Z

Removed tests for legacy advertiser. Kept low latency tests.

alxelax

It looks much better but still requires minor fixes. Please take a look at my comments.

alxelax · 2025-05-22T07:30:25Z

tests/bsim/bluetooth/mesh/compile.sh

@@ -15,24 +15,16 @@ app=tests/bsim/bluetooth/mesh conf_overlay=overlay_pst.conf compile
 app=tests/bsim/bluetooth/mesh conf_overlay=overlay_gatt.conf compile
 app=tests/bsim/bluetooth/mesh conf_overlay=overlay_gatt_separate.conf compile
 app=tests/bsim/bluetooth/mesh conf_overlay=overlay_low_lat.conf compile
-app=tests/bsim/bluetooth/mesh conf_overlay=overlay_psa.conf compile
 app=tests/bsim/bluetooth/mesh conf_overlay=overlay_workq_sys.conf compile
 app=tests/bsim/bluetooth/mesh conf_overlay=overlay_multi_adv_sets.conf compile
 app=tests/bsim/bluetooth/mesh conf_overlay=overlay_lpn_scan_on.conf compile
 app=tests/bsim/bluetooth/mesh \


I guess this line should be removed too. It doesn't throw error only because line 22 is empty.

alxelax · 2025-05-22T07:31:20Z

tests/bsim/bluetooth/mesh/overlay_pst.conf

@@ -15,3 +15,6 @@ CONFIG_BT_MESH_SEQ_STORE_RATE=1
 CONFIG_BT_MESH_RPL_STORE_TIMEOUT=1
 CONFIG_BT_MESH_STORE_TIMEOUT=1
 CONFIG_BT_MESH_COMP_PST_BUF_SIZE=600
+
+# Required for PSA
+CONFIG_SECURE_STORAGE=y


Please fix commit message:

- merging `overlay_ss.conf` into `overlay_pst.conf` as secure storage is required for running persistent storage bsim tests with PSA;

As TinyCrypt gets deprecated, we need to remove all bsim tests that use it. This commit switches all Bluetooth mesh bsim tests to PSA as a default crypto backend. This includes: - removing `CONFIG_BT_MESH_USES_TINYCRYPT` as PSA is default crypto backend for mesh. `CONFIG_BT_MESH_USES_MBEDTLS_PSA` is not required to be enabled; - merging `overlay_psa.conf` into `prj.conf` as this configuration becomes default for all mesh bsim tests; - merging `overlay_ss.conf` into `overlay_pst.conf` as secure storage is required for running persistent storage bsim tests with PSA; Signed-off-by: Pavel Vasilyev <pavel.vasilyev@nordicsemi.no>

sonarqubecloud · 2025-05-22T08:24:12Z

Please retry analysis of this Pull-Request directly on SonarQube Cloud

aescolar

From my POV I don't see any issue, so this can be considered a +1.
I do not know if you guys want any more reviews from the mesh side though.

aescolar

+1

PavelVPV force-pushed the remove_tc_support_split branch 4 times, most recently from 6aa2327 to 5e78f0a Compare May 20, 2025 12:04

PavelVPV changed the title ~~tests: bsim: bluetooth: mesh: Remove TinyCrypt bsim tests~~ tests: bsim: bluetooth: mesh: Remove TinyCrypt bsim tests and enable advertiser tests for legacy advertiser May 21, 2025

PavelVPV force-pushed the remove_tc_support_split branch from 5e78f0a to f6b500e Compare May 21, 2025 06:01

PavelVPV marked this pull request as ready for review May 21, 2025 06:02

github-actions bot added platform: nRF BSIM area: Bluetooth area: Bluetooth Mesh labels May 21, 2025

github-actions bot requested review from aescolar, akredalen, alxelax, Andrewpini, HaavardRei, jhedberg, KyraLengfeld, LingaoM and omkar3141 May 21, 2025 06:03

github-actions bot assigned aescolar May 21, 2025

PavelVPV force-pushed the remove_tc_support_split branch from f6b500e to 6f2a6be Compare May 21, 2025 06:17

alxelax requested changes May 21, 2025

View reviewed changes

PavelVPV requested a review from alxelax May 21, 2025 07:21

PavelVPV force-pushed the remove_tc_support_split branch from 6f2a6be to 92ff351 Compare May 21, 2025 08:11

PavelVPV force-pushed the remove_tc_support_split branch from 92ff351 to 325c890 Compare May 21, 2025 08:13

omkar3141 reviewed May 21, 2025

View reviewed changes

tests: bsim: bluetooth: mesh: Run low latency adv tests with PSA

fe7da67

This is a preparation commit for switching all bsim tests to PSA as TinyCrypt will be removed soon. Signed-off-by: Pavel Vasilyev <pavel.vasilyev@nordicsemi.no>

PavelVPV force-pushed the remove_tc_support_split branch from 325c890 to 05b6708 Compare May 21, 2025 08:42

PavelVPV changed the title ~~tests: bsim: bluetooth: mesh: Remove TinyCrypt bsim tests and enable advertiser tests for legacy advertiser~~ tests: bsim: bluetooth: mesh: Remove TinyCrypt bsim tests May 21, 2025

PavelVPV force-pushed the remove_tc_support_split branch 2 times, most recently from 6cb411d to 6b76107 Compare May 21, 2025 11:15

PavelVPV requested a review from omkar3141 May 21, 2025 13:26

alxelax reviewed May 22, 2025

View reviewed changes

PavelVPV force-pushed the remove_tc_support_split branch from 6b76107 to 6b9d10f Compare May 22, 2025 07:54

PavelVPV force-pushed the remove_tc_support_split branch from 6b9d10f to 340c91d Compare May 22, 2025 07:55

PavelVPV requested a review from alxelax May 22, 2025 08:29

alxelax approved these changes May 22, 2025

View reviewed changes

aescolar assigned PavelVPV May 26, 2025

aescolar reviewed May 26, 2025

View reviewed changes

aescolar approved these changes May 26, 2025

View reviewed changes

kartben merged commit 9cb00be into zephyrproject-rtos:main May 26, 2025
28 checks passed

PavelVPV deleted the remove_tc_support_split branch May 26, 2025 11:57

tests: bsim: bluetooth: mesh: Remove TinyCrypt bsim tests #90208

tests: bsim: bluetooth: mesh: Remove TinyCrypt bsim tests #90208

Uh oh!

Conversation

PavelVPV commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alxelax commented May 20, 2025

Uh oh!

PavelVPV commented May 21, 2025

Uh oh!

alxelax commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alxelax left a comment

Choose a reason for hiding this comment

Uh oh!

PavelVPV commented May 21, 2025

Uh oh!

alxelax commented May 21, 2025

Uh oh!

PavelVPV commented May 21, 2025

Uh oh!

PavelVPV commented May 21, 2025

Uh oh!

alxelax commented May 21, 2025

Uh oh!

alxelax commented May 21, 2025

Uh oh!

PavelVPV commented May 21, 2025

Uh oh!

alxelax commented May 21, 2025

Uh oh!

omkar3141 commented May 21, 2025

Uh oh!

PavelVPV commented May 21, 2025

Uh oh!

omkar3141 May 21, 2025

Choose a reason for hiding this comment

Uh oh!

alxelax May 21, 2025

Choose a reason for hiding this comment

Uh oh!

omkar3141 left a comment

Choose a reason for hiding this comment

Uh oh!

alxelax commented May 21, 2025

Uh oh!

PavelVPV commented May 21, 2025

Uh oh!

alxelax left a comment

Choose a reason for hiding this comment

Uh oh!

alxelax May 22, 2025

Choose a reason for hiding this comment

Uh oh!

alxelax May 22, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented May 22, 2025

Uh oh!

aescolar left a comment

Choose a reason for hiding this comment

Uh oh!

aescolar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

PavelVPV commented May 20, 2025 •

edited

Loading

alxelax commented May 21, 2025 •

edited

Loading