Skip to content

Extra socket options: SO_ACCEPTCONN, TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL#830

Merged
sunfishcode merged 4 commits intobytecodealliance:mainfrom
badeend:extra-socket-options
Sep 19, 2023
Merged

Extra socket options: SO_ACCEPTCONN, TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL#830
sunfishcode merged 4 commits intobytecodealliance:mainfrom
badeend:extra-socket-options

Conversation

@badeend
Copy link
Copy Markdown
Member

@badeend badeend commented Sep 17, 2023

Add getters and setters for:

  • SO_ACCEPTCONN
  • TCP_KEEPCNT
  • TCP_KEEPIDLE
  • TCP_KEEPINTVL

Also, add getter for SO_REUSEADDR. (Setter already exists)

@badeend badeend force-pushed the extra-socket-options branch from ebf5682 to 687918b Compare September 18, 2023 18:14
Copy link
Copy Markdown
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks a lot for adding these!

@sunfishcode sunfishcode merged commit 4358096 into bytecodealliance:main Sep 19, 2023
@sunfishcode
Copy link
Copy Markdown
Member

@badeend We don't have illumos in CI, but I do occasionally test rustix on illumos, but with this PR, on illumos, sockopt::test_sockets_ipv4 (but not sockopt::test_sockets_ipv6) now fails with this error:

thread 'sockopt::test_sockopts_ipv4' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 22, kind: InvalidInput, message: "Invalid argument" }', tests/net/sockopt.rs:167:78

At a quick search, I found this illumos bug report which on the surface looks similar.

It's not formally necessary for wasi-sockets to support illumos, but it is worth considering whether there's something we should do there.

@sunfishcode
Copy link
Copy Markdown
Member

In case it's relevant, the illumos version is openindiana 2023.05.

@badeend
Copy link
Copy Markdown
Member Author

badeend commented Sep 19, 2023

That bug report and the source code seem to suggest that the order in which the options are set matters. I haven't been able to decipher the illumos source, but the bug report mentions that set_tcp_keepintvl before set_tcp_keepcnt should work (?)

I don't have an illumos install, so I can't test it easily but do these changes work for you?: badeend@771f027 If not, how about when set_tcp_keepintvl and set_tcp_keepcnt are swapped around?


It's not formally necessary for wasi-sockets to support illumos, but it is worth considering whether there's something we should do there.

Couple of options:

  1. Nothing. Let it fail (semi-arbitrarily) like any other application already running on illumos.
  2. Let the set_tcp_keepintvl and set_tcp_keepcnt WASI equivalents return not-supported on illumos
  3. (If the order is indeed the culprit) Make a custom provision in the WASI implementation for illumos that defers making the actual syscalls until just before connect (for example)

@sunfishcode
Copy link
Copy Markdown
Member

I did some experimenting, and it seems to be related to the integer value. A value of 60 works, but the test has 61 which fails.

I think option 1 may be fine here. This seems sufficiently minor that we don't need to make any changes at this time.

@sunfishcode
Copy link
Copy Markdown
Member

This is now released in rustix 0.38.14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants