buglwIP - A Lightweight TCP/IP stack - Bugs: bug #49684, lwip_netconn_do_writemore():...

 
 

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

bug #49684: lwip_netconn_do_writemore(): non-blocking ERR_MEM treated as failure

Submitted by:  Joel Cunningham <jcunningham>
Submitted on:  Wed 23 Nov 2016 04:10:00 PM UTC  
 
Category: sockets/netconnSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Joel Cunningham <jcunningham>
Open/Closed: ClosedPlanned Release: None
lwIP version: git head

Wed 23 Nov 2016 09:09:21 PM UTC, comment #3:

Simon,

I agree with you that using sent_tcp/poll_tcp to check if the socket is writable again after a memory allocation failure could lead to some false positives because snd_buf/snd_queuelen doesn't necessarily have anything to do with the free space in the memory pools.

For my particular use case, sent_tcp is triggering the socket as writable again and my TCP segments are composed for PBUF_RAM and TCP_SEG allocations. At least one of these segments is freed when sent_tcp is called

Also, as mentioned in the original description, this is the same approach that blocking sockets uses, so the limitation exists there as well.

Committed in 5030fa81a020280c467126568027608fa08e5136

Joel Cunningham <jcunningham>
Project MemberIn charge of this item.
Wed 23 Nov 2016 08:12:54 PM UTC, comment #2:

That is, I think with the patch, it's better than without.

Simon Goldschmidt <goldsimon>
Project Administrator
Wed 23 Nov 2016 07:31:29 PM UTC, comment #1:

The idea is good. I think we've had it before, but saw this problem:

ERR_MEM is a global problem (e.g. heap is empty). So you would have to mark all netconns in this state as writable again when there is memory to consume (if it was the heap, maybe it was "out of struct tcp_seg" in that pool, or out of PBUF_REF for the nocopy case).

It might be marked as writable with the next call to poll_tcp() though, maybe that's enough?

Keeping that aside, the patch should be good I think (i.e. it should handle snd_buf/snd_queuelen limitations correctly).

Simon Goldschmidt <goldsimon>
Project Administrator
Wed 23 Nov 2016 04:10:00 PM UTC, original submission:

The current logic in lwip_do_writemore() treats ERR_MEM for non-blocking sockets as an error that should be propagated to the application. When ERR_MEM comes out of the sockets layer, it turns into ENOMEM which most applications treat as fatal and close the socket rather than waiting in select for the socket to be writable again

We can instead treat the ERR_MEM case the same as ERR_WOULDBLOCK: mark the socket as non-writeable, and then writable upon resources opening up. The later two steps are already done in lwip_netconn_do_writemore() and sent_tcp/poll_tcp. The only piece left is to return ERR_WOULDBLOCK in this case.

Further, this approach is very similar to how ERR_MEM is handled for blocking sockets except that the application thread stays blocked until sent_tcp/poll_tcp indicates resources are available.

Attached is a patch that fixes returning ERR_WOULDBLOCK

Joel Cunningham <jcunningham>
Project MemberIn charge of this item.

 

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by jcunningham (Submitted the item)
  •  

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

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 23 Nov 2016 09:09:21 PM UTCjcunninghamStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Wed 23 Nov 2016 04:10:00 PM UTCjcunninghamAttached File-=>Added 0001-api_msg-treat-non-blocking-ERR_MEM-as-ERR_WOULDBLOCK.patch, #39057

    Back to the top


    Powered by Savane 3.1-cleanup1