> From: Stephen Hemminger [mailto:step...@networkplumber.org] > Sent: Thursday, 9 November 2023 18.16 > > On Thu, 9 Nov 2023 08:50:10 +0100 > Morten Brørup <m...@smartsharesystems.com> wrote: > > > > rte_exit(EXIT_FAILURE, > > > "Packet dump enable on %u:%s failed %s\n", > > > intf->port, intf->name, > > > - rte_strerror(-ret)); > > > + rte_strerror(rte_errno)); > > > > This bugfix (the line above, not the patch itself) supports Tyler's > proposal to standardize on returning -1 with rte_errno set on failure, > instead of some functions returning -errno. Our dual convention for > function return values will cause many bugs like this. > > The error case here is when rte_pdump_enable_bpf() fails. > This is return from pdump_enable in pdump library. > The library does follow the rte_errno convention correctly.
I'm sorry about being unclear in my comment about rte_errno conventions; it was not targeted at this library. My comment was meant as general support for Tyler's suggestion, using this as an example of a bug that would not have been there if the return convention was always -1 with rte_errno. With the dual return convention, it's amazing that you caught this bug. > But the error message wasn't reporting correctly which would lead to > confusing error in case where > multiple invocations failed. > > It is not possible to do multiple captures on same interface. And not > worth modifying the > library (would require multiple copies and ref counts) to handle this > case.
