Re: [lwip-devel] Problem with UDP and ARP queuing

[David Haas]

Yes, I agree with you.

In fact I had made that change a few days ago, but had not gotton around 
to creating a patch yet. My code around this area looks like this:


#if ARP_QUEUEING /* queue the given q packet */
     /* copy any PBUF_REF referenced payloads into PBUF_RAM */
     /* (the caller of lwIP assumes the referenced payload can be
      * freed after it returns from the lwIP call that brought us here) */
     p = pbuf_take(q);
     /* packet could be taken over? */
     if (p != NULL) {
       /* queue packet */
       if (arp_table[i].p)
         pbuf_queue(arp_table[i].p, p);
       else
         arp_table[i].p = p;
     /* pbufs are queued, increase the reference count */
     pbuf_ref(p);
       LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("etharp_query: queued 
packet %p on ARP entry %d\n", (void *)q, i));
       result = ERR_OK;
     } else {
       LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("etharp_query: could not 
queue a copy of PBUF_REF packet %p (out of memory)\n", (void *)q));
       /* { result == ERR_MEM } through initialization */
     }
#else /* ARP_QUEUEING == 0 */

I've done some pretty extensive testing with TCP, but only limited 
testing with UDP.

David.

Mountifield, Tony wrote:

> Following up again:
>
>  
>
> > After further thought, an idea:
> >
> >    
> >
> > > It may be just that udp_send() has no reason to free the 
> > > header pbuf, and we can remove that bit of code, but I don't 
> > > know what else may rely on it.
> > >      
> > Perhaps it is a reference-counting issue. Does/should 
> > etharp_query increment a reference count in the chain/packet 
> > when queuing it on an ARP table entry? And then of course 
> > decrement it when dequeuing.
> >    
>
> I think the following is certainly needed, but we will need to make sure there 
> is a pbuf_free() in the right place (there may already be) to avoid a pbuf leak:
>
> Index: src/netif/etharp.c
> ===================================================================
> RCS file: /cvsroot/lwip/lwip/src/netif/etharp.c,v
> retrieving revision 1.80
> diff -u -r1.80 etharp.c
> --- src/netif/etharp.c  17 Aug 2004 08:39:43 -0000      1.80
> +++ src/netif/etharp.c  18 Aug 2004 11:19:14 -0000
> @@ -755,6 +755,7 @@
>         /* queue packet ... */
>         if (arp_table[i].p == NULL) {
>                /* ... in the empty queue */
> +               pbuf_ref(p);
>                arp_table[i].p = p;
>         } else {
>                /* ... at tail of non-empty queue */
>
> Cheers
> Tony
>
>
> ***********************************************************************************
> This email, its content and any attachments is PRIVATE AND
> CONFIDENTIAL to TANDBERG Television. If received in error please
> notify the sender and destroy the original message and attachments.
>
> www.tandbergtv.com
> ***********************************************************************************
>
>
>
> _______________________________________________
> lwip-devel mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/lwip-devel
>
>