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

Ensure that stack layers above transport manager never get chained packet buffers. #9599

Merged

Conversation

bzbarsky-apple
Copy link
Contributor

None of our upper-layer core really deals with chained packet buffers, starting with the way we do in-place decryption that assumes that we have a contiguous buffer.

The changes here:

  1. Flatten out possibly-chained buffers in the LwIP case in the UDP
    and raw endpoints.
  2. Ensure that we don't have a chained buffer in BTP. There was
    already a check for this in the "in progress" case; this change just
    adds the same check in the "idle" case.

Problem

Want to ensure that upper layers never have to deal with chained buffers.

Change overview

See above.

Testing

Not sure how to test LwIP, unfortunately, especially how to inject chained buffers into the receive path there.

@todo
Copy link

todo bot commented Sep 10, 2021

add support for BtpEngine messages longer than 1 pbuf

// TODO add support for BtpEngine messages longer than 1 pbuf
VerifyOrExit(!mRxBuf->HasChainedBuffer(), err = CHIP_ERROR_INBOUND_MESSAGE_TOO_BIG);
}
else if (mRxState == kState_InProgress)
{


This comment was generated by todo based on a TODO comment in 100b18c in #9599. cc @bzbarsky-apple.

src/inet/UDPEndPoint.cpp Outdated Show resolved Hide resolved
…cket buffers.

None of our upper-layer core really deals with chained packet buffers,
starting with the way we do in-place decryption that assumes that we
have a contiguous buffer.

The changes here:

1) Flatten out possibly-chained buffers in the LwIP case in the UDP
   endpoint.
2) Ensure that we don't have a chained buffer in BTP.  There was
   already a check for this in the "in progress" case; this change just
   adds the same check in the "idle" case.
@bzbarsky-apple bzbarsky-apple force-pushed the ensure-no-chained-buffers branch from 100b18c to b8423e2 Compare September 10, 2021 21:01
@github-actions
Copy link

Size increase report for "gn_qpg-example-build" from eda07d5

File Section File VM
chip-qpg6100-lighting-example.out .text 352 352
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.debug_info,0,1067
.debug_line,0,467
.text,352,352
.debug_loc,0,309
.debug_ranges,0,168
.symtab,0,96
.strtab,0,86
.debug_frame,0,76
.debug_abbrev,0,71
.debug_aranges,0,16
.debug_str,0,10
.shstrtab,0,2
[Unmapped],0,-352


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from eda07d5

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

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,215
.debug_loc,0,120
rodata,80,80
.debug_line,0,69
.debug_ranges,0,56
.debug_frame,0,52
.symtab,0,48
.strtab,0,46
.debug_abbrev,0,40
text,40,40
.debug_aranges,0,8
.shstrtab,0,2
device_handles,-8,-8

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

sections,vmsize,filesize
.debug_info,0,215
.debug_loc,0,120
rodata,80,80
.debug_line,0,69
.debug_ranges,0,56
.debug_frame,0,52
.symtab,0,48
.strtab,0,46
.debug_abbrev,0,40
text,40,40
.debug_aranges,0,8
.shstrtab,0,-2
device_handles,-8,-8


@github-actions
Copy link

Size increase report for "esp32-example-build" from eda07d5

File Section File VM
chip-ipv6only-app.elf .flash.text 172 172
chip-all-clusters-app.elf .flash.text 276 276
chip-all-clusters-app.elf .flash.rodata 144 144
chip-lock-app.elf .flash.text 216 216
chip-lock-app.elf .flash.rodata 144 144
chip-shell.elf .flash.rodata 144 144
chip-shell.elf .flash.text 92 92
chip-bridge-app.elf .flash.text 216 216
chip-bridge-app.elf .flash.rodata 144 144
chip-temperature-measurement-app.elf .flash.text 268 268
chip-temperature-measurement-app.elf .flash.rodata 144 144
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize

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

sections,vmsize,filesize
[Unmapped],0,3924
.flash.text,172,172

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

sections,vmsize,filesize
.debug_info,0,752
.debug_line,0,661
.debug_loc,0,553
.flash.text,276,276
.flash.rodata,144,144
.debug_ranges,0,120
.strtab,0,86
.debug_frame,0,80
.symtab,0,48
.debug_abbrev,0,31
.debug_aranges,0,16
.debug_str,0,12
.riscv.attributes,0,3
.shstrtab,0,-2
[Unmapped],0,-420

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

sections,vmsize,filesize

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

sections,vmsize,filesize
.debug_line,0,844
.debug_info,0,817
.debug_loc,0,339
.flash.text,216,216
.xt.prop._ZNK4chip9Transport11PeerAddress8ToStringEPcj,0,168
.flash.rodata,144,144
.debug_ranges,0,128
.shstrtab,0,110
.strtab,0,86
.symtab,0,80
[ELF Section Headers],0,80
.debug_frame,0,48
.debug_abbrev,0,28
.debug_aranges,0,16
.debug_str,0,12
.xt.lit._ZNK4chip9Transport11PeerAddress8ToStringEPcj,0,8
[Unmapped],0,-360

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

sections,vmsize,filesize
.debug_line,0,844
.debug_info,0,814
.debug_loc,0,342
.xt.prop._ZNK4chip9Transport11PeerAddress8ToStringEPcj,0,168
.flash.rodata,144,144
.debug_ranges,0,128
.shstrtab,0,110
.flash.text,92,92
[ELF Section Headers],0,80
.symtab,0,64
.debug_frame,0,48
.strtab,0,46
.debug_abbrev,0,28
.debug_aranges,0,16
.xt.lit._ZNK4chip9Transport11PeerAddress8ToStringEPcj,0,8
[Unmapped],0,-236

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

sections,vmsize,filesize
.debug_line,0,844
.debug_info,0,814
.debug_loc,0,334
.flash.text,216,216
.xt.prop._ZNK4chip9Transport11PeerAddress8ToStringEPcj,0,168
.flash.rodata,144,144
.debug_ranges,0,128
.shstrtab,0,110
.strtab,0,86
.symtab,0,80
[ELF Section Headers],0,80
.debug_frame,0,48
.debug_abbrev,0,28
.debug_aranges,0,16
.debug_str,0,12
.xt.lit._ZNK4chip9Transport11PeerAddress8ToStringEPcj,0,8
[Unmapped],0,-360

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

sections,vmsize,filesize
[Unmapped],0,3684
.debug_line,0,863
.debug_info,0,824
.debug_loc,0,321
.flash.text,268,268
.flash.rodata,144,144
.xt.prop._ZNK4chip9Transport11PeerAddress8ToStringEPcj,0,144
.shstrtab,0,110
.debug_ranges,0,104
.strtab,0,86
.symtab,0,80
[ELF Section Headers],0,80
.debug_frame,0,48
.debug_aranges,0,16
.debug_str,0,13
.xt.lit._ZNK4chip9Transport11PeerAddress8ToStringEPcj,0,8
.debug_abbrev,0,-1


@bzbarsky-apple
Copy link
Contributor Author

@LuDuda @jepenven-silabs @jmartinez-silabs @Damian-Nordic Please take a look?

@woody-apple woody-apple merged commit b976ebe into project-chip:master Sep 13, 2021
@bzbarsky-apple bzbarsky-apple deleted the ensure-no-chained-buffers branch September 13, 2021 20:14
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.

5 participants