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

sys/suit: avoid installing payload twice #17984

Merged
merged 1 commit into from
Apr 27, 2022

Conversation

fjmolinas
Copy link
Contributor

Contribution description

For some reason the generated manifest triggers the payload to be installed twice:

SUIT policy checks OK.
Formatted component name: .ram.0
Fetching firmware |█████████████████████████| 100%
Finalizing payload store
Verifying image digest
Starting digest verification against image
Install correct payload
Verifying image digest
Starting digest verification against image
Install correct payload

Why I'm not sure is whether this is an issue in the manifest generation or if it's something to handle in code. Pinging @bergzand on this.

Testing procedure

Run examples/suit_update payload is installed once:

SUIT policy check OK.
Formatted component name: .ram.0
Fetching firmware |█████████████████████████| 100%
Finalizing payload store
Verifying image digest
Starting digest verification against image
Install correct payload

@github-actions github-actions bot added Area: OTA Area: Over-the-air updates Area: sys Area: System labels Apr 22, 2022
@fjmolinas
Copy link
Contributor Author

For context this is called twice because in the manifest there are actually two condition-image-match:

ValidateSeq.append(mkCommand(cid, 'condition-image-match', None))

InstSeq.append(mkCommand(cid, 'condition-image-match', None))

Which in turn results in multiple calls to _dtv_verify_image_match which does the install/verify

@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 22, 2022
@fjmolinas fjmolinas force-pushed the pr_suit_install_once branch 3 times, most recently from e2acf2f to 127ffd3 Compare April 25, 2022 07:40
@fjmolinas
Copy link
Contributor Author

I removed setting the SUIT_COMPONENT_STATE_FINALIZED flag since this stage does not necessarily mean that it's finalized since there is actually a suit-validate sequence called after the suit-install sequence. I find it somewhat redundant when the install sequence already implements the condition-image-match, but since the validate sequence is mandatory, I would think it just means that the extra condition-image-match could be removed from install.

@fjmolinas
Copy link
Contributor Author

Note that my main motivation for this is to know when from a user perspective I can consider the payload "ready to use", and I would have thought that once installed if it succeeded then it should be good to go.

Copy link
Member

@bergzand bergzand left a comment

Choose a reason for hiding this comment

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

Ack, thanks for the fix

@bergzand bergzand merged commit cdf1d43 into RIOT-OS:master Apr 27, 2022
@fjmolinas fjmolinas deleted the pr_suit_install_once branch April 27, 2022 07:53
@fjmolinas
Copy link
Contributor Author

Thanks!

@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OTA Area: Over-the-air updates Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants