On Wed, Nov 30, 2022 at 02:54:27PM -0800, Tyler Retzlaff wrote:
> hi folks,
>
> i'd like to continue work moving to our platform abstracted rte_thread
> but ran into a hiccup. for some recent and not so recent apis it appears
> they managed to slip in without ever being __experimental.
>
> as a function of the dpdk project api/abi policy this means we can't
> change or remove some of these functions without following the
> deprecation process.
>
> the apis that are causing me immediate difficulty are
> rte_thread_setname and rte_ctrl_thread_create.
after looking in more detail at our current implementations of these
functions i would like to backtrack a little and limit the scope of
discussion to rte_thread_setname and rte_thread_getname.
as eal functions they aren't doing a good job in abstracting the
environment for applications, meaning an application would have to wrap
their use in platform conditional checks.
current status.
rte_thread_getname
* freebsd, no implementation and it appears no possible support
* linux, implementation conditional on __GLIBC_PREREQ(2, 12)
* windows, can be implemented but isn't, noop success
* fortunately is marked __rte_experimental
* called in 1 place only from eal (logging)
i would propose to present a consistent abstraction the best thing to do
here is just remove rte_thread_getname. providing a version that
requires an application to do conditional dances / compilation based on
platform gains nothing.
rte_thread_setname
* freebsd, implemented, imposes no limit on name length, suppresses errors
* linux, implementation conditional on __GLIBC_PREREQ(2, 12), imposes
limit of 16 (including terminating NUL) on name length, may return an
error
* windows, can be implemented, no explicit limit on name length, may
return errors
* unfortunately not marked __rte_experimental
i would propose to provide a replacement with the name
rte_thread_set_name with more consistent behavior across the 3 platforms.
* returns errors for platforms that return errors, but the caller
is free to ignore them.
* explicit limit of 16 (including terminating NUL) on name length,
names that are provided that exceed the limit are truncated without
error.
your feedback would be appreciated.
thanks
> i think the least painful path forward to deprecating and removing these
> apis is probably just to introduce the replacements with new names.
>
> 1. introduce functions with the following names marked as
> __experimental.
>
> rte_control_thread_create(rte_thread_t *, ...)
> rte_thread_set_name(rte_thread_t, ...)
> rte_thread_get_name(rte_thread_t, ...)
>
> along with the new functions, new unit tests will be included.
>
> 2. update dpdk internal implementation to use the new functions.
>
> 3. immediately remove the following functions from the public headers
> and issue an api deprecation notice for the functions not marked
> experimental.
>
> rte_ctrl_thread_create(pthread_t *, ...)
> rte_thread_setname(pthread_t *, ...)
>
> 4. when the new functions have their __experimental marking removed
> issue an abi deprecation notice for the functions from (2).
>
> i'm open to feedback/suggestions of a better approach if anyone has one
> to offer.
>
> thanks!
