> -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ananyev, Konstantin > Sent: Monday, December 9, 2019 2:42 PM > > > 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) > }; >
I like Konstantin's idea of providing the filter type as a flag, as it still leaves the filter parameter open for other filter types in the future. It also allows using the existing pdump_request structure (and associated client/server infrastructure) as is. And I appreciate very much that name of the flag explicitly indicates that the filter type is cBPF (not just BPF, which in librte_bpf actually means eBPF). Did I mention that I hate the use of the name "BPF" instead of "eBPF", because "BPF" used to mean what is today also known as "cBPF"... Med venlig hilsen / kind regards - Morten Brørup
