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

[OpenThread] Optimize memory usage of DNS client #8614

Merged
merged 1 commit into from
Jul 26, 2021

Conversation

Damian-Nordic
Copy link
Contributor

@Damian-Nordic Damian-Nordic commented Jul 24, 2021

Problem

In the worst-case scenario, the DNS client must correctly handle a commissionable node service which contains
a TXT entry with key "PI" and a pairing instruction up to 128 characters. Current DNS client implementation for OpenThread does not support such long TXT entries to limit the stack usage.

Change overview

Change the method of allocating memory for the resolved/discovered services to allow long TXT entries, yet not to consume too much memory on stack.
Fixes #8467.

Testing

Tested along with #8454 that nRF Connect Lock can both advertise and discover (with matter dns browse shell command) a node with the longest possible pairing instruction.

In the worst-case scenario, the DNS client must correctly
handle a commissionable node service which contains
a TXT entry with key "PI" and a pairing instruction up to
128 characters. Change the method of allocating memory
for the resolved/discovered services to allow such long
TXT entries yet not to consume too much memory on stack.
@github-actions
Copy link

Size increase report for "esp32-example-build" from 762a204

File Section File VM
chip-lock-app.elf .flash.text 64 64
chip-shell.elf .flash.text -32 -32
chip-temperature-measurement-app.elf .flash.text 60 60
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.debug_str,0,148
.debug_info,0,142
.flash.text,64,64
.debug_loc,0,-14
[Unmapped],0,-64

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
[Unmapped],0,32
.flash.text,-32,-32

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.flash.text,60,60
.debug_info,0,59
.debug_str,0,16
.debug_loc,0,13
[Unmapped],0,-60

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,59
.debug_str,0,17
.riscv.attributes,0,-1
.debug_loc,0,-3


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 762a204

File Section File VM
chip-shell.elf device_handles 8 8
chip-shell.elf text -56 -56
chip-lock.elf device_handles 8 8
chip-lock.elf text -56 -56
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,317
.debug_loc,0,301
.debug_str,0,203
.debug_abbrev,0,36
.debug_line,0,35
.debug_ranges,0,16
device_handles,8,8
.debug_frame,0,-4
text,-56,-56

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_loc,0,305
.debug_str,0,203
.debug_info,0,188
.debug_line,0,40
.debug_abbrev,0,36
.debug_ranges,0,16
device_handles,8,8
.shstrtab,0,-1
.debug_frame,0,-4
.strtab,0,-11
text,-56,-56


@woody-apple woody-apple merged commit a85a567 into project-chip:master Jul 26, 2021
@Damian-Nordic Damian-Nordic deleted the dns-allocator branch July 26, 2021 15:54
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
In the worst-case scenario, the DNS client must correctly
handle a commissionable node service which contains
a TXT entry with key "PI" and a pairing instruction up to
128 characters. Change the method of allocating memory
for the resolved/discovered services to allow such long
TXT entries yet not to consume too much memory on stack.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to max(commissionable TXT key size, operational TXT key size) after optim...
5 participants