Re: [lwip-users] Patch for persist timer and silly window bug

[Kieran Mansley]

On Tue, 2007-12-18 at 14:40 +0100, Per-Henrik Lundblom wrote:
> The patch should fix the following things:
> 
> - Silly window problem (bug #20199)

I've ported this portion of the patch to CVS head and attached a new
patch to the bug report.  I'd be grateful if others could check it looks
sensible.  The only functional difference to the one supplied by Per-
Henrik Lundblom is that it is careful to avoid rcv_ann_wnd going
negative when decreasing it.  There was one place (in tcp_receive) where
if for example rcv_ann_wnd < tcplen < rcv_wnd, and you subtract tcplen
from rcv_ann_wnd it could theoretically go negative. 

> - Implementation of the TCP persist timer which lwIP curretly lacks. The
>   persist timer is MANDATORY for any TCP implementation. This
>   implementation can be slimmed down. This implementation doesn't share
>   any code with the retransmit timer even though it should be possible.
>   Also, the tcp_split_unsent_seq() function has code copied from other
>   tcp_*() functions. I guess you could leave out the split part of the
>   persist timer if you want to reduce code size but then you don't fully
>   implement the functionality for the persist timer.

I'm working on merging this now.  The code to split the pbuf and unacked
head is pretty complex though.  I wonder if a simpler solution would be
to just make a new pbuf when you want to send a persist timer probe,
copy in the first byte from the unacknowledged queue, and send that
(without adding it to the unacknowledged queue).  This is rather similar
to our existing approach for keep alive probes.  The consequence of this
is that you will then get an ACK for a byte that is part of a larger
pbuf, and so you can't free the pbuf immediately, and you will then
retransmit that byte when you get enough window to send the whole pbuf,
but I think it should fulfill the requirements of the persist timer and
be a lot less code.  Any thoughts before I go to the effort of doing
this?

Kieran