--Shahaf
> -----Original Message----- > From: Wu, Jingjing [mailto:jingjing...@intel.com] > Sent: Monday, May 8, 2017 3:54 AM > To: Shahaf Shuler <shah...@mellanox.com> > Cc: dev@dpdk.org; Thomas Monjalon <tho...@monjalon.net>; > sta...@dpdk.org > Subject: RE: [PATCH v2] app/testpmd: support non contiguous socket ids > > > > > -----Original Message----- > > From: Shahaf Shuler [mailto:shah...@mellanox.com] > > Sent: Sunday, May 7, 2017 2:06 PM > > To: Wu, Jingjing <jingjing...@intel.com> > > Cc: dev@dpdk.org; Thomas Monjalon <tho...@monjalon.net>; > > sta...@dpdk.org > > Subject: RE: [PATCH v2] app/testpmd: support non contiguous socket ids > > > > Saturday, May 6, 2017 4:41 AM, Wu, Jingjing: > > > > > > > > The test assumes the socket ids are contiguous. This is not > > > > necessarily the case on all servers and may cause mempool creation to > fail. > > > > > > > > Fixing it by detecting the list of valid socket ids and use it for > > > > the mempool creation. > > > > > > > > Fixes: 7acf894d07d1 ("app/testpmd: detect numa socket count") > > > > > > > > CC: sta...@dpdk.org > > > > Signed-off-by: Shahaf Shuler <shah...@mellanox.com> > > > > --- > > > > on v2: > > > > * fix minor typo on commit message : be->by. > > > > --- > > > > app/test-pmd/parameters.c | 38 ++++++++++++++++++++++++++++- > ---- > > > ----- > > > > app/test-pmd/testpmd.c | 38 +++++++++++++++++++++++++++++- > ----- > > > --- > > > > app/test-pmd/testpmd.h | 4 +++- > > > > 3 files changed, 60 insertions(+), 20 deletions(-) > > > > > > > > [..] > > > > > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index > > > > dfe64427d..a556a8aff 100644 > > > > --- a/app/test-pmd/testpmd.c > > > > +++ b/app/test-pmd/testpmd.c > > > > [..] > > > > > > +/* > > > > * Setup default configuration. > > > > */ > > > > static void > > > > @@ -388,12 +405,14 @@ set_default_fwd_lcores_config(void) > > > > > > > > nb_lc = 0; > > > > for (i = 0; i < RTE_MAX_LCORE; i++) { > > > > - sock_num = rte_lcore_to_socket_id(i) + 1; > > > > - if (sock_num > max_socket) { > > > > - if (sock_num > RTE_MAX_NUMA_NODES) > > > > - rte_exit(EXIT_FAILURE, "Total sockets > > > greater > > > > than %u\n", RTE_MAX_NUMA_NODES); > > > > - max_socket = sock_num; > > > > + sock_num = rte_lcore_to_socket_id(i); > > > +1 is missed? > > > > I don't think so. > > On previous implementation this logic was meant to find the max_socket. > > max_socket was the first invalid socket_id and was used, for example : > > > > if (socket_id >= max_socket) { > > > > or > > > > for (i = 0; i < max_socket; i++) > > > > now, on above logic, we list the valid socket id. Therefore > > rte_lcore_to_socket_id return a valid id and not need to add 1. > > > > > OK, but at list the following check "if (sock_num > > RTE_MAX_NUMA_NODES)" > Should be "if (sock_num +1 > RTE_MAX_NUMA_NODES)", right? > Right, i prefer the following though, what do you think? for (i = 0; i < RTE_MAX_LCORE; i++) { sock_num = rte_lcore_to_socket_id(i); if (new_socket_id(sock_num)) { if (num_sockets >= RTE_MAX_NUMA_NODES) { rte_exit(EXIT_FAILURE, "Total sockets greater than %u\n", RTE_MAX_NUMA_NODES); } socket_ids[num_sockets++] = sock_num; } > One more thing, if this patch is fixing a bug, I think "fix" should be added > in > title. OK, I guess I can change the commit title and message. BTW note that there is a V3 with Thomas requested changes. If we agree on above I can submit a v4 with the last updates. > > Thanks > Jingjing
