Netlink extensions (issue #32)

[jreppnow]

Hi everyone,

this is a first step towards the requirements in #32. Specifically, it implements:

• creating vcan (and theoretically other) CAN interfaces
• deleting interfaces
• reading rudimentary interface data (name, MTU, up/down) (can be extended easily by adding more entries in the corresponding loop)
• setting MTU (switching VCAN from FD to non-FD and vice versa)

All of the tests added here need to be run as root and/or with CAP_NET_ADMIN capability. As such, I hid them behind another feature flag. They work locally for me.

I did not get to bitrate and such because that actually does not work with vcan out-of-the-box; and I don't have a physical CAN device at home atm.

For follow-up changes it might be interesting to know where I got the information from (which was quite painful as they documentation leaves some things to be desired..):

• netlink and rnetlink documentation (https://man7.org/linux/man-pages/man7/netlink.7.html and https://man7.org/linux/man-pages/man7/rtnetlink.7.html)
• iproute2 (aka iptools) source: https://github.com/shemminger/iproute2
• ultimately, I mostly ended up just using strace with the ip link commands I was interested in

[jreppnow]

Ah, there is one annoyance: There can only be one netlink socket per PID, so when running multiple tests, one has to pass --test-threads 1 to cargo.

At the moment (this I did not change from the old code) a NL socket is created on-demand per request. It would be reasonable to use an RAII wrapper for this, but it does not solve the threading issue. There could also be a process-wide global, but that kinda sits wrong with me.

[fpagliughi]

@jreppnow Thanks for getting on this!

I did not get to bitrate and such because that actually does not work with vcan out-of-the-box; and I don't have a physical CAN device at home atm.

OK. But please do not use any equipment that is owned by the company you work for, or any other company. I would not want any company claiming ownership of this code based on something like that!

We can get this PR merged and then try to do the bit rates in one of several ways:

• Try an implementation and put up a different PR for it marked "untested" or something like that. I can verify that it works and/or debug and finish it.
• If you have a basic idea of how to do it, create an issue describing it, and I can try to implement it when I have a moment.
• If you're done with this, just leave it, and I will look into it at some point.

[fpagliughi]

Ah, there is one annoyance: There can only be one netlink socket per PID, so when running multiple tests, one has to pass --test-threads 1 to cargo.

Ah, yeah, I've had a few crates that need to serialize tests. The problem with requiring it to be done manually with --test-threads=1 is that you always forget. (I know from experience!)

But there's a crate to help with this: serial_test.

Just add to Cargo.toml:

[dev-dependencies]
serial_test = "0.9"

and then tag any tests that need to run in isolation as #[serial], like:

#[test]
#[serial]
fn test_something() { ... }

[jreppnow]

Hi @fpagliughi,

I'll address the remarks in order:

1. I never have and I will not use work equipment for any tests of my open source contributions, so no need to worry. We (company) might use this library and specifically the netlink functionality at some point in the future, in which case I will attempt to get the company to sign the typical waiver so that I can safely push possible changes/fixes back upstream, but I'll address that when it comes to it.
2. I will probably get myself a USB CAN(-FD) adapter and the resistors necessary to build a small CAN network at home at some point in the future, but these aren't concrete plans yet.
3. Regarding further changes: I am open to implementing further functionality, but I am generally limited to weekends for bigger contributions (although I will have some time after Christmas). What I would suggest is that I write a small guide in the netlink Issue regarding how I got at the necessary information and how to implement these kinds of netlink requests (to the best of my also somewhat limited knowledge). Then it's up to you if you (or other contributors) pick up on this and add features on their own; and I'll also look into adding some more myself starting next weekend. The ones I cannot test I would mark as "needs testing" as you suggested. As we are talking about what is essentially a set of pretty much separate requests, incrementing the supported operations step-by-step would work quite nicely I believe.
4. Thanks a lot for the suggestion regarding the serial_test crate, I added it to the relevant tests in my PR (and will probably use it quite a bit in other projects in the future 👍 ).

I rebased the PR to the most recent version of the master branch.

[fpagliughi]

That all sounds good. I was actually out sick from work the last week+, so got more done than I thought on this, but I'm back so mostly relegated to nights and weekends going forward. But I may have time over Christmas as well.

Anyway, the main upstream crates we use (neli, libc, and nix) all seem receptive to Pull Requests, and the neli maintainer was explicit in his willingness to add CAN constants as needed. So, by all means, try to push support upstream where appropriate.

I currently have two PR's in to the nix crate to add CAN constants and a CanAddr implementation. That would allow us to move much of the socket code from unsafe libc calls to the safe nix wrappers.

I'll have a look at the PR this evening, and most likely just land it and keep moving forward.

Thanks again.

[jreppnow]

I've already forked libc to add the constants I temporarily added here ;) After/if that goes through I'll turn to neli to have them added with all of their traits, since neli is mostly just wrapping libc constants.

Edit: rust-lang/libc#3035