CONFIG_TCP_MSS used in AsyncUDP.cpp causes truncated packets

[troyhacks]

Board

ALL

Device Description

N/A

Hardware Configuration

N/A

Version

latest master (checkout manually)

Type

Bug

IDE Name

N/A

Operating System

N/A

Flash frequency

N/A

PSRAM enabled

yes

Upload speed

N/A

Description

In AsyncUDP.cpp there are two places where the payload size is checked against CONFIG_TCP_MSS and truncated to that length if longer.

if (size > CONFIG_TCP_MSS) {
  size = CONFIG_TCP_MSS;
}

This is fine for TCP packets, but not for UDP packets.

The default for CONFIG_TCP_MSS is 1436 - but the maximum value is 1460 - which is perfectly sane for UDP packets - or I think so anyway, and testing seems to confirm.

I propose this is a sane change to the two spots (1) (2) in AsyncUDP.cpp where is check introduces the bug.

if (size > 1460) {
  size = 1460;
}

One of the network lighting protocols I use is DDP, which as its own in-payload header of 10 bytes and then up to 1440 bytes of channel data.

This means 1450-1436 = 14 bytes truncated - which is 5 RGB pixels, except for 1 channel of the first of the five.

This photo explains it visually:

This issue wasn't noticed before in WLED because WiFiUDP was previously used for DDP packets, and WiFiUDP doesn't have this truncation bug - and Art-Net and E1.31/sCAN packets are much smaller, so they never tripped over the line.

When I hack around the faulty truncation logic, the image is perfectly transmitted.

Sketch

...a bit complicated to pull this apart but basically when my packetSize is 1450 (10+1440), which it usually is, it's truncated as it's more than the default value of CONFIG_TCP_MSS (1436).

#define DDP_MAX_DATALEN 1440
#define DDP_HEADER_LEN 10
#define DDP_DEFAULT_PORT 4048
IPAddress clientIP;
static AsyncUDP ddpUdp;
ddpUdp.writeTo(packet_buffer, packetSize + DDP_HEADER_LEN, client, DDP_DEFAULT_PORT);

Debug Message

No debug messages, the payload length is just silently truncated in AsyncUDP::writeTo and AsyncUDPMessage::AsyncUDPMessage when bigger than CONFIG_TCP_MSS (which defaults to 1436).

Other Steps to Reproduce

Truncated everywhere when payload is bigger than 1436 - unless I change CONFIG_TCP_MSS to 1460 and rebuild Arduno-ESP32 or just manually hack AsyncUDP.cpp to remove the check or fix it to 1460.

So based on not wanting to change CONFIG_TCP_MSS to a higher number because it's very reasonable for TCP, I think the length checks should be hard-coded to 1460.

I have checked existing issues, online documentation and the Troubleshooting Guide

• I confirm I have checked existing issues, online documentation and Troubleshooting guide.