Hi David, > OVS and some other applications have been hacking into DPDK internals to > fake EAL threads and avoid performance penalty of only having non-EAL > threads. > > This series proposes to add a new type of lcores and maps those threads > to such lcores. > non-EAL threads won't run the DPDK eal mainloop. > As a consequence, part of the EAL threads API cannot work. > > Having new lcores appearing during the process lifetime is not expected > by some DPDK components. This is addressed by introducing init/uninit > callacks invoked when hotplugging of such lcore. > > There is still some work/discussion: > - refuse new lcore role in incompatible EAL threads API (or document it > only as those API were already incompatible?), > - think about deprecation notices for existing RTE_FOREACH_LCORE macros > and consorts, it is probably worth discussing on how to iterate over > lcores, > > For the interested parties, I have a patch [1] against dpdk-latest OVS > branch that makes use of this series (this patch probably won't work with > v5, it will be rebased once dpdk side is ready). > > 1: > https://patchwork.ozlabs.org/project/openvswitch/patch/20200626123017.28555-1-david.march...@redhat.com/ > > Changes since v5: > - fixed windows build, > > Changes since v4: > - added separate API to control mp feature activation, > - addressed Konstantin and Olivier comments, > > Changes since v3: > - added init failure when trying to use in conjunction with multiprocess, > - addressed Andrew comments, > > Changes since v2: > - fixed windows build error due to missing trace stub, > - fixed bug when rolling back on lcore register, > > Changes since v1: > - rebased on master (conflicts on merged Windows series), > - separated lcore role code cleanup in a patch, > - tried to use a single naming, so kept non-EAL threads as the main > notion. non-EAL threads are then distinguished between registered and > unregistered non-EAL threads, > - added unit tests (still missing some coverage, marked with a FIXME), > - reworked callbacks call under a common rwlock lock which protects > lcores allocations and callbacks registration, > - introduced lcore iterators and converted the bucket mempool driver, >
LGTM, just 2 nits see below. Apart from that: Series Acked-by: Konstantin Ananyev <konstantin.anan...@intel.com> 1. +void +rte_lcore_callback_unregister(void *handle) +{ + struct rte_config *cfg = rte_eal_get_configuration(); + struct lcore_callback *callback = handle; + unsigned int lcore_id; Seems like forgot to add formal parameter check here: if (callback == NULL) ... + + rte_rwlock_write_lock(&lcore_lock); + if (callback->uninit == NULL) 2. +bool +rte_mp_disable(void) +{ + return set_mp_status(MP_STATUS_DISABLED); +} Probably name it rte_eal_multiprocess_enable (or so) to make it clear from naming and follow more closely our own name convention. + +bool +eal_enable_multiprocess(void) +{ + return set_mp_status(MP_STATUS_ENABLED); +} Might be worth to make that function public too. Then user will have a proper pair to use: rte_eal_multiprocess_(enable|disable).