-
Notifications
You must be signed in to change notification settings - Fork 2.1k
net/sock: Add access to auxiliary data #14622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
What about |
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.)
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. |
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 |
Now DTLS is also included. I ended up reusing the aux data types (but |
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.
Rebased on master (without squashing) |
@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.
Hmm, the implementations are entangled. It might not be such a good idea to split those apart. E.g. both the added parameter to |
What you could try is to leave the new functions unimplemented in one PR and then implement them per stack. |
@@ -102,6 +102,7 @@ PSEUDOMODULES += shell_hooks | |||
PSEUDOMODULES += slipdev_stdio | |||
PSEUDOMODULES += sock | |||
PSEUDOMODULES += sock_async | |||
PSEUDOMODULES += sock_aux% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. (But in #14703)
Split this PR as follows:
(The above list is also the dependency tree.) |
Contribution description
Add variants of
sock_<PROTO>_send()
andsock_<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:
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