Skip to content

Comments

feat(!): make socket creation, bind, listen async#377

Open
Lzzzzzt wants to merge 5 commits intobytedance:masterfrom
Lzzzzzt:uring-socket
Open

feat(!): make socket creation, bind, listen async#377
Lzzzzzt wants to merge 5 commits intobytedance:masterfrom
Lzzzzzt:uring-socket

Conversation

@Lzzzzzt
Copy link
Collaborator

@Lzzzzzt Lzzzzzt commented Nov 13, 2025

This PR is mainly to make socket, bind, listen using io uring.

To be noted, I choose to let bind and listen be features because using these needs your kernel version >= 6.11.

So, this PR finishes the following tasks:

  • add socket, bind, listen op
  • change all the related function to async function
  • replace the original sync syscall function to io uring op
  • disable ci on armv7 and s390x, because the crate io-uring doesn't provide the prebuild sys.rs for these arch.
  • other small fix

complete #348

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR makes socket creation, binding, and listening asynchronous using io_uring operations. The bind and listen operations are feature-gated as they require kernel version >= 6.11. This is a breaking change that converts previously synchronous bind() calls to async, requiring .await at all call sites.

Key Changes:

  • Added async socket creation operation using io_uring
  • Made bind() and listen() async with feature flags for kernel 6.11+ support
  • Updated all test files to use .await with bind operations
  • Added fallback behavior for legacy/poll-io features

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
monoio/tests/*.rs Updated all test bind calls to async with .await
monoio/src/net/tcp/listener.rs Made bind() and bind_with_config() async
monoio/src/net/tcp/stream.rs Updated socket creation to use async operation
monoio/src/net/udp.rs Made bind() async with fallback for unsupported kernels
monoio/src/net/unix/listener.rs Made Unix listener bind async with feature-gated bind/listen ops
monoio/src/net/unix/seq_packet/listener.rs Made seq packet listener bind async
monoio/src/net/unix/stream.rs Updated socket creation to async
monoio/src/net/unix/seq_packet/mod.rs Updated socket creation to async
monoio/src/net/unix/datagram/mod.rs Updated socket creation to async
monoio/src/net/mod.rs Converted new_socket() to async operation
monoio/src/driver/op/socket.rs New socket operation implementation
monoio/src/driver/op/bind.rs New bind operation implementation
monoio/src/driver/op/listen.rs New listen operation implementation
monoio/src/driver/op/recv.rs Fixed type cast issue (contains bug)
monoio/Cargo.toml Updated dependencies and added bind/listen features
README.md Updated example to use async bind

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

SIO_GET_EXTENSION_FUNCTION_POINTER,
&WSAID_WSARECVMSG as *const _ as *const std::ffi::c_void,
std::mem::size_of::<GUID> as usize as u32,
std::mem::size_of::<GUID> as *const () as usize as u32,
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Incorrect type cast chain. The expression std::mem::size_of::<GUID> as *const () as usize as u32 is invalid - you cannot cast a usize to *const (). This should remain as std::mem::size_of::<GUID>() as u32 or simply std::mem::size_of::<GUID>() as _.

Suggested change
std::mem::size_of::<GUID> as *const () as usize as u32,
std::mem::size_of::<GUID>() as u32,

Copilot uses AI. Check for mistakes.
@bytedance bytedance deleted a comment from Copilot AI Nov 14, 2025
Signed-off-by: Lzzzt <lzzzt.main@gmail.com>
Copy link
Member

@ihciah ihciah left a comment

Choose a reason for hiding this comment

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

It’s a good PR, and I’ve left a few inline comments.

Beyond those, I think we should address this:

As kernel support for io_uring ops becomes more fragmented (i.e., more ops may or may not be available depending on the kernel version),
relying only on feature flags is no longer sufficient. I suggest:

  • In iouring-only mode, decide per feature whether each new op should use a direct syscall or io_uring.
  • In fusion mode, probe each enabled op at startup (today we only do a coarse overall probe; ideally per-op), and introduce a dedicated
    abstraction layer to support this capability and decide whether to run in legacy or io_uring mode.

What do you think?

socket_type: libc::c_int,
pub(crate) async fn new_socket(
domain: socket2::Domain,
socket_type: socket2::Type,
Copy link
Member

Choose a reason for hiding this comment

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

Here socket_type has been changed to socket2::Type, but socket2::Type doesn’t implement BitOr, so

  let socket_type = socket_type | libc::SOCK_NONBLOCK | libc::SOCK_CLOEXEC;

will not compile.


/// Bind to address
pub fn bind<A: ToSocketAddrs>(addr: A) -> io::Result<Self> {
pub async fn bind<A: ToSocketAddrs>(addr: A) -> io::Result<Self> {
Copy link
Member

Choose a reason for hiding this comment

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

Changing bind from sync to async is a breaking API change for downstreams. Could we keep the old sync bind and add async_bind?

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.

2 participants