On Thu, Jul 2, 2020 at 3:09 PM Ilya Maximets <[email protected]> wrote: > > On 6/26/20 2:27 PM, David Marchand wrote: > > DPDK lcores range is [0, RTE_MAX_LCORE[. > > Trying to use a lcore out of this range expose OVS to crashes in DPDK > > mempool > > local cache array. > > Prefer a "clean" abort so that users know that something is wrong with > > their configuration. > > > > Signed-off-by: David Marchand <[email protected]> > > --- > > lib/dpdk.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/lib/dpdk.c b/lib/dpdk.c > > index 98d0e2643e..55ce9a9221 100644 > > --- a/lib/dpdk.c > > +++ b/lib/dpdk.c > > @@ -617,7 +617,14 @@ dpdk_set_lcore_id(unsigned cpu) > > { > > /* NON_PMD_CORE_ID is reserved for use by non pmd threads. */ > > ovs_assert(cpu != NON_PMD_CORE_ID); > > + if (cpu >= RTE_MAX_LCORE) { > > + cpu = LCORE_ID_ANY; > > + } > > RTE_PER_LCORE(_lcore_id) = cpu; > > + if (rte_lcore_id() == LCORE_ID_ANY) { > > + ovs_abort(0, "PMD thread init failed, trying to use more cores > > than " > > + "DPDK supports (RTE_MAX_LCORE %u).", RTE_MAX_LCORE); > > I don't think that random OVS abort after pmd-cpu-mask change or port addition > is a good thing to have. Maybe it's better to limit CPU core discovery? > This way OVS will never try to create thread on unwanted cores. > > Since ovs-numa module is mostly used by userspace datapath this will not > affect non-DPDK setups. > > Something like this (not tested): > > diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c > index 6d0a68522..7c8a3b036 100644 > --- a/lib/ovs-numa.c > +++ b/lib/ovs-numa.c > @@ -27,6 +27,7 @@ > #include <unistd.h> > #endif /* __linux__ */ > > +#include "dpdk.h" > #include "hash.h" > #include "openvswitch/hmap.h" > #include "openvswitch/list.h" > @@ -58,6 +59,13 @@ VLOG_DEFINE_THIS_MODULE(ovs_numa); > > #define MAX_NUMA_NODES 128 > > +#ifdef DPDK_NETDEV > +/* DPDK will not work correctly for cores with core_id >= RTE_MAX_LCORE. */ > +#define MAX_CORE_ID (RTE_MAX_LCORE - 1) > +#else > +#define MAX_CORE_ID INT_MAX > +#endif > + > /* numa node. */ > struct numa_node { > struct hmap_node hmap_node; /* In the 'all_numa_nodes'. */ > @@ -112,6 +120,12 @@ insert_new_numa_node(int numa_id) > static struct cpu_core * > insert_new_cpu_core(struct numa_node *n, unsigned core_id) > { > + if (core_id > MAX_CORE_ID) { > + VLOG_DBG("Ignoring CPU core with id %u ( %u > %d).", > + core_id, core_id, MAX_CORE_ID); > + return NULL; > + } > + > struct cpu_core *c = xzalloc(sizeof *c); > > hmap_insert(&all_cpu_cores, &c->hmap_node, hash_int(core_id, 0)); > @@ -278,6 +292,11 @@ ovs_numa_init(void) > if (ovsthread_once_start(&once)) { > const struct numa_node *n; > > + if (MAX_CORE_ID != INT_MAX) { > + VLOG_INFO("Maximum allowed CPU core id is %d. " > + "Other cores will not be available.", MAX_CORE_ID); > + } > + > if (dummy_numa) { > discover_numa_and_core_dummy(); > } else { > --- > > What do you think?
pmd threads will never run on any core >= RTE_MAX_LCORE ok, it works. Though, it binds OVS core id and DPDK lcore id. Should OVS really care about this? -- David Marchand _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
