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

Use libnet crate for grabbing link info. #304

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

luqmana
Copy link
Contributor

@luqmana luqmana commented Jan 24, 2023

Avoids just shelling out to dladm and some vnic-specific methods to work with other link classes.

Copy link
Collaborator

@pfmooney pfmooney left a comment

Choose a reason for hiding this comment

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

Thanks for getting this cleaned up.

Comment on lines +60 to +78
let info =
libnet::get_link(&libnet::LinkHandle::Name(vnic_name.to_string()))
.map_err(|e| {
io::Error::new(
io::ErrorKind::Other,
format!("failed to get link info: {e}"),
)
})?;

let mtu = if let Some(mtu) = info.mtu {
Some(u16::try_from(mtu).map_err(|_| {
io::Error::new(
io::ErrorKind::Other,
format!("MTU too large: {mtu}"),
)
})?)
} else {
None
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we want to keep propolis able to build (albeit useless binaries) on non-illumos platforms, perhaps we can wrap the libnet stuff pulled out into a function in cfg blocks?

@pfmooney
Copy link
Collaborator

Since @rmustacc pointed out that the libnet crate is consuming a bunch of Private OS interfaces in order to expose its APIs, @luqmana and I chatted about exploring options which stick to public Committed interfaces for that data. Either tweaking the info-gathering calls to dladm, and/or expanding libdladm upstream so it makes available the things we need.

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.

None yet

2 participants