Retransmit

[M1cha]

Data can get lost - especially with transports like UDP. But even with a UART you could loose packets due to data corruptions. I noticed this while doing updates over UDP, where smpmgr(which uses smpclient) just fails on a a timeout every few packets.

I think, that we can handle this in a lower layer without modifying e.g. smpmgr, because the SMP protocol has sequence counters and thus can prevent repetitive execution of request handlers - which could have unintended side-effects.

What do you think?

[JPHutchins]

Can you paste some logs of what is occurring? The upload routine waits for the response to the request before sending the next packet. If the server doesn't respond there would be a timeout exception.

Thanks!

[M1cha]

[06:10:08] ERROR    Timeout (2.5s) waiting for request header=Header(op=<OP.WRITE: 2>, version=<Version.V2: 1>,                __init__.py:162
                    flags=<Flag.UNUSED: 0>, length=492, group_id=<GroupId.IMAGE_MANAGEMENT: 1>, sequence=25,
                    command_id=<ImageManagement.UPLOAD: 1>) version=<Version.V2: 1> sequence=25
                    smp_data=b'...' image=None len=None sha=None upgrade=None - image_management:115

[JPHutchins]

I have a theory - are you fragmenting SMP frames on the UDP transport? I could never think of a way to make that work over UDP. It's what is done for BLE, I realize. Though, we've even seen that cause problems where the smpclient sends SMP packets faster than the SMP server can handle, which will eventually manifest as a timeout.

#30

If you're not fragmenting then I'll need to attempt to reproduce. Paste the logs with --loglevel DEBUG or use the logfile flag.

[M1cha]

I'm running Zephyrs smp_udp on a device, which is connected to an openthread network. The device is accessible in my local network through an openthread border router (otbr). While I'm not doing fragmentation specifically, thread has a maximum frame size of 127 bytes, so I assume that OTBR does the fragmentation.

[JPHutchins]

Now that I'm looking, you'd know if smpclient was fragmenting because it would spam warnings:

smpclient/smpclient/transport/udp.py

Lines 52 to 58 in 83cfd1c

	@override
	async def send(self, data: bytes) -> None:
	if len(data) > self.max_unencoded_size:
	logger.warning(
	"Fragmenting UDP packets is not recommended: "
	f"{len(data)=} B > {self.max_unencoded_size=} B"
	)

[JPHutchins]

OK, so DEBUG log level on client side should allow to verify exactly what bytes are being sent, then it's a matter of verifying that they are received on the remote server.

Any dropped part of a packet would cause a timeout error because the SMP server would never assemble a complete request and then never send a response.

[M1cha]

Any dropped part of a packet would cause a timeout error because the SMP server would never assemble a complete request and then never send a response.

I can create a debug log if it's still needed, but I think this right here is the core issue. The thread device in question has a bad connection and does randomly drop packets - it's expected from my perspective.

Also, I don't have this issue with a different thread device, which has a very good connection, because it's close to the OTBR.

[JPHutchins]

Interested in implementing an SMP TCP Transport over at Zephyr 🤣?

Your comment in #57 gave me an idea - can you set the MTU to 127 (or even lower) and try?

Inevitably, this is going in the direction of retries and error handling.

[JPHutchins]

Also ... if Thread transport is a special kind of transport, perhaps with auth or whatever, then it's always an option to subclass the UDP transport or create a new SMPTransport from scratch.

[M1cha]

Interested in implementing an SMP TCP Transport over at Zephyr 🤣?

Well the thing is that this can happen with serial as well, it's just very hard to trigger. Cause a bit-flip and the packet will be dropped due to e.g. an invalid checksum.

Your comment in #57 gave me an idea - can you set the MTU to 127 (or even lower) and try?

Learning about max_unencoded_size in #57 actually let me retry playing with the MTU. There are several insights here:

1. If I use an MTU < 116 for upgrade, Zephyr returns an error:

SMPUploadError: header=Header(op=<OP.WRITE_RSP: 3>, version=<Version.V2: 1>, flags=<Flag.UNUSED: 0>, length=19, group_id=1, sequence=1,
command_id=1) version=<Version.V2: 1> sequence=1 smp_data=b'\x0b\x00\x00\x13\x00\x01\x01\x01\xbfcerr\xbfegroup\x01brc\x16\xff\xff'
err=Err[IMG_MGMT_ERR](group=<GroupId.IMAGE_MANAGEMENT: 1>, rc=<IMG_MGMT_ERR.INVALID_IMAGE_HEADER: 22>)

2. 116 works, but is very slow. (1.2KB/s bytes/s instead of around 3-4KB/s on the device with a good connection. Down to <200 bytes/s on the one with a bad connection). I assume, that's due to the SMP frame overhead.
3. I still get timeouts with 116
4. The maximum MTU size I can choose is 1232, do you have an explanation for that? Zephyrs UDP transport uses the default of 1500. Above that I get a WARNING Error received: exc=OSError(71, 'Protocol error') - _udp_client:88 without any stack trace.

Inevitably, this is going in the direction of retries and error handling.

I'm afraid so, yes. 👀

[M1cha]

Also ... if Thread transport is a special kind of transport, perhaps with auth or whatever, then it's always an option to subclass the UDP transport or create a new SMPTransport from scratch.

Well the link layer is special, but it's translated into an ethernet layer2 by OTBR. layer 3(IPv6) works exactly the same, so clients generally don't have to know, that they are talking to a thread device. It can be useful if they know(e.g. to avoid fragmentation), but it generally "just works". For example, I'm using the telnet shell backend to get shell access to thread devices and the devices themselves send data to an mqttsn server, which is running on my homeserver and knows nothing about thread.

[JPHutchins]

I'll have to leave you to it for a bit.

The small MTU problem doesn't surprise me, it's a long standing issue where the SMP server expects the first 32 bytes of the MCUBoot header in the first packet.

Slow speeds are not surprising either, that's what we'd see over UART with small MTU.

The timeout in this case is more controllable, right? At least we'd know which packet to retry rather than having to send 7 fragmented packets and hoping that all 7 get through eventually?

[M1cha]

The timeout in this case is more controllable, right? At least we'd know which packet to retry rather than having to send 7 fragmented packets and hoping that all 7 get through eventually?

Well I guess it's mostly a question of optimizing the throughput. Depending on the error rate it can be worth it to save the overhead and to resend all 7 fragments in case a fragment gets lost. You could probably write a smart algorithm which automatically detects that and adjusts the MTU accordingly to maximize throughput, but IMO this is beyond the scope of this issue and maybe even of this project.

But the way it works right now, I don't think smpclient even has to know, that fragmentation exists. An SMP packet got lost and it can retry it. I don't think the code has to know, that the packet got fragmented to make that decision.

[JPHutchins]

Let's draft what would be ideal for retries. Here's the existing request method that has a timeout only.

async def request(
    self, request: SMPRequest[TRep, TEr1, TEr2], timeout_s: float = 120.000
) -> TRep | TEr1 | TEr2:

I like the idea of having a retry interval. The idea is that there is some time - typically like 50ms - in which the response is expected. Then, the user doesn't give up until say 1 second.

The defaults here are long because I don't like imposing strict timeouts on the user.

async def request(
    self,
    request: SMPRequest[TRep, TEr1, TEr2],
    timeout_s: float = 120.000,
    retry_interval_s: float = 1.000
) -> TRep | TEr1 | TEr2:

Then there is the problem of receiving the response after the timeout - sending the request 2x when it was in fact handled the first time. Imagine sending request A.

• request A0 sent
• request A0 timeout
• request A1 sent
• response A0 received
• response A1 received