RE: [PATCH 0/3] eal: mark API's as stable

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Jerin Jacob [mailto:jerinjacobk@gmail.com]
> Sent: Thursday, 5 September 2024 16.02
> 
> On Thu, Sep 5, 2024 at 3:14 PM Morten Brørup <mb@smartsharesystems.com> wrote:
> >
> > > From: David Marchand [mailto:david.marchand@redhat.com]
> > > Sent: Thursday, 5 September 2024 11.03
> > >
> > > On Thu, Sep 5, 2024 at 10:55 AM Morten Brørup <mb@smartsharesystems.com>
> > > wrote:
> > > >
> > > > > From: David Marchand [mailto:david.marchand@redhat.com]
> > > > > Sent: Thursday, 5 September 2024 09.59
> > > > >
> > > > > On Wed, Sep 4, 2024 at 8:10 PM Stephen Hemminger
> > > > > <stephen@networkplumber.org> wrote:
> > > > > >
> > > > > > The API's in ethtool from before 23.11 should be marked stable.
> > > > >
> > > > > EAL* ?
> > > > >
> > > > > > Should probably include the trace api's but that is more complex
> change.
> > > > >
> > > > > On the trace API itself it should be ok.
> > > >
> > > > No!
> > >
> > > *sigh*
> > >
> > > >
> > > > Trace must remain experimental until controlled by a meson option, e.g.
> > > "enable_trace", whereby trace can be completely disabled and omitted from
> the
> > > compiled application/libraries/drivers at build time.
> > >
> > > This seems unrelated to marking the API stable as regardless of the
> > > API state at the moment, this code is always present.
> >
> > I cannot foresee if disabling trace at build time will require changes to
> the trace API. So I'm being cautious here.
> >
> > However, if Jerin (as author of the trace subsystem) foresees that it will
> be possible to disable trace at build time without affecting the trace API, I
> don't object to marking the trace API (or some of it) stable.
> 
> I don't for foresee any ABI changes when adding disabling trace
> compile time support.

Based on Jerin's feedback, I'm retracting my objection.

> However, I don't understand why we need to do
> that.

To reduce code size.
Relevant for embedded/memory-constrained systems.

> In the sense, fast path functions are already having an option
> to compile out.
> Slow path functions can be disabled at runtime at the cost of 1 cycle
> as instrumentation cost. Having said that, I don't have any concern
> about disabling trace as an option.

Great.

> 
> 
> >
> > Before doing that, rte_trace_mode_get/set() and the accompanying enum
> rte_trace_mode should be changed to rte_trace_config_get/set() using a new
> struct rte_trace_config (containing the enum rte_trace_mode, and expandable
> with new fields as the need arises). This will prepare for e.g. tracing to
> other destinations than system memory, such as a remote trace collector on the
> network, like SYSLOG.

I'm also retracting this precondition...

If the need for further trace configuration should ever arise, rte_trace_config_get/set() can be added later.
And rte_trace_mode_get/set(), if not marked as experimental anymore, will be kept for backwards compatibility.

> >
> > > Patches welcome if you want it stripped.
> >
> > Don't have time myself, so I suggested it as a code challenge instead. :-)
> >