Fri 07 Aug 2009 04:20:41 AM UTC, original submission:
Credit to Mousom <mousom_b@yahoo.com> for bringing this up in lwip-users:
There is a bug in tcp_receive(), in its handling of the send window and associated handling of acknowledgements.
There are three places where the wrong sequence number has been used to calculate the right edge of the send window. Each of these refers to the "right_wnd_edge" variable so they are easy to spot.
right_wnd_edge = pcb->snd_wnd + pcb->snd_wl1;
if (pcb->snd_wl1 + pcb->snd_wnd == right_wnd_edge)
LWIP_DEBUGF(TCP_FR_DEBUG, ("tcp_receive: dupack averted %"U32_F" %"U32_F"\n",
pcb->snd_wl1 + pcb->snd_wnd, right_wnd_edge));
In each case, the use of 'pcb->snd_wl1' is incorrect and it shoudl be 'pcb->snd_wl2'.
Explanation:
pcb->snd_wl1 is set from the received seqno, while pcb->snd_wl2 is set from the received ackno.
This means that the duplicate ack and fast retransmit calculations are being done based on a calculated right window edge which is the send window size plus the RECEIVE sequence number, and comparing to the send sequence number. This is meaningless.
I haven't looked through the code in detail to determine the likely side effects, but it appears from a quick glance that this could have implications such as unnecessary retransmission, and never closing connections due to waiting for an acknowledgement which will never arrive.
This may be the cause for several of the bugs which are listed on the bug tracker.
I'm still using LWIP of approximately 1.1.1 vintage and the bug existed at least that far back, but I've confirmed it is still there in CVS head.
As a general comment: I think it would be a good idea to rename these fields. It is not at all obvious what "snd_wl1" and "snd_wl2" are supposed to do. They have similar names so can be easily confused, and there is nothing to associate them with the appropriate direction.
|