wasmtime / PR #7120 wasi-sockets: Simplify & clarify TCP ... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #7120 wasi-sockets: Simplify & clarify TCP ...

Wasmtime GitHub notifications bot (Oct 01 2023 at 08:05):

badeend opened PR #7120 from badeend:tcp-error-checking to bytecodealliance:main:

Allow accept() to return transient errors.
The original provision was added to align with preview3 streams that may only fail once. However, after discussing with Dan Gohman, we came to the conclusion that a stream of result<> could do the trick fine too.
Fixes: https://github.com/WebAssembly/wasi-sockets/issues/22

Fold ephemeral-ports-exhausted into address-in-use. There is no cross-platform way to know the distinction between them.

Remove concurrency-conflict clutter,
and just document it to be always possible.

Simplify "not supported", "invalid argument" and "invalid state" error cases.
There is a myriad of reasons why an argument might be invalid or an operation might be not supported. But there is few cross platform consistency in which of those error cases result in which error codes. Many wasi-sockets codes were unnecessarily detailed and had no standardized equivalent in POSIX, so wasi-libc will probably just map them all back into a single EOPNOTSUPP or EINVAL or ...

Remove create-tcp/udp-socket not supported errors.
These stem from back when the entire wasi-sockets proposal was one big single thing. In this day and age, when an implementation doesn't want to support TCP and/or UDP, it can simply _not_ implement that interface, rather than returning an error at runtime.

Document that connect may return ECONNABORTED

Clarify connect failure state:
    POSIX mentions:
    > If connect() fails, the state of the socket is unspecified. Conforming applications should
    > close the file descriptor and create a new socket before attempting to reconnect.

    WASI prescribes the following behavior:
    - If `connect` fails because an input/state validation error, the socket should remain usable.
    - If a connection was actually attempted but failed, the socket should become unusable for further network communication.
Clarify local-address behavior on unbound socket:
POSIX mentions:
> If the socket has not been bound to a local name, the value
> stored in the object pointed to by `address` is unspecified.

WASI is stricter and requires `local-address` to return `not-bound` when the socket hasn't been bound yet.
Additionally

Document the set of socket options that are inherited through accept

Added Linux & Windows specific fixes.

Added integration tests

Move example_body out of lib.rs into tcp_sample_application.rs to make room for other tests.

Keep track of the address family ourselves. This is used in various places to provide cross-platform behavior.

Fix get/set_unicast_hop_limit. It didn't work for IPv4 sockets on Linux because Linux returns OPNOTSUPP instead of NOPROTOOPT.

Use Rustix::Errno to base all error code checks on.

Wasmtime GitHub notifications bot (Oct 01 2023 at 08:05):

badeend requested wasmtime-core-reviewers for a review on PR #7120.

Wasmtime GitHub notifications bot (Oct 01 2023 at 08:05):

badeend requested pchickey for a review on PR #7120.

Wasmtime GitHub notifications bot (Oct 01 2023 at 08:11):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 02 2023 at 18:24):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 02 2023 at 22:55):

pchickey requested alexcrichton for a review on PR #7120.

Wasmtime GitHub notifications bot (Oct 04 2023 at 16:04):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 04 2023 at 16:16):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 05 2023 at 07:41):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 05 2023 at 19:20):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 07 2023 at 09:36):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 08 2023 at 10:10):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 08 2023 at 10:11):

badeend edited PR #7120:

Allow accept() to return transient errors.
The original provision was added to align with preview3 streams that may only fail once. However, after discussing with Dan Gohman, we came to the conclusion that a stream of result<> could do the trick fine too.
Fixes: https://github.com/WebAssembly/wasi-sockets/issues/22

Fold ephemeral-ports-exhausted into address-in-use. There is no cross-platform way to know the distinction between them.

Remove concurrency-conflict clutter,
and just document it to be always possible.

Simplify "not supported", "invalid argument" and "invalid state" error cases.
There is a myriad of reasons why an argument might be invalid or an operation might be not supported. But there is few cross platform consistency in which of those error cases result in which error codes. Many wasi-sockets codes were unnecessarily detailed and had no standardized equivalent in POSIX, so wasi-libc will probably just map them all back into a single EOPNOTSUPP or EINVAL or ...

Remove create-tcp/udp-socket not supported errors.
These stem from back when the entire wasi-sockets proposal was one big single thing. In this day and age, when an implementation doesn't want to support TCP and/or UDP, it can simply _not_ implement that interface, rather than returning an error at runtime.

Document that connect may return ECONNABORTED

Clarify connect failure state:
    POSIX mentions:
    > If connect() fails, the state of the socket is unspecified. Conforming applications should
    > close the file descriptor and create a new socket before attempting to reconnect.

    WASI prescribes the following behavior:
    - If `connect` fails because an input/state validation error, the socket should remain usable.
    - If a connection was actually attempted but failed, the socket should become unusable for further network communication.
Clarify local-address behavior on unbound socket:
POSIX mentions:
> If the socket has not been bound to a local name, the value
> stored in the object pointed to by `address` is unspecified.

WASI is stricter and requires `local-address` to return `not-bound` when the socket hasn't been bound yet.
Additionally

Document the set of socket options that are inherited through accept

Added Linux & Windows specific fixes.

Added integration tests

Move example_body out of lib.rs into tcp_sample_application.rs to make room for other tests.

Keep track of the address family & IPV6_V6ONLY ourselves. This is used in various places to provide cross-platform behavior.

Fix get/set_unicast_hop_limit. It didn't work for IPv4 sockets on Linux because Linux returns OPNOTSUPP instead of NOPROTOOPT.

Use Rustix::Errno to base all error code checks on.

Wasmtime GitHub notifications bot (Oct 08 2023 at 10:13):

badeend edited PR #7120:

Allow accept() to return transient errors.
The original provision was added to align with preview3 streams that may only fail once. However, after discussing with Dan Gohman, we came to the conclusion that a stream of result<> could do the trick fine too.
Fixes: https://github.com/WebAssembly/wasi-sockets/issues/22

Fold ephemeral-ports-exhausted into address-in-use. There is no cross-platform way to know the distinction between them.

Remove concurrency-conflict clutter,
and just document it to be always possible.

Simplify "not supported", "invalid argument" and "invalid state" error cases.
There is a myriad of reasons why an argument might be invalid or an operation might be not supported. But there is few cross platform consistency in which of those error cases result in which error codes. Many wasi-sockets codes were unnecessarily detailed and had no standardized equivalent in POSIX, so wasi-libc will probably just map them all back into a single EOPNOTSUPP or EINVAL or ...

Remove create-tcp/udp-socket not supported errors.
These stem from back when the entire wasi-sockets proposal was one big single thing. In this day and age, when an implementation doesn't want to support TCP and/or UDP, it can simply _not_ implement that interface, rather than returning an error at runtime.

Document that connect may return ECONNABORTED

Clarify connect failure state:
    POSIX mentions:
    > If connect() fails, the state of the socket is unspecified. Conforming applications should
    > close the file descriptor and create a new socket before attempting to reconnect.

    WASI prescribes the following behavior:
    - If `connect` fails because an input/state validation error, the socket should remain usable.
    - If a connection was actually attempted but failed, the socket should become unusable for further network communication.
Clarify local-address behavior on unbound socket:
POSIX mentions:
> If the socket has not been bound to a local name, the value
> stored in the object pointed to by `address` is unspecified.

WASI is stricter and requires `local-address` to return `not-bound` when the socket hasn't been bound yet.
Additionally

Document the set of socket options that are inherited through accept

Added Linux, MacOS & Windows specific fixes.

Added integration tests

Move example_body out of lib.rs into tcp_sample_application.rs to make room for other tests.

Keep track of the address family & IPV6_V6ONLY ourselves. This is used in various places to provide cross-platform behavior.

Fix get/set_unicast_hop_limit. It didn't work for IPv4 sockets on Linux because Linux returns OPNOTSUPP instead of NOPROTOOPT.

Use Rustix::Errno to base all error code checks on.

Wasmtime GitHub notifications bot (Oct 08 2023 at 10:50):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 08 2023 at 12:41):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 08 2023 at 13:15):

badeend updated PR #7120.

Wasmtime GitHub notifications bot (Oct 09 2023 at 03:22):

alexcrichton submitted PR review:

Thanks!

Wasmtime GitHub notifications bot (Oct 09 2023 at 04:38):

alexcrichton merged PR #7120.

Last updated: Nov 22 2024 at 16:03 UTC