Wed 12 Mar 2008 04:59:53 PM UTC, comment #11:
> the lwip_recvfrom for a SOCK_RAW should also returns the IP header
If that's the case, everything looks fine to me.
|
Wed 12 Mar 2008 11:52:27 AM UTC, comment #10:
Uhmm, there is always a "small" problem: the netbuf::addr is initialized with an address inside the "p" packet, and not inside the "q" one.
In fact, it's the same kind of problem than "[bug #22435] lwip_recvfrom with TCP break".
The fastest way to fix it should be to write :
buf->addr = &(((struct ip_hdr*)(q->payload))->src);
No objections ?
|
Wed 12 Mar 2008 11:01:50 AM UTC, comment #9:
It's check in. I think we can close it if there is no objections...
|
Wed 12 Mar 2008 10:51:13 AM UTC, comment #8:
Answer : the lwip_recvfrom for a SOCK_RAW should also returns the IP header (thank you jifl)
http://linux.die.net/man/7/raw ("...For receiving the IP header is always included in the packet.")
http://msdn2.microsoft.com/en-us/library/ms740548.aspx ("An application always gets the IP header at the front of each received datagram ...")
So, the patch is good. Note, the contrib/apps/ping is wrong, and I will fix it...
|
Wed 12 Mar 2008 10:27:00 AM UTC, comment #7:
I see a "problem" during my tests: what is supposed to return a raw pcb at the netconn/socket layer? Should the payload have to point on the IP header, or on the IP payload ?
I ask that because a hidden problem with the current code is - I suppose - the input processing packet should do a pbuf_header so, sample like the ping.c (with LWIP_SOCKET=1) in the contrib folder failed (see ping.c, line~162) : the code suppose that lwip_recvfrom return the IP payload (so, the ICMP header), but is it the normal behavior?
Is it the stack which should add a "pbuf_header( q, -PBUF_IP_HLEN)" (or better, use IPH_HL), or to the application ?
|
Mon 10 Mar 2008 07:47:53 PM UTC, comment #6:
Definitively should go in for 1.3.0 since this is a real bug fix! Good work!
|
Mon 10 Mar 2008 03:10:27 PM UTC, comment #5:
Looks good to me. If it's to be checked in before 1.3.0 it would probably be good to test it just in case, as there won't be much more testing after now.
|
Mon 10 Mar 2008 02:31:21 PM UTC, comment #4:
I attach a patch file. Since I have the case with ping sender&ping receive in my application, I will test it if you want...
(file #15219)
|
Mon 10 Mar 2008 02:14:44 PM UTC, comment #3:
Yes, which is why I don't think the API layer should eat the packet, as it's not definite that the API user is the proper (and only) place for the packet.
|
Mon 10 Mar 2008 02:03:05 PM UTC, comment #2:
If we return 1 in recv_raw (in api_msg.c), I suppose the stack would stop to response to "ping" if one of the application open a raw pcb with IP_PROTO_ICMP proto (by example, to send ping), right?
|
Mon 10 Mar 2008 01:25:29 PM UTC, comment #1:
I agree that this is a bug. recv_raw should not assume that if it hasn't "eaten" the data it will continue unmodified.
Although I don't think the suggested fix is correct. That would produce odd behaviour that would change depending on exactly when an application makes a call to receive a packet or not. It would not be reliable.
The correct answer is probably to still not "eat" the packet, but to copy the pbuf entirely into a new pbuf. The reason we don't "eat" the packet is because we don't know whether the data is definitely for us or not. It's the same principle as raw_input() which tries all the raw PCBs in turn until one of them eats the data - it doesn't know which one the data is really for. It would be more wrong to "eat" the data before it reaches the PCB that actually uses it, than to duplicate the data.
|
Mon 10 Mar 2008 01:17:11 PM UTC, original submission:
From lwip-users:
-=-=-=-=-=-=-=-
I have an application that opens up a raw socket (SOCK_RAW), and is expecting to receive an ICMP packet.
In this case, depending on the kernel scheduling, sometimes the packet received includes the IP header, and sometimes not. This occurs because recv_raw (api_msg.c) posts the received packet into the applications mbox. However, it does not say that the packet has been processed (or "eaten" according to the sorrounding comments), so further layers continue processing the packet. Eventually it reaches icmp_input which moves the payload past the ip header.
It seems like the code in recv_raw should return 1 in cases where there is an application waiting for the input. I can see that this is ok to do for ICMP packets, but I'm not sure if it is generally applicable.
The issue exists in lwip-1.2.0, and at first glance appears to exist in lwip-1.3.0rc1 also.
-=-=-=-=-=-=-=-=-
|