-
Notifications
You must be signed in to change notification settings - Fork 151
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
base: master
Are you sure you want to change the base?
Conversation
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 :
|
This looks good, would you like to add a test? |
@quininer done |
4497757
to
403d61b
Compare
|
||
pub struct ReadMulti { | ||
fd: { impl sealed::UseFixed }, | ||
buf_group: { u16 }, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not true, ReadMulti is more similar to Read than to RecvMulti.
see https://github.com/torvalds/linux/blob/v6.13/io_uring/rw.c#L366 and https://github.com/axboe/liburing/blob/liburing-2.9/src/include/liburing.h#L801
let mut sqe = sqe_zeroed(); | ||
sqe.opcode = Self::CODE; | ||
assign_fd!(sqe.fd = fd); | ||
sqe.__bindgen_anon_3.msg_flags = flags as _; |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this 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 }, |
There was a problem hiding this comment.
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 _; |
There was a problem hiding this comment.
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
Simple addition, already have the CODE in the /sys/