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.

Sign up for GitHub

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

Jump to bottom

LwM2M: Fix incorrect last_block handling in the firmware write callback #16158

Closed
lodup29 opened this issue May 14, 2019 · 7 comments · Fixed by #18757
Closed

LwM2M: Fix incorrect last_block handling in the firmware write callback #16158

lodup29 opened this issue May 14, 2019 · 7 comments · Fixed by #18757
Assignees
@mike-scott
Labels
area: LWM2M bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@lodup29
Copy link
Contributor

lodup29 commented May 14, 2019 •
edited
Loading

At least using a coap to http proxy (I never manager to trigger the callback otherwise) the last_block parameter provided though the callback function (implemented through firmware_block_received_cb in the LWM2M client example) is never set to true although the firmware is fully downloaded.

To Reproduce

  • Build and run a firmware distribution point along with the Leshan demo server using docker-compose (adjust the host-side directory of the volume mounted on /usr/share/nginx/html):
version: '3'

services:
  cf-proxy-coap-http:
    image: linarotechnologies/cf-proxy-coap-http:latest
    container_name: coap-proxy
    restart: unless-stopped
    ports:
      - "5682:5682/udp"
  nginx:
    image: opensourcefoundries/nginx:latest
    container_name: firmware-server
    restart: unless-stopped
    ports:
      - "8080:80"
    volumes:
      - /home/admin-local/data:/usr/share/nginx/html
  leshan-server:
    image: linarotechnologies/leshan:latest
    read_only: true
    container_name: leshan
    restart: unless-stopped
    ports:
      - "5683:5683/udp"
      - "5684:5684/udp"
      - "8081:8080"
    tmpfs:
      - /tmp
  • Add support for firmware pull using COAP proxy to the LWM2M client example:
CONFIG_LWM2M_FIRMWARE_UPDATE_PULL_SUPPORT=y
CONFIG_LWM2M_FIRMWARE_UPDATE_OBJ_SUPPORT=y
CONFIG_LWM2M_FIRMWARE_UPDATE_PULL_COAP_PROXY_SUPPORT=y
CONFIG_LWM2M_FIRMWARE_UPDATE_PULL_COAP_PROXY_ADDR="coap://FIRMWARE_IP_ADDRESS:5682"
CONFIG_LWM2M_COAP_BLOCK_SIZE=1024

and monitor last_block.

  • Place a file named update.bin in the directory mounted on /usr/share/nginx/html (see docker-compose configuration above) and write http://FIRMWARE_IP_ADDRESS:8080/update.bin to /5/0/1 (Firmware Update/Instance 0/Package URI)

The download should go fine but last_block is never set to to true.
@lodup29 lodup29 added the bug The issue is a bug, or the PR is fixing a bug label May 14, 2019
@mike-scott
Copy link
Contributor

@lodup29 Thank you for bringing this up. It might be that over time the logic to the last_block handling has been broken (from the callback perspective).

We calculate last_block here:
https://github.com/zephyrproject-rtos/zephyr/blob/master/subsys/net/lib/lwm2m/lwm2m_obj_firmware_pull.c#L295

And then check for a write callback here:
https://github.com/zephyrproject-rtos/zephyr/blob/master/subsys/net/lib/lwm2m/lwm2m_obj_firmware_pull.c#L295

With the actual callback happening here:
https://github.com/zephyrproject-rtos/zephyr/blob/master/subsys/net/lib/lwm2m/lwm2m_obj_firmware_pull.c#L337

As I get some time, I'll run a few tests and see what needs to happen there.

@nashif nashif added the priority: medium Medium impact/importance bug label May 21, 2019
@ioannisg
Copy link
Member

@mike-scott is this bug still present? If so, will you address this for 2.0.0?

@mike-scott
Copy link
Contributor

Yes, I will test and address for 2.0 if there's a bug.

@ioannisg
Copy link
Member

@mike-scott ping :)

@mike-scott
Copy link
Contributor

@ioannisg thanks for the ping. seems I had a rather large backlog for LwM2M this cycle.

@mike-scott
Copy link
Contributor

@lodup29 I think I know what's causing the issue you're seeing. The proxy uses ETAG CoAP options which are longer than the default of 12. Can you add these 2 configs to the exact setup that you have and see if it fixes it?

CONFIG_COAP_EXTENDED_OPTIONS_LEN=y
CONFIG_COAP_EXTENDED_OPTIONS_LEN_VALUE=32

On a side note, I did find a bug in how the last_block calculation is sent to the callbacks. When the user buffer has a smaller size than the BLOCK_SIZE, multiple callbacks are sent with last_block=true. I'll submit that PR today.

@mike-scott
Copy link
Contributor

mike-scott commented Aug 28, 2019 •
edited
Loading

To be clear, I reproduced the behavior in the issue and after adding the above 2 items to my prj.conf I see the following (this includes a patch here: #18757):

[00:00:13.890,000] <inf> net_lwm2m_client_app: FIRMWARE: BLOCK RECEIVED: len:64 last_block:0
[00:00:13.890,000] <inf> net_lwm2m_client_app: FIRMWARE: BLOCK RECEIVED: len:64 last_block:0
[00:00:13.890,000] <inf> net_lwm2m_client_app: FIRMWARE: BLOCK RECEIVED: len:64 last_block:0
[00:00:13.890,000] <inf> net_lwm2m_client_app: FIRMWARE: BLOCK RECEIVED: len:64 last_block:0
[00:00:13.890,000] <inf> net_lwm2m_client_app: FIRMWARE: BLOCK RECEIVED: len:64 last_block:0
[00:00:13.890,000] <inf> net_lwm2m_client_app: FIRMWARE: BLOCK RECEIVED: len:64 last_block:0
[00:00:13.890,000] <inf> net_lwm2m_client_app: FIRMWARE: BLOCK RECEIVED: len:64 last_block:0
[00:00:13.890,000] <inf> net_lwm2m_client_app: FIRMWARE: BLOCK RECEIVED: len:37 last_block:1

@mike-scott mike-scott changed the title LWM2M : last_block is not set through firmware write callback LwM2M: Fix incorrect last_block handling in the firmware write callback Aug 28, 2019
@mike-scott mike-scott mentioned this issue Aug 28, 2019
Merged
mike-scott added a commit to mike-scott/zephyr that referenced this issue Aug 29, 2019
@mike-scott
net: lwm2m: firmware_pull: fix multiple last_block notifications
f23a56f 
When the firmware_pull mechansim sends the callback to notify the
sample of a new firmware block, the user supplied buffer can be
smaller than the CoAP BLOCK_SIZE setting.  To handle this case,
we loop through the payload and fill the user supplied buffer with
smaller chunks.

Unfortunately, the last_block calculation is done outside this loop
which causes several callbacks (while in this loop) to have
last_block true.   Let's fix this by adding a small check to make
sure we're at the end of the current payload block before notifying
the user of a last_block.

Fixes: zephyrproject-rtos#16158

Signed-off-by: Michael Scott <mike@foundries.io>
ioannisg pushed a commit that referenced this issue Aug 29, 2019
@mike-scott @ioannisg
net: lwm2m: firmware_pull: fix multiple last_block notifications
7841ebf 
When the firmware_pull mechansim sends the callback to notify the
sample of a new firmware block, the user supplied buffer can be
smaller than the CoAP BLOCK_SIZE setting.  To handle this case,
we loop through the payload and fill the user supplied buffer with
smaller chunks.

Unfortunately, the last_block calculation is done outside this loop
which causes several callbacks (while in this loop) to have
last_block true.   Let's fix this by adding a small check to make
sure we're at the end of the current payload block before notifying
the user of a last_block.

Fixes: #16158

Signed-off-by: Michael Scott <mike@foundries.io>
LeiW000 pushed a commit to LeiW000/zephyr that referenced this issue Sep 2, 2019
@mike-scott
net: lwm2m: firmware_pull: fix multiple last_block notifications
f90fb22 
When the firmware_pull mechansim sends the callback to notify the
sample of a new firmware block, the user supplied buffer can be
smaller than the CoAP BLOCK_SIZE setting.  To handle this case,
we loop through the payload and fill the user supplied buffer with
smaller chunks.

Unfortunately, the last_block calculation is done outside this loop
which causes several callbacks (while in this loop) to have
last_block true.   Let's fix this by adding a small check to make
sure we're at the end of the current payload block before notifying
the user of a last_block.

Fixes: zephyrproject-rtos#16158

Signed-off-by: Michael Scott <mike@foundries.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mike-scott mike-scott

Labels
area: LWM2M bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

net: lwm2m: firmware_pull: fix multiple last_block notifications mike-scott/zephyr
5 participants
@nashif @ioannisg @lodup29 @mike-scott @carlescufi