zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.49k stars 6.42k forks source link

LwM2M block-wise transfer: add callback to notify upper layer that a transfer has restarted #71269

Closed marco85mars20 closed 4 months ago

marco85mars20 commented 5 months ago

When a Zephyr device receives data via Block-Transfer, each block is passed to the upper layers via a post-write callback. The receiving function knows:

However, if the transfer fails, depending on the CoAP configuration of the client (sending the data), the transfer might be restarted from scratch, ie. from block #0. Zephyr detects the reception of the first block for an already existing transfer, removes the old context for the block transfer and creates a new one. However, the application is not notified about it and keeps appending the incoming blocks for the new transfers to those from the old one.

Describe the solution you'd like

SeppoTakalo commented 5 months ago

As you have found a real issue here, my proposal would be even a third option: refactor the post-write callback

  1. Remove any Block-Wise parameters from post-write. It should only receive a full payload, if that is available.
  2. Define a new callback and resource type that can handle block-wise transfers. Even without having full memory for it.

This new resource type should allow both read&write using block-wise.

rlubos commented 5 months ago

I'm not sure if I'm convinced about introducing yet another callback type (although I'm of course open for discussion), wouldn't that be confusing to have two different types of write callbacks, depending on whether you expect to receive resource data via block transfer or not? Especially, that post-write wouldn't be reliably called then, for example during FW updates.

For me the most appealing option would be to simply add the offset parameter to the callback. It would not only allow to detect when the transfer starts from scratch, but also detect data stream inconsistencies (although I believe that the LwM2M engine already guarantees that out-of-order blocks are not forwarded to the application).

I don't think though that we could drop last_block parameter - total_size is not always available, i.e. if the CoAP peer does not send the Size1/2 option, we have no information about the total resource size. Maybe we should group block parameters given to the callback function into some structure, and provide a pointer to the structure in the callback instead - that way apps could easily determine whether block transfer is used or not.