io-uring

io-uring copied to clipboard

Follow-up to #123, this enables the use of io_uring's automatic buffer selection for those operations.

Would you like to add a test?

Added.

It looks like ci hangs on tcp_buffer_select, and it looks like the kernel that ci is currently using doesn't support this feature. we may need to mark it as unstable until we update the ci kernel.

It looks like ci hangs on tcp_buffer_select, and it looks like the kernel that ci is currently using doesn't support this feature. we may need to mark it as unstable until we update the ci kernel.

This pr is great with supporting buf_group field to Readv and RecvMsg opcode. When would the pr be merged ?

@glenn-wang We need to mark the feature as unstable and pass the ci test.

@glenn-wang We need to mark the feature as unstable and pass the ci test.

Hi, @quininer

This pr still not be merged, even could not be as unstable feature.

The buf_group field may not be tested for the Read op either. Maybe it is enough to add the support without adding unit tests for it. Users of this crate can always report if they find a problem and even provide a fix. The users of this crate seem to be very good at uring and Rust issues.

And maybe someone wants to propose a way for the unit tests to check the version of kernel they are running on. The kernel uring API itself isn't great at always providing a backwards compatible way of checking that one of its features is supported; they try for many features but many fell through the cracks - in terms of being able to check for support. The liburing documentation always falls back on describing what version of kernel the feature started being supported in.

We can use not(feature = "ci") cfg to allow ci to skip this test temporarily. like https://github.com/tokio-rs/io-uring/blob/38de039be7827b3a11566bf45e1745377c4bc29c/io-uring-test/src/main.rs#L89-L92

@quininer Don't merge this yet. I've run the test on a 6.1 kernel and found a unit test failure. Let me investigate and push something else up to fix it.

@quininer I think it is good to merge now. Squash merge I hope. You are welcome to look at this last merge of mine and see if the difference between a 5.15 kernel and a 6.1 kernel, that I was getting with this branch, in case you can make more sense out of it than I did.

@quininer @glenn-wang @mrakh While I'm getting the unit test better setup it, it has become clear recvmsg and BUFFER_SELECT are generally incompatible unless one sets the msg_iovlen value to 1, as the API doesn't allow for returning multiple buffer IDs. Just one buffer ID can be returned in the cqe, there is no field in the iovec for placing it, so the application would not know what buffers had been selected. I've seen both a 5.15 and a 6.1 kernel return a result of EINVAL when the msg_iovlen is 2, so while the liburing man pages don't document it, I believe recvmsg and BUFFER_SELECT is only valid when a single iovec is specified and the kernel uring device enforces that - which will beg the question for most apps of why even bother using recvmsg with provided buffers.

And I'm not going to go through the same trials with readv but I suspect the same. There's no sense in trying to gather into multiple buffers with a readv length other than 1 because the API would have no way of returning multiple buffer ids.

Hey @FrankReh, thanks for following up with this. Having buffer selection for recvmsg and readv is still useful even with the 1-iovec limitation because they have additional functionality not available through the more generic interfaces:

1. recvmsg returns important auxiliary data through the struct msghdr* argument. AFAIK, it's the only way using io_uring to get the source IP of an incoming UDP packet.
2. readv op contains a flags argument to specify things like RWF_DSYNC or RWF_SYNC on a per-invocation basis.

@mrakh Good points.

@quininer Ready for another review.

Thank you! @FrankReh @mrakh