tokio icon indicating copy to clipboard operation
tokio copied to clipboard

net: expose more socket options on `TcpSocket`

Open hawkw opened this issue 4 years ago • 15 comments

Is your feature request related to a problem? Please describe. While updating Hyper to use Tokio 0.3 (hyperium/hyper#2317), it turned out that most of the socket options that Hyper needs to set are no longer available on TcpStream or on the new TcpSocket type. Instead, it was necessary to to open the socket using socket2 and use socket2 to set the socket options, resulting in some fairly unpleasant code for converting between socket2::Socket and TcpSocket, and then performing an asynchronous connect via TcpSocket. It would be nice if TcpSocket supported these socket options directly.

In particular, it's not great to have to manually convert a socket2 Socket to a Tokio TcpSocket via IntoRawFd/FromRawFd. It's relatively easy for users to mess this up in the following ways:

  • Using AsRawFd rather than IntoRawFd, failing to transfer ownership of the file descriptor, so that dropping the Socket closes the file descriptor
  • Forgetting to set O_NONBLOCK before converting to TcpSocket. If the socket isn't put in non-blocking mode, using it will block, potentially causing the runtime to silently hang.
  • Missing platform support for Unix or for Windows, depending on what platform the user builds on. This is an easy mistake to make when locally built docs don't show the traits for other operating systems (e.g. a user on MacOS or Linux might miss the existence of IntoRawSocket/FromRawSocket traits on Windows).

Also, it requires unsafe code in downstream crates that would be nice to abstract over.

It would be nice if all of these operations could be performed on TcpSocket without having to also use socket2. Or, at least, it would be nice if the conversion between socket2 and TcpSocket was less complex.

Describe the solution you'd like Ideally, TcpSocket would provide the ability to set at least the socket options required by Hyper:

  • TCP keepalive
  • send buffer size
  • receive buffer size

My understanding is that adding these options would require an upstream addition in mio, which I'm happy to make.

Describe alternatives you've considered

Alternatively, we could:

  • Leave things as they are, requiring users to use socket2 for setting any socket options other than SO_REUSEADDR. I discussed why this is not great above.

  • Rather than adding these additional options on TcpSocket, we could add a conversion between socket2::Socket and tokio::net::TcpSocket that abstracts over the complexities I discussed above. Adding a conversion would hide the unsafe code behind Tokio's API, ensure that the socket is put in non-blocking mode, and ensure that ownership of the file descriptor/Windows SOCKET is transferred.

    The downside of this is that it would require taking a socket2 dependency in Tokio, which may not be desirable for stability reasons. A dependency could be feature-flagged, though...

  • Add a From<T: IntoRawFd> or similar to TcpSocket that sets O_NONBLOCK etc. This is definitely a bad idea since there is no way to ensure that the file descriptor is actually a TCP socket. Let's not do that.

hawkw avatar Oct 31 '20 18:10 hawkw

It's worth noting that Mio's TcpSocket already exposes some additional socket options that Tokio doesn't expose:

We may want to expose these as well

hawkw avatar Oct 31 '20 18:10 hawkw

Opened tokio-rs/mio#1384 to expose buffer size, and tokio-rs/mio#1385 to expose the keepalive interval. We may also want to add SO_NODELAY...

hawkw avatar Oct 31 '20 22:10 hawkw

I think it makes total sense to add these methods to TcpSocket. :+1:

Darksonn avatar Nov 01 '20 15:11 Darksonn

I ran into a similar difficulty while updating tarssh to Tokio 0.3 - I was setting send/recv buffers on the TcpStream from an accept() call, but there seems no way to (sensibly) replicate this behaviour in 0.3?

Freaky avatar Nov 26 '20 22:11 Freaky

So how to set TCP keepalive for socket from accept()?

biluohc avatar Apr 23 '21 07:04 biluohc

If Tokio does not expose a method for doing it, you can do it with the socket2 crate using something like SockRef.

Darksonn avatar Apr 23 '21 08:04 Darksonn

Having used these methods from tokio 0.2 — namely set_keepalive — it would be great to have these in tokio 1.0 as well. For now, I'm using SockRef as @Darksonn mentioned (thanks @Darksonn).

thedodd avatar Jun 07 '21 16:06 thedodd

Is there any reasoning why available socket options are not exposed? This kind of scratches Tokio out from some application

esavier avatar Jun 08 '21 11:06 esavier

is there a better way to set keepalive? The way I'm doing it right now is inelegant. #4028

silence-coding avatar Aug 05 '21 11:08 silence-coding

yeah, i have no idea why setting options is not in the tokio api, but currently this is only option that i am aware of. Basically you have to create external socket structure and hopefully cast it to the tokio's version after setting the correct option.

esavier avatar Aug 05 '21 12:08 esavier

BIND_DEVICE, please!!!

wutianze avatar May 05 '22 07:05 wutianze

Two years have passed, any update on this issue please? Noticed that @hawkw 's commit is closed: net: expose keepalive configuration on TcpSocket But it seems no indirect solution provided, I mean, do we have to use non-safe methods?

howellzhu avatar Nov 08 '22 09:11 howellzhu

The socket2 crate can generally set any socket option safely. Other than that, please open a new issue if there is some specific option that you think Tokio should provide.

Darksonn avatar Nov 08 '22 09:11 Darksonn

@Darksonn thanks , following safe code can work:

use socket2::{SockRef, TcpKeepalive};
...
        info!("connecting to {addr}");
        let tcp = TcpStream::connect(&addr).await?;
        info!("tcp connected to {addr}");
        let ka = TcpKeepalive::new().with_time(std::time::Duration::from_secs(30));
        let sf = SockRef::from(&tcp);
        sf.set_tcp_keepalive(&ka)?;
...

howellzhu avatar Nov 08 '22 09:11 howellzhu

@howellzhu's code worked great for me!

msdrigg avatar Dec 15 '23 04:12 msdrigg