On Tue, Sep 24, 2024 at 9:23 PM Morten Brørup <m...@smartsharesystems.com> wrote: > > > From: Jerin Jacob [mailto:jerinjac...@gmail.com] > > Sent: Tuesday, 24 September 2024 11.42 > > > > On Tue, Sep 24, 2024 at 7:10 PM Morten Brørup <m...@smartsharesystems.com> > > wrote: > > > > > > Some applications want to omit the trace feature. > > > Either to reduce the memory footprint, to reduce the exposed attack > > > surface, or for other reasons. > > > > > > This patch adds an option in rte_config.h to include or omit trace in > > the > > > build. Trace is included by default. > > > > > > Omitting trace works by omitting all trace points. > > > For API and ABI compatibility, the trace feature itself remains. > > > > > > Signed-off-by: Morten Brørup <m...@smartsharesystems.com> > > > --- > > > v4: > > > * Added check for generic trace enabled when registering trace points, > > in > > > RTE_INIT. (Jerin Jacob) > > > * Test application uses function instead of macro to check if generic > > > trace is enabled. (Jerin Jacob) > > > * Performance test application uses function to check if generic trace > > is > > > enabled. > > > v3: > > > * Simpler version with much fewer ifdefs. (Jerin Jacob) > > > v2: > > > * Added/modified macros required for building applications with trace > > > omitted. > > > > > > > > +/** > > > + * @internal > > > > Since it is used in app/test. Can we remove it as internal and make > > symbol as rte_trace_point_is_enabled > > I don't think we should make functions public if only used for test purposes. > > Do you think it is useful for normal usage too? Does rte_trace_is_enabled() > not suffice?
It will be good for an app to know, Is trace feature disabled if the application cares about. Yes. rte_trace_is_enabled() suffice. > > > > > > + * > > > + * Test if the tracepoint compile-time option is enabled. > > > + * > > > + * @return > > > + * true if tracepoint enabled, false otherwise. > > > + */ > > > +__rte_experimental > > > +static __rte_always_inline bool > > > +__rte_trace_point_generic_is_enabled(void) > > > > Do we need to keep _generic_ > > Other internal functions have _generic_ too, so I added it. > If we decide to make it public, I agree _generic_ should be removed. > > > > > Rest looks good to me.
