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.
Cheers
Tony
-----Original Message-----
From: David Haas [mailto:address@hidden
Sent: 18 August 2004 15:27
To: lwip-devel
Subject: Re: [lwip-devel] Problem with UDP and ARP queuing
Its not outside of the "if (p != NULL) block." My indentation just
seems to be wrong.
The inner "if (arp_table[i].p)" statement just decides how
to queue the
pbuf.
The pbuf is still queued no matter which branch is taken.
David.
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?
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