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

[Mountifield, Tony]

Hi David,

I'm not 100% conversant with how reference counts are supposed to work within 
queues and chains. but I see that your pbuf_ref() is outside the if, and 
therefore called also after pbuf_queue(). Is this correct?

Cheers
Tony

> -----Original Message-----
> From: David Haas [mailto:address@hidden
> Sent: 18 August 2004 13:29
> To: lwip-devel
> Subject: Re: [lwip-devel] Problem with UDP and ARP queuing
> 
> 
> 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
> >
> >  
> >
> 
> 
> _______________________________________________
> lwip-devel mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/lwip-devel
> 


***********************************************************************************
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
***********************************************************************************