[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [lwip-devel] Problem with UDP and ARP queuing
From: |
Leon Woestenberg |
Subject: |
RE: [lwip-devel] Problem with UDP and ARP queuing |
Date: |
Wed, 18 Aug 2004 17:01:36 +0200 |
Hello Tony, David,
On Wed, 2004-08-18 at 15:42, Mountifield, Tony wrote:
> 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?
>
The reference count is equal to the number of pointers to the pbuf.
Both lwip-internal and lwip-external pointers.
There are usage contracts in the API, that clearly describe when you
may no longer reference a pbuf, or when you may.
I will double-check the proposed changes by David.
Regards,
Leon.
> 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
> ***********************************************************************************
>
>
>
> _______________________________________________
> lwip-devel mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/lwip-devel
- [lwip-devel] Problem with UDP and ARP queuing, Mountifield, Tony, 2004/08/18
- RE: [lwip-devel] Problem with UDP and ARP queuing, Mountifield, Tony, 2004/08/18
- RE: [lwip-devel] Problem with UDP and ARP queuing, Mountifield, Tony, 2004/08/18
- RE: [lwip-devel] Problem with UDP and ARP queuing, Mountifield, Tony, 2004/08/18
- RE: [lwip-devel] Problem with UDP and ARP queuing, Mountifield, Tony, 2004/08/18
- RE: [lwip-devel] Problem with UDP and ARP queuing, Mountifield, Tony, 2004/08/19
- RE: [lwip-devel] Problem with UDP and ARP queuing, Mountifield, Tony, 2004/08/20