|
Hi Leon,
Thanks for your advice.
I found this problem since I have met it as starting TCP connecting.
The sequence number of first packet of three-way handshake was changed before
TCP
got remote ack and then it failed to establish tcp connection. Of course,
it only occurs as ETHARP_QUEUEING=1.
As you said, I made some mistakes in my patch. For TCP, it seems never to
use a pbuf which has PBUF_FLAG_REF been set in its flag. But this kind
of pbuf
is always used as using UDP and I think it realy will meet the problem
I mentioned in previous mail. Besides that, TCP will lose some packets
which is chained
in the unacked segments list, since pbuf_take doesn't make a copy as
PBUF_FLAG_REF is not set and ARP will free its queue packets after sending
them out.
According to above situation, we should not make original packets be
freed before they are queued in ARP queue and after they are sent out
from ARP queue.
Therefore, I made a new fix which is shown as follows.
In pbuf.c
828 struct pbuf *
829
pbuf_take(struct pbuf *p)
.....
841 - pbuf_free(p);
.....
868 /* remove linkage from original pbuf
*/
869 - p->next = NULL;
.....
901 /* p->flags != PBUF_FLAG_REF
*/
902 } else {
+
pbuf_ref(p);
903 LWIP_DEBUGF(PBUF_DEBUG | DBG_TRACE |
1, ("pbuf_take: skipping pbuf not of type
PBUF_REF\n"));
904 }
Just like you said, it may not be suitable to modifiy pbuf_take to resolve
this problem. Or we could say that ARP queueing should not use pbuf_take to make
a copy
of the original packet. Just for easy modifing and understanding,
I decided to make a modification in pbuf_take. You could
make modification in another place instead
of pbuf_take since I am not sure where is the suitable one.
Maybe I am missing something again. Please let me know on the
list.
Regards,
Bo-Er
----- Original Message -----
Sent: Thursday, December 16, 2004 9:30
PM
Subject: Re: [lwip-users] Bug of
ARP?
Hello Jason,
thanks for your bug report.
Did you
experience premature free()ing of pbufs?
I have tried following your
analysis. Maybe you do have a point that some
packets could be free
prematurely, but I think the problem might be
somewhere
else, and I
think your patch fixes the wrong stuff.
Wei Bo-Er (Jason)
wrote:
> Hi all,
> I'm using lwIP 1.0.0 and found a problem
look like a lwIP's bug. I
> don't know if there is
> anyone had
mentioned it before(There seems to be many bugs fixing
> recently). I
just explain
> what I found here.
Out of interest, you found this
in what way? Actual debugging or code
analysis?
Also, did you *see*
this happen with TCP only, or UDP and ICMP as well?
> In gereral,
LwIP will queue packets as there is no matching entry in
> the ARP
table. It uses
Yes, more exactly, only if ETHARP_QUEUEING is 1, this
downstream
queueing occurs
(if resources are available).
> p =
pbuf_take(q) to copy sent packet 'q' to internal pbuf 'p' and make
>
q=p.The 'p' will be
The documentation of pbuf_take() says:
* Go
through a pbuf chain and replace any PBUF_REF buffers
* with PBUF_POOL (or
PBUF_RAM) pbufs, each taking a copy of
* the referenced data.
* ...
*
@note Any replaced pbufs will be freed through pbuf_free().
* This may
deallocate them if they become no longer referenced.
So, it only makes
copies for PBUF_REF type pbufs. This makes sure the
caller can safely
re-use
the memory references by PBUF_REF pbufs after submitting them to
lwIP
for transmission.
For non-PBUF_REF types, nothing
happens.
> queued in arp_table[i].p and be freed after timeout or it
can be sent
> out. This will cause some
> problems in TCP. Since
TCP needs to keep sent packets in unacked queue
> until ack is
>
arriving, those packets may be freed in ARP before ack is received by
>
TCP. Except TCP,
Maybe TCP should reference the pbufs it decides to
queue and free them
when no longer
queueing them?
> ICMP and
UDP will also free sent packets after return of sent-out
> function. It
may also
> causes packets in ARP queue be freed before timeout or they
can be
> sent out.
To prevent that, etharp increments the pbuf
reference count when
queueing it.
> I made some modification in
pbuf_take which is shown as follow to
> resolve it.
> In
pbuf.c
> 828 struct pbuf *
> 829 pbuf_take(struct pbuf *p)
>
830 {
> 831 - struct pbuf *q , *prev, *head;
> + struct pbuf *q ,
*prev, *head, *ori_head;
> .....
> 836 - head = p;
> + head
= ori_head = p;
> .....
> 841 - pbuf_free(p);
>
.....
> 911 + p=ori_head;
This patch does nothing except remove
pbuf_free() !
Please note that p has local scope and it not used
afterwards.
> 912 return head;
> 913 }
> This
modification keeps 'p' as original one and doesn't free it. All
> sent
packets will be freed by up layer
> instead of ARP.
I think the
real problem is that the upper TCP layer should increment
the reference
count of a pbuf during
queueing and decrement it once it decides to drop it
(by receiving an
ACK for it).
UDP and ICMP do not queue and have no
extra references to a pbuf.
Maybe I am missing the point. Please let me
know on the
list.
Regards,
Leon.
_______________________________________________
lwip-users
mailing list
address@hidden
http://lists.nongnu.org/mailman/listinfo/lwip-users
|