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

Fix missing footer space during WiFi provisioning #4868

Merged

Conversation

mspang
Copy link
Contributor

@mspang mspang commented Feb 15, 2021

As of a868187 ("Reduce use of maximum-sized packet buffers (#4434)"),
network provisioning does not allocate enough space to encrypt the WiFi
credentials payload, resulting in the following error:

CHIP:NP: Failed in sending Network Creds. error Error 4047 (0x00000FCF)

Pending API improvements to make these errors less likely, manually add
the needed extra space at allocation time.

Fixes #4823

As of a868187 ("Reduce use of maximum-sized packet buffers (project-chip#4434)"),
network provisioning does not allocate enough space to encrypt the WiFi
credentials payload, resulting in the following error:

 CHIP:NP: Failed in sending Network Creds. error Error 4047 (0x00000FCF)

Pending API improvements to make these errors less likely, manually add
the needed extra space at allocation time.

Fixes project-chip#4823
@mspang mspang added hotfix urgent fix needed, can bypass review and removed hotfix urgent fix needed, can bypass review labels Feb 15, 2021
@mspang mspang added the hotfix urgent fix needed, can bypass review label Feb 15, 2021
@github-actions
Copy link

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

File Section File VM
chip-lock.elf shell_root_cmds_sections 12 12
chip-lock.elf text 4 4
chip-lighting.elf shell_root_cmds_sections 12 12
chip-lighting.elf text 4 4
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,7101
.debug_line,0,641
.debug_abbrev,0,278
.debug_loc,0,68
shell_root_cmds_sections,12,12
text,4,4

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

sections,vmsize,filesize
.debug_info,0,7101
.debug_line,0,645
.debug_abbrev,0,278
.debug_loc,0,72
shell_root_cmds_sections,12,12
text,4,4

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

sections,vmsize,filesize


@github-actions
Copy link

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

File Section File VM
chip-all-clusters-app.elf .flash.text 4 4
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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,5842
.debug_line,0,830
.debug_abbrev,0,322
.debug_loc,0,32
.flash.text,4,4
.xt.prop._ZTVN4chip11DeviceLayer37DeviceNetworkProvisioningDelegateImplE,0,2


Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

This is OK as far as it goes, but:

  1. NetworkProvisioning::SendThreadCredentials has the same issue.
  2. The uses in BdxTransferSession would have the same issue, if we ever sent that data anywhere.
  3. The use in src/app/encoder.cpp would have this issue if it did not ridiculously over-allocate.

Maybe just like we have MessagePacketBuffer we should have MessagePacketBufferWriter, which is automatically little-endian and allocates the footer space?

@kpschoedel

@bzbarsky-apple bzbarsky-apple merged commit 95b7359 into project-chip:master Feb 15, 2021
Damian-Nordic added a commit to Damian-Nordic/connectedhomeip that referenced this pull request Feb 16, 2021
Thread provisioning message construction needs fixing in
the same way as Wi-Fi provisioning in project-chip#4868.
By the way, fix rendezvous clean up to make sure that
the app doesn't crash when trying to commision a device
for the second time.
@kpschoedel
Copy link
Contributor

Maybe just like we have MessagePacketBuffer we should have MessagePacketBufferWriter, which is automatically little-endian and allocates the footer space?

On seeing this case, I think PacketBufferWriter is already overspecialized. I think it would be better for it to take ownership of an existing PacketBufferHandle, however allocated, just like PacketBufferTLVWriter.

@bzbarsky-apple
Copy link
Contributor

@kpschoedel That seems fine to me. Want to switch it around, or should I?

bzbarsky-apple pushed a commit that referenced this pull request Feb 16, 2021
Thread provisioning message construction needs fixing in
the same way as Wi-Fi provisioning in #4868.
By the way, fix rendezvous clean up to make sure that
the app doesn't crash when trying to commision a device
for the second time.
@kpschoedel
Copy link
Contributor

PR #4874

@mspang mspang deleted the for-chip/fix-wifi-provisioning branch February 16, 2021 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotfix urgent fix needed, can bypass review transport
Projects
None yet
Development

Successfully merging this pull request may close these issues.

raspi: Network Provisioning step Failed in sending Network Creds
4 participants