[dpdk-dev] [PATCH v2] app/testpmd: support non contiguous socket ids
Shahaf Shuler
shahafs at mellanox.com
Mon May 8 19:35:48 CEST 2017
--Shahaf
> -----Original Message-----
> From: Wu, Jingjing [mailto:jingjing.wu at intel.com]
> Sent: Monday, May 8, 2017 3:54 AM
> To: Shahaf Shuler <shahafs at mellanox.com>
> Cc: dev at dpdk.org; Thomas Monjalon <thomas at monjalon.net>;
> stable at dpdk.org
> Subject: RE: [PATCH v2] app/testpmd: support non contiguous socket ids
>
>
>
> > -----Original Message-----
> > From: Shahaf Shuler [mailto:shahafs at mellanox.com]
> > Sent: Sunday, May 7, 2017 2:06 PM
> > To: Wu, Jingjing <jingjing.wu at intel.com>
> > Cc: dev at dpdk.org; Thomas Monjalon <thomas at monjalon.net>;
> > stable at 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: stable at dpdk.org
> > > > Signed-off-by: Shahaf Shuler <shahafs at 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
More information about the dev
mailing list