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

[Sebastian Gerhardt-2]

URL:
  <http://savannah.nongnu.org/bugs/?22110>

                 Summary: Recv() makes receive window update for data, that
was not received by application
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: olegreen
            Submitted on: Friday 01/25/2008 at 14:26
                Category: TCP
                Severity: 3 - Normal
              Item Group: Faulty Behaviour
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release:

    _______________________________________________________

Details:

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.





    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?22110>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sebastian Gerhardt-2]

Follow-up Comment #1, bug #22110 (project lwip):

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.

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?22110>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sebastian Gerhardt-2]

Follow-up Comment #2, bug #22110 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?22110>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sebastian Gerhardt-2]

Update of bug #22110 (project lwip):

         Planned Release:                         => 1.3.1                  


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?22110>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sebastian Gerhardt-2]

Update of bug #22110 (project lwip):

            lwIP version:                    None => CVS Head              

    _______________________________________________________

Follow-up 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)?

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?22110>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sebastian Gerhardt-2]

Follow-up Comment #4, bug #22110 (project lwip):

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.



    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?22110>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sebastian Gerhardt-2]

Follow-up Comment #5, bug #22110 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?22110>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sebastian Gerhardt-2]

Follow-up Comment #6, bug #22110 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?22110>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sebastian Gerhardt-2]

Update of bug #22110 (project lwip):

         Planned Release:                   1.3.1 => 1.4.0                  

    _______________________________________________________

Follow-up 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.

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?22110>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sebastian Gerhardt-2]

Follow-up Comment #8, bug #22110 (project lwip):

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().

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?22110>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel