rte_memzone_reserve and invalid socket id

rte_memzone_reserve and invalid socket id

[Tyler Retzlaff]

hi,

there is a repeatable test failure in test_memzone when running
dpdk-test.exe --no-huge for memzone_autotest

it's clear why the test fails but what isn't clear if what
rte_memzone_reserve is doing when provided an invalid socket id is
sensible or not.

as a matter of luck the system i'm using to test is a single socket
system and as a result has only socket_id 0. the test however tries to
use rte_memzone_reserve with a socket_id of 1 which is not a valid
socket_id on the system.

	memzone3 = rte_memzone_reserve(TEST_MEMZONE_NAME("testzone3"), 1000,
				1, 0);
                                ^ socket_id (to repeat just make it invalid)

the parameter documentation provided for reference.

 * @param socket_id
 *   The socket identifier in the case of
 *   NUMA. The value can be SOCKET_ID_ANY if there is no NUMA
 *   constraint for the reserved zone.

of interest is should rte_memzone_reserve fail when provided a
completely invalid socket_id?

when running with --no-huge it does not because when --no-huge the
socket_id no matter the value is silently re-mapped to SOCKET_ID_ANY
though without --no-huge if a completely garbage socket_id were provided
it seems the allocation would fail.

so you get different behavior for an invalid socket_id depending on
--no-huge vs with.

	if (!rte_eal_has_hugepages() && socket_id < RTE_MAX_NUMA_NODES)
		socket_id = SOCKET_ID_ANY;

the test later fails at this check. where it compares the memzone3
socket_id to what was used in the call to rte_memzone_reserve.

	if (memzone3 != NULL && memzone3->socket_id != 1)
		return -1;                ^ SOCKET_ID_ANY if --no-huge

if the allocation had failed, the test would pass instead of failing at
this point.

so what's wrong here? the test should be changed to expect different
behavior with --no-huge vs huge or should rte_memzone_reserve be
explicitly requiring SOCKET_ID_ANY instead of re-mapping invalid socket
id?

if it isn't the test that is wrong then a compatibility discussion is of
interest but i'm avoiding that until someone confirms the intended
design/behavior.

thanks

Re: rte_memzone_reserve and invalid socket id

[Tyler Retzlaff]

On Mon, Mar 28, 2022 at 11:04:36PM -0700, Tyler Retzlaff wrote:
> hi,
> 
> there is a repeatable test failure in test_memzone when running
> dpdk-test.exe --no-huge for memzone_autotest
> 
> it's clear why the test fails but what isn't clear if what
> rte_memzone_reserve is doing when provided an invalid socket id is
> sensible or not.
> 
> as a matter of luck the system i'm using to test is a single socket
> system and as a result has only socket_id 0. the test however tries to
> use rte_memzone_reserve with a socket_id of 1 which is not a valid
> socket_id on the system.
> 
> 	memzone3 = rte_memzone_reserve(TEST_MEMZONE_NAME("testzone3"), 1000,
> 				1, 0);
>                                 ^ socket_id (to repeat just make it invalid)
> 
> the parameter documentation provided for reference.
> 
>  * @param socket_id
>  *   The socket identifier in the case of
>  *   NUMA. The value can be SOCKET_ID_ANY if there is no NUMA
>  *   constraint for the reserved zone.
> 
> of interest is should rte_memzone_reserve fail when provided a
> completely invalid socket_id?
> 
> when running with --no-huge it does not because when --no-huge the
> socket_id no matter the value is silently re-mapped to SOCKET_ID_ANY
> though without --no-huge if a completely garbage socket_id were provided
> it seems the allocation would fail.
> 
> so you get different behavior for an invalid socket_id depending on
> --no-huge vs with.
> 
> 	if (!rte_eal_has_hugepages() && socket_id < RTE_MAX_NUMA_NODES)
> 		socket_id = SOCKET_ID_ANY;
> 
> the test later fails at this check. where it compares the memzone3
> socket_id to what was used in the call to rte_memzone_reserve.
> 
> 	if (memzone3 != NULL && memzone3->socket_id != 1)
> 		return -1;                ^ SOCKET_ID_ANY if --no-huge
> 
> if the allocation had failed, the test would pass instead of failing at
> this point.
> 
> so what's wrong here? the test should be changed to expect different
> behavior with --no-huge vs huge or should rte_memzone_reserve be
> explicitly requiring SOCKET_ID_ANY instead of re-mapping invalid socket
> id?
> 
> if it isn't the test that is wrong then a compatibility discussion is of
> interest but i'm avoiding that until someone confirms the intended
> design/behavior.
> 
> thanks

ping? does the community have an opinion here?

Re: rte_memzone_reserve and invalid socket id

[Dmitry Kozlyuk]

2022-04-13 00:54 (UTC-0700), Tyler Retzlaff:
> On Mon, Mar 28, 2022 at 11:04:36PM -0700, Tyler Retzlaff wrote:
[...]
> > 	memzone3 = rte_memzone_reserve(TEST_MEMZONE_NAME("testzone3"), 1000,
> > 				1, 0);
> >                                 ^ socket_id (to repeat just make it invalid)
> > 
> > the parameter documentation provided for reference.
> > 
> >  * @param socket_id
> >  *   The socket identifier in the case of
> >  *   NUMA. The value can be SOCKET_ID_ANY if there is no NUMA
> >  *   constraint for the reserved zone.
> > 
> > of interest is should rte_memzone_reserve fail when provided a
> > completely invalid socket_id?

I think it should.

> > 
> > when running with --no-huge it does not because when --no-huge the
> > socket_id no matter the value is silently re-mapped to SOCKET_ID_ANY
> > though without --no-huge if a completely garbage socket_id were provided
> > it seems the allocation would fail.

It's an implementation detail.
NUMA could be respected for --no-huge if there was a need.

> > 
> > so you get different behavior for an invalid socket_id depending on
> > --no-huge vs with.
> > 
> > 	if (!rte_eal_has_hugepages() && socket_id < RTE_MAX_NUMA_NODES)
> > 		socket_id = SOCKET_ID_ANY;
> > 
> > the test later fails at this check. where it compares the memzone3
> > socket_id to what was used in the call to rte_memzone_reserve.
> > 
> > 	if (memzone3 != NULL && memzone3->socket_id != 1)
> > 		return -1;                ^ SOCKET_ID_ANY if --no-huge
> > 
> > if the allocation had failed, the test would pass instead of failing at
> > this point.
> > 
> > so what's wrong here? the test should be changed to expect different
> > behavior with --no-huge vs huge or should rte_memzone_reserve be
> > explicitly requiring SOCKET_ID_ANY instead of re-mapping invalid socket
> > id?

memzone3->socket_id == SOCKET_ID_ANY should not be possible,
because it's a specific selected socket ID.
Rather, the check should be relaxed depending on rte_eal_has_hugepages().

Re: rte_memzone_reserve and invalid socket id

[Tyler Retzlaff]

On Thu, Apr 14, 2022 at 10:03:10PM +0300, Dmitry Kozlyuk wrote:
> 2022-04-13 00:54 (UTC-0700), Tyler Retzlaff:
> > On Mon, Mar 28, 2022 at 11:04:36PM -0700, Tyler Retzlaff wrote:
> [...]
> > > 	memzone3 = rte_memzone_reserve(TEST_MEMZONE_NAME("testzone3"), 1000,
> > > 				1, 0);
> > >                                 ^ socket_id (to repeat just make it invalid)
> > > 
> > > the parameter documentation provided for reference.
> > > 
> > >  * @param socket_id
> > >  *   The socket identifier in the case of
> > >  *   NUMA. The value can be SOCKET_ID_ANY if there is no NUMA
> > >  *   constraint for the reserved zone.
> > > 
> > > of interest is should rte_memzone_reserve fail when provided a
> > > completely invalid socket_id?
> 
> I think it should.
> 
> > > 
> > > when running with --no-huge it does not because when --no-huge the
> > > socket_id no matter the value is silently re-mapped to SOCKET_ID_ANY
> > > though without --no-huge if a completely garbage socket_id were provided
> > > it seems the allocation would fail.
> 
> It's an implementation detail.
> NUMA could be respected for --no-huge if there was a need.
> 
> > > 
> > > so you get different behavior for an invalid socket_id depending on
> > > --no-huge vs with.
> > > 
> > > 	if (!rte_eal_has_hugepages() && socket_id < RTE_MAX_NUMA_NODES)
> > > 		socket_id = SOCKET_ID_ANY;
> > > 
> > > the test later fails at this check. where it compares the memzone3
> > > socket_id to what was used in the call to rte_memzone_reserve.
> > > 
> > > 	if (memzone3 != NULL && memzone3->socket_id != 1)
> > > 		return -1;                ^ SOCKET_ID_ANY if --no-huge
> > > 
> > > if the allocation had failed, the test would pass instead of failing at
> > > this point.
> > > 
> > > so what's wrong here? the test should be changed to expect different
> > > behavior with --no-huge vs huge or should rte_memzone_reserve be
> > > explicitly requiring SOCKET_ID_ANY instead of re-mapping invalid socket
> > > id?
> 
> memzone3->socket_id == SOCKET_ID_ANY should not be possible,
> because it's a specific selected socket ID.
> Rather, the check should be relaxed depending on rte_eal_has_hugepages().

okay, i think we are in agreement. my interpreted summary of how things
should be are as follows.

* rte_memzone_reserve() should fail if the provided socket_id is invalid
  for both huge and no-huge.

* the test of memzone3 should be conditional on rte_eal_has_hugepages()
  instead of allocation succcess/failure of memzone3.
  note: if this is corrected it would have masked the bug/difference
        in behavior.

* a separate test should exist that checks rte_memzone_reserve()
  correctly fails when given an invalid socket_id with both huge
  and no-huge.

if the 3 points above were to be addressed the issue is it is a
compatibility break. so while it should work a certain way the question
for the community now what should we do?

thanks