[En-Nut-Discussion] [PATCH] Implement TCP sequence number wraparound

[Henrik Maier]

Hi Marti,

What happens to the connection if your patch is not applied? Does it crash,
stall or simply disconnect?

I am asking in order to assess what impact this issue has on existing
systems and whether they need to be updated.

I could live with the fact the connection just drops and works again after
reconnection.

Kind Regards

Henrik

> -----Original Message-----
> From: en-nut-discussion-bounces at egnite.de [mailto:en-nut-discussion-
> bounces at egnite.de] On Behalf Of Marti Raudsepp
> Sent: Thursday, 25 March 2010 1:59 AM
> To: Ethernut User Chat (English)
> Cc: Maidu Raudsepp
> Subject: [En-Nut-Discussion] [PATCH] Implement TCP sequence number
> wraparound
> 
> Hello list,
> 
> It seems that neither Ethernut 4.8 nor 4.9 currently attempt to handle
> TCP sequence number wraparound. This means that each TCP connection is
> limited to transmitting/receiving 4 GB of data in the best case.
> Technically it's in violation of the TCP spec.
> 
> Transmitting at the maximum rate of our application (1.10 MB/s both ways
> by our measurements) means that we can hit this limit in just 1 hour.
> 
> Further, since the initial TX sequence number is initialized from
> NutGetMillis(), the maximum TCP session lifetime approaches 0 as uptime
> approaches 49.71 days. Nut/OS has no control at all over RX sequence
> numbers which are assigned by the other endpoint.
> 
> So I went through every sequence number comparison in the TCP code and
> changed it. My initial approach was to change all sequence number values
> to signed int32_t and change comparisons of style "a > b" into
> "a - b > 0", but I figured it might be confusing to people, because
> sequence numbers are usually represented as positive integers and the
> two comparisons would seem equivalent mathematically.
> 
> In the end I settled for adding two macros to tcputil.h:
> 
> #define SeqIsAfter(x, low) \
>    ((int32_t)(x - low) > 0)
> 
> #define SeqIsBetween(x, low, high) \
>    ((uint32_t)(x - low) <= (uint32_t)(high - low))
> 
> SeqIsBetween is my replacement for the IsInLimits function; the macro is
> very simple, so a function seemed like overkill.
> 
> The patch applies cleanly to both 4.8.6 and 4.9.8, but I have only
> tested with 4.8.6 on AT91SAM7X. So far I have tested wraparound in the
> common case (no packet loss). I feel confident that I haven't broken
> anything that worked before. But no guarantees, test and scrutinize it
> yourself. :)
> 
> However, testing wraparound around packet loss/reordering edge cases is
> pretty complicated, and Nut/OS's short TCP window doesn't help. I'll see
> what I can do with Linux netem network emulator.
> 
> ----
> Hereby I submit 3 patches for review:
> 
> 01_tcp_seqno_wraparound.patch:
> Implements handling of TCP sequence number wraparound. This is the
> actual patch that I described above.
> 
> 02_remove_bogus_todo.patch:
> This just removes a bogus TODO item that has already been implemented.
> 
> 03_tcp_ntohl_to_htonl.patch:
> I found a few places in the TCP code where the "htonl" macro was used
> insteasd of "ntohl". Since these operations are intechangeable, it's a
> coding style fix only.
> 
> 
> Regards,
> Marti Raudsepp
> voicecom.ee
> 
> _______________________________________________
> http://lists.egnite.de/mailman/listinfo/en-nut-discussion