search

smoltcp-rs / smoltcp

a smol tcp/ip stack
BSD Zero Clause License
3.63k stars 402 forks source link

UDP: Read and set local address on unbound sockets (IPV6_PKTINFO style functionality) #903

Closed chrysn closed 2 months ago

chrysn commented 5 months ago

When UDP sockets are created and bound to a port but no IP address (the POSIX equivalent being being bound to [::]:${PORT}), applications often need to know which local address the packet was sent to, so that responses can be sent from the same address. This is requirement for implementing CoAP and (AIU) also QUIC -- and part of the embedded-nal-async (where more extensive rationale is given).

I can put in work on smoltcp to make this possible, but being new to the project I'll need a bit of guidance as to where to do that. My naive approach would be to add a field localendpoint to PacketMetadata, possibly behind a pktinfoish flag. Odd thing is that this is really not phy-level information -- it'd just as well fit in UdpMetadata, but that is not #[non_exhaustive] (should it be?).

For actually making it functional on the incoming side, I think I'd just need to update socket::udp::Socket::process to propagate data that is available in the ip_repr down into the metadata. On the send side, this would be in its dispatch(), where we'd have to decide whether we just take the application's word that the source address is good, or whether we check if it really matches self.endpoint.addr / is a valid source address of cx.

Dirbaio commented 5 months ago

yes, UdpMetadata would be the correct place to add this.

chrysn commented 5 months ago

So, I add it to UdpMetadata in a PR and worry about non_exhaustive and semver breakage later? Should I still make it opt-in, or make it unconditional? (Any implementation of embedded-nal-async on it, such as the one I'd plan for embassy, would enable the flag anyway, and it's about a 128bit copy and per-message storage that's added, maybe even saving on get_source_address calls in the outbound direction.)

Dirbaio commented 5 months ago

don't worry about breakage, next release will likely be 0.12 (breaking bump) anyway.

I'm not sure about making it optional.. it's 16 extra bytes per packet, which is not much but also not insignificant. i'm sort of leaning towards not making it optional, because we already have enough feature flags :smiling_face_with_tear:

chrysn commented 2 months ago

I've updated the PR as you requested -- there is now a local_address on UdpMetadata, and it is not optional. (Precisely: It is an Option<...> because a sender may not know where to send from / rely on the stack to set that, but not build-time configurable).

(Oups, I was looking at the issue and not the PR that has advanced -- but still, the PR is https://github.com/smoltcp-rs/smoltcp/pull/904 is rather ready, so nonetheless nonetheless:)

We're still before the 0.12 release -- is this good to go in?