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

Implemented non-blocking pipe and socketpair #101

Merged
merged 1 commit into from
Sep 30, 2023
Merged

Implemented non-blocking pipe and socketpair #101

merged 1 commit into from
Sep 30, 2023

Conversation

nikarh
Copy link
Contributor

@nikarh nikarh commented Sep 26, 2023

Motivation:

In Rust ecosystem many libraries rely on a tokio async runtime, which internally requires nonblocking unix pipes (as a fallback for os'es without epoll or kqueue).

This PR contains:

  • Added socketpair call. This just creates a IPV4 socket pair yolo ignoring the domain passed as an argument since usually socketpair is used with AF_UNIX, and they are not supported
  • Added pipe2 call for nonblocking unix pipes. Depending on the options it delegates either to the original pipe or to sockerpair call. This allows having select, poll, read, write calls for nonblocking pipes without any additional code, because pipe in that case is just a pair of sockets. fcntl is not implemented, since I don't want to touch/break the original pipe implementation in case blocking pipes are needed.
  • Fixed read and write calls for pipes, previously they returned buffer length instead of actually read bytes length

Comment on lines +514 to +544
if (bind(listener, (struct sockaddr*)&server_addr, addr_len) == -1) {
close(listener);
return -1;
}

if (listen(listener, 1) == -1) {
close(listener);
return -1;
}

if (getsockname(listener, (struct sockaddr *)&server_addr, &addr_len) == -1) {
close(listener);
return -1;
}

if ((sockfds[1] = socket(AF_INET, type, protocol)) == -1) {
close(listener);
return -1;
}

if (connect(sockfds[1], (struct sockaddr*)&server_addr, addr_len) == -1) {
close(sockfds[1]);
close(listener);
return -1;
}

if ((sockfds[0] = accept(listener, (struct sockaddr*)&server_addr, &addr_len)) == -1) {
close(sockfds[1]);
close(listener);
return -1;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikarh i love this pr.
just one question, don't we need to set errno?

maybe almost errors already set by these base calls.
if you confirm it, i'll merge it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this impl delegates to other newlib methods (connect, accept, socket), they already set errno, so this shouldn't be a problem

@isage
Copy link
Contributor

isage commented Sep 29, 2023

You'll still ignore it, but I'll leave this note here anyway: async pipes will break (without proper error) if network is in adhoc mode.

@sleirsgoevy
Copy link

If this is only for Tokio, maybe it's better to implement kqueue/epoll instead?

bors added a commit to rust-lang/libc that referenced this pull request Sep 29, 2023
Added socketpair and pipe2 for Vita target

As an effort to port `tokio` to Vita, `socketpair` and `pipe2` calls were implemented in Vita newlib.

A PR to newlib with these methods while limitations/implementation details are discussed (vitasdk/newlib/pull/101), but until it is merged there is [vita-newlib-shims](https://crates.io/crates/vita-newlib-shims) crate with these methods implemented.

The implementation may change but methods will certainly remain, and the interface will remain POSIX.

`socketpair` will be needed for `rust-lang/rust` std, `rust-lang/socket2`, `rust-lang/mio`.
`pipe2` will be required for `mio` pipe-based Waker.
@d3m3vilurr
Copy link
Contributor

You'll still ignore it, but I'll leave this note here anyway: async pipes will break (without proper error) if network is in adhoc mode.

  • who is you? (nikarh or me?)
  • so does @sleirsgoevy 's mention(implement with epoll) be better option to prevent an issue of the adhoc mode?

@isage
Copy link
Contributor

isage commented Sep 29, 2023

  • who is you? (nikarh or me?)

You

  • so does @sleirsgoevy 's mention(implement with epoll) be better option to prevent an issue of the adhoc mode?

Obviously not. 1) You can't have tcp sockets in adhoc mode
2) select/poll is implemented through epoll

@d3m3vilurr
Copy link
Contributor

d3m3vilurr commented Sep 29, 2023

why do you think i ignore you. but nvm i dont want meanless conversation in here. if you want please send message in discord or email.


well, you right. our select and poll uses epoll and currently it's limitated with socket.

idk sceIoOpen with SCE_O_NOWAIT is callable and can it work with epoll. hm

@isage
Copy link
Contributor

isage commented Sep 29, 2023

idk sceIoOpen with SCE_O_NOWAIT is callable and can it work with epoll. hm

no. sceIoOpen also isn't relevant. MsgPipes can be async. It just requires additional work to multiplex them with sockets in poll/select

@d3m3vilurr
Copy link
Contributor

hey isage. do you have any idea or suggestions? or just share the side effects?

@isage
Copy link
Contributor

isage commented Sep 29, 2023

Yes. Use async MsgPipes

@d3m3vilurr
Copy link
Contributor

d3m3vilurr commented Sep 30, 2023

Yes. Use async MsgPipes

yep you make sense.

I'll add the FIXME tag of it.

@d3m3vilurr d3m3vilurr closed this Sep 30, 2023
@d3m3vilurr d3m3vilurr reopened this Sep 30, 2023
@d3m3vilurr d3m3vilurr merged commit c69045c into vitasdk:vita Sep 30, 2023
@zetanumbers zetanumbers deleted the nonblock-pipe branch September 30, 2023 12:05
@nikarh
Copy link
Contributor Author

nikarh commented Oct 1, 2023

The reason I did not go with MsgPipes is that they would require a clever reimplementation of epoll/select, and the only solution I can think of would be a spin loop doing syscalls to check pipe state for each pipe fds, and a netEpoll call that instantly returns for all net fds. And I don't know how justified syscalls in a spin loop are for pipes in terms of performance.

As for ad-hoc mode - I never tested it with ad-hoc, but at least opening a socket on loopback works with wifi disabled and in airplane mode.

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 this pull request may close these issues.

4 participants