net/sock: Add access to auxiliary data by maribu · Pull Request #14622 · RIOT-OS/RIOT

maribu · 2020-07-27T19:59:07Z

Contribution description

Add variants of sock_<PROTO>_send() and sock_<PROTO>_recv() that allow fetching auxiliary data. As a motivating example, the local address is provided to allow virtual hosting to be implemented on top of the sock API. Additionally, the API for timestamps as auxiliary data is implemented; but the corresponding feature is not yet implemented by any network stack.

These are the functions that were added for the UDP protocol:

ssize_t sock_udp_recv_aux(sock_udp_t *sock, void *data, size_t max_len,
                          uint32_t timeout, sock_udp_ep_t *remote,
                          sock_udp_aux_rx_t *aux);
ssize_t sock_udp_recv_buf_aux(sock_udp_t *sock, void **data, void **buf_ctx,
                              uint32_t timeout, sock_udp_ep_t *remote,
                              sock_udp_aux_rx_t *aux);
ssize_t sock_udp_send_aux(sock_udp_t *sock, const void *data, size_t len,
                          const sock_udp_ep_t *remote, sock_udp_aux_tx_t *aux);

The same was provided for IP and DTLS.

Testing procedure

make -C tests/gnrc_sock_udp flash test
make -C tests/gnrc_sock_ip flash test
LWIP_IPV4=1 LWIP_IPV6=1 make -C tests/lwip_sock_udp flash test
LWIP_IPV4=1 LWIP_IPV6=1 make -C tests/lwip_sock_ip flash test

Issues/PRs references

Supersedes: #14402

miri64 · 2020-07-28T10:02:52Z

What about sock_dtls and sock_tcp? Regarding sock_tcp, I'm not sure if you aim to have this only work for datagram-based protocols, but sock_dtls is definitely missing from this.

maribu · 2020-07-28T10:48:49Z

Regarding sock_tcp, I'm not sure if you aim to have this only work for datagram-based protocols

I think that this feature might be useful there, but the API proposed here is likely a poor match. (E.g. we don't expect the destination address to change after accepting the connection.)

but sock_dtls is definitely missing from this.

I agree that this should also be provided that. But I felt this might be a bit more involved there (e.g. should auxiliary data also be provided in the session initialization?).

But how about this: I extend this PR to adapt the DTLS socks in the same manner, and follow up PRs can extend APIs unique to DTLS (like session initialization) if there is a use case for it.

maribu · 2020-07-28T10:58:14Z

May I squash to fix the following commit message:

- net/sock_udp: Extend IP for auxiliary data
+ net/sock_udp: Extend API for auxiliary data

I tend to forget this, if I don't do such things right away.

miri64 · 2020-07-28T11:00:13Z

May I squash to fix the following commit message:
- net/sock_udp: Extend IP for auxiliary data
+ net/sock_udp: Extend API for auxiliary data
I tend to forget this, if I don't do such things right away.

Sure! I usually do git commit --allow-empty --squash=<commit in question> -e and add the new one to the body btw.

maribu · 2020-07-28T13:48:07Z

Now DTLS is also included. I ended up reusing the aux data types (but typedefed to a new name, to allow revising this decision). That allowed to reuse sock_udp_recv_aux() within sock_dtls_recv_aux() to provide the auxiliary data.

sys/include/net/sock/dtls.h

maribu · 2020-07-31T08:56:08Z

I split #14665 out of this, as this is not really related to the target of this PR.

Provide address the IP packet / UDP datagram was received on in the auxiliary data, if module sock_aux_local is used.

Provide address the IP packet / UDP datagram was received on in the auxiliary data, if module sock_aux_ip is used.

maribu · 2020-07-31T12:34:31Z

Rebased on master (without squashing)

maribu · 2020-08-04T07:10:46Z

@miri64 I could split this up into three PRs, one for IP, one for UDP, and one for DTLS. Maybe this would ease review?

Provide address the IP packet / UDP datagram was received on in the auxiliary data, if module sock_aux_local is used.

maribu · 2020-08-04T07:32:28Z

Hmm, the implementations are entangled. It might not be such a good idea to split those apart. E.g. both the added parameter to gnrc_sock() requires updating of both the UDP and IP sock API in GNRC anyway...

miri64 · 2020-08-04T21:10:40Z

Hmm, the implementations are entangled. It might not be such a good idea to split those apart. E.g. both the added parameter to gnrc_sock() requires updating of both the UDP and IP sock API in GNRC anyway...

What you could try is to leave the new functions unimplemented in one PR and then implement them per stack.

miri64 · 2020-08-04T21:18:15Z

makefiles/pseudomodules.inc.mk

 PSEUDOMODULES += slipdev_stdio
 PSEUDOMODULES += sock
 PSEUDOMODULES += sock_async
+PSEUDOMODULES += sock_aux%


I personally don't like using wildcards for pseudo-modules. It is basically just laziness and it confuses the build system mightily when there is an actual module that matches that wildcard as well. It is unlikely that this will happen for sock_aux% and this is not the first offender, so I won't block. However, I would prefer them written out.

Fixed. (But in #14703)

maribu · 2020-08-05T07:56:58Z

Split this PR as follows:

net/sock: Add access to auxiliary data (API only) #14703: API only
- net/gnrc/sock: Implement sock_aux_local #14704: GNRC implementation of sock_aux_local
  - pkg/tinydtls: Implement sock_aux_local #14706: tinydtls implementation of sock_aux_local
- pkg/lwip: Implement sock_aux_local #14705: lwIP implementation of sock_aux_local

(The above list is also the dependency tree.)

maribu · 2020-09-14T08:02:32Z

Sorry, confused this with #14703. This should remain closed in favor of #14703

maribu added the Area: network Area: Networking label Jul 27, 2020

maribu requested a review from miri64 July 27, 2020 19:59

maribu requested review from PeterKietzmann, cgundogan and haukepetersen as code owners July 27, 2020 19:59

maribu mentioned this pull request Jul 27, 2020

[WIP] sys/net/sock/udp: Provide access to local IP addr #14402

Closed

maribu force-pushed the sock-aux branch from 3968dc9 to 763c77b Compare July 28, 2020 12:39

maribu requested a review from leandrolanzieri as a code owner July 28, 2020 12:39

miri64 reviewed Jul 28, 2020

View reviewed changes

sys/include/net/sock/dtls.h Outdated Show resolved Hide resolved

sys/include/net/sock/dtls.h Outdated Show resolved Hide resolved

miri64 added Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Jul 28, 2020

maribu mentioned this pull request Jul 31, 2020

pkg/lwip: Use IS_ACTIVE() and bugfix #14665

Merged

maribu added the State: waiting for other PR State: The PR requires another PR to be merged first label Jul 31, 2020

maribu added 11 commits July 31, 2020 14:23

net/sock_ip: Extend API for auxiliary data

47c4bce

net/sock_udp: Extend API for auxiliary data

c5abe9e

sys/net/gnrc: Implement sock_aux_local

0daf706

Provide address the IP packet / UDP datagram was received on in the auxiliary data, if module sock_aux_local is used.

tests/gnrc_sock_udp: Added test for sock_aux_local

a85a727

tests/gnrc_sock_ip: Added test for sock_aux_local

ff1101b

pkg/lwip: Implement sock_aux_local

79768ea

Provide address the IP packet / UDP datagram was received on in the auxiliary data, if module sock_aux_ip is used.

tests/lwip_sock_udp: Added test for sock_aux_local

cecf633

tests/lwip_sock_ip: Added test for sock_aux_local

cd16045

net/sock_dtls: Extend API for auxiliary data

7b62f23

fixup! net/sock_ip: Extend API for auxiliary data

17b620e

fixup! net/sock_udp: Extend API for auxiliary data

28b1b7f

maribu added 3 commits July 31, 2020 14:29

pkg/tinydtls: Implement sock_aux_local

058c482

fixup! net/sock_dtls: Extend API for auxiliary data

35cc893

fixup! pkg/lwip: Implement sock_aux_local

865cb7c

maribu force-pushed the sock-aux branch from c03ce78 to 865cb7c Compare July 31, 2020 12:34

maribu removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jul 31, 2020

maribu mentioned this pull request Jul 31, 2020

pkg/lwip: Use IS_ACTIVE() and bugfix [backport 2020.07] #14671

Merged

maribu added 2 commits August 4, 2020 09:29

fixup! pkg/lwip: Implement sock_aux_local

e13b75e

squash! pkg/lwip: Implement sock_aux_local

7d4d5a8

Provide address the IP packet / UDP datagram was received on in the auxiliary data, if module sock_aux_local is used.

fjmolinas requested review from benpicco and bergzand August 4, 2020 08:19

miri64 reviewed Aug 4, 2020

View reviewed changes

maribu mentioned this pull request Aug 5, 2020

net/sock: Add access to auxiliary data (API only) #14703

Merged

maribu closed this Aug 5, 2020

maribu requested review from chrysn and kaspar030 September 14, 2020 08:01

maribu reopened this Sep 14, 2020

maribu closed this Sep 14, 2020

maribu deleted the sock-aux branch January 25, 2021 09:46

Conversation

maribu commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

miri64 commented Jul 28, 2020

Uh oh!

maribu commented Jul 28, 2020

Uh oh!

maribu commented Jul 28, 2020

Uh oh!

miri64 commented Jul 28, 2020

Uh oh!

maribu commented Jul 28, 2020

Uh oh!

Uh oh!

Uh oh!

maribu commented Jul 31, 2020

Uh oh!

maribu commented Jul 31, 2020

Uh oh!

maribu commented Aug 4, 2020

Uh oh!

maribu commented Aug 4, 2020

Uh oh!

miri64 commented Aug 4, 2020

Uh oh!

miri64 Aug 4, 2020

Choose a reason for hiding this comment

Uh oh!

maribu Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

maribu commented Aug 5, 2020

Uh oh!

maribu commented Sep 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maribu commented Jul 27, 2020 •

edited

Loading