-
Notifications
You must be signed in to change notification settings - Fork 65
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
Introduce VirtIONetRaw to allow custom NIC buffer management and used buffer notification suppression #111
Conversation
Arceos PR : arceos-org/arceos#129 |
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.
Thanks! This mostly looks good, but some minor things to fix.
src/device/net/dev_raw.rs
Outdated
/// | ||
/// [`fill_buffer_header`]: Self::fill_buffer_header | ||
pub fn transmit_wait(&mut self, tx_buf: &[u8]) -> Result<usize> { | ||
let token = unsafe { self.transmit_begin(tx_buf)? }; |
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.
Please add safety comments for the unsafe blocks, or better, use add_notify_wait_pop
to avoid them entirely.
…fication suppression
80d1652
to
b2adb57
Compare
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.
Thanks for tidying this up! Just two minor comments then I'm happy to merge it.
src/device/net/dev_raw.rs
Outdated
} | ||
|
||
/// Whether can receive packet. | ||
pub fn can_recv(&self) -> bool { |
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.
Maybe just remove this, poll_receive
fills the same purpose.
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.
you mean peek_used
?
Since it was used by ArceOS, so I reserve it, it has been deleted!
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 meant the poll_receive
method that you added to VirtIONetRaw
, which calls peek_used
internally.
I've pushed a commit to keep can_recv
on VirtIONet
, by calling VirtIONetRAW::peek_used
.
d0e772f
to
ceb6ee1
Compare
Some tiny modifications need to be completed within ArceOS. @equation314