buglwIP - A Lightweight TCP/IP stack - Bugs: bug #22110, Recv() makes receive window update...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #22110: Recv() makes receive window update for data, that was not received by application

Submitter:  Oleg Tyshev <olegreen>
Submitted:  Fri 25 Jan 2008 02:26:48 PM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Jump to the original submission

Tue 09 Feb 2010 08:27:19 PM UTC, comment #9: 

I've implemented the idea from comment #8, but throught a flag in conn->flags rather than an additional argument to netconn_recv():

- the socket code now calls netconn_set_noautorecved(conn, 1) after creating a connection,
- this lets netconn_recv skip the do_recv part,
- the socket code calls netconn_recved() before it returns from lwip_recvfrom with off > 0

This is right now not integrated with close/shutdown (as mentioned in comment #4), but I see that as a different problem. It might be solved by changing the netconn_recved() implementation, but it might also be solved differently...

Simon Goldschmidt <goldsimon>
Group administrator
Mon 26 Oct 2009 08:44:03 AM UTC, comment #8: 

This is rather minor, too: I'd change the function netconn_recv() to take an argument whether to call do_recv or not (keeping a define without that argument for backwards compatibility and UDP), and then simply call do_recv from lwip_recvfrom().

Simon Goldschmidt <goldsimon>
Group administrator
Wed 24 Jun 2009 03:20:20 PM UTC, comment #7: 

As I've moved BUG #24830, and this sort of depends on it (see comment #4) I'm moving this back to 1.4.0 too.

Kieran Mansley <kieranm>
Group Member
Sun 26 Apr 2009 05:50:07 PM UTC, comment #6: 


> The change needed here - being able to find out if the layer
> above is buffering data - is similar to the one needed to
> support shutdown and close properly.


You could implement it that way, but I'd rather think of it as a limitation of the sockets API being built upon the netconn API: the netconn API simply does not provide the functionality that we need here: we would need a netconn_recved callback that effectively does the window update.


A little off topic: I think this is a major performance whole in TCP RX: we should try to prevent calling back into the stack (do_recv) every time a packet is received. Most of the time, tcp_recved does nothing and the places where tcp_ack_now is called that could maybe also be solved differently (e.g. on next input packet)?

Simon Goldschmidt <goldsimon>
Group administrator
Wed 22 Apr 2009 10:38:54 AM UTC, comment #5: 


> ACKing the data before its delivered to the application is fine.


Yeah, I messed that up. Actually I was referring to the call to do_recv: of course, this updates the window, not sends an ACK.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 22 Apr 2009 08:10:54 AM UTC, comment #4: 

ACKing the data before its delivered to the application is fine.  The problem here is updating the available window while we're still buffering the data.

The change needed here - being able to find out if the layer above is buffering data - is similar to the one needed to support shutdown and close properly.  (BUG #24830)  While this bug is a minor problem, BUG #24830 is more serious, and if I can fix that we might get the solution to this one for free.


Kieran Mansley <kieranm>
Group Member
Tue 21 Apr 2009 08:08:05 PM UTC, comment #3: 

Having a look through the code, this is clearly a bug resulting in the mix of netconn- and socket-API (netconn_recv acknowledges the data but the socket layer buffers it before presenting it to the application).

Being that, I don't know if we really should solve this (it would require changing/extending the netconn API to either not ACK or not receive all data), or if we should just hope that "sockets2" gives us the chance of cleaning up the mess of having 3 APIs (converting to 2 - raw and sequential - by having a socket-API that provides fast netconn-like functions)?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 25 Jan 2008 02:54:02 PM UTC, comment #2: 

Agree with you, it is minor bug. It can be postponed.

Oleg Tyshev <olegreen>
Fri 25 Jan 2008 02:29:41 PM UTC, comment #1: 

This sounds like a genuine bug, but minor enough that we don't need to worry about it for 1.3.0, assuming that once the application has processed all the data our window updates will be correct again.

Kieran Mansley <kieranm>
Group Member
Fri 25 Jan 2008 02:26:48 PM UTC, original submission:  

One sample
receive mailbox contains 4 packets each 1000 bytes
(all this data already ACKed)

Application makes Recv for 1 byte.
It would be receive window update sent as it was 1000 bytes
received.

The problem is that receive buffer has two parts
receive mailbox and lastdata.
Lwip considers, that all data in lastdata
always processed and immediately after receive data from
receive mailbox sends window update.
But we should do it only for actual processed data.

Oleg Tyshev <olegreen>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by olegreen (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-02-09 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2009-06-24 kieranm Planned Release1.3.1 1.4.0
    2008-03-04 kieranm Planned Release 1.3.1

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code