[lwip-members] [Fwd: Re: [lwip-users] I think I found a bug/problem in lwIP stack 1.0.0 (regarding retransmissions)]

[K.J. Mansley]

Hi lwip-members folks

Bonny Gijzen sent me this fix for a problem related to our recent
changes to the handling of retransmitted tcp packets.  Basically when a
single packet is queued for retranmission, it is moved to the unsent
list, and then back to the unacked list when it is actually
retransmitted.  This results in the unacked list being out of order, and
when received ACKs are compared to the head of the list, it obviously
doesn't see the "next" in-sequence packet.

Previously, when we dumped the whole unacked list on to the unsent list
and retransmitted all of them (even if just one retransmission was
required) we didn't have this problem as everything remained ordered.  

However, the fix involves searching the unacked list every time a packet
is sent.  This may be a significant overhead and so I'd prefer a
solution that didn't require this list searching for normal
(unretransmitted) packets.

Unfortunately I don't have the time right now to investigate if this
overhead is significant, or come up with an alternative.  So, could
someone else either have a think about it, or just check and apply the
fix as Bonny suggests.

Thanks!

Kieran

-----Forwarded Message-----

From: Bonny Gijzen <address@hidden>
To: address@hidden
Subject: Re: [lwip-users] I think I found a bug/problem in lwIP stack 1.0.0     
 (regarding retransmissions)
Date: 24 Nov 2004 08:45:00 +0100

Hi Kieran,


I hope you can receive my mail from this account. Please let me know.
I have replaced the code using a FIX1 define.

replaced code in tcp_output() :

/* put segment on unacknowledged list if length > 0 */
    if (TCP_TCPLEN(seg) > 0)
    {

#ifndef FIX1
      //original code
      seg->next = NULL;
      if (pcb->unacked == NULL)
      {
        pcb->unacked = seg;
        useg = seg;
      }
      else
      {
        useg->next = seg;
        useg = useg->next;
      }

#else
      //Check if segment is already on the unacked list
   //If not then add it
      seg2 = pcb->unacked;
   while (seg2 !=NULL)
   {
    if (seg2->p==seg->p)  //compare using pbuf pointer??
    {
            trace8(0xC0480000, 0, "segment already on unacked list!\n",0);
   break;
    }
    seg2 = seg2->next;
   }

      if (seg2->p!=seg->p)
   {
        trace8(0xC0480000, 0, "segment not on unacked list yet, so add
it\n",0);

  seg->next = NULL;
        if (pcb->unacked == NULL)
  {
          pcb->unacked = seg;
          useg = seg;
  }
        else
  {
          useg->next = seg;
          useg = useg->next;
  }

   }
   else
   {
        trace8(0xC0480000, 0, "Free it\n",0);
  tcp_seg_free(seg);
   }
#endif



Replaced code in tcp_rexmit() :

void
tcp_rexmit(struct tcp_pcb *pcb)
{
  struct tcp_seg *seg;



  if (pcb->unacked == NULL) {
    return;
  }


#ifndef FIX1
  //original code
  /* Move the first unacked segment to the unsent queue */
  seg = pcb->unacked->next;
  pcb->unacked->next = pcb->unsent;
  pcb->unsent = pcb->unacked;        //unsent=unacked,
unacked->next=unsent -> so "unacked" is inserted at the front
  pcb->unacked = seg;                //unacked becomes unacked->next (so the
newly unsent is removed from unacked)
  //unacked becomes unacked->next
  //So orig unacked is the 1st unacked segment in the list

#else

  //Move first unacked segment to unsent queue, but leave it on unacked
list!!
  //This means we need to make a COPY of the segment
  seg2=tcp_seg_copy((void*)pcb->unacked);
  if (seg2 == NULL) {
   trace8(0xC0480000, 0, "rexmit: tcp_seg_copy() failed\n",0);
    }
  else
  {
    seg2->next = pcb->unsent;  //Append the unsent list to it
    pcb->unsent = seg2;        //unsent list is updated with 1st unacked
segment
  }

#endif