[En-Nut-Discussion] MSS not calculated correctly

[Henrik Maier]

Hi,

I came across a TCP/IP comms issue when a machine connects to Nut/OS (AVR,
any 4.x version) and advertises a MSS of 1024 instead of the more common
1460 bytes. Suddenly I saw Nut/OS replying a SYNC/ACK with a MSS of 4 and
this was the end of all sensible TCP/IP comms as from then on segment size
was 4 bytes which brakes most applications. (Most Linux and Windows PCs use
a MSS of 1460 but some other embedded TCP/IP stacks or WAN routers may use a
different size.)

After analysing what is going on I found the following code snippet in
tcpsm.c to be the culprit:

            /* Read MAXSEG option */
            case TCPOPT_MAXSEG:
                s = ntohs(((u_short)cp[2] << 8) | cp[3]);
                if (s < sock->so_mss)
                    sock->so_mss = s;

The ntohs() call is not necessary here as byte swapping is already done by
the shift and logic operation. ntohs swaps again and gives the following
results:

(Note: sock->so_mss is initialised to 536)

a) Common case of MSS=1460:

                s = ntohs( 0x05 | 0xB4) = 0xb405 = 46085
                if (46085 < sock->so_mss)
                    sock->so_mss = s;

Result: No change to sock->so_mss, it stays at Nut/OS default of 536.

b) Special case of MSS=1024:

                s = ntohs( 0x04 | 0x00) = 0x0004 = 4
                if (4 < sock->so_mss)
                    sock->so_mss = 4;

Result: sock->so_mss is now 4 and this is the maximum size of sent TCP
packets. All further TCP/IP comms will most likely not work because of the
small segment size. 

I believe the correct implementation should be:

            /* Read MAXSEG option */
            case TCPOPT_MAXSEG:
                s = ((u_short)cp[2] << 8) | cp[3];
                if (s < sock->so_mss)
                    sock->so_mss = s;

Any thoughts/agreements/disagreements?


Regards

Henrik


PS1: This also may have been the explanation of the issue András Szemző has
reported on the 30/9/2009.

PS2: I am also not sure if topics like this should go to the mailing list or
rather be lodged as bug report on SourceForge instead (or both)?