search

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.71k stars 6.54k forks source link

mgmt/mcumgr/lib: SMP processing should be always able to at least return "rc" code #44536

Open de-nordic opened 2 years ago

de-nordic commented 2 years ago

Is your enhancement proposal related to a problem? Please describe. The mcumgr is able to process SMP frame only when it is able to allocate buffer for it. The allocation happens at transport level, when transport wants receives SMP data and needs buffer to store them. The problem is that, when there are no more free buffers, it is left for transport to respond with an error and often happens that the the transport does not have such capability, for example serial will just drop the frame. This causes some problems as for example a client will not be informed what a problem was and may retry sending causing more frame drops, instead of just lowering the pace the frames are send at.

Describe the solution you'd like Every time when transport is not able to obtain "request" frame for received data, from mcumgr, it should be given by SMP a rc=MGMT_ERR_ENOMEM frame that it can send back to the client. The frame should have proper response header (group,command id, operation opposite to request and sequence number) and only payload of the rc code.

Describe alternatives you've considered Adding some special SMP header to respond; modifying transport (not practical, not possible everywhere); leaving as is.

Additional context BT dropping SMP frame: https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/mgmt/mcumgr/smp_bt.c#L232-L236 Serial dropping SMP frame when allocation fails, https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/mgmt/mcumgr/smp_uart.c#L41-L52, due to https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/mgmt/mcumgr/serial_util.c#L76-L81

nordicjm commented 1 year ago

I parked my changes here https://github.com/nordicjm/zephyr/tree/mcu_err_buf

This is a bit more complex that first anticipated, because depending upon the data received, which cannot be analysed due to the lack of a net buffer, there might or might not be sufficient information to construct a valid error response - the header information from the request is needed, group, ID, etc. and so if there is a problem with this information or if it is lacking is unknown as e.g. the point in the serial transport where this code is needed will not have been able to verify any of the incoming data, so a valid error response might not be able to be constructed.

zephyrbot commented 8 months ago

Hi @nordicjm,

This issue, marked as an Enhancement, was opened a while ago and did not get any traction. It was just assigned to you based on the labels. If you don't consider yourself the right person to address this issue, please re-assing it to the right person.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@de-nordic you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!