[lwip-users] A flaw in lwIP's keepalive implementation?

[Uri Yurman]

Hi all,

I have found something that seems to me like a bug in lwIP's keepalives.

I'll try to explain the problem/situation and offer a correction. If
I'm wrong please correct me, and if I'm right I'd really like to know
if the correction I suggest doesn't affect other aspects of lwIP.

lwIP's keepalive packet is an ACK packet with zero data. This means
that when lwIP sends a keepalive it doesn't wait for an ACK for that
keepalive - acks are only sent for packets with data.

The keepalive packets are sent when time comes in specific intervals
according to the TCP_KEEPINTVL value and the inactivity timer. If the
inactivity timer is reset before time comes, the keepalive won't be
sent.

lwIP resets the inactivity timer when data is received, in
tcp_process() using the following code:
   /* Update the PCB (in)activity timer. */
   pcb->tmr = tcp_ticks;
   pcb->keep_cnt = 0;
This is done even before checking the current state and acting
accordingly (the big "switch (pcb->state)"), and therefore this code
runs for any incoming packet.

The problem is that when the other side is sending an empty ACK
(sometimes a keepalive) it causes these 2 lines of code to reset the
inactivity timer.
Therefore if you connect two lwIP systems, where the first is sending
keepalives in a higher frequency than the second, the second won't
send keepalives at all (!!).

My solution is to make sure that in ESTABLISHED state, the inactivity
timer will be reset only when we need to send an ACK. I think lwIP
should reset the inactivity timer inside the "switch (pcb->state)" in
states that don't call tcp_receive(), and inside tcp_receive() for
other states.
inside tcp_receive, the timer should be reset *only* for packets
carrying bytes of data.

Waiting for your comments,
-Uri