Skip to content

Misc fixes of PR #24 #29

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.

Sign up for GitHub

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

Merged
cathay4t merged 2 commits into from
Jan 4, 2026
Merged

Misc fixes of PR #24 #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9978485
Use netlink-packet-core to replace netlink-packet-util
cathay4t Jan 4, 2026
98f38ed
Run cargo fmt and clippy
cathay4t Jan 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 18 additions & 26 deletions src/eeprom/attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
// SPDX-License-Identifier: MIT

use anyhow::Context;
use byteorder::{ByteOrder, NativeEndian};
use netlink_packet_utils::{
nla::{DefaultNla, Nla, NlaBuffer, NlasIterator, NLA_F_NESTED},
DecodeError, Emitable, Parseable,
use netlink_packet_core::{
emit_u32, DecodeError, DefaultNla, Emitable, ErrorContext, Nla, NlaBuffer,
NlasIterator, Parseable, NLA_F_NESTED,
};

use crate::{EthtoolAttr, EthtoolHeader};


const ETHTOOL_A_MODULE_EEPROM_HEADER: u16 = 1;
const ETHTOOL_A_MODULE_EEPROM_OFFSET: u16 = 2;
const ETHTOOL_A_MODULE_EEPROM_LENGTH: u16 = 3;
Expand All @@ -35,11 +32,8 @@ impl Nla for EthtoolModuleEEPROMAttr {
match self {
Self::Header(hdrs) => hdrs.as_slice().buffer_len(),
Self::Data(data) => data.len(),
Self::Page(_)
| Self::Bank(_)
| Self::I2CAddress(_) => 1,
Self::Offset(_)
| Self::Length(_) => 4,
Self::Page(_) | Self::Bank(_) | Self::I2CAddress(_) => 1,
Self::Offset(_) | Self::Length(_) => 4,
Self::Other(attr) => attr.value_len(),
}
}
Expand All @@ -61,11 +55,10 @@ impl Nla for EthtoolModuleEEPROMAttr {
match self {
Self::Header(ref nlas) => nlas.as_slice().emit(buffer),
Self::Data(d) => buffer.copy_from_slice(d.as_slice()),
Self::Page(d)
| Self::Bank(d)
| Self::I2CAddress(d) => buffer[0] = *d,
Self::Offset(d)
| Self::Length(d) => NativeEndian::write_u32(buffer, *d),
Self::Page(d) | Self::Bank(d) | Self::I2CAddress(d) => {
buffer[0] = *d
}
Self::Offset(d) | Self::Length(d) => emit_u32(buffer, *d).unwrap(),
Self::Other(ref attr) => attr.emit(buffer),
}
}
Expand All @@ -79,7 +72,8 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable<NlaBuffer<&'a T>>
Ok(match buf.kind() {
ETHTOOL_A_MODULE_EEPROM_HEADER => {
let mut nlas = Vec::new();
let error_msg = "failed to parse module eeprom header attributes";
let error_msg =
"failed to parse module eeprom header attributes";
for nla in NlasIterator::new(payload) {
let nla = &nla.context(error_msg)?;
let parsed =
Expand All @@ -88,13 +82,10 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable<NlaBuffer<&'a T>>
}
Self::Header(nlas)
}
ETHTOOL_A_MODULE_EEPROM_DATA => Self::Data(
Vec::from(payload),
),
kind => Self::Other(
DefaultNla::parse(buf)
.context(format!("invalid ethtool module eeprom NLA kind {kind}"))?,
),
ETHTOOL_A_MODULE_EEPROM_DATA => Self::Data(Vec::from(payload)),
kind => Self::Other(DefaultNla::parse(buf).context(format!(
"invalid ethtool module eeprom NLA kind {kind}"
))?),
})
Comment on lines +85 to 89
Copy link

@gemini-code-assist gemini-code-assist bot Jan 4, 2026

Choose a reason for hiding this comment

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

critical

The Parseable implementation for EthtoolModuleEEPROMAttr is incomplete. It's missing handlers for ETHTOOL_A_MODULE_EEPROM_OFFSET, ETHTOOL_A_MODULE_EEPROM_LENGTH, ETHTOOL_A_MODULE_EEPROM_PAGE, ETHTOOL_A_MODULE_EEPROM_BANK, and ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS. This will cause these attributes to be incorrectly parsed as EthtoolModuleEEPROMAttr::Other when they are present in a netlink message from the kernel. This is a critical issue as it prevents correct parsing of ETHTOOL_MSG_MODULE_EEPROM_GET_REPLY messages.

            ETHTOOL_A_MODULE_EEPROM_OFFSET => Self::Offset(
                netlink_packet_core::parse_u32(payload)
                    .context("invalid ETHTOOL_A_MODULE_EEPROM_OFFSET value")?,
            ),
            ETHTOOL_A_MODULE_EEPROM_LENGTH => Self::Length(
                netlink_packet_core::parse_u32(payload)
                    .context("invalid ETHTOOL_A_MODULE_EEPROM_LENGTH value")?,
            ),
            ETHTOOL_A_MODULE_EEPROM_PAGE => {
                if payload.len() != 1 {
                    return Err(DecodeError::from(
                        "invalid ETHTOOL_A_MODULE_EEPROM_PAGE value: length is not 1",
                    ));
                }
                Self::Page(payload[0])
            }
            ETHTOOL_A_MODULE_EEPROM_BANK => {
                if payload.len() != 1 {
                    return Err(DecodeError::from(
                        "invalid ETHTOOL_A_MODULE_EEPROM_BANK value: length is not 1",
                    ));
                }
                Self::Bank(payload[0])
            }
            ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS => {
                if payload.len() != 1 {
                    return Err(DecodeError::from(
                        "invalid ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS value: length is not 1",
                    ));
                }
                Self::I2CAddress(payload[0])
            }
            ETHTOOL_A_MODULE_EEPROM_DATA => Self::Data(Vec::from(payload)),
            kind => Self::Other(DefaultNla::parse(buf).context(format!(
                "invalid ethtool module eeprom NLA kind {kind}"
            ))?),
        })

}
}
Expand All @@ -104,8 +95,9 @@ pub(crate) fn parse_module_eeprom_nlas(
) -> Result<Vec<EthtoolAttr>, DecodeError> {
let mut nlas = Vec::new();
for nla in NlasIterator::new(buffer) {
let error_msg =
format!("Failed to parse ethtool module eeprom message attribute {nla:?}");
let error_msg = format!(
"Failed to parse ethtool module eeprom message attribute {nla:?}"
);
let nla = &nla.context(error_msg.clone())?;
let parsed = EthtoolModuleEEPROMAttr::parse(nla).context(error_msg)?;
nlas.push(EthtoolAttr::ModuleEEPROM(parsed));
Expand Down
27 changes: 18 additions & 9 deletions src/eeprom/get.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ pub struct EthtoolModuleEEPROMGetRequest {
}

impl EthtoolModuleEEPROMGetRequest {
pub(crate) fn new(handle: EthtoolHandle, iface_name: Option<&str>,
offset: u32,
length: u32,
page: u8,
bank: u8,
i2c_address: u8
pub(crate) fn new(
handle: EthtoolHandle,
iface_name: Option<&str>,
offset: u32,
length: u32,
page: u8,
bank: u8,
i2c_address: u8,
) -> Self {
EthtoolModuleEEPROMGetRequest {
handle,
Expand All @@ -30,7 +32,7 @@ impl EthtoolModuleEEPROMGetRequest {
length,
page,
bank,
i2c_address
i2c_address,
}
}

Expand All @@ -45,10 +47,17 @@ impl EthtoolModuleEEPROMGetRequest {
length,
page,
bank,
i2c_address
i2c_address,
} = self;

let ethtool_msg = EthtoolMessage::new_module_eeprom_get(iface_name.as_deref(), offset, length, page, bank, i2c_address);
let ethtool_msg = EthtoolMessage::new_module_eeprom_get(
iface_name.as_deref(),
offset,
length,
page,
bank,
i2c_address,
);
ethtool_execute(&mut handle, iface_name.is_none(), ethtool_msg).await
}
}
28 changes: 19 additions & 9 deletions src/eeprom/handle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,25 @@ impl EthtoolModuleEEPROMHandle {
EthtoolModuleEEPROMHandle(handle)
}

/// Retrieve the module eeprom data pages of a interface (used by `ethtool -m
/// eth1`)
pub fn get(&mut self, iface_name: Option<&str>,
offset: u32,
length: u32,
page: u8,
bank: u8,
i2c_address: u8
/// Retrieve the module eeprom data pages of a interface (used by `ethtool
/// -m eth1`)
pub fn get(
&mut self,
iface_name: Option<&str>,
offset: u32,
length: u32,
page: u8,
bank: u8,
i2c_address: u8,
) -> EthtoolModuleEEPROMGetRequest {
EthtoolModuleEEPROMGetRequest::new(self.0.clone(), iface_name, offset, length, page, bank, i2c_address)
EthtoolModuleEEPROMGetRequest::new(
self.0.clone(),
iface_name,
offset,
length,
page,
bank,
i2c_address,
)
}
}
7 changes: 4 additions & 3 deletions src/handle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ use netlink_packet_core::{
use netlink_packet_generic::GenlMessage;

use crate::{
try_ethtool, EthtoolChannelHandle, EthtoolCoalesceHandle, EthtoolError, EthtoolFeatureHandle,
EthtoolFecHandle, EthtoolLinkModeHandle, EthtoolMessage, EthtoolPauseHandle, EthtoolRingHandle,
EthtoolTsInfoHandle, EthtoolModuleEEPROMHandle
try_ethtool, EthtoolChannelHandle, EthtoolCoalesceHandle, EthtoolError,
EthtoolFeatureHandle, EthtoolFecHandle, EthtoolLinkModeHandle,
EthtoolMessage, EthtoolModuleEEPROMHandle, EthtoolPauseHandle,
EthtoolRingHandle, EthtoolTsInfoHandle,
};

#[derive(Clone, Debug)]
Expand Down
5 changes: 4 additions & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ pub use coalesce::{
#[cfg(feature = "tokio_socket")]
pub use connection::new_connection;
pub use connection::new_connection_with_socket;
pub use eeprom::{EthtoolModuleEEPROMAttr, EthtoolModuleEEPROMGetRequest, EthtoolModuleEEPROMHandle};
pub use eeprom::{
EthtoolModuleEEPROMAttr, EthtoolModuleEEPROMGetRequest,
EthtoolModuleEEPROMHandle,
};
pub use error::EthtoolError;
pub use feature::{
EthtoolFeatureAttr, EthtoolFeatureBit, EthtoolFeatureGetRequest,
Expand Down
52 changes: 34 additions & 18 deletions src/message.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use netlink_packet_generic::{GenlFamily, GenlHeader};
use crate::{
channel::{parse_channel_nlas, EthtoolChannelAttr},
coalesce::{parse_coalesce_nlas, EthtoolCoalesceAttr},
eeprom::{parse_module_eeprom_nlas, EthtoolModuleEEPROMAttr},
feature::{parse_feature_nlas, EthtoolFeatureAttr},
fec::{parse_fec_nlas, EthtoolFecAttr},
eeprom::{parse_module_eeprom_nlas, EthtoolModuleEEPROMAttr},
link_mode::{parse_link_mode_nlas, EthtoolLinkModeAttr},
pause::{parse_pause_nlas, EthtoolPauseAttr},
ring::{parse_ring_nlas, EthtoolRingAttr},
Expand Down Expand Up @@ -80,7 +80,9 @@ impl From<EthtoolCmd> for u8 {
EthtoolCmd::ChannelGetReply => ETHTOOL_MSG_CHANNELS_GET_REPLY,
EthtoolCmd::ChannelSet => ETHTOOL_MSG_CHANNELS_SET,
EthtoolCmd::ModuleEEPROMGet => ETHTOOL_MSG_MODULE_EEPROM_GET,
EthtoolCmd::ModuleEEPROMGetReply => ETHTOOL_MSG_MODULE_EEPROM_GET_REPLY,
EthtoolCmd::ModuleEEPROMGetReply => {
ETHTOOL_MSG_MODULE_EEPROM_GET_REPLY
}
}
}
}
Expand Down Expand Up @@ -305,29 +307,43 @@ impl EthtoolMessage {
}
}


pub fn new_module_eeprom_get(
iface_name: Option<&str>,
offset: u32,
length: u32,
page:u8,
bank:u8,
i2c_address:u8) -> Self {
iface_name: Option<&str>,
offset: u32,
length: u32,
page: u8,
bank: u8,
i2c_address: u8,
) -> Self {
let mut nlas = match iface_name {
Some(s) => {
vec![EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::Header(vec![
EthtoolHeader::DevName(s.to_string()),
]))]
vec![EthtoolAttr::ModuleEEPROM(
EthtoolModuleEEPROMAttr::Header(vec![
EthtoolHeader::DevName(s.to_string()),
]),
)]
}
None => {
vec![EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::Header(vec![]))]
vec![EthtoolAttr::ModuleEEPROM(
EthtoolModuleEEPROMAttr::Header(vec![]),
)]
}
};
nlas.push(EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::Offset(offset)));
nlas.push(EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::Length(length)));
nlas.push(EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::Page(page)));
nlas.push(EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::Bank(bank)));
nlas.push(EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::I2CAddress(i2c_address)));
nlas.push(EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::Offset(
offset,
)));
nlas.push(EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::Length(
length,
)));
nlas.push(EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::Page(
page,
)));
nlas.push(EthtoolAttr::ModuleEEPROM(EthtoolModuleEEPROMAttr::Bank(
bank,
)));
nlas.push(EthtoolAttr::ModuleEEPROM(
EthtoolModuleEEPROMAttr::I2CAddress(i2c_address),
));
EthtoolMessage {
cmd: EthtoolCmd::ModuleEEPROMGet,
nlas,
Expand Down