On 1/31/2023 6:46 PM, Jerin Jacob wrote: > On Wed, Feb 1, 2023 at 12:09 AM Ferruh Yigit <ferruh.yi...@amd.com> wrote: >> >> On 1/30/2023 4:01 PM, Ankur Dwivedi wrote: >> >> <...> >> >>>>> diff --git a/lib/ethdev/meson.build b/lib/ethdev/meson.build index >>>>> 39250b5da1..f5c0865023 100644 >>>>> --- a/lib/ethdev/meson.build >>>>> +++ b/lib/ethdev/meson.build >>>>> @@ -24,6 +24,7 @@ headers = files( >>>>> 'rte_ethdev.h', >>>>> 'rte_ethdev_trace.h', >>>>> 'rte_ethdev_trace_fp.h', >>>>> + 'rte_ethdev_trace_fp_burst.h', >>>> >>>> Why these trace headers are public? >>>> Aren't trace points only used by the APIs, so I expect them to be >>>> internal, so >>>> applications shouldn't need them. Why they are expsed to user. >>> 'rte_ethdev_trace.h' can be made as internal. Not sure about >>> 'rte_ethdev_trace_fp.h' and 'rte_ethdev_trace_fp_burst.h' as the >>> tracepoints in fast path may be called from public inline functions. >> >> Trace calls used by inline functions needs to be public, in this case at >> least 'rte_ethdev_trace_fp_burst.h' needs to be public. >> >> Can you please at least move all trace points that are called by inline >> functions to the same file, 'rte_ethdev_trace_fp_burst.h', to reduce >> number of the header files to make public? Feel free to rename header if >> required. >> >> Meanwhile not sure about adding new header as dependency to end user. >> @Jerin, @Andrew, what do you think > > rte_ethdev_trace_fp_burst.h will be installed through ninja install > and application does not need to directly include this. So this scheme > is OK. Right? I dont see any downside. >
Right. It is installed automatically with above meson change, and it is included by 'rte_ethdev.h', so user doesn't need to include it explicitly. Overall there is no functional problem here. Only this header file needs to be included (directly or indirectly) by every application that use ethdev. I would much prefer to have an internal header but not able to because of technical reasons (inline functions). After lots of effort we did to hide as much ethdev internals as we can, now we are exposing some new trace stuff to user. As we can't prevent header to be public, I am just questioning below options to reduce exposure of this header, hoping that it may lead a better solution. > >> 1) to move these trace points to 'rte_ethdev_core.h' >> OR >> 2) disable trace calls in inline functions on compile time, possibly >> with existing 'RTE_ETHDEV_DEBUG_*' macro
