ethtool: Add support of channel#8
Conversation
Equivalent to `ethtool -l <nic_name>` command. Example code included.
DmytroLinkin
left a comment
DmytroLinkin
left a comment
There was a problem hiding this comment.
Left few small comments.
I've tested the code - it works.
|
|
||
| use futures::stream::TryStreamExt; | ||
|
|
||
| // Once we find a way to load netsimdev kernel module in CI, we can convert this |
There was a problem hiding this comment.
This comment is copied from other examples provided in the project.
I'll submit a dedicated PR to fix this in all the code examples.
src/message.rs
Outdated
| const ETHTOOL_MSG_LINKMODES_GET_REPLY: u8 = 4; | ||
| const ETHTOOL_MSG_RINGS_GET: u8 = 15; | ||
| const ETHTOOL_MSG_RINGS_GET_REPLY: u8 = 16; | ||
| const ETHTOOL_MSG_CHANNELS_GET: u8 = 17; | ||
| const ETHTOOL_MSG_CHANNELS_GET_REPLY: u8 = 18; | ||
| const ETHTOOL_MSG_COALESCE_GET: u8 = 19; | ||
| const ETHTOOL_MSG_COALESCE_GET_REPLY: u8 = 20; | ||
| const ETHTOOL_MSG_TSINFO_GET: u8 = 25; |
There was a problem hiding this comment.
This const list is quite confusing, because it actually mixes values from two separate lists. Isn't it better to have "userspace to kernel" and "kernel to userspace" separately?
the const list comes from two different enums. Put them into two different enums to make it clear.
cathay4t
left a comment
cathay4t
left a comment
There was a problem hiding this comment.
Introducing new dependency should be carefully reviewed. In this case, it is not a stuff we cannot do elegantly by ourselves, hence please remove it the use of num_enum.
| const ETHTOOL_MSG_TSINFO_GET: u8 = 25; | ||
| const ETHTOOL_MSG_TSINFO_GET_REPLY: u8 = 26; | ||
| #[repr(u8)] | ||
| #[derive(Debug, PartialEq, Eq, Clone, Copy, IntoPrimitive, FromPrimitive)] |
There was a problem hiding this comment.
In stead of introducing new dependency, please use From<u8> and remove #[repr(u8)].
| PauseSet = 22, | ||
| TsInfoGet = 25, | ||
| #[num_enum(catch_all)] | ||
| UnSupport(u8), |
There was a problem hiding this comment.
Please use Other(u8) like other code. Yes this forbid you from using repr[u8], please impl From trait for it when needed.
| PauseGetReply = 22, | ||
| TsInfoGetReply = 26, | ||
| #[num_enum(catch_all)] | ||
| UnSupport(u8), |
|
Hello, I'm interested in this feature being merged (as well as the @SteamedFish if you don't have time or have shifted priorities, do you mind if I jump in and complete this? |
Sure, I got several issues managing ethernet devices in virtual machines with netlink, I've switched to ioctl. |
|
@SteamedFish Is that OK for me to close this PR since you moved to ioctl() approach? |
4 participants
Equivalent to
ethtool -l <nic_name>command.Example code included.