Skip to content
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

Merge bind and listen? #29

Open
sunfishcode opened this issue Mar 16, 2023 · 8 comments
Open

Merge bind and listen? #29

sunfishcode opened this issue Mar 16, 2023 · 8 comments

Comments

@sunfishcode
Copy link
Member

I know this has been iterated on in other contexts, but

Implementing wasi-sockets on top of the Rust standard library, I have to bridge the gap that Rust's bind function does socket, bind, and listen all in one step. I have it implemented with a little state machine that collects up the socket and bind info and does everything at the point of the listen. But this does mean that if users pass port 0 to bind and then do getsockname afterwards to find out what port it picked, I can't implement it because I haven't actually created a socket at that point.

The answer might be that the Rust standard library is too high-level here and I need to use lower-level APIs, and I can do that. But I suspect Rust is not the only language that does something like this. If so, it's worth considering merging these calls.

@badeend
Copy link
Collaborator

badeend commented Mar 17, 2023

Yeah, I was about to suggest something similar in response to #28.

Preferably, our interface would be:

connect: func(tcp-socket, network, remote-address) -> ...
listen: func(tcp-socket, network, local-address) -> ...

and drop the separate bind step completely.

I guess the question then becomes: how much are we willing to stray from POSIX/BSD compatibility?
By merging bind and listen we basically assume that every call to bind will be followed by a listen call.
In terms of POSIX compatibility; libc's bind function would then map to our listen method, and libc's listen function would simply be a no-op.

This will probably work fine for the majority of cases. However, bind is perfectly valid for clients too. Mainly to force which local interface the connection will be made from.
Given how niche this usecase is, and that: without bind the interface will be simpler and work OK for 99+% of the cases, I think it's fine to drop the explicit bind for now and just wait to see if this issue ever pops up in the real world.


Aside: Regardless of this specific issue, I doubt the rust standard library is going to cut it anyway, as it doesn't provide any async methods nor access to IPV6_V6ONLY, SO_KEEPALIVE, IPV6_UNICAST_HOPS, SO_RCVBUF, SO_SNDBUF. socket2 looks promising.

@sunfishcode
Copy link
Member Author

A major consideration here is whether other major host APIs do something similar. I looked around a little and found that while Rust's standard library merges listen into bind, Java merges listen into accept, while Python, C#, Ruby, libuv all appear to follow POSIX in keeping bind, listen, and accept separate.

Consequently, I now think we should keep bind, listen, and accept separate, as wasi-sockets already does.

@badeend
Copy link
Collaborator

badeend commented Apr 10, 2023

I don't know if it affects your conclusion at all, but Java's ServerSocket implementation calls listen as part of bind, not accept.

@sunfishcode
Copy link
Member Author

Oh, interesting, thanks for finding that! That is worth thinking about then. I don't think we necessarily need to make a change right now, but is is worth thinking about.

@badeend
Copy link
Collaborator

badeend commented Apr 12, 2023

I'll reopen this as a reminder for myself to re-evaluate this in the future.

@badeend badeend reopened this Apr 12, 2023
@badeend
Copy link
Collaborator

badeend commented May 23, 2023

Another consideration when merging bind and listen at the WASI layer is: how to handle the backlog parameter passed to the listen wasi-libc call?

It seems that you'd have to give up on either: being able to specify the backlog size, or: being able to call getsockname after bind

Edit: because not every platform supports changing the backlog size after the initial listen call.

@badeend
Copy link
Collaborator

badeend commented Aug 9, 2023

Maybe we can solve this entirely in libc.

Suppose wasi-sockets provides only:

  • listen: func(tcp-socket, network, local-address)
  • connect: func(tcp-socket, network, remote-address, option<local-address>)
  • and no bind method.

Then, by default, libc's bind function will call WASI's listen method, and libc's listen function would simply be a no-op. Calling getsockname directly after bind then works as expected. And in general, most server & client applications would "just work", which is why I think this should be the default.

For the edge case where a client want to make the connection from a specific local address, we can invent a SO_DEFER_BIND socket option. This option delays binding the socket up to the first libc listen or connect call; Gaining the ability to specify a local address, but giving up on the ability to query getsockname in between bind and listen/connect.

libc pseudo code:

fn bind(sock, local_addr) {
	if (sock.SO_DEFER_BIND) {
		sock.local_addr = local_addr;
	} else {
		wasi_sockets::tcp::listen(sock, local_addr);
	}
}

fn getsockname(sock) {
	if (sock.SO_DEFER_BIND) {
		sock.local_addr;
	} else {
		wasi_sockets::tcp::local_address(sock);
	}
}

fn listen(sock, backlog) {
	wasi_sockets::tcp::set_backlog_size(sock, backlog);

	if (sock.SO_DEFER_BIND) {
		wasi_sockets::tcp::listen(sock, local_addr);
	} else {
		// Do nothing
	}
}

fn connect(sock, remote_addr) {
	wasi_sockets::tcp::connect(sock, remote_addr, sock.local_addr);
}

The downside is that existing client software may need to be modified to target WASI.

I think this is justified, because:

  • most client software doesn't need this,
  • the change is pretty trivial (#ifdef WASI \n setsockopt(sock, SOL_SOCKET, SO_DEFER_BIND, true); \n #endif)
  • it increases the likelihood that higher-level libraries & languages can implement wasi-sockets.
  • it simplifies the WASI interface and removes the current edge case where one can call bind and connect with different network handles.

@sunfishcode What do you think?

@sunfishcode
Copy link
Member Author

Then, by default, libc's bind function will call WASI's listen method, and libc's listen function would simply be a no-op.

This change sounds risky to me, as it changes the externally-visible behavior. listen is the point at which incoming connection requests start being accepted by the TCP protocol.

The change for client code making connections with custom local addresses is also concerning.

Both of those are relatively obscure, but I think overall, wasi-sockets should err on the side of preserving fundamental POSIX semantics.

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 a pull request may close this issue.

2 participants