RE: [lwip-users] Bug in lwip ARP caching

[Goldschmidt Simon]

Hi,

 

see bug #11400 for an explanation of this. Multiple packet queueing should be turned of by an #if 0 statement in etharp.c and (at least with a current version), you should not be seeing this problem.

 

Simon

 

---

From: address@hidden [mailto:address@hidden On Behalf Of Reither Robert
Sent: Tuesday, February 13, 2007 1:05 PM
To: address@hidden
Subject: [lwip-users] Bug in lwip ARP caching

Hi,

i guess i've found a serious bug in lwip arp-caching.

Let me explain:

Tcp connection, the arp does not know the MAC-Addr, because the destination does not exit.
ARP-cache stores the (unsendable) pbuf-packet in its cache.

So far so good. But after some time, a TCP-retransmit attempt tries to resend the same data (pbuf).
The ARP-cache tries to save this packet and appends it to the last one.

But in this case, the last packet (and so the pbuf) is the same as the new one !
So it creates a pbuf-chain which points with the next-pointer to itself !!!

And guess what happens if, for example, the checksum routine walks over the pbuf-chain at a second retransmit attempt !
This will be the end if line, we loop 4ever ....

I tried to solve it within pbuf.c/pbuf_queue() call:

I do not enqueue a new pbuf if the new pbuf is already within the given ARP pbuf-chain.

code snipplet from pbuf.c/pbuf_queue()

.......
  /* iterate through all packets on queue */
  while (p->next != NULL) {
/* be very picky about pbuf chain correctness */
#if PBUF_DEBUG
    /* iterate through all pbufs in packet */
    while (p->tot_len != p->len) {
      /* make sure invariant condition holds */
      LWIP_ASSERT("p->len < p->tot_len", p->len < p->tot_len);
      /* make sure each packet is complete */
      LWIP_ASSERT("p->next != NULL", p->next != NULL);
      p = p->next;
      /* { p->tot_len == p->len => p is last pbuf of a packet } */
    }
    /* { p is last pbuf of a packet } */
    /* proceed to next packet on queue */
#endif

//inserted code
    // RR: Never append a pbuf already in chain, we would create a loop !!!!!
    if (p == n)
    {
    //  nr_printf("Trying to append a pbuf (0x%x) to arp_queue (0x%x) already in chain !!\n",(unsigned)p,(unsigned)n);
      return;
    } 
//inserted end
    /* proceed to next pbuf */
    if (p->next != NULL) p = p->next;
  }

//inserted code
  // Need it here too in case of a single pbuf in p (old arp-cache holds one a single pbuf)
  // RR: Never append a pbuf already in chain, we would create a loop !!!!!
  if (p == n)
  {
  //  nr_printf("Trying to append a pbuf (0x%x) to arp_queue (0x%x) already in chain !!\n",(unsigned)p,(unsigned)n);
    return;
  }
//inserted end

.......

Maybe someone finds a better solution.

Greetings

Robert

Robert Reither
Research & Development
AV Digital Audio- Videotechnik GmbH
Rampengasse 3-5
A-1190 Wien/Austria
Commercial Register No.: FN 201615v
Commercial Court: Handelsgericht Wien
VAT-No.: ATU 50461904
Tel.: +43/1/3680368 43
Visit: <http://www.av-digital.at>