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

feat: extend support to ipv6 for default MACAddres #534

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

Conversation

sarmahaj
Copy link
Contributor

@sarmahaj sarmahaj commented Jul 18, 2023

This PR adds to mfg_string_type=MACAddress feat

  • PR feat(manufacturing-client): use default ipv4 iface route if not provided #491 adds support for ipv4 based network interfaces
  • This PR extends to support ipv6 based network interfaces as well.
  • In case of mfg_string_type selected as MACAddress and user has not provided which interface to use then get the default route and use that interface to get MACAddress.
  • test in PR#491 covers this scenario as well.

Fixes #535

- in case of mfg_string_type selected as  MACAddress and user has not
provided which interface to use then get the default route and use that
interface to get MACAddress.
- add support for ipv6

Signed-off-by: Sarita Mahajan <sarmahaj@redhat.com>
Comment on lines +1015 to +1025
for line in reader.lines().skip(1) {
let line = line?;
let fields: Vec<_> = line.split_whitespace().collect();
if fields.is_empty() {
continue;
}
if fields[1] == IPV6_DEFAULT && fields[9] != "lo" {
let iface = fields[0].to_string();
log::info!("Default network interface is ipv6 based {}", iface);
return Ok(Some(iface));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic is not going to work for parsing /proc/net/ipv6_route, because the layout of the fields is quite different from /proc/net/route

Skipping the first line of the output is likely going to drop one of the IPv6 loopback addresses; on two of my Fedora systems the line looks like:

00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000100 00000001 00000000 00000001 lo

fields[0] in this table is going to be "IPv6 destination network displayed in 32 hexadecimal chars without colons as separator", so it is not the right field to use for the interface name.

It's also possible for an interface to have a single IPv6 address that is "link local" and will never be routed, so it is unlikely to be usable for broader communication. I would think we would need to check for this kind of address before we decide to use the interface name.

Copy link
Contributor Author

@sarmahaj sarmahaj Jul 19, 2023

Choose a reason for hiding this comment

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

Agree!
I think instead of extracting the network interface at low level, it will be better to use some networking related crate/s which provides info on the active ipv6 network interface etc . I would like to convert this PR to draft, go back and check for a proper crate so that even if something changes with the kernel files (e.g. columns or way of representation this will fail!) whereas with crate it will be(probably) handled in case of any changes .
CC: @nullr0ute @7flying

Copy link
Contributor

Choose a reason for hiding this comment

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

my only concern is that the crate (if any) should ideally be already packaged in Fedora, as well as its dependencies

Copy link
Contributor

Choose a reason for hiding this comment

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

is there a standard network crate that provides all this information for both IPv4 and IPv6 that is widely used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

last time when I had looked for such crates, there were many options but

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.

Allow to default to a single available iface when MACAddress is used as the mfg_string_type (IPv6)
4 participants