missing multishot commands

[FrankReh]

I'm going to be adding more multishot commands, some or all in separate PRs. But I wanted a place where we could discuss the naming of these opcodes and options. I'm going to list the names that come from the liburing API as a starting point.

• recv_multishot(sockfd, buf_group, flags) (see opcode: add RecvMulti #188)
• recvmsg_multishot (see types: add RecvMsgOut helper structure #192)
• poll_multishot(fd, flags) (see opcode: add support for IORING_POLL_ADD_MULTI #191)

The multishot version of recv should not take a buffer pointer or length so a separate opcode seems warranted to me, maybe RecvMulti as a convention I think we're using is to place modifiers in a name after the primary part of the name? If we wanted to
use the same opcode, we could add an optional buf_group field, but a buf_group id value of 0 is allowed so we might have to wrap that optional type in an Option in that case or default it to a max value.

The multishot version of recvmsg I'm still confused about. I think parts of the msghdr are still relevant to the function even though the buffer it is designed to point to would not be. I would be tempted to tackle this one last because it's not clear to me from even the liburing documentation how it works or what it does. It might require looking through the kernel source or asking on the liburing repo.

The poll_multishot ends up taking the same arguments as the single-shot poll_add operation, so maybe that doesn't deserve its own opcode here, maybe an optional multishot boolean? But if you want a separate opcode, how about the name PollMulti to go with the PollAdd that is there now?

@quininer @lucab

[lucab]

The multishot version of recvmsg I'm still confused about. I think parts of the msghdr are still relevant to the function even though the buffer it is designed to point to would not be. I would be tempted to tackle this one last because it's not clear to me from even the liburing documentation how it works or what it does. It might require looking through the kernel source or asking on the liburing repo.

I was playing with this one and I have a locally working branch (on kernel 6.1), but I still need to polish and write tests before submitting.

What I've found out so far is that:

• the userland needs to pass some real msghdr, a NULL pointer is invalid and rejected by kernel
• the structure is not used for storing results from the kernel, but only read from for input arguments
• the kernel only uses two values from that structure (name length and control length), this way:
  • msg_namelen is the reserved fixed length (in bytes) to store the name result, in the provided-buffer (0 is a valid input)
  • msg_controllen is the reserved fixed length (in bytes) to store the control data, in the provided-buffer (0 is a valid input)
• those two values aren't reflected back by the kernel on the multishot CQE, so userland should somehow keep them around to properly parse the resulting data in the provided buffer

[FrankReh]

I was playing with this one and I have a locally working branch (on kernel 6.1), but I still need to polish and write tests before submitting.

That is great. Anytime in the next week or two is probably not a problem. I was hoping to get a tokio-uring release out when all of these PRs were reviewed and merged but there are a number that haven't been reviewed in that repo either so a new release is likely a week or two out anyway. I feel your pain, getting the test in is often harder than adding the binding.

[FrankReh]

They are all supported now. See the unit test named test_udp_recvmsg_multishot for how to use the existing opcode::RecvMsg but with proper flags for multishot and see the same test for how to use the new types::RecvMsgOut::parse function to create a RecvMsgOut with methods to access various parts of the data that was received.

[lucab]

There is still one UX question open about how to expose this:

io-uring/io-uring-test/src/tests/net.rs

Lines 1295 to 1296 in 6785e66

	// TODO(lucab): make this more ergonomic to use.
	const IORING_RECV_MULTISHOT: u16 = 2;

On the same topic, it is also relevant to note that multishot recvmsg also needs to have the BUFFER_SELECT flag set:

io-uring/io-uring-test/src/tests/net.rs

Lines 1298 to 1304 in 6785e66

	let recvmsg_e = io_uring::opcode::RecvMsg::new(socket_slot, &mut msghdr as *mut _)
	.ioprio(IORING_RECV_MULTISHOT)
	.buf_group(BUF_GROUP)
	.build()
	.flags(io_uring::squeue::Flags::BUFFER_SELECT)
	.user_data(77)
	.into();

At this point, it looks like all other multishot operations have been placed behind their own FooMulti types, right?
If so it may make sense to also introduce a RecvMsgMulti for this, taking care of setting all the relevant flags internally.

[FrankReh]

@lucab I agree. See what @quininer says or just create a PR and see if @quininer accepts it?

[quininer]

We already have RecvMulti, it's nice to have RecvMsgMulti.

[FrankReh]

@lucab Do you want to finish this? Do you have time?
I'll give it a shot now.

• opcode: add RecvMsgMulti #201

[FrankReh]

@lucab Are you aware of anything else, after 201, that should go in before we ask @quininer for a new release to be tagged?

I would suggest updating the sys module to make the next round of work potentially easier on people, but it's not necessary for the currently supported opcodes of course.

[lucab]

Sorry my availability is a bit spotty as I'm busy taking care of other stuff offline. No I don't have other things to merge before the next release.