> -----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?
One more thing, if this patch is fixing a bug, I think "fix" should be added in title. Thanks Jingjing
