smoltcp icon indicating copy to clipboard operation
smoltcp copied to clipboard

ACK sequence number check too strict

Open jsnell opened this issue 8 years ago • 1 comments

In process():

// Every acknowledgement must be for transmitted but unacknowledged data

This is incorrect. In RFC 793 the ACK sequence number being out of that range would have little effect; the payload would still be processed. It's just that the window or SND.UNA would not be updated based on the ACK sequence number.

This was then made more strict in RFC 5961 (which is the version worth implementing).

The ACK value is considered acceptable only if it is in the range of
((SND.UNA - MAX.SND.WND) <= SEG.ACK <= SND.NXT). All incoming segments
whose ACK value doesn't satisfy the above condition MUST be discarded and
an ACK sent back. 

So the changes to the code would be: a) expanding the range of acceptable ACKs to cover up to one window's worth of already ACKed data; b) sending a challenge ACK instead of just dropping the packet.

This is not just a theoretical concern. The too-strict check will cause trouble for bidirectional traffic in networks with reordering.

jsnell avatar Apr 12 '17 23:04 jsnell

Thanks, will fix!

whitequark avatar Apr 13 '17 01:04 whitequark