sys/net/sock: add sock_udp_sendv() API

[benpicco]

Contribution description

Both GNRC and LWIP support the notion of packet snips/slices: A packet does not have to be in continuous memory.
This is useful when e.g. sending a blockwise CoAP message where the CoAP header can be a small buffer and the CoAP block payload can be just a pointer into a larger buffer (possibly on ROM).

However, the sock API completely throws this out of the window by only allowing a single payload buffer in sock_udp_send().
This means we need another buffer to copy the payload and the header into, artificially doubling memory requirements and introducing an unnecessary copy step.

Fix this by introducing a new sock_udp_sendv() function. This allows to specify an array of payload chunks instead of a single buffer. sock_udp_send() is now simple a wrapper around the new function.

Implemented for

• GNRC
• LWIP
• OpenWSN

Testing procedure

Issues/PRs references

[maribu]

I do like the idea very much. One comment inline.

[kaspar030]

Why an array? There's reasons why gnrc and netdev use iolists instead of arrays. They're much more stack friendly, especially for layered protocols.
And, iolists are used at the lowest level. I don't think gnrc can do it, because it needs pktsnips, but theoretically an application could provide an iolist, and the network stack just has to prepend the udp header and pass it on. Maybe this can be added to gnrc's machinery, with a special "iolist" pktsnip type.
An array would need to be reallocated, potentially multiple times.

[benpicco]

We are on the sock layer here so we can't use network stack native list representations anyway.
I don't see how lists or arrays differ in stack usage (in fact the list will store an additional pointer, so 4 more bytes per element) but lists are more awkward to use:

• Both GNRC and LWIP expect the last element to be written first. This would either mean that the user has to supply the list in an 'unnatural' order, use double-linked lists or we reverse the list inside the network stack
• Array can be statically initialized (and be compile time const)

I agree that lists provide additional flexibility in case someone wants to add multiple layers on top of sock though.

[kaspar030]

I don't see how lists or arrays differ in stack usage (in fact the list will store an additional pointer, so 4 more bytes per element)

Think of an application writing some content using protocol X. This is a potential call stack using iolists:

proto_X_send(..., buf, len) {
  iolist_t entry = { .iol_data=buf, iol_len=len }

 proto_X_udp_send(..., &entry);
}

proto_X_udp_send(..., iolist_t entry) {
 buf, len = proto_X_create_hdr(...);
 iolist_t proto_x = {.iol_data = buf, .iol_len=len, iol_next=entry };
udp_sendv(..., &proto_x);
}

udp_sendv(..., &proto_x) {
buf, len = udp_build_hdr(...);
 iolist_t udp = {.iol_data = buf, .iol_len=len, iol_next=proto_x };
ip_send(..., &udp);
}

...

Using arrays, each layer would probably get an array as parameter. to add data, that array either needs to be in reverse order and large enough (who knows how large?), or, be recreated on each layer, but again, how big? variable length?.

IMO, lists save a lot here.

[benpicco]

How about this? I'm not entirely happy with having to reverse the list depending on which network stack is used, but then again this is neatly hidden from the user and the list based interface does indeed provide more flexibility to the application.

[kaspar030]

I still don't understand why the iolist has to be reversed. just to be clear, iolists are in-order, meaning, first entry (list head) contains bytes 0-x, second contains x+1 . ..., and so on, right?

That's how it is passed from the application, and that's how the good old netdev expects (or expected) it.

In the PR, you're reversing that for lwip and openwsn, but not gnrc? Even though lwip writes in-order using netcon_write_partly() and openwsn also copies in-order? does gnrc_pktbuf_add prepend?

It seems to me something is turned around. if gnrc_pktbuf_add prepends, we need something that appends. There's gnrc_pkt_append, that sounds like it could be used (but might need allocation of a pktsnip, even though the data could be just pointed to?

but, are we clear on the supposed order of the iolists?

(You really don't want to re-use iolist.h?)

[benpicco]

Sometimes one does not see the forest due to all the trees 😄
I've added gnrc_pktbuf_add_tail() and reworked the buffer chaining in lwip_sock_sendv() so now we can always use the buffer list in the right order.

I'll split up the commits and force push shortly.

[yarrick]

I am not familiar with the netconn/netbuf API, but it seems the TCP send can be simplified.

[benpicco]

Fine, I still maintain that the iolist_t API is fugly, but if you prefer it that way so be it.

[kaspar030]

Fine, I still maintain that the iolist_t API is fugly

you mean because of the member names, or because of the const?

[kaspar030]

From my perspective you can squash this!

[benpicco]

you mean because of the member names, or because of the const?

both really, e.g.

[kaspar030]

ACK.

Code looks fine. Did some basic UDP testing, all good.

[benpicco]

Thank you for the review!