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

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 9/5/2024 3:01 PM, Jerin Jacob wrote:
> 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. However, I don't understand why we need to do
> that. 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.
> 

I agree with Jerin, I don't see motivation to disable slow path traces
when they can be disabled in runtime.
And fast path traces already have compile flag to disable them.

Build time configurations in long term has problems too, so I am for not
using them unless we don't have to.

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