Re: [dpdk-dev] [PATCH v2] eal: rename state values in rte_lcore_state

[David Marchand <david.marchand@redhat.com>]

On Tue, Apr 2, 2019 at 5:57 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> C language does not really treat enum's as first class symbols.
> The values in an enum live in a global namespace.  That means if
> DPDK defines "RUNNING" it can't be used by another enum in an
> application using DPDK.
>
> To solve this add a prefix "RTE_LCORE_" to the enum values, and
> make them grammatically consistent.
>

Well, I understand this is not clean, but this patch breaks the API.



> Also, simplify the inline thread_is_running() which is copied
> in softnic and ip_pipeline.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v2 - change prefix to RTE_LCORE_
>
>  drivers/event/octeontx/ssovf_evdev_selftest.c |  2 +-
>  drivers/event/sw/sw_evdev_selftest.c          |  4 ++--
>  drivers/net/softnic/rte_eth_softnic_thread.c  |  5 +----
>  examples/ip_pipeline/thread.c                 |  5 +----
>  examples/l2fwd-keepalive/main.c               |  2 +-
>  lib/librte_eal/common/eal_common_launch.c     | 12 ++++++------
>  lib/librte_eal/common/include/rte_launch.h    |  6 +++---
>  lib/librte_eal/common/rte_service.c           |  2 +-
>  lib/librte_eal/freebsd/eal/eal_thread.c       |  6 +++---
>  lib/librte_eal/linux/eal/eal.c                |  2 +-
>  lib/librte_eal/linux/eal/eal_thread.c         |  8 ++++----
>  11 files changed, 24 insertions(+), 30 deletions(-)
>

Missed a few places:

# should probably not access lcore_config directly...
examples/bond/main.c:           if
(lcore_config[global_flag_stru_p->LcoreMainCore].state != WAIT)      {
examples/bond/main.c:   if (lcore_config[slave_core_id].state != WAIT)

# BSD must have trouble building
lib/librte_eal/freebsd/eal/eal.c:               lcore_config[i].state =
WAIT;

# I suppose most comments are quite useless, but still unaligned with your
change
examples/ip_pipeline/thread.c: *    (B) Its data plane thread is in RUNNING
state.
lib/librte_eal/common/eal_common_launch.c: * Check that every SLAVE lcores
are in WAIT state, then call
lib/librte_eal/common/include/rte_eal.h: * It puts the SLAVE lcores in the
WAIT state.
lib/librte_eal/common/include/rte_launch.h: * is in the WAIT state (this is
true after the first call to
lib/librte_eal/common/include/rte_launch.h: * the RUNNING state, then calls
the function f with argument arg. Once the
lib/librte_eal/common/include/rte_launch.h: * execution is done, the remote
lcore switches to a FINISHED state and
lib/librte_eal/common/include/rte_launch.h: *   - (-EBUSY): The remote
lcore is not in a WAIT state.
lib/librte_eal/common/include/rte_launch.h: * Check that each SLAVE lcore
is in a WAIT state, then call
lib/librte_eal/common/include/rte_launch.h: *   - (-EBUSY): At least one
remote lcore is not in a WAIT state. In this
lib/librte_eal/common/include/rte_launch.h: * If the slave lcore identified
by the slave_id is in a FINISHED state,
lib/librte_eal/common/include/rte_launch.h: * switch to the WAIT state. If
the lcore is in RUNNING state, wait until
lib/librte_eal/common/include/rte_launch.h: * the lcore finishes its job
and moves to the FINISHED state.
lib/librte_eal/common/include/rte_launch.h: *   - 0: If the lcore
identified by the slave_id is in a WAIT state.
lib/librte_eal/common/include/rte_launch.h: *     FINISHED or RUNNING
state. In this case, it changes the state
lib/librte_eal/common/include/rte_launch.h: *     of the lcore to WAIT.
lib/librte_eal/common/include/rte_launch.h: * that all slave lcores are in
a WAIT state.
lib/librte_eal/freebsd/eal/eal_thread.c: * remote lcore switch in FINISHED
state.
lib/librte_eal/linux/eal/eal_thread.c: * remote lcore switch in FINISHED
state.
lib/librte_eal/linux/eal/eal_thread.c:          /* when a service core
returns, it should go directly to WAIT


-- 
David Marchand