Limit size of sctp_event_subscribe on Linux

[attah]

Use only the size which contains the last used option. This will help with compatibility since some vendor kernels have backported SCTP options turning simple backwards compatibility into breaking forward compatibility even for relatively similar versions. The Linux kernel is robust against using arbitrary sized structs.

=== Commit message ends ===

NB: Still slightly WIP - i do want to add inline comments, but i think it is better i do that after initial discussion so they are on the correct level.
Only the inet code is tested as of writing this.

=== Extra info ===

The problem

The struct for the SCTP_EVENTS socket option (sctp_event_subscribe) breaks ABI/API and is only backwards compatible.

We are making a single OTP build that is used across a few different Linux versions.
Yes, that's not the best way to do things - but given the low amount of dependencies and that we have taken some care to select build host, it has been working fine for our purposes for many many years.

Up until now, that is.

It turns out some vendor kernels have backported options all the way from 5.5 to 4.18, causing what should otherwise have been a simple minor backward compatibility situation to instead be a forward compatibility problem (which breaks, as it should).

This option, SCTP_EVENTS, is known to have this issue and has been deprecated.
However, it seems to early to stop using it (see Alternative solutions).

Proposed solution

Use a smaller struct size

Linux, though late on the suggested proper solution (SCTP_EVENT), has very robust handling of the sctp_event_subscribe struct which is pretty much as good and supports arbitrary sizes as long as the fields are in the correct order (and they are).
The only thing that will return an error is if we would pass it a larger struct than what it knows what to do with when setting options. Doing that could indicate the user had set options which the kernel did not accept.

Therefore i propose to clamp the struct size used to include only the highest used/usable option.
The socket api exposes options 9 and 10, beyond the 8 that inet does, behind individual config-tested ifdefs.
This makes the proposed code in inet and socket slightly different to keep in line with the respective module.

(Sidenote: Currently the socket API is limited to only the events defined in the RFC. This is good practice; so if/when further options are needed, add the SCTP_EVENT option for individual options instead).
(Sidenote 2: Should trying to set option 9 or 10 when not supported not return an error?)

Unfortunately; this will have to be a Linux-only solution.
FreeBSD and IllumOS both have no recent updates to this struct - but also basically no usable backward compatibility mechnisms.
FreeBSD will return EINVAL if the provided size is smaller than the kernel's definition of the struct, both for reading and writing, i.e. silently discard too-new options(!).
IllumOS does not check the allocated size whatsoever and just casts to the Kernel's definition of the struct, both potentially segfaulting and ignoring options with non-matching definitions.

It is fair to assume FreeBSD and IllumOS will adopt some sort of robustness features if they ever add more fields, but what they will look like is anyone's guess.

Alternative solutions

SCTP_EVENT (singular) socket options

This would allow us to set individual options and not care about struct size.
OTP could even own a RFC-like struct definiton (which it basically does already) and translate to individual SCTP_EVENT calls.
This was added in FreeBSD 9 (2012), Linux 5.0 (2019), but does not exost in IllumOS.
Not sure what te status of IllumOS support actually is, but dropping support for Linux 4.X seems a little too drastic.
I.e. i agree with the decision to keep using the deprecated SCTP_EVNTS (plural) socket option for a while longer.
(Especially since the Linux implementation is so good).

History of the options struct

struct sctp_event_subscribe {
  uint8_t sctp_data_io_event;
  uint8_t sctp_association_event;
  uint8_t sctp_address_event;
  uint8_t sctp_send_failure_event;
  uint8_t sctp_peer_error_event;
  uint8_t sctp_shutdown_event;
  uint8_t sctp_partial_delivery_event;
  uint8_t sctp_adaptation_layer_event;
  uint8_t sctp_authentication_event;
  uint8_t sctp_sender_dry_event;
};

The ABI up until including sctp_sender_dry_event is in RFC 6458.
No idea where the rest comes from - but at lest Linux and FreeBSD agree on what the 11th event is.
IllumOS, then Solaris, development seems to have stopped at the first published revision of the struct.

RFC 6458 (and preceeding IETF draft)

• 10 options, since version 18 (2008)
• 9 since version 11 (2005)
• 8 at first definition version 3 (2002)

https://datatracker.ietf.org/doc/html/rfc6458

Linux

• 14 options since version 5.5
• 13 since version 4.12
• 11 since version 4.11,
• 10 since version 3.0
• ...and then we are into 2.6 prehistorics.

https://github.com/torvalds/linux/blame/6548d364a3e850326831799d7e3ea2d7bb97ba08/include/uapi/linux/sctp.h#L611

FreeBSD

• 11 options, last addition in version 9 (2010).

https://github.com/freebsd/freebsd-src/blame/131dc2b7ad1b147b3b2775090f9eb55a7c2112ba/sys/netinet/sctp_uio.h#L61

IllumOS

• 8 options since OpenSolaris launched (typo fix in 2007)

https://github.com/illumos/illumos-gate/blame/7f3d7c9289dee6488b3cd2848a68c0b8580d750c/usr/src/uts/common/netinet/sctp.h#L159

[github-actions]

CT Test Results

    3 files    142 suites   50m 19s ⏱️
1 651 tests 1 594 ✅ 57 💤 0 ❌
2 374 runs  2 297 ✅ 77 💤 0 ❌

Results for commit 00bda45.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[attah]

Brief update on testing the socket API: i'm failing.
On the same machines where i successfully strace:d my changes to inet having the desired effect, the socket API gives me this:

1> {ok, Sock} = socket:open(inet, seqpacket, sctp).
{ok,{'$socket',#Ref<0.3959534975.147980289.3318>}}
2> socket:setopt(Sock, sctp, events, #{}).
{error,{invalid,{socket_option,{sctp,events},#{}}}}
3> Events = #{data_in          => true,
                             association      => true,
                             address          => true,
                             send_failure     => true,
                             peer_error       => true,
                             shutdown         => true,
                             partial_delivery => true,
                             adaptation_layer => true,
                             authentication   => true,
                             sender_dry       => true}.
#{shutdown => true,adaptation_layer => true,address => true,
  association => true,authentication => true,
  partial_delivery => true,peer_error => true,
  sender_dry => true,send_failure => true,data_in => true}
4> socket:setopt(Sock, sctp, events, Events).
{error,{invalid,{socket_option,{sctp,events},
                               #{shutdown => true,adaptation_layer => true,address => true,
                                 association => true,authentication => true,
                                 partial_delivery => true,peer_error => true,
                                 sender_dry => true,send_failure => true,data_in => true}}}}

Usage above is inspired by socket_server:do_manager_init().
(And it fails the same without my changes).

[bmk]

It looks like you could add a "#if defined(linux) ... #endif" around all the last_opt assignments.
Unless you are counting on the compiler optimizing those away on non-linux?

[attah]

Yes, indeed i could.
I thought that would be too messy and since this isn't a hot code path at all, i did not consider it for manual optimization.
If that's how you would like it to look though, i'll gladly add that.

[bmk]

Please do.

[bmk]

Note that the sctp support is currently limited.
I am working on a branch that is intended to bring 'socket' up to par with 'inet'.