lib: pdn: use heap for AT response buffer and make size configurable and increase CONFIG_HEAP_MEM_POOL_SIZE to 1280 for net samples

[eivindj-nordic]

• Add CONFIG_PDN_DYNAMIC_INFO_AT_BUF_SIZE to make the AT response buffer
  size for AT+CGCONTRDP configurable, as it depends on the APN.
  Use k_malloc for AT buffer to avoid increasing system workqueue stack
  size requirement.
• Increase CONFIG_HEAP_MEM_POOL_SIZE to 1280 for samples that previously
  used a lower value as it is required by the zephyr kernel.

[NordicBuilder]

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 4

Inputs:

Sources:

sdk-nrf: PR head: 35fd00e33bcba549e50c10759b872ccb4a63f875

more details

sdk-nrf:

PR head: 35fd00e33bcba549e50c10759b872ccb4a63f875
merge base: 10e197ecd2b3c9cd4c58565efa35e1b71061d0fb
target head (main): 115359d6f2e3970e2bf5a17fefb2608c86a3917e
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 (8)

doc
│  ├── nrf
│  │  ├── releases_and_maturity
│  │  │  ├── releases
│  │  │  │  │ release-notes-changelog.rst
lib
│  ├── pdn
│  │  ├── Kconfig
│  │  │ pdn.c
samples
│  ├── net
│  │  ├── azure_iot_hub
│  │  │  ├── boards
│  │  │  │  │ native_sim.conf
│  │  ├── download
│  │  │  │ prj.conf
│  │  ├── https_client
│  │  │  │ prj.conf
│  │  ├── udp
│  │  │  │ prj.conf
tests
│  ├── lib
│  │  ├── pdn
│  │  │  │ CMakeLists.txt

Outputs:

Toolchain

Version: 342151af73
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:342151af73_912848a074

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

• ◻️ Toolchain - Skipped: existing toolchain is used
• ✅ Build twister
  • sdk-nrf test count: 337
• ✅ Integration tests
  • ✅ test-fw-nrfconnect-nrf-iot_samples
  • ✅ test-fw-nrfconnect-nrf-iot_lwm2m
  • ✅ test-fw-nrfconnect-nrf-iot_thingy91
  • ✅ test-fw-nrfconnect-nrf-iot_mosh

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_positioning
  • test-fw-nrfconnect-nrf-iot_serial_lte_modem
  • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
  • test-fw-nrfconnect-nrf_crypto
  • test-fw-nrfconnect-ps
  • test-fw-nrfconnect-rpc
  • test-fw-nrfconnect-rs
  • test-fw-nrfconnect-tfm
  • test-fw-nrfconnect-thread
  • test-fw-nrfconnect-zigbee
  • test-low-level
  • test-sdk-audio
  • test-sdk-dfu
  • test-sdk-find-my
  • test-sdk-mcuboot
  • test-sdk-pmic-samples
  • test-sdk-sidewalk
  • test-sdk-wifi
  • test-secdom-samples-public

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

[NordicBuilder]

You can find the documentation preview for this PR at this link.

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

[jtguggedal]

NULL check here? Or is that handled by nrf_modem_at_cmd()?

[eivindj-nordic]

It is checked in nrf_modem_at_cmd(), though I agree that an extra check would be useful for debugging purposes.

[lemrey]

I don't like that we allocate on the stack for this. Using nrf_modem_at_scanf() avoided this problem, parsing from shared memory directly. Perhaps it is possible to provide the address family as a parameter to this function, and let it parse the response using nrf_modem_at_scanf() with a format to match the desired family? If the application wants to retrieve the parameters for both address families it can query twice.

[eivindj-nordic]

I don't like that we allocate on the stack for this. Using nrf_modem_at_scanf() avoided this problem, parsing from shared memory directly. Perhaps it is possible to provide the address family as a parameter to this function, and let it parse the response using nrf_modem_at_scanf() with a format to match the desired family? If the application wants to retrieve the parameters for both address families it can query twice.

Rewriting it like that could reduce the need of stack/heap, though we would still need space for storing the IPv4 and IPv6 primary and secondary DNS addresses in string form. Currently net samples are failing in CI, so I would rather get a fix merged to main, then we can discuss improvements for NCS 3.0.0.

[lemrey]

Rewriting it like that could reduce the need of stack/heap, though we would still need space for storing the IPv4 and IPv6 primary and secondary DNS addresses in string form. Currently net samples are failing in CI, so I would rather get a fix merged to main, then we can discuss improvements for NCS 3.0.0.

ok, but can you leave a comment about what you mean "1280 is required by the Zephyr kernel", what happened? :)

[eivindj-nordic]

ok, but can you leave a comment about what you mean "1280 is required by the Zephyr kernel", what happened? :)

Yes, if CONFIG_HEAP_MEM_POOL_SIZE is less than 1280 you get a warning during compilation:

CMake Warning at /home/eivind/ncs/zephyr/kernel/CMakeLists.txt:171 (message):
  
          CONFIG_HEAP_MEM_POOL_SIZE is less than requested minimum:
            1024 < 1280
          Setting the system heap size to 1280

[jorgenmk]

@tejlmand or @nordicjm Can one of you take a look at this one?