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

Remove NORDIC_SECURITY_BACKEND selections for BLE/FAST_PAIR #17961

Merged
merged 4 commits into from
Oct 22, 2024

Conversation

frkv
Copy link
Contributor

@frkv frkv commented Oct 17, 2024

This PR allows usage of nrf_oberon when TF-M is enabled without additionally selecting NORIDC_SECURITY_BACKEND
which would favour legacy API support (when it is unneeded)

ref: NCSDK-29217

@frkv frkv added this to the 2.8.0 milestone Oct 17, 2024
@frkv frkv requested review from maciejbaczmanski and a team October 17, 2024 08:48
@frkv frkv requested review from a team as code owners October 17, 2024 08:48
@github-actions github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Oct 17, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Oct 17, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
find-my https://github.com/nrfconnect/sdk-find-my/commit/3c6a0c09b03a8fa5b3eef2b08b3166ce6d730a9e https://github.com/nrfconnect/sdk-find-my/commit/e3ea0a87fbb5917b2e34edba7ad4f00f3086e80c (main) nrfconnect/[email protected]
nrfxlib nrfconnect/sdk-nrfxlib@d433afe nrfconnect/sdk-nrfxlib@bf32f98 (main) nrfconnect/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@frkv frkv requested a review from kapi-no October 17, 2024 08:50
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Oct 17, 2024

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 15

Inputs:

Sources:

sdk-nrf: PR head: 18c243e91f12b40e772464f6ffef4a5121e811ce
nrfxlib: PR head: bf32f988ed75536e2df4c318a7e3879466125353
find-my: PR head: e3ea0a87fbb5917b2e34edba7ad4f00f3086e80c

more details

sdk-nrf:

PR head: 18c243e91f12b40e772464f6ffef4a5121e811ce
merge base: 936fc574d73dbbfc37ffaec3a87c00c940f2a685
target head (main): d42d5c25cd5de964a89864d645b6339680af1941
Diff

nrfxlib:

PR head: bf32f988ed75536e2df4c318a7e3879466125353
merge base: d433afe9346130233557aef11c2428c6b24dc9c6
Diff

find-my:

PR head: e3ea0a87fbb5917b2e34edba7ad4f00f3086e80c
merge base: 3c6a0c09b03a8fa5b3eef2b08b3166ce6d730a9e
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (5)
nrfxlib
│  ├── crypto
│  │  │ CMakeLists.txt
subsys
│  ├── bluetooth
│  │  ├── controller
│  │  │  │ Kconfig
│  │  ├── services
│  │  │  ├── fast_pair
│  │  │  │  ├── fp_crypto
│  │  │  │  │  │ Kconfig.fp_crypto
│  ├── nrf_security
│  │  ├── src
│  │  │  │ CMakeLists.txt
west.yml

File list hidden for private repositories.
If you have access you can see the diff via the github link.
find-my Diff

Outputs:

Toolchain

Version: 3dd8985b56
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:3dd8985b56_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 1706
  • ✅ Integration tests
    • ✅ desktop52_verification
    • ✅ test_ble_nrf_config
    • ✅ test-fw-nrfconnect-ble_samples
    • ✅ test-fw-nrfconnect-chip
    • ✅ test-fw-nrfconnect-nrf-iot_cloud
    • ✅ test-fw-nrfconnect-nrf_crypto
    • ✅ test-fw-nrfconnect-rs
    • ✅ test-fw-nrfconnect-fem
    • ✅ test-fw-nrfconnect-tfm
    • ✅ test-fw-nrfconnect-thread
    • ✅ test-fw-nrfconnect-zigbee
    • ✅ test-sdk-find-my
    • ✅ test-sdk-sidewalk
    • ✅ test-sdk-wifi
    • ✅ test-sdk-dfu
Disabled integration tests
    • doc-internal
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_mosh
    • test-fw-nrfconnect-nrf-iot_nrf_provisioning
    • test-fw-nrfconnect-nrf-iot_positioning
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-proprietary_esb
    • test-fw-nrfconnect-ps
    • test-fw-nrfconnect-rpc
    • test-low-level
    • test-sdk-audio
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

@kapi-no
Copy link
Contributor

kapi-no commented Oct 17, 2024

@frkv, could you also align the Find My part and revert my change from upmerge?

https://github.com/nrfconnect/sdk-find-my/commit/42d731a502932ccec09a75461b2d4316d4c3aa6b

west.yml Outdated Show resolved Hide resolved
@@ -203,7 +203,7 @@ endif()
add_subdirectory(drivers)

# Add legacy Mbed TLS APIs
if(CONFIG_MBEDTLS_LEGACY_CRYPTO_C)
if(CONFIG_MBEDTLS_LEGACY_CRYPTO_C OR (CONFIG_NRF_OBERON AND CONFIG_BUILD_WITH_TFM))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why AND CONFIG_BUILD_WITH_TFM?

Copy link
Contributor Author

@frkv frkv Oct 17, 2024

Choose a reason for hiding this comment

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

Because we use the same code in both TF-M (secure image) and NS image builds. We don't want to load legacy libraries just based on CONFIG_NRF_OBERON. Checking that CONFIG_BUILD_WITH_TFM is set means we can be sure we are in an NS image...

@frkv
Copy link
Contributor Author

frkv commented Oct 17, 2024

@frkv, could you also align the Find My part and revert my change from upmerge?

nrfconnect/sdk-find-my@42d731a

Contacting you offline

@frkv frkv force-pushed the deprecate_legacy branch 6 times, most recently from be63881 to bf1fbb5 Compare October 21, 2024 10:41
@frkv frkv force-pushed the deprecate_legacy branch 3 times, most recently from 2a5e49e to 921bdad Compare October 22, 2024 10:03
-This allows for direct calls to nrf_oberon with TF-M builds without
 also enabling NORDIC_SECURITY_BACKEND

ref: NCSDK-29217

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
-This commit allows linking to nrf_oberon for samples/apps that
 enable TF-M but still rely on ocrypto_ prefixed APIs because some
 of the crypto not being available in PSA crypto APIs.

Note: Previous to this it was required to enable NORDIC_SECURITY_BACKEND
to get access to nrf_oberon library (and ocrypto_ prefixed APIs). This
commit will allow CONFIG_NRF_OBERON to be used as a signal that
ocrypto_ APIs are required with NRF_SECURITY being enabled

ref: NCSDK-29217

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
-This commit removes selection of legacy crypto just to get acces
 to nrf_oberon (and ocrypto_ prefixed APIs)
-This commit enables NRF_OBERON regardless if TF-M is enabled or not

ref: NCSDK-29217

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
-This commit allows direct calls to nrf_oberon (ocrypto_ prefixed APIs)
 without simultaneously requiring NORDIC_SECURITY_BACKEND being set.
-This commit enables NRF_OBERON regardless if TF-M is enabled or not

ref: NCSDK-29217

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
@NordicBuilder NordicBuilder removed the DNM label Oct 22, 2024
@rlubos rlubos merged commit 6f5194b into nrfconnect:main Oct 22, 2024
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. manifest manifest-find-my manifest-nrfxlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants