Skip to content

samples: wifi: provisioning: ble: Fix stack overflow #22529

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

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

Conversation

D-Triveni
Copy link
Contributor

Increase bluetooth RX stack size to prevent stack overflow.

Fixes SHEL-3623.

Increase bluetooth RX stack size to prevent stack overflow.

Fixes SHEL-3623.

Signed-off-by: Triveni Danda <[email protected]>
@D-Triveni D-Triveni requested a review from sachinthegreen May 27, 2025 10:30
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label May 27, 2025
@D-Triveni D-Triveni requested a review from krish2718 May 27, 2025 10:31
@NordicBuilder
Copy link
Contributor

NordicBuilder commented May 27, 2025

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 72ec6f505b8f8d125e57729f840847029b820b16

more details

sdk-nrf:

PR head: 72ec6f505b8f8d125e57729f840847029b820b16
merge base: d0189a6959cded3ca35737a37359d2fde26b0fef
target head (main): d0189a6959cded3ca35737a37359d2fde26b0fef
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 (1)
samples
│  ├── wifi
│  │  ├── provisioning
│  │  │  ├── ble
│  │  │  │  │ prj.conf

Outputs:

Toolchain

Version: 4aa3467a6d
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:4aa3467a6d_e85602c25f

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

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 7
  • ❌ Integration tests
    • ❌ test-sdk-wifi
Disabled integration tests
    • desktop52_verification
    • doc-internal
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_cloud
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_mosh
    • 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-nrf_crypto
    • test-fw-nrfconnect-proprietary_esb
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread-main
    • test-low-level
    • test-sdk-audio
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-secdom-samples-public

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

@@ -80,7 +80,7 @@ CONFIG_BT_BUF_ACL_RX_SIZE=151
CONFIG_BT_L2CAP_TX_MTU=147
CONFIG_BT_BUF_ACL_TX_SIZE=151

CONFIG_BT_RX_STACK_SIZE=5120
CONFIG_BT_RX_STACK_SIZE=5400
Copy link
Contributor

Choose a reason for hiding this comment

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

I see max of 5120 in the NCS, so, a few queries:

  1. What was the max usage?
  2. Is this platform dependent?
  3. Do we understand why the increase?
  4. Do we need to update any other samples (not just Wi-Fi)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@krish2718

  1. Will check and update the PR with required maximum value.
  2. No, we are seeing issue on all supported platforms.
  3. Looks like the issue is seen after the recent upmerge.
  4. No, the change is required only for this sample.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I guess !4 is more important to understand, why does this sample cause BT Rx thread stack to increase? IIUC, it should apply for any Wi-Fi/BLE combo case as the thread is part of HCI which is sample independent and I see no other sample bumping this.

@krish2718 krish2718 requested a review from Copilot May 28, 2025 11:38
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR increases the Bluetooth RX stack size in the BLE provisioning sample to prevent stack overflow (SHEL-3623).

  • Increased CONFIG_BT_RX_STACK_SIZE from 5120 to 5400 in the BLE WiFi provisioning sample.

@@ -80,7 +80,7 @@ CONFIG_BT_BUF_ACL_RX_SIZE=151
CONFIG_BT_L2CAP_TX_MTU=147
CONFIG_BT_BUF_ACL_TX_SIZE=151

Copy link
Preview

Copilot AI May 28, 2025

Choose a reason for hiding this comment

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

[nitpick] Add a comment explaining why the RX stack size was increased to 5400 (e.g., based on profiling or safe margins) to help future maintainers understand the rationale.

Suggested change
# Increased RX stack size to 5400 based on profiling results to ensure sufficient buffer space for Bluetooth RX operations.

Copilot uses AI. Check for mistakes.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants