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

clearing TcpStream's writable event #3873

Closed
zonyitoo opened this issue Jun 19, 2021 · 5 comments
Closed

clearing TcpStream's writable event #3873

zonyitoo opened this issue Jun 19, 2021 · 5 comments
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-net Module: tokio/net

Comments

@zonyitoo
Copy link
Contributor

zonyitoo commented Jun 19, 2021

Is your feature request related to a problem? Please describe.

I am trying to make TcpStream to support TCP Fast Open. On Linux and FreeBSD, TFO is enabled with:

  1. Enable TCP_FASTOPEN sockopt on the socket
  2. Call sendto with the first packet
  3. Call send as the normal TCP sockets

Since TcpStream::poll_write actually calls send, so I have to manually do:

  1. poll_write_ready
  2. sendto, and check EAGAIN or EINPROGRESS
  3. If errno == EINPROGRESS, I have to clear the write ready state and call poll_write_ready again

Describe the solution you'd like

  1. Provide a method clear_write_ready to clear the writable state of the internal io
  2. poll_write_ready returns a WriteReadyGuard just like AsyncFd

Describe alternatives you've considered

There shouldn't be an alternative way to make it possible.

Additional context

[pid  9423] sendto(15, "POST /api HTTP/1.1\r\nHost: 149.15"..., 266, MSG_FASTOPEN, {sa_family=AF_INET, sin_port=htons(80), sin_addr=inet_addr("...")}, 16) = -1 EINPROGRESS (Operation now in progress)

ref shadowsocks/shadowsocks-rust#555

@zonyitoo zonyitoo added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Jun 19, 2021
@Darksonn Darksonn added the M-net Module: tokio/net label Jun 19, 2021
@zonyitoo
Copy link
Contributor Author

Which solution you prefer? @Darksonn I could make a PR.

@zonyitoo
Copy link
Contributor Author

zonyitoo commented Jun 19, 2021

Or maybe, add try_io method like this:

pub fn try_io<R>(&self,
                 cx: &mut Context<'_>,
                 interest: Interest,
                 f: impl FnOnce() -> io::Result<R>) -> io::Result<()> {
    self.io
        .registration()
        .try_io(interest, f)
}

Or poll_write_io method like this:

pub fn poll_write_io<R>(
    &self,
    cx: &mut Context<'_>,
    f: impl FnMut() -> io::Result<R>,
) -> Poll<io::Result<R>> {
    self.io.registration().poll_write_io(cx, f)
}

@carllerche
Copy link
Member

Adding a method to clear readiness is tricky as it opens up the API to potential races. An event may be received after the last time readiness is observed and the call to clear readiness, resulting in a lost event. This is why AsyncFd returns a readiness guard and that is used to clear readiness (the guard prevents the race).

For this case, I am wondering if exposing try_io on TcpStream is the best option. It would most likely not need the cx as you can call writable().await before calling try_io.

Thoughts @Darksonn ?

@Darksonn
Copy link
Contributor

A try_io like API seems reasonable.

@zonyitoo
Copy link
Contributor Author

zonyitoo commented Jun 22, 2021

With try_io, I can convert EINPROGRESS to an io::Error with ErrorKind::WouldBlock, which should work as expected.

let mut connecting = false;
let result = stream.try_io(Interest::writable(), || {
    let ret = libc::sendto(sockfd, buf.as_ptr(), buf.len(), MSG_FASTOPEN, saddr.as_ptr(), saddr.len());
    if ret < 0 {
        let err = Error::last_os_error();
        if (err.kind() != ErrorKind::WouldBlock) {
            if let Some(libc::EINPROGRESS) = err.raw_os_error() {
                connecting = true;
                Err(ErrorKind::WouldBlock.into())
            } else {
                Err(err)
            }
        } else {
            Err(err)
        }
    } else {
        Ok(ret as usize)
    }
});

// .. Handling result

if connecting {
    ready!(stream.poll_write_ready());
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-net Module: tokio/net
Projects
None yet
Development

No branches or pull requests

3 participants