[EXT] Re: [PATCH v2 1/4] ethdev: add trace points

[David Marchand]

On Wed, Oct 12, 2022 at 11:50 AM Jerin Jacob <jerinjacobk at gmail.com> wrote:
> On Thu, Oct 6, 2022 at 1:27 PM David Marchand <david.marchand at redhat.com> wrote:
> > On Thu, Oct 6, 2022 at 9:50 AM Andrew Rybchenko
> > <andrew.rybchenko at oktetlabs.ru> wrote:
> > > >>>>> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map index
> > > >>>>> 3def7bfd24..e3d603cc9a 100644
> > > >>>>> --- a/lib/ethdev/version.map
> > > >>>>> +++ b/lib/ethdev/version.map
> > > >>>>> @@ -288,6 +288,150 @@ EXPERIMENTAL {
> > > >>>>>
> > > >>>>>           # added in 22.11
> > > >>>>>           rte_flow_async_action_handle_query;
> > > >>>>> + __rte_eth_trace_add_first_rx_callback;
> > > >>>>
> > > >>>> Why is it in EXPERIMENTAL section, but not INTERNAL?
> > > >>> [Ankur] Because the functions for which trace is added are not internal
> > > >> functions.
> > > >>
> > > >> Sorry, but I don't understand. I agree that tracing of public inline functions
> > > >> must be part of ABI, but why everything else should be a part of ABI?
> > > > [Ankur] I see that there are some already existing trace functions added in EXPERIMENTAL in version.map like __rte_ethdev_trace_configure, __rte_ethdev_trace_rxq_setup. So not sure will it be internal or experimental.
> > > >
> > > > But you are right the trace function will not be called as a public api. Should I make the newly added trace as internal then?
> > >
> > > @David, do I understand correctly that trace points in
> > > EXPERIMENTAL is a mistake in majority of cases?
> >
> > The trace point global variables (__rte_trace_foo)  are only exposed
> > for inline helpers that might call their associated trace point helper
> > (rte_trace_foo()).
> > An application is not supposed to directly manipulate them.
> > Any tp manipulation should be through the rte_trace_point_* API.
> >
> > Jerin, do you see any other uses for them?
>
> No.  Expect the following ones, which can be used by application directly.
>
>         __rte_eal_trace_generic_float;
>         __rte_eal_trace_generic_func;
>         __rte_eal_trace_generic_i16;
>         __rte_eal_trace_generic_i32;
>         __rte_eal_trace_generic_i64;
>         __rte_eal_trace_generic_i8;
>         __rte_eal_trace_generic_int;
>         __rte_eal_trace_generic_long;
>         __rte_eal_trace_generic_ptr;
>         __rte_eal_trace_generic_str;
>         __rte_eal_trace_generic_u16;
>         __rte_eal_trace_generic_u32;
>         __rte_eal_trace_generic_u64;
>         __rte_eal_trace_generic_u8;

Indeed, that's something I discussed offlist after with Andrew but
forgot to put back on the mailing list.
As long as the trace point is called from a helper in a header exposed
to applications, we can't mark the trace point variable internal.


-- 
David Marchand