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

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 5 Sep 2024 16:18:13 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:

> > 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. :-)
> > >  

My feeling is that the the experimental flag is not intended as permanent "get out of ABI compatiablity"