Skip to content
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

Bump netlink-packet-utils and netlink-packet-core crates #156

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

qmonnet
Copy link

@qmonnet qmonnet commented Apr 3, 2025

“Bump netlink-packet-utils, get rid of anyhow, and adjust the code accordingly.”

In practice, the PR is not ready yet; but I could do with some feedback.

What's missing:

  • A newer version for netlink-packet-core
  • It feels like we're losing a lot of context by discarding all error messages when doing the adjustments, so I'm not sure I've been using the right approach (at the same time, I really hope I don't need to redo all adjustments 😅)
  • There are some changes that I had to do on error types to make the code compile, but I don't think they're correct. I'm not sure what the right approach is, though.
  • Two tests break; but that's probably something we want to address last.

Bump netlink-packet-utils, get rid of anyhow, and adjust the code
accordingly.

Signed-off-by: Quentin Monnet <[email protected]>
Comment on lines -135 to +161
.map(|nla| nla.and_then(|nla| InfoBondPort::parse(&nla)))
.map(|nla| {
nla.and_then(|nla| {
InfoBondPort::parse(&nla)
// Placeholder error, as we need to return a NlaError
.map_err(|_| NlaError::InvalidLength { nla_len: 0 })
})
})
.collect::<Result<Vec<_>, _>>()
.map(InfoPortData::BondPort),
InfoPortKind::Bridge => NlasIterator::new(payload)
.map(|nla| nla.and_then(|nla| InfoBridgePort::parse(&nla)))
.map(|nla| {
nla.and_then(|nla| {
InfoBridgePort::parse(&nla)
// Placeholder error, as we need to return a NlaError
.map_err(|_| NlaError::InvalidLength { nla_len: 0 })
})
})
.collect::<Result<Vec<_>, _>>()
.map(InfoPortData::BridgePort),
InfoPortKind::Vrf => NlasIterator::new(payload)
.map(|nla| nla.and_then(|nla| InfoVrfPort::parse(&nla)))
.map(|nla| {
nla.and_then(|nla| {
InfoVrfPort::parse(&nla)
// Placeholder error, as we need to return a NlaError
.map_err(|_| NlaError::InvalidLength { nla_len: 0 })
})
})
Copy link
Author

Choose a reason for hiding this comment

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

This is one of the chunks that I don't believe are the right way to address the changes.

Comment on lines -83 to +85
TCA_ACT_KIND => Some(
parse_string(nla.value())
.context("failed to parse TCA_ACT_KIND"),
),
TCA_ACT_KIND => {
Some(parse_string(nla.value()).map_err(|_| {
// Placeholder error, as we need to return a NlaError
NlaError::InvalidLength { nla_len: 0 }
}))
}
Copy link
Author

Choose a reason for hiding this comment

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

This one, too.

Comment on lines -730 to +708
match RouteNetlinkMessage::parse_with_param(&buf, header.message_type) {
Err(e) => Err(e),
Ok(message) => Ok(message),
}
RouteNetlinkMessage::parse_with_param(&buf, header.message_type)
.map_err(|_| NlaError::InvalidLength { nla_len: 0 })
Copy link
Author

Choose a reason for hiding this comment

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

And this seems not correct either.

bitflags = "2"
byteorder = "1.3.2"
libc = "0.2.66"
log = { version = "0.4.20", features = ["std"] }
netlink-packet-core = { version = "0.7.0" }
netlink-packet-utils = { version = "0.5.2" }
netlink-packet-core = { git = "https://github.com/rust-netlink/netlink-packet-core.git", rev = "01e8dd1" }
Copy link
Author

Choose a reason for hiding this comment

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

This is temporary, obviously.

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.

1 participant