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

[Mountifield, Tony]

Thanks David. Yes, I think it would mainly manifest itself as a pbuf leak when 
trying to send to unresponsive addresses.

I checked the correction into CVS this morning.

Cheers
Tony

> -----Original Message-----
> From: David Haas [mailto:address@hidden
> Sent: 20 August 2004 13:46
> To: lwip-devel
> Subject: Re: [lwip-devel] Problem with UDP and ARP queuing
> 
> 
> Tony,
> 
> You are right. Good catch! I did a fair amount of testing and 
> did'nt see 
> a problem, but I guess it would manifest itself as a slow memory leak 
> and then only if there is more than one frame on the arp 
> queue (which in 
> my case is not that likely). I corrected my version.
> 
> David.
> 
> 
> Mountifield, Tony wrote:
> 
> >OK, I've just looked in more detail at pbuf_queue(). Just 
> before the end (pbuf.c:754), it does a pbuf_ref() on the 
> packet being queued, so I think etharp_queue() only needs to 
> do a pbuf_ref() when putting the pointer directly into the 
> ARP  table, and not when doing a pbuf_queue().
> >
> >Cheers
> >Tony
> >
> >  
> >
> >>-----Original Message-----
> >>From: David Haas [mailto:address@hidden
> >>Sent: 18 August 2004 16:34
> >>To: lwip-devel
> >>Subject: Re: [lwip-devel] Problem with UDP and ARP queuing
> >>
> >>
> >>It turns out that
> >>
> >>arp_table[i] = p;
> >>
> >>and
> >>
> >>pbuf_queue(arp_table[i].p, p);
> >>
> >>really do the same thing: put p on the arp_table queue. The 
> >>reason they 
> >>are separate is that pbuf_queue() does not handle the case 
> where the 
> >>queue is empty and therefore the first argument would be 
> zero. If the 
> >>pbuf is being queued at all, then the arp table has a 
> pointer to the 
> >>pbuf. Later when it takes the pbuf off the queue it will call 
> >>pbuf_free() with that pbuf pointer. So it needs to increment the 
> >>reference count when the pbuf is being queued.
> >>
> >>David.
> >>
> >>
> >>Mountifield, Tony wrote:
> >>
> >>    
> >>
> >>>Sorry, wrong "if".
> >>>
> >>>You have (effectively):
> >>>
> >>>       if (arp_table[i].p == NULL) {
> >>>           arp_table[i].p = p;
> >>>       } else {
> >>>           pbuf_queue(arp_table[i].p, p);
> >>>       }
> >>>       pbuf_ref(p);
> >>>
> >>>whereas I have:
> >>>
> >>>       if (arp_table[i].p == NULL) {
> >>>           pbuf_ref(p);
> >>>           arp_table[i].p = p;
> >>>       } else {
> >>>           pbuf_queue(arp_table[i].p, p);
> >>>       }
> >>>
> >>>I'm not sure which of the two is correct. It depends whether 
> >>>      
> >>>
> >>queueing requires a refcnt increment, and if so, whether 
> >>pbuf_queue() handles it.
> >>    
> >>
> >
> >
> >*************************************************************
> **********************
> >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
***********************************************************************************