[v2] eal: fix undetected NUMA nodes

In cases where the number of cores on a given socket is greater than
RTE_MAX_LCORES, then EAL will be unaware of all the sockets/numa nodes
on a system. Fix this limitation by having the EAL probe the NUMA node
for cores it isn't going to use, and recording that for completeness.

This is necessary as memory is tracked per node, and with the --lcores
parameters our app lcores may be on different sockets than the lcore ids
may imply. For example, lcore 0 is on socket zero, but if app is run
with --lcores=0@64, then DPDK lcore 0 may be on socket one, so DPDK
needs to be aware of that socket.

Fixes: 952b20777255 ("eal: provide API for querying valid socket ids")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

---
v2: handle case where RTE_MAX_LCORE > CPU_SETSIZE (i.e. >1024)
---
 lib/eal/common/eal_common_lcore.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
  

Recheck-request: iol-intel-Performance

There was an infra failure with the Arm Grace server about 12 hours ago
which caused this failure - sending a retest request.
  

On Wed, Mar 05, 2025 at 04:24:58PM +0000, Bruce Richardson wrote:
> In cases where the number of cores on a given socket is greater than
> RTE_MAX_LCORES, then EAL will be unaware of all the sockets/numa nodes
> on a system. Fix this limitation by having the EAL probe the NUMA node
> for cores it isn't going to use, and recording that for completeness.
> 
> This is necessary as memory is tracked per node, and with the --lcores
> parameters our app lcores may be on different sockets than the lcore ids
> may imply. For example, lcore 0 is on socket zero, but if app is run
> with --lcores=0@64, then DPDK lcore 0 may be on socket one, so DPDK
> needs to be aware of that socket.
> 
> Fixes: 952b20777255 ("eal: provide API for querying valid socket ids")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> ---
> v2: handle case where RTE_MAX_LCORE > CPU_SETSIZE (i.e. >1024)
> ---
>  lib/eal/common/eal_common_lcore.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
Ping for review.

For anyone wanting to test:
	 To reproduce the issue, do a build of DPDK with max_lcores option
set to less than the number of physical cores you have on a socket.
Then when running DPDK, the number of NUMA nodes printed at startup
will be incorrect. Applying this patch will then correct that.

Thanks,
/Bruce
  

On Tue, Mar 18, 2025 at 6:42 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Mar 05, 2025 at 04:24:58PM +0000, Bruce Richardson wrote:
> > In cases where the number of cores on a given socket is greater than
> > RTE_MAX_LCORES, then EAL will be unaware of all the sockets/numa nodes
> > on a system. Fix this limitation by having the EAL probe the NUMA node
> > for cores it isn't going to use, and recording that for completeness.
> >
> > This is necessary as memory is tracked per node, and with the --lcores
> > parameters our app lcores may be on different sockets than the lcore ids
> > may imply. For example, lcore 0 is on socket zero, but if app is run
> > with --lcores=0@64, then DPDK lcore 0 may be on socket one, so DPDK
> > needs to be aware of that socket.
> >
> > Fixes: 952b20777255 ("eal: provide API for querying valid socket ids")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> > ---
> > v2: handle case where RTE_MAX_LCORE > CPU_SETSIZE (i.e. >1024)
> > ---
> >  lib/eal/common/eal_common_lcore.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> >
> Ping for review.
>
> For anyone wanting to test:
>          To reproduce the issue, do a build of DPDK with max_lcores option
> set to less than the number of physical cores you have on a socket.

You also need to have those cores contiguous for a numa node which is
quite frequent on the systems in my lab.
But anyway, the bug is not hard to understand, I'll reply on the patch itself.
  

On Wed, Mar 5, 2025 at 5:25 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> In cases where the number of cores on a given socket is greater than
> RTE_MAX_LCORES, then EAL will be unaware of all the sockets/numa nodes
> on a system. Fix this limitation by having the EAL probe the NUMA node
> for cores it isn't going to use, and recording that for completeness.
>
> This is necessary as memory is tracked per node, and with the --lcores
> parameters our app lcores may be on different sockets than the lcore ids
> may imply. For example, lcore 0 is on socket zero, but if app is run
> with --lcores=0@64, then DPDK lcore 0 may be on socket one, so DPDK
> needs to be aware of that socket.
>
> Fixes: 952b20777255 ("eal: provide API for querying valid socket ids")
> Cc: stable@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

On the principle, the fix lgtm.

I have one comment.

>
> ---
> v2: handle case where RTE_MAX_LCORE > CPU_SETSIZE (i.e. >1024)
> ---
>  lib/eal/common/eal_common_lcore.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/lib/eal/common/eal_common_lcore.c b/lib/eal/common/eal_common_lcore.c
> index 2ff9252c52..820a6534b1 100644
> --- a/lib/eal/common/eal_common_lcore.c
> +++ b/lib/eal/common/eal_common_lcore.c
> @@ -144,7 +144,11 @@ rte_eal_cpu_init(void)
>         unsigned lcore_id;
>         unsigned count = 0;
>         unsigned int socket_id, prev_socket_id;
> -       int lcore_to_socket_id[RTE_MAX_LCORE];
> +#if CPU_SETSIZE > RTE_MAX_LCORE
> +       int lcore_to_socket_id[CPU_SETSIZE] = {0};
> +#else
> +       int lcore_to_socket_id[RTE_MAX_LCORE] = {0};
> +#endif

This initialisation was unneeded so far because, in the next loop (on
each possible lcore), eal_cpu_socket_id() (returning 0 even for
errors) was called regardless of eal_cpu_detected().
Moving this call after eal_cpu_detected() would be consistent with the
rest of this patch.


It is unrelated to this patch itself, but I also have some doubt about
the socket_id value stored per lcore, as no check against
RTE_MAX_NUMA_NODES is done afterwards.
(it is probably never hit since the default value for RTE_MAX_NUMA_NODES is 32).
  

On Wed, Mar 19, 2025 at 05:31:45PM +0100, David Marchand wrote:
> On Wed, Mar 5, 2025 at 5:25 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > In cases where the number of cores on a given socket is greater than
> > RTE_MAX_LCORES, then EAL will be unaware of all the sockets/numa nodes
> > on a system. Fix this limitation by having the EAL probe the NUMA node
> > for cores it isn't going to use, and recording that for completeness.
> >
> > This is necessary as memory is tracked per node, and with the --lcores
> > parameters our app lcores may be on different sockets than the lcore ids
> > may imply. For example, lcore 0 is on socket zero, but if app is run
> > with --lcores=0@64, then DPDK lcore 0 may be on socket one, so DPDK
> > needs to be aware of that socket.
> >
> > Fixes: 952b20777255 ("eal: provide API for querying valid socket ids")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> On the principle, the fix lgtm.
> 
> I have one comment.
> 
> >
> > ---
> > v2: handle case where RTE_MAX_LCORE > CPU_SETSIZE (i.e. >1024)
> > ---
> >  lib/eal/common/eal_common_lcore.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/eal/common/eal_common_lcore.c b/lib/eal/common/eal_common_lcore.c
> > index 2ff9252c52..820a6534b1 100644
> > --- a/lib/eal/common/eal_common_lcore.c
> > +++ b/lib/eal/common/eal_common_lcore.c
> > @@ -144,7 +144,11 @@ rte_eal_cpu_init(void)
> >         unsigned lcore_id;
> >         unsigned count = 0;
> >         unsigned int socket_id, prev_socket_id;
> > -       int lcore_to_socket_id[RTE_MAX_LCORE];
> > +#if CPU_SETSIZE > RTE_MAX_LCORE
> > +       int lcore_to_socket_id[CPU_SETSIZE] = {0};
> > +#else
> > +       int lcore_to_socket_id[RTE_MAX_LCORE] = {0};
> > +#endif
> 
> This initialisation was unneeded so far because, in the next loop (on
> each possible lcore), eal_cpu_socket_id() (returning 0 even for
> errors) was called regardless of eal_cpu_detected().
> Moving this call after eal_cpu_detected() would be consistent with the
> rest of this patch.
> 

So keep the zero-init, and move the function call to set the initial values
in the array then?

> 
> It is unrelated to this patch itself, but I also have some doubt about
> the socket_id value stored per lcore, as no check against
> RTE_MAX_NUMA_NODES is done afterwards.
> (it is probably never hit since the default value for RTE_MAX_NUMA_NODES is 32).
> 

Well, it's an open question whether RTE_MAX_NUMA_NODES is the max value for a
node id, or the maximum number of ids which can be handled. I imagine most
of the code assumes both - that we have sequential numa nodes with value <
MAX.

/Bruce
  

On Wed, Mar 19, 2025 at 5:55 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Mar 19, 2025 at 05:31:45PM +0100, David Marchand wrote:
> > On Wed, Mar 5, 2025 at 5:25 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > In cases where the number of cores on a given socket is greater than
> > > RTE_MAX_LCORES, then EAL will be unaware of all the sockets/numa nodes
> > > on a system. Fix this limitation by having the EAL probe the NUMA node
> > > for cores it isn't going to use, and recording that for completeness.
> > >
> > > This is necessary as memory is tracked per node, and with the --lcores
> > > parameters our app lcores may be on different sockets than the lcore ids
> > > may imply. For example, lcore 0 is on socket zero, but if app is run
> > > with --lcores=0@64, then DPDK lcore 0 may be on socket one, so DPDK
> > > needs to be aware of that socket.
> > >
> > > Fixes: 952b20777255 ("eal: provide API for querying valid socket ids")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> > On the principle, the fix lgtm.
> >
> > I have one comment.
> >
> > >
> > > ---
> > > v2: handle case where RTE_MAX_LCORE > CPU_SETSIZE (i.e. >1024)
> > > ---
> > >  lib/eal/common/eal_common_lcore.c | 17 ++++++++++++-----
> > >  1 file changed, 12 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/eal/common/eal_common_lcore.c b/lib/eal/common/eal_common_lcore.c
> > > index 2ff9252c52..820a6534b1 100644
> > > --- a/lib/eal/common/eal_common_lcore.c
> > > +++ b/lib/eal/common/eal_common_lcore.c
> > > @@ -144,7 +144,11 @@ rte_eal_cpu_init(void)
> > >         unsigned lcore_id;
> > >         unsigned count = 0;
> > >         unsigned int socket_id, prev_socket_id;
> > > -       int lcore_to_socket_id[RTE_MAX_LCORE];
> > > +#if CPU_SETSIZE > RTE_MAX_LCORE
> > > +       int lcore_to_socket_id[CPU_SETSIZE] = {0};
> > > +#else
> > > +       int lcore_to_socket_id[RTE_MAX_LCORE] = {0};
> > > +#endif
> >
> > This initialisation was unneeded so far because, in the next loop (on
> > each possible lcore), eal_cpu_socket_id() (returning 0 even for
> > errors) was called regardless of eal_cpu_detected().
> > Moving this call after eal_cpu_detected() would be consistent with the
> > rest of this patch.
> >
>
> So keep the zero-init, and move the function call to set the initial values
> in the array then?

I see no elegant way with current code.
I would completely separate this socket discovery from the rest...

Anyway, this is not the subject of this fix, so I'll withdraw this comment.

>
> >
> > It is unrelated to this patch itself, but I also have some doubt about
> > the socket_id value stored per lcore, as no check against
> > RTE_MAX_NUMA_NODES is done afterwards.
> > (it is probably never hit since the default value for RTE_MAX_NUMA_NODES is 32).
> >
>
> Well, it's an open question whether RTE_MAX_NUMA_NODES is the max value for a
> node id, or the maximum number of ids which can be handled. I imagine most
> of the code assumes both - that we have sequential numa nodes with value <
> MAX.

Regardless of the meaning, we can end up in a situation where a lcore
has a socket_id set in lcore_config[] / rte_lcore_XX API, that is
outside the list of numa nodes stored in config->numa_nodes[] /
rte_socket_XX API, which is used for memory init for example.
  

19/03/2025 17:31, David Marchand:
> On Wed, Mar 5, 2025 at 5:25 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > In cases where the number of cores on a given socket is greater than
> > RTE_MAX_LCORES, then EAL will be unaware of all the sockets/numa nodes
> > on a system. Fix this limitation by having the EAL probe the NUMA node
> > for cores it isn't going to use, and recording that for completeness.
> >
> > This is necessary as memory is tracked per node, and with the --lcores
> > parameters our app lcores may be on different sockets than the lcore ids
> > may imply. For example, lcore 0 is on socket zero, but if app is run
> > with --lcores=0@64, then DPDK lcore 0 may be on socket one, so DPDK
> > needs to be aware of that socket.
> >
> > Fixes: 952b20777255 ("eal: provide API for querying valid socket ids")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> On the principle, the fix lgtm.

It is very late in the release cycle, but we have discussed it
and decided to merge this fix in 25.03.

Applied, thanks.