net: mqtt: Add write message handler

[rlubos]

Add new transport handler for MQTT, with sendmsg-like functionality.
This allows TCP transport to send PUBLISH packets w/o fragmentation at
the TCP layer. Implement this new functionality for all existing
transports.

Fixes #22679

Signed-off-by: Robert Lubos robert.lubos@nordicsemi.no

[rlubos]

FYI @JusbeR

[mrfuchs]

Works in my setup with mqtt_client.transport.type = MQTT_TRANSPORT_SECURE_WEBSOCKET.

[jhedberg]

@jukkar this seems more of an enhancement than a bug fix. Were you planning on requesting TSC approval for it (considering that you added the 2.2 milestone)?

[jukkar]

this seems more of an enhancement than a bug fix

True, I will set the milestone to 2.3 which is more appropriate in this case.

[WilliamGFish]

Can we double check this PR as my MQTT connections now reject upon first message send...

[00:00:45.067,413] [0m net_mqtt.mqtt_publish: (0x20001840): [CID 0x200018ec]:[State 0x06]: >> Topic size 0x00000022, Data size 0x00000078[0m
[00:00:45.067,504] [0m net_mqtt_enc.pack_utf8_str: (0x20001840): >> str_size:00000024 cur:0x20007530, end:0x2000772b[0m
[00:00:45.067,504] [0m net_mqtt_enc.pack_uint16: (0x20001840): >> val:0022 cur:0x20007530, end:0x2000772b[0m
[00:00:45.067,535] [0m net_mqtt_enc.pack_uint16: (0x20001840): >> val:1723 cur:0x20007554, end:0x2000772b[0m
[00:00:45.067,535] [0m net_mqtt_enc.mqtt_encode_fixed_header: (0x20001840): << msg type:0x32 length:0x0000009e[0m
[00:00:45.067,535] [0m net_mqtt_enc.packet_length_encode: (0x20001840): >> length:0x0000009e cur:0x00000000, end:0x00000000[0m
[00:00:45.067,535] [0m net_mqtt_enc.mqtt_encode_fixed_header: (0x20001840): Fixed header length = 03[0m
[00:00:45.067,565] [0m net_mqtt_enc.pack_uint8: (0x20001840): >> val:32 cur:0x2000752d, end:0x2000772b[0m
[00:00:45.067,565] [0m net_mqtt_enc.packet_length_encode: (0x20001840): >> length:0x0000009e cur:0x2000752e, end:0x2000772b[0m
[00:00:45.067,565] [0m net_mqtt.client_write_msg: (0x20001840): [0x200018ec]: Transport writing message.[0m
[00:00:45.067,687] [0m net_mqtt.client_write_msg: (0x20001840): Transport write failed, err_code = -127, closing connection[0m
[00:00:45.067,718] [0m net_mqtt_sock_tcp.mqtt_client_tcp_disconnect: (0x20001840): Closing socket 0[0m
[mqtt_evt_handler:188] MQTT client disconnected -127

[rlubos]

@WilliamGFish Any more details on your setup perhaps? I've tested this change with qemu/local mosquitto server and nRF91 (offloaded sockets) with nRF Cloud, both worked fine.

[WilliamGFish]

@WilliamGFish Any more details on your setup perhaps? I've tested this change with qemu/local mosquitto server and nRF91 (offloaded sockets) with nRF Cloud, both worked fine.

I am using the PArticle Argon with esp32 (esp8266 modem driver) with offloaded sockets. This was working two days ago and had sen t over 500,000 messages without a failure.

This wold be using the NET_TCP option therefore using the mqtt_client_tcp_write_msg() function. Not sure quite was is happening as error code does not appear to make sense (EKEYEXPIRED )

[WilliamGFish]

Found there is a bug/s in the modem driver that has been dramatically exposed by these changes.

[mike-scott]

@WilliamGFish Cool! Which driver?

[WilliamGFish]

I am using the Particle Argon with esp32 (esp8266 modem driver) with offloaded sockets. This was working two days ago and had sent over 500,000 messages without a failure.

The offending logic in {esp8266_offload.c} esp8266_sendto()

	if (sock->type == SOCK_STREAM) {
		if (!esp8266_socket_connected(sock)) {
			esp8266_unlock(dev);
			return -ENOTCONN;
// 		} else if (dst_addr) {
// 			esp8266_unlock(dev);
// LOG_DBG("ERROR: dst_addr %d", ret);				
// 			return -EISCONN;

The logic seems confusing but by removing this the driver is back working again.

I am looking at other drivers but haven't found the same unusual logic.

[jbrzozoski]

Doh. This pull request breaks MQTT for sufficiently large payloads. The call to sendmsg only checks pass/fail, not how many bytes were sent... Our application is stuck on this, so I'll be working on a fix.

[jbrzozoski]

And it looks like the implementation of sendmsg over in the NRF sockets version falls back for large paylods to looping with sendto on each iovec and also doesn't notice partial transmits. 🤯
Somebody owes me a cookie when I'm done with this.

[jbrzozoski]

After a bit more reading, I realized sendmsg is supposed to be all or nothing, and never return partial success. My issue was entirely in the NRF sendmsg implementation. I just opened a pull request with a fix on the NRF repository here.