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

Submitted by:  Oleg Tyshev <olegreen>
Submitted on:  Fri 25 Jan 2008 02:26:48 PM UTC  
 
Category:  TCP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  Simon Goldschmidt <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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project 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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project 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>
Project AdministratorIn charge of this item.
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>
Project 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>

 

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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    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.5