Issue with User-Defined Payload Transmission Over SMP: Payload Length Calculation and Memory Address Handling

[yossriK]

Describe the bug

I am attempting to send a user-defined payload as a response over SMP by enabling the CONFIG_MCUMGR_MGMT_CUSTOM_PAYLOAD flag and setting custom_payload to true in the handler's mgmt_group struct.

In the implementation, I am writing directly into the network buffer net_buf using Zephyr's net_buf library functions. However, when attempting to send the response, no data is being transmitted to the client, and the payload length is incorrectly set to 0 in the smp_hdr.

Upon further investigation, I discovered that the payload length in smp_hdr is being calculated as the difference between the payload_mut and net_buf memory addresses. This suggests that, for a correct calculation, one of the addresses has to change during the writing process, which is what occurs in fact in the case of using CBOR encoding functions such as zcbor_tstr_put_lit where the writer->zs->payload_mut address is moved to the tail of the memory buffer. However, when writing data to net_buf using the Zephyr API, neither of the memory addresses changes, leading to the payload length issue. As a temporary solution, I manually updated the payload_mut address to point to the tail of the allocated memory by adjusting it with payload_mut = payload_mut + bytes_written

To Reproduce

Enable the CONFIG_MCUMGR_MGMT_CUSTOM_PAYLOAD flag.
Set custom_payload to true in the handler's mgmt_group struct.
Write custom data into the net_buf using net_buf library functions.
Attempt to send the SMP response.
Observe that no data is transmitted, and the payload length in the smp_hdr is set to 0.
Expected behavior

The custom payload should be correctly written into the network buffer and transmitted as part of the SMP response. The payload length in smp_hdr should reflect the actual data written.

Actual Behavior

No data is transmitted to the client, and the payload length in smp_hdr is 0.
Impact

Logs and console output

Environment (please complete the following information):

• OS: Windows 10
• Toolchain : Zephyr SDK 3.6.0, Nordic Connect SDK 2.9

Additional context/Suggested Enhancement

• Investigate the payload length calculation and ensure that memory address changes during data writing are handled correctly.
• Ensure that payload_mut moves properly in memory during operations like those with CBOR encoding (e.g., zcbor_tstr_put_lit).
• Consider providing clearer guidance in the documentation for handling custom payloads and memory management in SMP responses, especially when using the net_buf API (there was almost no documentation on how to send user-defined payloads over SMP except for the release notes suggesting that a custom payload is now feasible)

[github-actions]

Hi @yossriK! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[nordicjm]

Have submitted a fix here: #85604
That code was submitted by a zephyr user for their custom use-case, I have not used it though after looking at it I'm not sure how it ever worked for them. Here is a quick diff you can try with the above PR:

diff --git a/subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c b/subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c
index 3fc4d42cfe1..014b127f2fd 100644
--- a/subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c
+++ b/subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c
@@ -162,8 +162,7 @@ static int os_mgmt_echo(struct smp_streamer *ctxt)
 		return MGMT_ERR_EINVAL;
 	}
 
-	ok = zcbor_tstr_put_lit(zse, "r")	&&
-	     zcbor_tstr_encode(zse, &data);
+	net_buf_add_mem(ctxt->writer->nb, data.value, data.len);
 
 	return ok ? MGMT_ERR_EOK : MGMT_ERR_EMSGSIZE;
 }
@@ -1122,6 +1121,7 @@ static struct mgmt_group os_mgmt_group = {
 #ifdef CONFIG_MCUMGR_GRP_ENUM_DETAILS_NAME
 	.mg_group_name = "os mgmt",
 #endif
+.custom_payload = true,
 };
 
 static void os_mgmt_register_group(void)

Have tested this and the response is now valid instead of being a constant 0 byte payload

[yossriK]

thanks @nordicjm.

On a side note (could be a separate issue), the MGMT_ERR_* cases seem to still be wrapped in a CBOR format, forexample MGMT_ERR_ENOTUP when a handler is not found thatll return an error which is not custom? how would you recommend setting up the calling client in that case.