[API break] link: Reorganize the code base

[cathay4t]

• This patch is first preparation on stable release 1.0 for submodule
  link. The detailed checklist for developer is stated in
  README.md file.

• Stored constants into where it been used. Will remove constants.rs
  once all components finished.

• Replace pub use self::* to explicit expose.

• Removed data folder, README.md file has expanded to hold steps to
  capture netlink packets.

• Removed Unspec as kernel not use it.

• Removed benches folder as never used.

• Removed dependency on lazy_static.

• Added dependency on log. Considering rtnetlink also has it, no
  burden added to end user.

• Many member of InfoVxlan changed from u8 to bool after confirmed
  with kernel code.

• Changed API:

  • Rename:
    • link::Nla to link::LinkAttribute because Nla is a trait
    • link:Info to link::LinkInfo
    • link::VethInfo to link::InfoVeth
    • link::InfoXfrmTun to link::InfoXfrm
  • The AfSpec for ipv4 and ipv6 is rework. Please check AfSpecUnspec
  • Removed link::Nla::AltIfName. Use
    link::LinkAttribute::PropList<vec![link::Prop::AltIfName]>
    instead.
  • Changed InfoVlan::Protocol<u16> to
    InfoVlan::Protocol<VlanProtocol>.
  • LinkHeader changed to use enum instead raw integer on flags,
    address family and link layer type.
  • Change LinkMessage.nlas to LinkMessage.attributes.

• Added:

  • link::LinkFlag and LinkFlags for IFF_UP and etc
  • link::LinkLayerType for ARPHRD_ETHER and etc
  • crate::AddressFamily for AF_UNSPEC and etc

[codecov]

Codecov Report

Merging #56 (9e9b73f) into main (37f9c5c) will increase coverage by 3.99%.
The diff coverage is 63.35%.

@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
+ Coverage   51.40%   55.40%   +3.99%     
==========================================
  Files          76       86      +10     
  Lines        6744     6328     -416     
==========================================
+ Hits         3467     3506      +39     
+ Misses       3277     2822     -455     

Files	Coverage Δ
src/link/af_spec/inet6_cache.rs	100.00% <ø> (ø)
src/link/af_spec/inet6_devconf.rs	100.00% <100.00%> (ø)
src/link/af_spec/inet6_icmp.rs	0.00% <ø> (ø)
src/link/af_spec/inet6_stats.rs	0.00% <ø> (ø)
src/link/af_spec/unspec.rs	100.00% <100.00%> (ø)
src/link/header.rs	100.00% <100.00%> (ø)
src/link/link_info/bond.rs	44.66% <ø> (ø)
src/link/link_info/bond_port.rs	79.38% <ø> (ø)
src/link/link_state.rs	52.00% <ø> (ø)
src/link/map.rs	100.00% <ø> (ø)
... and 30 more

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

[cathay4t]

@little-dude @JohnTitor Please take a look when you have time. I am planning to do 1.0 release before end of 2024. I am still adding more test cases to make sure this fundamental network crate is trustable.

[JohnTitor]

Didn't check the diff exhaustively (it's huge!) but the summary on OP sounds good to me.

[cathay4t]

Patch is ready for review.

I will create new PRs for follow up works:

• Avoid using Vec<u8> unless kernel state so.
• Convert integer flag to enum.
• Convert bit-map flags to Vec<enum>.
• More tests.
• Do the same for address, tc and other subsystems.

[ydirson]

I'm not convinced it is a good idea to remove the dependency on libc and duplicate here the job of maintaining mappings for address families and other things. Portability-wise, where the current implementation makes the compiler point to places where work is needed, this proposal just pretends the Linux constants are the truth, bypassing the value checks done by the libc crate's tests. This would effectively make this crate Linux-only.

[ydirson]

Didn't check the diff exhaustively (it's huge!)

Indeed, not having individual reworks separated does not help to investigate alternative changes.

[cathay4t]

I'm not convinced it is a good idea to remove the dependency on libc and duplicate here the job of maintaining mappings for address families and other things. Portability-wise, where the current implementation makes the compiler point to places where work is needed, this proposal just pretends the Linux constants are the truth, bypassing the value checks done by the libc crate's tests. This would effectively make this crate Linux-only.

Regarding this libc dependency, I actually got email requesting support on FreeBSD which suggest me to remove the dependency on libc as FreeBSD does not have libc constants we depend on,

These netlink crates are mainly developed and tested on Linux, and I don't have resource to test or maintain it on other platforms. In stead of future use case, so if you have proof that Unix and Linux holding different integer on constants used by this crate, please state them, we can discuss in new thread on whether bring libc dependency back or use conditional compiling.

Regarding the large patch, I have tried my best to refrain myself on changing more codes. Sorry. Mostly the code changes is just moving code around.

[cathay4t]

@ydirson Oh, just realize it is you who requested FreeBSD support. Could you state which constants has conflicts between Unix and Linux and protected by libc?

[ydirson]

Regarding this libc dependency, I actually got email requesting support on FreeBSD which suggest me to remove the dependency on libc as FreeBSD does not have libc constants we depend on,

Well, my suggestion was not to remove the dependency on libc, rather to decorrelate the netlink-packet-route enums from the libc ones.

Could you state which constants has conflicts between Unix and Linux and protected by libc?

The main problem was not that of differing values, but as you mentioned earlier of not all constants not being available on FreeBSD. However, now that you mention it, even though the NETLINK values present today on FreeBSD are the same as on Linux, netlink has to manipulate eg. address families, and we have for example:

rust-libc (freebsd-netlink)$ git grep AF_APPLETALK: src
src/fuchsia/mod.rs:pub const AF_APPLETALK: ::c_int = 5;
src/unix/aix/mod.rs:pub const AF_APPLETALK: ::c_int = 16;
src/unix/bsd/apple/mod.rs:pub const AF_APPLETALK: ::c_int = 16;
src/unix/bsd/freebsdlike/mod.rs:pub const AF_APPLETALK: ::c_int = 16;
src/unix/bsd/netbsdlike/mod.rs:pub const AF_APPLETALK: ::c_int = 16;
src/unix/haiku/mod.rs:pub const AF_APPLETALK: ::c_int = 2;
src/unix/linux_like/mod.rs:pub const AF_APPLETALK: ::c_int = 5;
src/unix/nto/mod.rs:pub const AF_APPLETALK: ::c_int = 16;
src/unix/solarish/mod.rs:pub const AF_APPLETALK: ::c_int = 16;

These netlink crates are mainly developed and tested on Linux, and I don't have resource to test or maintain it on other platforms.

That's the case for lots of software. It would be one more reason to rely on the work done in the libc crate, and follow libc as well in adding CI jobs for FreeBSD. And users of those other platforms can also participate ;)

What I think would be a good solution would look like:

• all enum values supported by the crate should be publicly exposed regardless of their libc availability, with their actual values left to the compiler
• only when we come to the point of filling the C struct, then use the libc constants

This would allow something like:

fn numeric_address_family(af: AddressFamily) -> Option<u32> {
  match af {
  AddressFamily::Inet => Some(libc::AF_INET),
  ...
  #[cfg(target_os = "linux")]
  AddressFamily::Ax25 => Some(libc::AF_AX25),
  ...
  _ => None,
  }
}

Now there is extra maintenance work with this, especially as more OS will gain Netlink support, and their protocol coverage changes over time. So maybe something more clever could be done, like having build.rs check for availability of known-missing-somewhere defines, autoconf-style, and export using println!("cargo:rustc-env=HAS_ASX25={}", ...) so conditional compilation expression don't have to change over time.

[cathay4t]

Thanks for the suggestions. It seems the way we are handling AddressFamily need some work. Let me amend that.

[cathay4t]

@ydirson New patch uploaded for the cross-platform supports:

• Use conditional compiling for FreeBSD in address_family_freebsd.rs.
• Use conditional compiling for Linux and Fuchsia in address_family_linux.rs.
• Other platform are using address_family_fallback.rs which hold minim libc supported AF_XXX constants.
• CI has expanded to test the build on these rust targets:
  • x86_64-unknown-linux-gnu
  • x86_64-unknown-freebsd
  • x86_64-unknown-fuchsia
  • x86_64-apple-darwin
  • x86_64-linux-android

[ydirson]

@cathay4t that looks great!

Eager to see libc 0.3 debut so this can be applied to the netlink constants too!

I guess that to be able to try that on xen-guest-agent the next step would be propagating the API change to rtnetlink?

[cathay4t]

I have waited long enough for reviews. Let's move on and fix issue in new PR.