-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support more socket options #53
Conversation
/// - `already-listening`: (set) The socket is already in the Listener state. | ||
/// - `concurrency-conflict`: (set) A `bind`, `connect` or `listen` operation is already in progress. (EALREADY) | ||
traffic-class: func(this: tcp-socket) -> result<u8, error-code> | ||
set-traffic-class: func(this: tcp-socket, value: u8) -> result<_, error-code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going to have this, we should define the meaning of the bits of the u8
value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in #58
/// # Typical errors | ||
/// - `concurrency-conflict`: (set) A `bind`, `connect` or `listen` operation is already in progress. (EALREADY) | ||
dont-fragment: func(this: udp-socket) -> result<u8, error-code> | ||
set-dont-fragment: func(this: udp-socket, value: u8) -> result<_, error-code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a bool
instead of a u8
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes :)
FIxed in #58
/// # Typical errors | ||
/// - `concurrency-conflict`: (set) A `bind`, `connect` or `listen` operation is already in progress. (EALREADY) | ||
traffic-class: func(this: udp-socket) -> result<u8, error-code> | ||
set-traffic-class: func(this: udp-socket, value: u8) -> result<_, error-code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above, we should define the meaning of the u8
bits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in #58
/// The value of the Hop Limit field of the IP packet. (Also known as "Time To Live (TTL)" in IPv4) | ||
/// | ||
/// Equivalent to the IP_TTL & IPV6_HOPLIMIT ancillary messages. | ||
hop-limit: option<u8>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This datagram
type is used for both sending and receiving. It seems like all of these additional fields only make sense for sending. Would it make sense to make these be parameters to the send
function, or perhaps to have an alternate send
function with more options?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're also applicable when receiving. See:
- IP_RECVTTL/IPV6_RECVHOPLIMIT
- IP_RECVTOS/IPV6_RECVTCLASS
- IP_RECVPKTINFO/IPV6_RECVPKTINFO
/// # Typical errors | ||
/// - `concurrency-conflict`: (set) A `bind`, `connect` or `listen` operation is already in progress. (EALREADY) | ||
no-push: func(this: tcp-socket) -> result<bool, error-code> | ||
set-no-push: func(this: tcp-socket, value: bool) -> result<_, error-code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we be confident that TCP_CORK
and TCP_NOPUSH
are the same? This blog post suggests there may still be some differences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we be confident that TCP_CORK and TCP_NOPUSH are the same?
Do we need to? I consider no-push
to be an optimization hint, telling the host "Hey, I'm about to make many consecutive Write calls. Try to optimize for network efficiency instead of latency and feel free to optimize the TCP frames accordingly". Both Linux' TCP_CORK and BSD's TCP_NOPUSH fit that description. Minor implementation differences are to be expected. For example, Windows doesn't support it at all, but can fall back to regular non-NODELAY behavior (or just error out). In the end, exactly the same bytes will be received on the other end, either way.
The blog post you referenced mentions a difference in what happens when the cork ends. Both implementations are equally "correct" in my mind. If an application wanted the uncork to also immediately send out the data without delay (Linux' behavior), then the application should have enabled no-delay
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind adding a section to the documentation to discuss issues like this, and mapping IP_DONTFRAG
to IP_MTU_DISCOVER
, and other portability concerns? The Notes in the table are good, but it'd be helpful to have more detail somewhere. My experience here as someone familiar with sockets but has never had to know what eg. TCP_CORK
does is that the implementation work shifts from "wiring things up" to "scouring through documentation and learning all about TCP protocol flags and kernel buffering policies and interactions with other obscure socket options to figure out if I need to do anything special to wire things up".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a couple of # Implementors note
sections in the new PRs
TCP Fast Open is still experimental. And, as discussed in the "Duplicate Data in SYNs" section, it seems difficult for applications to know whether they can accept the risks, if they don't have knowledge of the underlying network characteristics. I think we might consider adding Fast Open to wasi-sockets as an extension in the future, but to start with, I think it'd be best for wasi-sockets to start with a core set of standard functionality and to make that work well first, and once that's established, then look at adding extensions. |
| IPV6_RECVTCLASS | ✅ | ❌ | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | ❌ | ❌ | ❌ | ❌ | ❌ | ❌ | ✅ | ❌ | See [`udp::receive`](udp) | | ||
| IP_RECVPKTINFO | ✅ | ❌ | ✅ | ✅ | ✅ | ✅ | ❌ | ✅ | ❌ | ❌ | ❌ | ✅ | ✅ | ❌ | ✅ | ❌ | See [`udp::receive`](udp) <br/><br/> IP_PKTINFO on Linux & Windows, IP_RECVDSTADDR+IP_RECVIF on MacOS & FreeBSD. | | ||
| IPV6_RECVPKTINFO | ✅ | ❌ | ✅ | ✅ | ✅ | ✅ | ❌ | ✅ | ❌ | ❌ | ❌ | ✅ | ✅ | ❌ | ✅ | ❌ | See [`udp::receive`](udp) <br/><br/> IPV6_PKTINFO on Windows. | | ||
| IP_DONTFRAG | ✅ | ❌ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | ❌ | ✅ | ✅ | ❌ | ✅ | ❌ | [`udp::(set-)dont-fragment`](udp) <br/><br/> IP_DONTFRAGMENT on Windows, implementable using IP_MTU_DISCOVER on Linux. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see an IP_DONTFRAG
in Linux. It appears to only have IP64_DONTFRAG
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's implementable using IP_MTU_DISCOVER on Linux
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the Notes column wasn't visible on my screen. I have to specifically sideways-scroll on the table in order to see it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand. It's a bit of a beast :P
@sunfishcode Apologies for the churn in PR's, but I've split this one up again in:
I will get back to TCP_CORK & TCP_FASTOPEN at a later moment. |
To increase compatibility with existing software, this PR adds functionalities I consider simple enough to be copied almost verbatim from BSD-sockets into wasi-sockets.
Plain getter/setters for
Enable send-like behaviour
by making
datagram::remote-address
optional.Fixes #49
Send and receive ancillary data
Fixes #38
The following options are enabled by default; I doubt these 32-ish additional bytes are going to cause major performance issues.
TCP Fast Open
initial-data
parameter. When streams land, will be replaced by the stream input parameter.I intend to create followup PRs for:
After those, usage of the remaining socket options seems to quickly taper off.