Evaluate usage of features

[Trolldemorted]

https://github.com/ebfull/pcap/blob/5eddf7f839d51374e23e9df62b4fdd31f04ed546/Cargo.toml#L25-L40

libpcap 1.7.2 was released in 2015 and 1.5 in 2013. Are there any distros still shipping those versions? Debian buster ships 1.8.1, CentOS 8 ships 1.9.

[Wojtek242]

One of the PRs I was intending to submit shortly (once I finish with the tokio update) was to improve handling of version-gated features. On my fork I did dynamic version detection: https://github.com/Wojtek242/pcap/pull/3/files inspired by #68. One good question would be to decide what is the oldest version to support. I completely agree with you that Linux distros are well ahead of 1.0.0. WinPcap is based on 1.0.0 though (but then again I don't really know what people use on Windows). To me 1.5.0 would be already good as that introduces immediate_mode.

I'm open to suggestions and concerns about back-compatibility.

[Wojtek242]

Debian jessie (still supported I think) ships 1.6.2

[Wojtek242]

Centos 6 (still maintained until December) ships with 1.4.0

[Trolldemorted]

On Linux, with previous releases of libpcap, capture devices are always in immediate mode; however, in 1.5.0 and later, they are, by default, not in immediate mode, so if pcap_set_immediate_mode() is available, it should be used.

So if versions below 1.5 are still around you have to use version detection o_O? Your version detection is build-time static, correct? So if you ship binaries you'll have to take care of it manually?

Jessie support ended at the end of june, but Centos 6 is indeed still maintained until December. Winpcap was abandoned, it won't get security updates or support. Npcap has a higher libpcap api level, but their license might be unsuitable for some.

[sekineh]

Is tokio feature very useful in pcap crate? I'm not sure whether it is essential.

My perception is that tokio is for high performance NW things where thousands of users/contexts exist simultaneously. My guess is that if you capture thousands of interfaces simultaneously, it might be useful.... Do you know specific use cases?

[Wojtek242]

The same thought crossed my mind, but based on the issues some people do appear to use it #90 and #108.

I would keep the feature for now while we get everything up-to-date and then re-evaluate. Perhaps some other people will chip in.

[Wojtek242]

My guess is that if you capture thousands of interfaces simultaneously, it might be useful.... Do you know specific use cases?

Not thousands, but my original use case for this package was to develop a software switch for P4 (there is a C++ software switch, I just wanted to make a Rust one) and in that case I would like to capture and send on an arbitrary number of interfaces. The C++ software-switch also uses libpcap.

Despite the fact that libpcap was made for packet captures, it is also the most suitable library there is if you want to send packets with arbitrary headers (so potentially not even Ethernet).

[sekineh]

Software switch is a nice use case. Thank you for the explanation.

My guess is that if you capture thousands of interfaces simultaneously, it might be useful....

Do you know specific use cases?

Not thousands, but my original use case for this package was to develop a software switch for P4 (there is a C++ software switch, I just wanted to make a Rust one) and in that case I would like to capture and send on an arbitrary number of interfaces. The C++ software-switch also uses libpcap.

Despite the fact that libpcap was made for packet captures, it is also the most suitable library there is if you want to send packets with arbitrary headers (so potentially not even Ethernet).

[Wojtek242]

Right, so CentOS 6 reaches EOL by December so I think it's fair to ignore it. CentOS 7 uses libpcap 1.5.3 which is a much more convenient boundary.

Then the other question is how to handle this. As @Trolldemorted the solution on my fork (https://github.com/Wojtek242/pcap/pull/3/files) is build-time static. A dynamic version should be doable though since libpcap exposes a function to detect versions. I will have a look into it

[sekineh]

Features other than tokio (Now `capture-stream') have the purpose of switching individual APIs on/off. Because the availability of APIs are dependent of the PCAP version (1.5.3 or 1.8.1, etc.), so the libpcap version "level" will better serve than each feature names such as "pcap-savefile-append", "pcap-fopen-offline-precision", etc.

Configuation through enviroment variable might be a bit controvertial, and we need to pick proper name since it's like a global variable. If there's better configuration mechanism, we might want to try, but I don't have a concrete idea for it. So I'm positive to your plan of "dynamic version probe (nice fit for compile-and-run people) + environment variable override (nice for ship-and-run people)" approach.

I tried @Wojtek242 's folk, about 3 months ago, and it looked working. What I noticed was when I changed the environment variable, I had to run cargo clean manually. This feeled quite normal thing to me.

[sekineh]

Another approach would be:

Version-based feature aliases

[features]
libpcap-1.8.1 = ["aaa", "bbb"]
libpcap-1.5.3 = ["aaa"]

Pro:

• It is more standard way of doing feature gating.
• Edit: Users won't need to set env.

  [dependencies]
  pcap = { version = "0.8.0", features = ["libpcap-1.8.1"] }

Con:

• It does not allow dynamic detection. (But in a sense, it feels like a right thing.)
• Features are defined in two layer ("libpcap-x.x.x" and "aaa"/"bbb").

[sekineh]

What do you think? @Wojtek242 @Trolldemorted

Version-based features Rev.2

Cargo.toml:

[features]
libpcap-1.8.1 = [libpcap-1.7.2] # includes all previous APIs
libpcap-1.7.2 = [libpcap-1.5.3] # includes all previous APIs
libpcap-1.5.3 # minimum supported version + 1 increment

In *.rs files, we mark functions as below:

    #[cfg(feature = "libpcap-x.y.z")]
    pub fn pcap_fopen_offline_with_tstamp_precision(arg1: *mut FILE, arg2: c_uint,
                                                    arg3: *mut c_char) -> *mut pcap_t;

Pro:

• It is more standard way of doing feature gating. (compared to environmet variables or emitting cargo:rustc-cfg= in build.rs)
• Users don't need to set env. Instead they can specify libpcap version naturally. Users' Cargo.toml:

  [dependencies]
  pcap = { version = "0.8.0", features = ["libpcap-1.8.1"] }

Con:

• It does not allow dynamic detection. (But in a sense, it feels like a right thing.)
• ~Features are defined in two layer ("libpcap-x.x.x" and "aaa"/"bbb").~

[sekineh]

Windows: npcap claims to support libpcap-1.9.1

https://nmap.org/npcap/vs-winpcap.html The above page claims that WinPcap is libpcap-1.0.0.

Windows: WinPcap 4.1.3/Wpdpack 4.1.2 supports ~2.4~ libpcap-1.0.0?

2.4 is a file format version, not libpcap version. ~They say 2.4, but probably it's not the information we want....~ https://github.com/wireshark/winpcap/blob/267327e28031d2d3d74c28cf18a08dfbc515071b/dox/libpcap/incs/pcap.h#L56-L57

• Edit: wpcap.lib exported functions are listed in the gist below. Looks like only plain features are supported. https://gist.github.com/sekineh/8add20a6df916697f2c9c1c62e98c217

[sekineh]

Q) What is the minimum libpcap version we should support?

A) WinPcap coverage or libpcap-1.0.0 coverage

WinPcap looks like very old (libpcap-1.0.0?) and Unix platforms ship relatively recent versions.

So in raw.rs,

• We can ~use~ declare any functions WinPcap support without any cfg check.
• Anything other than the above requires cfg check.

[Wojtek242]

What do you think? @Wojtek242 @Trolldemorted

Version-based features Rev.2

Cargo.toml:

[features]
libpcap-1.8.1 = [libpcap-1.7.2] # includes all previous APIs
libpcap-1.7.2 = [libpcap-1.5.3] # includes all previous APIs
libpcap-1.5.3 # minimum supported version + 1 increment

I'm not really in favour of this solution. Cargo features have some annoying properties. One of them is that you cannot have exclusive features - that is you cannot make it so that if you enable libpcap-1.8.1 then libpcap-x.x.x for all other versions is disabled and I think it's messy to define a feature libpcap-1.8.1 which includes all previous APIs, but then you can also define libpcap-1.7.2. I think it's confusing and not very consistent. It's also slightly questionable to me why would the user of the library have to go and read documentation to figure out which version he should use to enable the API he wants to use.

The solution on my branch is build-time static so it isn't really fully dynamic. It's dynamic only if you're the one compiling the code. If that's the solution we opt for, I don't think an environment variable is a problem in this case, because you can always just set it exclusively during compilation.

Another option is to have it fully dynamic at runtime. This should be possible as libpcap exposes the function pcap_liv_version. Then we use dynamic dispatch to throw an error if an API that is not available in the current version is being called, but otherwise to work normally. I think this is the best solution from the pcap library user's point of view. The user just writes code using whatever API they feel like. Then if somebody else takes their code and tries to run it on an older libpcap version they will get an error that they need to update libpcap.

[Wojtek242]

The last option I mentions is more complicated, but I could try a proof-of-concept this weekend maybe.

[Wojtek242]

WinPcap looks like very old (libpcap-1.0.0?) and Unix platforms ship relatively recent versions.

WinPcap is awkward. It is based on 1.0.0, but it does support some API calls from later versions

We can use declare any functions WinPcap support without any cfg check.

Sounds like the best solution

[sekineh]

I'm not really in favour of this solution. Cargo features have some annoying properties. One of them is that you cannot have exclusive features - that is you cannot make it so that if you enable libpcap-1.8.1 then libpcap-x.x.x for all other versions is disabled and I think it's messy to define a feature libpcap-1.8.1 which includes all previous APIs, but then you can also define libpcap-1.7.2. I think it's confusing and not very consistent. It's also slightly questionable to me why would the user of the library have to go and read documentation to figure out which version he should use to enable the API he wants to use.

I think this

[features]
libpcap-1.8.1 = [libpcap-1.7.2] # includes all previous APIs
libpcap-1.7.2 = [libpcap-1.5.3] # includes all previous APIs
libpcap-1.5.3 # minimum supported version + 1 increment

and this

    // build.rs
    let api_vers: Vec<Version> = vec![
        Version::new(1, 2, 1),
        Version::new(1, 5, 0),
        Version::new(1, 7, 2),
        Version::new(1, 9, 0),
        Version::new(1, 9, 1),
    ];

    for v in api_vers.iter().filter(|&v| v <= &version) {
        println!("cargo:rustc-cfg=pcap_{}_{}_{}", v.major, v.minor, v.micro);
    }

are the same thing, but just expressed in a different language. I feel build.rs approach has a bit bigger mental load. https://github.com/Wojtek242/pcap/blob/7f20f08eaa420791f59d524565982d98865aa479/build.rs#L94-L110

The solution on my branch is build-time static so it isn't really fully dynamic. It's dynamic only if you're the one compiling the code. If that's the solution we opt for, I don't think an environment variable is a problem in this case, because you can always just set it exclusively during compilation.

Another option is to have it fully dynamic at runtime. This should be possible as libpcap exposes the function pcap_liv_version. Then we use dynamic dispatch to throw an error if an API that is not available in the current version is being called, but otherwise to work normally. I think this is the best solution from the pcap library user's point of view. The user just writes code using whatever API they feel like. Then if somebody else takes their code and tries to run it on an older libpcap version they will get an error that they need to update libpcap.

Fully dynamic apparently require greater endeaver. It feels like very opinionated , have many pros and cons.

Con: I'm a big fan of the "fail early" strategy. If the runtime platform does not have required function, it should fail on startup, not emitting Err when you try to call it.

I think we should release 0.8.0 without big change such as version probe (at runtime).

[sekineh]

Question:

• we only have two features which are version dependent.
  • pcap-savefile-append
  • pcap-fopen-offline-precision
• --features full serves great on modern linux and macosx platforms.

What is the problem you are trying to fix by version probe and version-based cfg control? Even 'Version-based features Rev.2' is not needed, in my opinion.

[sekineh]

Here's my evaluation of each version probe.

Build-time version probe

Probe version in build.rs and use it for cfging raw functions.

Pros

• It's automatic: users typically don't know what features are required by themselves. It just works for many people.
• Serves as a infra to append more version-dependent functions

Con

• ENV override is not good way of software configuration.
  • Add to bashrc and you'll forget that. It will become a source of problems. (reduces build reproducibility)
  • More difficult to change: If you use IDE, you need to relanch it after ENV change.
• Implicitness is a bad thing. The below is the more standard and explicit way of crate configuration.

  [dependencies]
  pcap = { version = "0.8.0", features = ["libpcap-1.8.1"] }

Fully Dynamic (runtime) version probe

Probe version in runtime and change behaviour using that information.

Pros

• You can ship software that run on verious environment. (Think of wireshark-like end user tool, for example.)

Cons

• API change is required (returning Result, etc.)
• Against fail-early strategy.

[sekineh]

I created #136 (build-time verion probe) and #137 (runtime version probe) for each possibility.

[Wojtek242]

I see your point about fail-early. As I thought what the use case would be for dynamic runtime checking it would mostly be essentially indistinguishable from a static build-time check upfront. A developer using pcap would have to choose a minimum required libpcap version and all of their users would need to have libpcap version.

However, one benefit of runtime checking is that it would allow for said developer to support a super new version of libpcap, but only the features that use the newest API calls would be unavailable to the user. A potential scenario is that 98% of a particular library uses the 1.5.0 API, but the 2% uses API from 1.7.2. With dynamic runtime checking the users of that library still get to use 98% of the functionality rather than 0%. However, this is easier said than done as libpcap also sometimes changes the behaviour of existing API calls across versions (e.g. immediate mode on Linux).

The runtime checking has potential, but doesn't justify the complexity. Let's leave #137 open and see if people express interest in it. If they don't then we don't implement it.

[Wojtek242]

think this

[features]
libpcap-1.8.1 = [libpcap-1.7.2] # includes all previous APIs
libpcap-1.7.2 = [libpcap-1.5.3] # includes all previous APIs
libpcap-1.5.3 # minimum supported version + 1 increment

and this

    // build.rs
    let api_vers: Vec<Version> = vec![
        Version::new(1, 2, 1),
        Version::new(1, 5, 0),
        Version::new(1, 7, 2),
        Version::new(1, 9, 0),
        Version::new(1, 9, 1),
    ];

    for v in api_vers.iter().filter(|&v| v <= &version) {
        println!("cargo:rustc-cfg=pcap_{}_{}_{}", v.major, v.minor, v.micro);
    }

are the same thing, but just expressed in a different language. I feel build.rs approach has a bit bigger mental load. https://github.com/Wojtek242/pcap/blob/7f20f08eaa420791f59d524565982d98865aa479/build.rs#L94-L110

There is still a very important difference. The first one requires the user to intentionally opt into later libpcap versions even if they already have it installed. This may be confusing if said user just downloaded pcap and some API call he could call from C is not being called from Rust. The latter option is opt-out. That is a user must consciously choose to compile against an older API version. Features depending on features is a fancy feature I haven't thought about though. If it can be made opt-out in a non-confusing manner then it would be the best solution though.

[Wojtek242]

In this case, I'll wait with submitting any PR until we decide if we want to use build.rs vs features. I'm happy to drop runtime for now. However, I do think we need this for 0.8.0, because without this PRs by others can get really complex and lead to a proliferation of forks (see #55).

[Wojtek242]

My vote is for build.rs unless there is a convenient way to make newer versions opt-out rather than opt-in using features.

[sekineh]

I also vote for build.rs. It will simplify most cases.

Prior art study: How openssl-sys handle the same problem? = build.rs

They support many versions of openssles and libressl with the single crate. build.rs approach is used (emitting many cargo:rustc-cfg={} lines).

https://github.com/sfackler/rust-openssl/blob/0a0da84f939090f72980c77f40199fc76245d289/openssl-sys/build/main.rs#L193-L195