> 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. > > 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. >
My preference is probably #4, with some extra changes: make actual type for 'filter' be determined by flags, something like: enum { RTE_PDUMP_FLAG_RX = 1, /* receive direction */ RTE_PDUMP_FLAG_TX = 2, /* transmit direction */ + RTE_PDUMP_FLAG_CBPF = 4, /* filter points to struct bpf_program */ /* both receive and transmit directions */ RTE_PDUMP_FLAG_RXTX = (RTE_PDUMP_FLAG_RX|RTE_PDUMP_FLAG_TX) };