OpenFastPath / ofp

OpenFastPath project
BSD 3-Clause "New" or "Revised" License
349 stars 126 forks source link

TCP zero window can not be sent correctly #285

Open HsuJv opened 1 year ago

HsuJv commented 1 year ago

This issue is created to describe PR #273 for the owners.

To reproduce the issue, you can simply accept a socket from an OFP listening socket. Then try to do nothing and ignore it on the server. Keep sending messages from the client slide, you'll find the advised receive window from the server side never get down to zero.

It's actually the what's difference between OFP & BSD stack: In the BSD stack all data get copied from eth raw packet but in OFP we just put a reference of that odp_packet in so->so_rcv.put. That might cause the problem that the advertised window will not shrink byte by byte! It shall be packet by packet.

Assuming that I've got a SOCKBUF_LEN setting to 256, which may have an initial advertised window up to 475136 (256 * 1856). Then I sent 256 individual packets from the client, including only little (to say 10) bytes in each. Let's see what happens when the OFP performs the ack.

https://github.com/OpenFastPath/ofp/blob/f6580bb675386b0599390daaabbb0a06c1c1be0c/src/ofp_tcp_output.c#L1017-L1029

The recwin here should be zero as there is no slot in so->so_rcv (all 256 slots get occupied, no read from app level). Then it shall compare the (rev_adv - rcv_nxt) with recwin, and get the maximum one. But the rev_adv was initialized as 475k and the rcv_nxt only grew up in 10 * 256 = 2560, it will always set recwin to a very large number which is unacceptable at all.

My fix in #273 was to remove lines 1021 - 1026. As we shrink & increase the TCP windows packet by packet, I don't think that we should consider the silly window syndrome so that lines 1021-1023 are unnecessary. And lines 1024-1026 are where the bug actually locates, to reduce the unnecessary calculations, we can simply use the remaining slots * packet size as the size of the advertised window.

BRs.