add Eio_unix.Sockopt for setting/getting socket options

[avsm]

There are some socket options that are Linux-specific which are quite handy to have. This adds a Eio_unix.Sockopt module which works on Eio_unix.Fd.t values.

This is a draft PR to check on the interface: the current PR replaces the need for Unix.setsockopt and has a single simple GADT for the socket options. Alternatively we could just expose the extended ones in the same style as upstream OCaml as a separate set of types, too. Opinions welcome.

[talex5]

Seems like socket options could be useful on non-Unix systems too. We could define the type in Eio.Net as an extensible variant and put the common ones there, adding Unix specific ones in Eio_unix.Net, etc. Would have to decide what to do about unsupported options.

[avsm]

Seems like socket options could be useful on non-Unix systems too. We could define the type in Eio.Net as an extensible variant and put the common ones there, adding Unix specific ones in Eio_unix.Net, etc. Would have to decide what to do about unsupported options.

SGTM. Right now, if a given sockopt isn't defined, it becomes (-1) and a call to it raises EINVAL (similar behaviour to the OCaml Unix module).

I'll move the common definitions that work on Windows as well to Eio.Net, and then put the remaining ones in Eio_unix.

[talex5]

In #322, @create2000 is asking about helping to finish this. Are we happy with the current design? What needs to happen beyond rebasing it?

It looks to me like some of the options (those not specific to Unix) should move to Eio.Net (e.g. TCP_NODELAY would also make sense in a unikernel). Possibly all the options should move there; it's OK to talk about an option, even if it's not supported.

Possibly eio_unix should define e.g. type _ t += Unix_bool of Unix.socket_bool_option : bool t, so all existing OCaml options can be used.

Might need a way to pass a list of options when connecting or binding (since we do that in one step, and maybe it's too late to set some options by then?). Might want some fancy heterogeneous list to hold them, since they have various types.

@create2000 I don't know if it's a good first issue or not. It uses some fancy types and involves C code. On the other hand, it's mostly done!

[patricoferris]

Re:design -- my impression is that we should definitely move parts to Eio.Net that are portable, and in this instance perhaps all of the options and add the Unix version to allow for all existing OCaml options to just work. I think we will want to be able to get/set after connecting/binding anyway so this seems like a reasonable chunk of work to have in a single PR (basically what is already here, shuffled around a little)?

Perhaps a follow up PR can work to support options being set at the point of connecting/binding (as you mentioned, this seems very related to #713)?

[avsm]

I've pushed a rebase with all the suggestions here. There are loads of Linux and Windows specific ones to add, but I wanted to check if this is version is heading in the right direction first @talex5 @patricoferris

[talex5]

This approach looks good to me.

[avsm]

I've added a bunch of Linux specific calls now (skipping multicast for now, and some of the more exotic fast connect and tcp_info ones), and also dropped SO_ACCEPTCON as it's not portable to macOS with no reasonable alternative (have to do a listen and wait for an EINVAL). It can still be passed directly via the Unix passthrough, but it's not much use in Eio where you know the state of your sockets anyway.

[avsm]

Tests should be fixed now as I've pushed a fix to ignore ENOPROTOOPT errors for IP_LOCAL IP sockopts that only work on Linux >6.3

[avsm]

Squashed commits, should be ready for merge.

[talex5]

(only reviewed the mock bit so far)

[avsm]

Rebased to trunk, and some minor fixes also added while re-reviewing it.

[avsm]

@talex5 @patricoferris I'm happy with the low-level implementation here, but I'm not sure if the Pi integration is right. We've got a new SOCKET module type now, and I think it won't be too onerous to mock, but I'm not sure...

[avsm]

Need to ensure this is compatible with whatever approach we pick for #713 as well

[talex5]

I've merged some of this separately as #858 and #859 and rebased the remaining bit (Linux options) on them.

I then added another commit with some suggested changes:

• Move all options to Eio.Net.Sockopt so you can try them, even if the backend might not be Linux.
• Simplify code and tests a bit.

[avsm]

Nothing good comes from returning a truncated string; the caller wont be able to use it meaningully. Instead of strnlen, why not check len after the getsockopt and raise an exception if it's the same as sizeof(buffer)?

[talex5]

Yes, could do. Not sure what exception to raise, though.

[avsm]

Invalid_argument is reasonable here. It's a rare violation that indicates something's quite wrong, and not something expected to be caught and handled.

[talex5]

OK, done!