Fix udp socket

[hannesm]

No description provided.

[hannesm]

@mirage/core this came out of using stack-socket. unfortunately SO_REUSEPORT is only available from OCaml 4.12 onwards -- anyone has problems with dropping earlier OCaml releases in tcpip?

[djs55]

I'm personally happy with OCaml 4.12+

[djs55]

Is this to allow multiple Unix processes to bind to the same port, to share responsibility for handling the requests?

[hannesm]

This is to allow the same unix process to bind to the same port several times, i.e. in DNS:

• we listen on UDP Ipaddr_any, 53 (via a listen callback) -- calls bind(socket, *, 53)
• once a UDP frame is received, we take some time to find an answer, and eventually send from (, 53) to the client -- this means we have to bind as well to (, 53).

Without the REUSEPORT we can't bind to the same ip, port pair multiple times.

[dinosaure]

@mirage/core this came out of using stack-socket. unfortunately SO_REUSEPORT is only available from OCaml 4.12 onwards -- anyone has problems with dropping earlier OCaml releases in tcpip?

Hmmhmm, 4.12 is a bit higher for me. We should keep, at least, the support of the 3 last version of OCaml I think.

[hannesm]

4.12 is a bit higher for me

ok, so what are the options?

• leave tcpip broken as is (the socket stack can't receive on the same port as it wants to send on (via UDP))
• provide a C stub that allows to set SO_REUSEPORT
• rework this PR to be able to avoid SO_REUSEPORT (the commit 76b8578#diff-2df233b86746a505609a07125cd5a1ca625c8ddc4faba8ab44ddf9c2a0d9b578 introduced the create_socket asymmetry in Udpv4v6_socket) -- the create_socket exists since when sending UDP data we only want to use a single socket (and the get_udpv4v6_listening_fds may return two sockets (one for IPv4, one for IPv6 if both a static v4 and a v6 address is configured). Likely this can be refactored.

Eventually, the SO_REUSEPORT is not required if -- similar to how Udpv4_socket (and Udpv6_socket) re-use listening sockets for writing. Unfortunately the lifetime of a socket is then infinite (with file descriptors leakage) -- I guess the stack-socket needs some enhancements anyways (and there should be the counterpart to llsten_tcp/listen_udp that stops the listener).

3 last version of OCaml

From my memory, we agreed to the last 2 OCaml versions -- and 4.13 is upcoming soon. I also think that the tcpip stack is barely used by others than MirageOS unikernels. IMHO it is fine to bump to 4.12 (and 4.13) for this library.

So:

• this PR here fixes some issues if you want to use UDP and the socket stack, esp. with dual stacks
• there's more work needed to avoid leaking file descriptors
• it is not urgent since it seems like nobody really uses the socket stack and UDP (we use it for developing and testing a dns resolver, but we can work with a local pin for now)

[hannesm]

I opened #450 describing the leaks and potential fix(es). related to #446. I also moved the less controversial part to #451.

[dinosaure]

From my memory, we agreed to the last 2 OCaml versions -- and 4.13 is upcoming soon. I also think that the tcpip stack is barely used by others than MirageOS unikernels. IMHO it is fine to bump to 4.12 (and 4.13) for this library.

So if we agree with that, it should fine so 👍. I largely prefer a non-buggy stack.

[avsm]

A minimum of 4.12 is just fine by me for this package

…

On 15 Jul 2021, at 20:49, David Scott ***@***.***> wrote:  @djs55 approved this pull request. — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[hannesm]

we revised the implementation to address the file descriptor leaks (should solve #446 / #450).
also, we removed the REUSEPORT socket option (which was really only needed in udpv4v6_socket), and instead reuse the same socket for listen and write (by adding some more logic into get_udpv4v6_listening_fd).

TL;DR: the socket stack disconnect function now cleanup file descriptors. the tests suite should pass nicely, and we don't require OCaml 4.12. we tested this with the dns-resolver unikernel which reads from and writes to a lot of udp sockets.

any reviews welcome, I'm keen to merge and release a new version of tcpip without the leaks (plus the other changes in previous PRs).