Re: [dpdk-dev] RFC - adding filter to packet capture API

[Ray Kinsella <mdr@ashroe.eu>]

On 06/12/2019 22:11, Stephen Hemminger wrote:
> In the process of updating packet capture to take a filter program, there
> is one open question about API/ABI.
> 
> The example is:
> 
> int
> rte_pdump_enable(uint16_t port, uint16_t queue, uint32_t flags,
> 		struct rte_ring *ring,
> 		struct rte_mempool *mp,
> 		void *filter);
> 
> For the new version want to add ability to pass a BPF (classic) program
> from libcap. This is described in most pcap API's as "struct bpf_program".
> 
> The filter parameter was left as a placeholder, but in typical YAGNI
> fashion. When we do need to use it, it is not going to work out.
> 
> Since the existing filter argument was never used, there are a bunch
> of options (in order from best to worse).
> 
> 1. Introduce new API which takes a filter. 
> 
> int
> rte_pdump_enable_bpf(uint16_t port, uint16_t queue, uint32_t flags,
> 		struct rte_ring *ring,
> 		struct rte_mempool *mp,
> 		const struct bpf_program *filter);
> 
> The old API is just the same as calling new API with NULL as filter.
> This solution is safe but adds minor bloat.
> 
> 2. Use API versioning.  This solves the ABI problem but it is still
>    an API breakage since program that was passing junk would still
>    not compile.

I honestly think this is the best option.
Our commitment with ABI Stability is only not to break applications linked against 19.11.

If someone is rebuilding against v20.02 and is passing something into the void *.
It is probably no-harm the compiler is giving them an FYI that things have changed.

> 
> 3. Change the function signature of existing API. This risks breaking
>    at compile time some program that was passing some other value.
>    Similarly, a program could have passed garbage, we never checked.
> 
> 4. Keep existing function signature, but be type unsafe.
>    This keeps API, but still is ABI breakage for programs that passed
>    garbage. Plus C is unsafe enough already.

True, but implementing ABI versioning is really trivial in this case, and avoids this.