DFU Migration From SDK v15.0 to SDK v15.3

[suhaibish]

Hello,

I am looking into migrating DFU and Bootloader over BLE from SDK v15.0 to SDK v15.3. I have a few questions about the code changes which I was hoping I could get some clarification on:

• Why is s_dfu_settings.boot_validation_* being populated after data transfer? Wouldn’t it make more sense to do this after activation, since the new image is in its final position at this point? I think currently if there’s some failure between postvalidate and the start of activation, we will no longer be able to boot into the old application, even though we should be able to.
• Why is nrf_dfu_validation_boot_validate being called in boot_validation_extract? It seems redundant as we’re recomputing a crc/hash/signature check that we just performed, on data that isn’t in its boot location (for dual bank updates.)
• Why does postvalidate perform a crc32_compute? In SDK v15.0, s_dfu_settings.progress.firmware_image_crc was being used, but that's been changed to compute it in postvalidate. It seems to me that an application update has up to 4 computations of CRC/hashes (fw_hash_ok, boot_validation_extract, nrf_dfu_validation_boot_validate, postvalidate,) which aren't all necessary.

I'm curious about the reasoning behind the design decisions for the questions I've asked. I realize that the library supports many different use cases, and that might lead to some inefficiencies here and there, but I think these issues should be resolvable.

Thanks,

Suhaib

[Øyvind Rønningstad]

suhaibish said:

Why is s_dfu_settings.boot_validation_* being populated after data transfer?

You mean, during postvalidate()? This has become convention. You're right that it could instead be done during activation, but there isn't anything that happens between postvalidation and activation (except, possibly a reboot), so it hasn't been viewed as an issue.

suhaibish said:

I think currently if there’s some failure between postvalidate and the start of activation, we will no longer be able to boot into the old application, even though we should be able to.

I see your point, but there isn't anything that can go wrong in a recoverable way in that time frame, the data is just "at rest", so a failure at this point (highly unlikely) means the whole flash (including the old app) is untrustable.

[Øyvind Rønningstad]

suhaibish said:

Why is nrf_dfu_validation_boot_validate being called in boot_validation_extract? It seems redundant as we’re recomputing a crc/hash/signature check that we just performed, on data that isn’t in its boot location (for dual bank updates.)

This is part of checking the integrity/validity of the init command. It's a better-safe-than-sorry thing, to make absolutely sure it will boot properly. The CRC and hash would be the same as just calculated, so they could probably be streamlined, but the signature is not quite the same BTW.

[Øyvind Rønningstad]

suhaibish said:

Why does postvalidate perform a crc32_compute? In SDK v15.0, s_dfu_settings.progress.firmware_image_crc was being used, but that's been changed to compute it in postvalidate. It seems to me that an application update has up to 4 computations of CRC/hashes (fw_hash_ok, boot_validation_extract, nrf_dfu_validation_boot_validate, postvalidate,) which aren't all necessary.

You're absolutely right that not all these are necessary. It hasn't been optimized in this way, but we might do so in the future.

[suhaibish]

Yeah, you're right, the second point is an unlikely failure mode. I think my real issue is that for dual bank updates, we're computing the "boot validation" CRC on the image in bank 1 even though it will be copied to bank 0 during activation. I would think this CRC would be populated by the crc_compute after image_copy is called in app_activate . Since you're saying it's convention to do it in postvalidate, I'll probably make this change locally. 

[suhaibish]

Ah yeah, good point, the boot validation info is copied from the signature in boot_validation_extract then computed and verified in nrf_dfu_validation_boot_validate. That seems more reasonable than the crc32 and hash cases.