Skip to content

feat: add support for ReadMulti opcode (6.7+) #317

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kp-omer-shamash
Copy link

Simple addition, already have the CODE in the /sys/

@kp-omer-shamash
Copy link
Author

Right now I'm doing what feels like a crime against humanity by implementing like this and having tons of structures copy-pasta'd from the /sys/ part of the crate, would really appreciate if we can merge so I don't use this abomination :

pub struct ReadMulti {
    fd: i32,
    buf_group: u16,
    flags: i32,
}

impl ReadMulti {
    #[inline]
    pub fn new(fd: i32, buf_group: u16) -> Self {
        ReadMulti {
            fd,
            buf_group,
            flags: 0,
        }
    }

    #[inline]
    pub fn build(self) -> Entry {
        let sqe = CustomSQE {
            opcode: IORING_OP_READ_MULTISHOT as _,
            flags: io_uring::squeue::Flags::BUFFER_SELECT.bits(),
            fd: self.fd,
            buf_index: PackedU16 {
                buf_index: self.buf_group,
            },
            msg_flags: Union3 {
                msg_flags: self.flags as _,
            },
            ..Default::default()
        };

        // Safety: CustomSQE has identical memory layout to io_uring_sqe
        #[allow(unsafe_code)]
        unsafe {
            std::mem::transmute(sqe)
        }
    }
}

@quininer
Copy link
Member

This looks good, would you like to add a test?

@kp-omer-shamash
Copy link
Author

kp-omer-shamash commented Feb 25, 2025

@quininer done
also, not sure what your release-plans are, but would appreciate if it's possible to bump to 0.7.5 soon after


pub struct ReadMulti {
fd: { impl sealed::UseFixed },
buf_group: { u16 },
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to provide a way to specify len and offset?

Copy link
Author

Choose a reason for hiding this comment

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

No, it should act the exact same as RecvMulti - you cannot specify anything other then which buffer-group to use, fd and flags (which I think none are not used at kernel-side right now)

Copy link
Member

@quininer quininer Mar 10, 2025

Choose a reason for hiding this comment

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

let mut sqe = sqe_zeroed();
sqe.opcode = Self::CODE;
assign_fd!(sqe.fd = fd);
sqe.__bindgen_anon_3.msg_flags = flags as _;
Copy link
Member

Choose a reason for hiding this comment

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

What does this flags mean and why does it write to msg_flags?

Copy link
Author

Choose a reason for hiding this comment

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

same as above - the API is similar to RecvMulti
flags are here in case kernel maintainers support any custom flags and we would like to use them, but default to 0

Copy link
Member

Choose a reason for hiding this comment

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

I think the kernel doesn't read msg_flags, right?

Copy link
Author

@kp-omer-shamash kp-omer-shamash left a comment

Choose a reason for hiding this comment

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

Sorry for late reply, I'm currently no longer available in this account, I hope this can be merged


pub struct ReadMulti {
fd: { impl sealed::UseFixed },
buf_group: { u16 },
Copy link
Author

Choose a reason for hiding this comment

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

No, it should act the exact same as RecvMulti - you cannot specify anything other then which buffer-group to use, fd and flags (which I think none are not used at kernel-side right now)

let mut sqe = sqe_zeroed();
sqe.opcode = Self::CODE;
assign_fd!(sqe.fd = fd);
sqe.__bindgen_anon_3.msg_flags = flags as _;
Copy link
Author

Choose a reason for hiding this comment

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

same as above - the API is similar to RecvMulti
flags are here in case kernel maintainers support any custom flags and we would like to use them, but default to 0

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