tasklwIP - A Lightweight TCP/IP stack - Tasks: task #14395, recv of short buffer should...

 
 

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

task #14395: recv of short buffer should eventually time out

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Thu 02 Mar 2017 12:55:10 PM UTC
   
 
Category:  socket/netconn Should Start On:  Thu 02 Mar 2017 12:00:00 AM UTC
Should be Finished on:  Thu 02 Mar 2017 12:00:00 AM UTC Priority:  3 - Low
Status:  Cancelled Privacy:  Public
Assigned to:  goldsimon Percent Complete:  0%
Open/Closed:  Closed Planned Release:  None
Effort:  0.00

Jump to the original submission

Thu 02 Mar 2017 09:27:49 PM UTC, comment #8: 

Hmm, correct. Thanks for pointing that out :-)
I hate that rcvevnt thing :-/ Sorry for the confusion.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 02 Mar 2017 09:16:25 PM UTC, comment #7: 


if (NETCONNTYPE_GROUP(netconn_type(sock->conn)) == NETCONN_TCP) {
      LWIP_ASSERT("invalid copylen, len would underflow", len >= copylen);
      len -= copylen;
      if ((len <= 0) ||
          (p->flags & PBUF_FLAG_PUSH) ||
          (sock->rcvevent <= 0) ||
          ((flags & MSG_PEEK) != 0)) {
        done = 1;
      }
    } else {
      done = 1;
    }


Just looking at this code again (in lwip_recvfrom), but won't the check against sock->rcvevent cause the done state to be set, which then exits the loop preventing blocking after the recvmbox has been drained?

Joel Cunningham <jcunningham>
Group Member
Thu 02 Mar 2017 07:02:29 PM UTC, comment #6: 

I think that ignoring PSH and returning any data instead of waiting is a far better solution than the winsock waiting thing.

I'll change it to that. However, MSG_WAITALL seems like a good idea (since it's standard and we even support the non-standard MSG_DONTWAIT - we don't support OOB though...)

However, that will be after moving nonblocking to the netconn layer (task #14396) - a thing that has disturbed me often but I never took the time to change...

Simon Goldschmidt <goldsimon>
Group administrator
Thu 02 Mar 2017 03:44:36 PM UTC, comment #5: 

I've done some auditing of this behavior in LwIP and wanted to share a couple of things:

My port has LWIP_SO_RCVTIMEO enabled and for sockets that don't set SO_RCVTIMEO, it uses 0 for a timeout, thus providing the behavior of returning whatever is in the conn->recvmbox and not blocking.  The push flag doesn't matter in this case either, meaning we don't wait if PSH is not set.  If I had SO_RCVTIMEO set to a non-zero value, I think it block waiting to satisfy the full amount

My understanding of BSD semantics is that a recv() call always returns if there is data available without blocking.  SO_RCVTIMEO controls blocking when no data was available and returns EWOULDBLOCK when timing out

Man page for Linux recv summarizes:

If no messages are available at the socket, the receive calls wait for a message to arrive, unless the socket is nonblocking (see fcntl(2)), in which case the value -1 is returned and the external variable errno is set to EAGAIN or EWOULDBLOCK. The receive calls normally return any data available, up to the requested amount, rather than waiting for receipt of the full amount requested.

https://linux.die.net/man/2/recv

Also, in Steven's TCP/IP Illustrated Vol. 1: The Protocols, it has this to say about receiver side handling of PSH:

"Berkeley-derived implementations ignore a received PUSH flag because they normally never delay the delivery of received data to the application"

So in summary, I see the BSD behavior if LWIP_SO_RCVTIMEO is enabled and application never use SO_RCVTIMEO, otherwise it has non-standard behavior

Joel Cunningham <jcunningham>
Group Member
Thu 02 Mar 2017 01:50:38 PM UTC, comment #4: 

I would not bother with SO_SNDLOWAT/SO_RCVLOWAT by the way. In hindsight they were not worth the trouble for me. I added them just because they're is part of the traditional BSD set of socket options, but semantics differ between platforms and few if any applications actually use them. MSG_WAITALL is nice in theory but not (yet) portable and as such probably not used much either.

David van Moolenbroek <dcvmoole>
Thu 02 Mar 2017 01:46:27 PM UTC, comment #3: 
David van Moolenbroek <dcvmoole>
Thu 02 Mar 2017 01:45:24 PM UTC, comment #2: 


> http://smallvoid.com/article/winnt-tcp-push-flag.htm


--> page not found

MSG_WAITALL, SO_RCVLOWAT: TODO :-)

Thanks for the rest of the input. I also think the windows behaviour is strange...

Simon Goldschmidt <goldsimon>
Group administrator
Thu 02 Mar 2017 01:28:31 PM UTC, comment #1: 

FWIW! After looking into this I decided to disregard the PSH flag in my implementation altogether, instead always returning only whatever is available when anything is available at all (modulo MSG_WAITALL, SO_RCVLOWAT etc). My PoV is that while not considering the PSH flag may cause performance issues, not returning available data may cause correctness issues. As far as I can tell, the BSDs and Linux also follow the same behavior (ie no timeout), but don't quote me on that! On Windows it appears there's a 0.5sec timeout which, as can be expected, causes problems in some scenarios: http://smallvoid.com/article/winnt-tcp-push-flag.htm

David van Moolenbroek <dcvmoole>
Thu 02 Mar 2017 12:55:10 PM UTC, original submission:  

When recv(1024) is called but we only have 1022 bytes, we currently wait forever (unless the PSH bit is set or the netconn is nonblocking/has a timeout set).

It seems like other platforms time out in this case and return 1022.

--> check how linux/windows/xBSD behave?

Simon Goldschmidt <goldsimon>
Group administrator

 

(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 jcunningham (Posted a comment)
  • -email is unavailable- added by dcvmoole (Posted a comment)
  • -email is unavailable- added by goldsimon (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-03-02 goldsimon StatusPostponed Cancelled
        Open/ClosedOpen Closed
    2017-03-02 goldsimon StatusNone Postponed
        Assigned toNone goldsimon

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code