buglwIP - A Lightweight TCP/IP stack - Bugs: bug #44805, sendmsg implementation to support...

 
 

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

bug #44805: sendmsg implementation to support scatter/gather IO

Submitter:  Joel Cunningham <jcunningham>
Submitted:  Fri 10 Apr 2015 09:20:17 PM UTC
   
 
Category:  sockets/netconn Severity:  3 - Normal
Item Group:  Feature Request Status:  Fixed
Privacy:  Public Assigned to:  jcunningham
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

Jump to the original submission

Wed 16 Sep 2015 09:22:33 PM UTC, comment #11: 

Submitted, thanks :)

Joel Cunningham <jcunningham>
Group Member
Wed 16 Sep 2015 08:13:35 PM UTC, comment #10: 

Why don't you go ahead and commit the patch?

Simon Goldschmidt <goldsimon>
Group administrator
Thu 10 Sep 2015 07:29:09 PM UTC, comment #9: 

I agree there is no disadvantage or introduced risk, sounds great :)

Joel Cunningham <jcunningham>
Group Member
Thu 10 Sep 2015 07:05:55 PM UTC, comment #8: 

ALthough  I can't really test this right now, why don't we just commit it? It's only extra functions, no change to existing code, so adding it couldn't harm, right?

I even think it wouldn't be necessary to #ifdef it out (so no need to change opt.h) as any smart linker should throw the function away if not used.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 09 Sep 2015 09:37:40 PM UTC, comment #7: 

Should this maybe be moved to a patch report from a bug report (/w feature request)?

I plan on maintaining the sendmsg patch.  Maybe it would be more useful to others as a patch report (maybe people look at the open patches to see if there are any extra features, but probably not at open bugs?)

Joel Cunningham <jcunningham>
Group Member
Thu 14 May 2015 06:18:55 PM UTC, comment #6: 

Updated patchset for lwip repo (0003-Sockets-add-sendmsg.patch)

Changes:

  • Built patch with LWIP_IPV6, identified incorrect cast of msg->msg_name to (struct sockaddr_in ) instead of (struct sockaddr ) when calling SOCKADDR_TO_IPADDR_PORT()


(file #34014)

Joel Cunningham <jcunningham>
Group Member
Thu 30 Apr 2015 06:25:39 PM UTC, comment #5: 

New set of patches (0002-).

Changes:

  • Added support for LWIP_NETIF_TX_SINGLE_PBUF
  • Added support for LWIP_CHECKSUM_ON_COPY
  • Added LWIP_SENDMSG compile option (defaults to off)
  • Rebased on top of tip of master


Let me know if anything else needs to be re-worked before the patches are accepted.

Thanks

(file #33846, file #33847)

Joel Cunningham <jcunningham>
Group Member
Tue 28 Apr 2015 01:46:42 PM UTC, comment #4: 

I did find some documentation in the function pbuf_alloc().  It mentions the pbuf should be copied with pbuf_take() if it's being queued.


I did not ifdef msghdr because open group specification lists sys/socket.h as where struct msghdr is defined.  struct iovec is defined in sys/uio.h.  I included a define for struct iovec for systems that don't have a sys/uio.h.  If there are systems that define struct msghdr in other headers, maybe an ifdef would be appropriate.

http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/uio.h.html
http://pubs.opengroup.org/onlinepubs/007908799/xns/syssocket.h.html

I can rebase the patchsets and add handling for LWIP_NETIF_TX_SINGLE_PBUF.  One final question I have is should we guard the sendmsg() implementation with an LWIP_SENDMSG flag to allow reduced code size for systems that don't need it?  I don't have a good picture on what features are considered optional vs not.



Joel Cunningham <jcunningham>
Group Member
Mon 27 Apr 2015 06:19:09 PM UTC, comment #3: 


> I based the UDP support on how sendto() is implemented.


Ok, good point. Checking back on the multiple discussions we had about zero copy, it seems like PBUF_REF is only zero-copy for non-DMA (i.e. non-delayed) transmission. When you have a netif that enqueues data for sending (like DMA-enabled interfaces would normally do), only PBUF_ROM allows true zero-copy. PBUF_REF has to be copied to temporary buffers to prevent the buffer being altered after netif->linkoutput returns.

I'm not too sure how wide this knowledge is spread around netif driver developers, though... It seems like there is room for improvement in documenting this.

Given that, the implementation looks correct.

BTW: why did you ifdef 'iovec' but not 'msghdr'?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 24 Apr 2015 09:07:49 PM UTC, comment #2: 

Simon,

Thanks for looking at the patch.  I based the UDP support on how sendto() is implemented.

For configurations where LWIP_NETIF_TX_SINGLE_PBUF is not enabled, sendto() skips the memory copy and uses a reference.  Specifically, it declares a struct netbuf on the stack called "buf" and then adds the data via reference with netbuf_ref().  Then it calls netbuf_send(), afterwards returning control to application.

Is the sendto implementation also incorrect, or did I overlook something?

Or is the issue that my patch doesn't work when LWIP_NETIF_TX_SINGLE_PBUF is enabled? I can add support for it and when enabled, flatten the vectors into a contiguous pbuf.

Joel Cunningham <jcunningham>
Group Member
Fri 24 Apr 2015 07:36:54 PM UTC, comment #1: 

I'm afraid the code won't work. The reason is that you removed memory copies (using netbuf_ref, not copying the data into the netbuf).

You simply can't expect all data being sent when netconn_send() returns. This is because you might have a DMA-capable netif that puts packets on a tx list in 'linkoutput' and because the application thread might have a higher priority than lwIP.

Summing it up, to implement zero-copy TX from sockets, you would have to let the application thread wait for a transmit-confirmation from the netif driver (e.g. the point where pbuf_free() is being called for the TX pbufs). If we had that, we could use sendto() and modify it to send the data by reference instead of copying it.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 10 Apr 2015 09:20:17 PM UTC, original submission:  

I've developed a sendmsg() addition to the sockets API.  It follows the Open Group specification to provide support for scatter/gather IO from applications.

http://pubs.opengroup.org/onlinepubs/009695399/functions/sendmsg.html

My use case was to reduce memory bandwidth utilization in an embedded system by removing memory copies.  The implementation does not support control messages as that is not used for scatter/gather IO

sendmsg helps with this use case because it allows applications to send vectored I/O data through LwIP without memory copying into a contiguous buffer.

I have compiled and tested the implementation against the master branch of lwip.  I have also included two tests in socket_examples.c that exercise the TCP and UDP cases.

I would like to work to get these patches submitted to master.

Thanks!






Joel Cunningham <jcunningham>
Group Member

 

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

Attached Files
file #34014:  0003-Sockets-add-sendmsg.patch added by jcunningham (7KiB - application/octet-stream)
file #33846:  0002-Sockets-add-sendmsg.patch added by jcunningham (8KiB - application/octet-stream)
file #33632:  0001-Sockets-add-sendmsg.patch added by jcunningham (6KiB - application/octet-stream)

 

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 jcunningham (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 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-09-16 jcunningham Open/ClosedOpen Closed
    2015-09-16 jcunningham StatusNone Fixed
    2015-09-10 goldsimon StatusPostponed None
        Assigned toNone jcunningham
    2015-08-19 goldsimon StatusNone Postponed
    2015-05-14 jcunningham Attached File- Added 0003-Sockets-add-sendmsg.patch, #34014
    2015-04-30 jcunningham Attached File- Added 0002-Sockets-add-sendmsg.patch, #33846
        Attached File- Added 0002-Added-sendmsg-tests-to-socket-examples.patch, #33847
    2015-04-27 goldsimon StatusInvalid None
    2015-04-24 goldsimon StatusNone Invalid
    2015-04-10 jcunningham Attached File- Added 0001-Sockets-add-sendmsg.patch, #33632
        Attached File- Added 0001-Added-sendmsg-tests-to-socket-examples.patch, #33633

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code