DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] test: test zero socket-mem as valid
       [not found] <CGME20190125075604eucas1p2ecf0177c3f953e889721d784ef387254@eucas1p2.samsung.com>
@ 2019-01-25  7:55 ` Ilya Maximets
  2019-01-25  8:06   ` Ilya Maximets
  0 siblings, 1 reply; 7+ messages in thread
From: Ilya Maximets @ 2019-01-25  7:55 UTC (permalink / raw)
  To: dev, Anatoly Burakov
  Cc: Thomas Monjalon, ShuaiX Zhu, Xueqin Lin, WenjieX A Li,
	FengqinX Wang, Ilya Maximets, stable

Dynamic memory mode allowes zero socket-mem because all the
required memory could be allocated on demand.

Fixes: 339c2244b4f1 ("eal: fix parsing zero socket memory and limits")
Cc: stable@dpdk.org

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 test/test/test_eal_flags.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index e3a60c7ae..81e345b87 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -1158,7 +1158,7 @@ test_memory_flags(void)
 	const char *argv1[] = {prgname, "-c", "10", "-n", "2",
 			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE};
 
-	/* invalid (zero) --socket-mem flag */
+	/* valid (zero) --socket-mem flag */
 	const char *argv2[] = {prgname, "-c", "10", "-n", "2",
 			"--file-prefix=" memtest, "--socket-mem=0,0,0,0"};
 
@@ -1256,8 +1256,8 @@ test_memory_flags(void)
 		printf("Error - process failed with valid -m flag!\n");
 		return -1;
 	}
-	if (launch_proc(argv2) == 0) {
-		printf("Error - process run ok with invalid (zero) --socket-mem!\n");
+	if (launch_proc(argv2) != 0) {
+		printf("Error - process failed with valid (zero) --socket-mem!\n");
 		return -1;
 	}
 
-- 
2.17.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] test: test zero socket-mem as valid
  2019-01-25  7:55 ` [dpdk-dev] [PATCH] test: test zero socket-mem as valid Ilya Maximets
@ 2019-01-25  8:06   ` Ilya Maximets
  2019-01-25  9:53     ` David Marchand
  0 siblings, 1 reply; 7+ messages in thread
From: Ilya Maximets @ 2019-01-25  8:06 UTC (permalink / raw)
  To: dev, Anatoly Burakov
  Cc: Thomas Monjalon, ShuaiX Zhu, Xueqin Lin, WenjieX A Li,
	FengqinX Wang, stable

On 25.01.2019 10:55, Ilya Maximets wrote:
> Dynamic memory mode allowes zero socket-mem because all the
> required memory could be allocated on demand.
> 
> Fixes: 339c2244b4f1 ("eal: fix parsing zero socket memory and limits")
> Cc: stable@dpdk.org
> 

Reported-by: Shuai Zhu <shuaix.zhu@intel.com>

> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  test/test/test_eal_flags.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
> index e3a60c7ae..81e345b87 100644
> --- a/test/test/test_eal_flags.c
> +++ b/test/test/test_eal_flags.c
> @@ -1158,7 +1158,7 @@ test_memory_flags(void)
>  	const char *argv1[] = {prgname, "-c", "10", "-n", "2",
>  			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE};
>  
> -	/* invalid (zero) --socket-mem flag */
> +	/* valid (zero) --socket-mem flag */
>  	const char *argv2[] = {prgname, "-c", "10", "-n", "2",
>  			"--file-prefix=" memtest, "--socket-mem=0,0,0,0"};
>  
> @@ -1256,8 +1256,8 @@ test_memory_flags(void)
>  		printf("Error - process failed with valid -m flag!\n");
>  		return -1;
>  	}
> -	if (launch_proc(argv2) == 0) {
> -		printf("Error - process run ok with invalid (zero) --socket-mem!\n");
> +	if (launch_proc(argv2) != 0) {
> +		printf("Error - process failed with valid (zero) --socket-mem!\n");
>  		return -1;
>  	}
>  
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] test: test zero socket-mem as valid
  2019-01-25  8:06   ` Ilya Maximets
@ 2019-01-25  9:53     ` David Marchand
  2019-01-25 13:48       ` Burakov, Anatoly
  0 siblings, 1 reply; 7+ messages in thread
From: David Marchand @ 2019-01-25  9:53 UTC (permalink / raw)
  To: Ilya Maximets
  Cc: dev, Anatoly Burakov, Thomas Monjalon, ShuaiX Zhu, Xueqin Lin,
	WenjieX A Li, FengqinX Wang, dpdk stable

On Fri, Jan 25, 2019 at 9:06 AM Ilya Maximets <i.maximets@samsung.com>
wrote:

> On 25.01.2019 10:55, Ilya Maximets wrote:
> > Dynamic memory mode allowes zero socket-mem because all the
> > required memory could be allocated on demand.
> >
> > Fixes: 339c2244b4f1 ("eal: fix parsing zero socket memory and limits")
> > Cc: stable@dpdk.org
> >
>
> Reported-by: Shuai Zhu <shuaix.zhu@intel.com>
>
> > Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>
> ---
> >  test/test/test_eal_flags.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
> > index e3a60c7ae..81e345b87 100644
> > --- a/test/test/test_eal_flags.c
> > +++ b/test/test/test_eal_flags.c
> > @@ -1158,7 +1158,7 @@ test_memory_flags(void)
> >       const char *argv1[] = {prgname, "-c", "10", "-n", "2",
> >                       "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE};
> >
> > -     /* invalid (zero) --socket-mem flag */
> > +     /* valid (zero) --socket-mem flag */
> >       const char *argv2[] = {prgname, "-c", "10", "-n", "2",
> >                       "--file-prefix=" memtest, "--socket-mem=0,0,0,0"};
> >
> > @@ -1256,8 +1256,8 @@ test_memory_flags(void)
> >               printf("Error - process failed with valid -m flag!\n");
> >               return -1;
> >       }
> > -     if (launch_proc(argv2) == 0) {
> > -             printf("Error - process run ok with invalid (zero)
> --socket-mem!\n");
> > +     if (launch_proc(argv2) != 0) {
> > +             printf("Error - process failed with valid (zero)
> --socket-mem!\n");
> >               return -1;
> >       }
> >
> >
>

Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] test: test zero socket-mem as valid
  2019-01-25  9:53     ` David Marchand
@ 2019-01-25 13:48       ` Burakov, Anatoly
  2019-01-25 14:00         ` Ilya Maximets
  0 siblings, 1 reply; 7+ messages in thread
From: Burakov, Anatoly @ 2019-01-25 13:48 UTC (permalink / raw)
  To: David Marchand, Ilya Maximets
  Cc: dev, Thomas Monjalon, ShuaiX Zhu, Xueqin Lin, WenjieX A Li,
	FengqinX Wang, dpdk stable

On 25-Jan-19 9:53 AM, David Marchand wrote:
> 
> 
> On Fri, Jan 25, 2019 at 9:06 AM Ilya Maximets <i.maximets@samsung.com 
> <mailto:i.maximets@samsung.com>> wrote:
> 
>     On 25.01.2019 10:55, Ilya Maximets wrote:
>      > Dynamic memory mode allowes zero socket-mem because all the
>      > required memory could be allocated on demand.
>      >
>      > Fixes: 339c2244b4f1 ("eal: fix parsing zero socket memory and
>     limits")
>      > Cc: stable@dpdk.org <mailto:stable@dpdk.org>
>      >
> 
>     Reported-by: Shuai Zhu <shuaix.zhu@intel.com
>     <mailto:shuaix.zhu@intel.com>>
> 
>      > Signed-off-by: Ilya Maximets <i.maximets@samsung.com
>     <mailto:i.maximets@samsung.com>>
> 
>      > ---
>      >  test/test/test_eal_flags.c | 6 +++---
>      >  1 file changed, 3 insertions(+), 3 deletions(-)
>      >
>      > diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
>      > index e3a60c7ae..81e345b87 100644
>      > --- a/test/test/test_eal_flags.c
>      > +++ b/test/test/test_eal_flags.c
>      > @@ -1158,7 +1158,7 @@ test_memory_flags(void)
>      >       const char *argv1[] = {prgname, "-c", "10", "-n", "2",
>      >                       "--file-prefix=" memtest, "-m",
>     DEFAULT_MEM_SIZE};
>      >
>      > -     /* invalid (zero) --socket-mem flag */
>      > +     /* valid (zero) --socket-mem flag */
>      >       const char *argv2[] = {prgname, "-c", "10", "-n", "2",
>      >                       "--file-prefix=" memtest,
>     "--socket-mem=0,0,0,0"};
>      >
>      > @@ -1256,8 +1256,8 @@ test_memory_flags(void)
>      >               printf("Error - process failed with valid -m flag!\n");
>      >               return -1;
>      >       }
>      > -     if (launch_proc(argv2) == 0) {
>      > -             printf("Error - process run ok with invalid (zero)
>     --socket-mem!\n");
>      > +     if (launch_proc(argv2) != 0) {
>      > +             printf("Error - process failed with valid (zero)
>     --socket-mem!\n");
>      >               return -1;
>      >       }
>      >
>      >
> 
> 
> Reviewed-by: David Marchand <david.marchand@redhat.com 
> <mailto:david.marchand@redhat.com>>
> 
> 
> -- 
> David Marchand

Now that i think of it, maybe it's not that simple.

--socket-mem/-m flag with zero is still an invalid value *if* 
--legacy-mem is involved. However, it is a valid value in non-legacy mode.

So maybe the test should reflect this, and the previous fix should have 
instead added a check for legacy mode rather than disabling the zero 
check outright.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] test: test zero socket-mem as valid
  2019-01-25 13:48       ` Burakov, Anatoly
@ 2019-01-25 14:00         ` Ilya Maximets
  2019-01-25 14:12           ` Burakov, Anatoly
  0 siblings, 1 reply; 7+ messages in thread
From: Ilya Maximets @ 2019-01-25 14:00 UTC (permalink / raw)
  To: Burakov, Anatoly, David Marchand
  Cc: dev, Thomas Monjalon, ShuaiX Zhu, Xueqin Lin, WenjieX A Li,
	FengqinX Wang, dpdk stable

On 25.01.2019 16:48, Burakov, Anatoly wrote:
> On 25-Jan-19 9:53 AM, David Marchand wrote:
>>
>>
>> On Fri, Jan 25, 2019 at 9:06 AM Ilya Maximets <i.maximets@samsung.com <mailto:i.maximets@samsung.com>> wrote:
>>
>>     On 25.01.2019 10:55, Ilya Maximets wrote:
>>      > Dynamic memory mode allowes zero socket-mem because all the
>>      > required memory could be allocated on demand.
>>      >
>>      > Fixes: 339c2244b4f1 ("eal: fix parsing zero socket memory and
>>     limits")
>>      > Cc: stable@dpdk.org <mailto:stable@dpdk.org>
>>      >
>>
>>     Reported-by: Shuai Zhu <shuaix.zhu@intel.com
>>     <mailto:shuaix.zhu@intel.com>>
>>
>>      > Signed-off-by: Ilya Maximets <i.maximets@samsung.com
>>     <mailto:i.maximets@samsung.com>>
>>
>>      > ---
>>      >  test/test/test_eal_flags.c | 6 +++---
>>      >  1 file changed, 3 insertions(+), 3 deletions(-)
>>      >
>>      > diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
>>      > index e3a60c7ae..81e345b87 100644
>>      > --- a/test/test/test_eal_flags.c
>>      > +++ b/test/test/test_eal_flags.c
>>      > @@ -1158,7 +1158,7 @@ test_memory_flags(void)
>>      >       const char *argv1[] = {prgname, "-c", "10", "-n", "2",
>>      >                       "--file-prefix=" memtest, "-m",
>>     DEFAULT_MEM_SIZE};
>>      >
>>      > -     /* invalid (zero) --socket-mem flag */
>>      > +     /* valid (zero) --socket-mem flag */
>>      >       const char *argv2[] = {prgname, "-c", "10", "-n", "2",
>>      >                       "--file-prefix=" memtest,
>>     "--socket-mem=0,0,0,0"};
>>      >
>>      > @@ -1256,8 +1256,8 @@ test_memory_flags(void)
>>      >               printf("Error - process failed with valid -m flag!\n");
>>      >               return -1;
>>      >       }
>>      > -     if (launch_proc(argv2) == 0) {
>>      > -             printf("Error - process run ok with invalid (zero)
>>     --socket-mem!\n");
>>      > +     if (launch_proc(argv2) != 0) {
>>      > +             printf("Error - process failed with valid (zero)
>>     --socket-mem!\n");
>>      >               return -1;
>>      >       }
>>      >
>>      >
>>
>>
>> Reviewed-by: David Marchand <david.marchand@redhat.com <mailto:david.marchand@redhat.com>>
>>
>>
>> -- 
>> David Marchand
> 
> Now that i think of it, maybe it's not that simple.
> 
> --socket-mem/-m flag with zero is still an invalid value *if* --legacy-mem is involved. However, it is a valid value in non-legacy mode.
> 
> So maybe the test should reflect this, and the previous fix should have instead added a check for legacy mode rather than disabling the zero check outright.
> 

I don't think that it's a big deal, because "--socket-mem=0 --legacy-mem"
quickly fails with clear:

  EAL: WARNING: Master core has no memory on local socket!

IMHO, It's actually more informative than previous:

  EAL: invalid parameters for --socket-limit

I agree that we could add a test for a legacy-mem cases, but that's a bit
different task.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] test: test zero socket-mem as valid
  2019-01-25 14:00         ` Ilya Maximets
@ 2019-01-25 14:12           ` Burakov, Anatoly
  2019-01-26 18:20             ` Thomas Monjalon
  0 siblings, 1 reply; 7+ messages in thread
From: Burakov, Anatoly @ 2019-01-25 14:12 UTC (permalink / raw)
  To: Ilya Maximets, David Marchand
  Cc: dev, Thomas Monjalon, ShuaiX Zhu, Xueqin Lin, WenjieX A Li,
	FengqinX Wang, dpdk stable

On 25-Jan-19 2:00 PM, Ilya Maximets wrote:
> On 25.01.2019 16:48, Burakov, Anatoly wrote:
>> On 25-Jan-19 9:53 AM, David Marchand wrote:
>>>
>>>
>>> On Fri, Jan 25, 2019 at 9:06 AM Ilya Maximets <i.maximets@samsung.com <mailto:i.maximets@samsung.com>> wrote:
>>>
>>>      On 25.01.2019 10:55, Ilya Maximets wrote:
>>>       > Dynamic memory mode allowes zero socket-mem because all the
>>>       > required memory could be allocated on demand.
>>>       >
>>>       > Fixes: 339c2244b4f1 ("eal: fix parsing zero socket memory and
>>>      limits")
>>>       > Cc: stable@dpdk.org <mailto:stable@dpdk.org>
>>>       >
>>>
>>>      Reported-by: Shuai Zhu <shuaix.zhu@intel.com
>>>      <mailto:shuaix.zhu@intel.com>>
>>>
>>>       > Signed-off-by: Ilya Maximets <i.maximets@samsung.com
>>>      <mailto:i.maximets@samsung.com>>
>>>
>>>       > ---
>>>       >  test/test/test_eal_flags.c | 6 +++---
>>>       >  1 file changed, 3 insertions(+), 3 deletions(-)
>>>       >
>>>       > diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
>>>       > index e3a60c7ae..81e345b87 100644
>>>       > --- a/test/test/test_eal_flags.c
>>>       > +++ b/test/test/test_eal_flags.c
>>>       > @@ -1158,7 +1158,7 @@ test_memory_flags(void)
>>>       >       const char *argv1[] = {prgname, "-c", "10", "-n", "2",
>>>       >                       "--file-prefix=" memtest, "-m",
>>>      DEFAULT_MEM_SIZE};
>>>       >
>>>       > -     /* invalid (zero) --socket-mem flag */
>>>       > +     /* valid (zero) --socket-mem flag */
>>>       >       const char *argv2[] = {prgname, "-c", "10", "-n", "2",
>>>       >                       "--file-prefix=" memtest,
>>>      "--socket-mem=0,0,0,0"};
>>>       >
>>>       > @@ -1256,8 +1256,8 @@ test_memory_flags(void)
>>>       >               printf("Error - process failed with valid -m flag!\n");
>>>       >               return -1;
>>>       >       }
>>>       > -     if (launch_proc(argv2) == 0) {
>>>       > -             printf("Error - process run ok with invalid (zero)
>>>      --socket-mem!\n");
>>>       > +     if (launch_proc(argv2) != 0) {
>>>       > +             printf("Error - process failed with valid (zero)
>>>      --socket-mem!\n");
>>>       >               return -1;
>>>       >       }
>>>       >
>>>       >
>>>
>>>
>>> Reviewed-by: David Marchand <david.marchand@redhat.com <mailto:david.marchand@redhat.com>>
>>>
>>>
>>> -- 
>>> David Marchand
>>
>> Now that i think of it, maybe it's not that simple.
>>
>> --socket-mem/-m flag with zero is still an invalid value *if* --legacy-mem is involved. However, it is a valid value in non-legacy mode.
>>
>> So maybe the test should reflect this, and the previous fix should have instead added a check for legacy mode rather than disabling the zero check outright.
>>
> 
> I don't think that it's a big deal, because "--socket-mem=0 --legacy-mem"
> quickly fails with clear:
> 
>    EAL: WARNING: Master core has no memory on local socket!
> 
> IMHO, It's actually more informative than previous:
> 
>    EAL: invalid parameters for --socket-limit
> 
> I agree that we could add a test for a legacy-mem cases, but that's a bit
> different task.
> 

Good point. Maybe leave it as is then :)

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] test: test zero socket-mem as valid
  2019-01-25 14:12           ` Burakov, Anatoly
@ 2019-01-26 18:20             ` Thomas Monjalon
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2019-01-26 18:20 UTC (permalink / raw)
  To: Ilya Maximets
  Cc: dev, Burakov, Anatoly, David Marchand, ShuaiX Zhu, Xueqin Lin,
	WenjieX A Li, FengqinX Wang, dpdk stable

25/01/2019 15:12, Burakov, Anatoly:
> On 25-Jan-19 2:00 PM, Ilya Maximets wrote:
> > On 25.01.2019 16:48, Burakov, Anatoly wrote:
> >> On 25-Jan-19 9:53 AM, David Marchand wrote:
> >>>
> >>> Reviewed-by: David Marchand <david.marchand@redhat.com <mailto:david.marchand@redhat.com>>
> >>
> >> Now that i think of it, maybe it's not that simple.
> >>
> >> --socket-mem/-m flag with zero is still an invalid value *if* --legacy-mem is involved. However, it is a valid value in non-legacy mode.
> >>
> >> So maybe the test should reflect this, and the previous fix should have instead added a check for legacy mode rather than disabling the zero check outright.
> >>
> > 
> > I don't think that it's a big deal, because "--socket-mem=0 --legacy-mem"
> > quickly fails with clear:
> > 
> >    EAL: WARNING: Master core has no memory on local socket!
> > 
> > IMHO, It's actually more informative than previous:
> > 
> >    EAL: invalid parameters for --socket-limit
> > 
> > I agree that we could add a test for a legacy-mem cases, but that's a bit
> > different task.
> 
> Good point. Maybe leave it as is then :)
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-01-26 18:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20190125075604eucas1p2ecf0177c3f953e889721d784ef387254@eucas1p2.samsung.com>
2019-01-25  7:55 ` [dpdk-dev] [PATCH] test: test zero socket-mem as valid Ilya Maximets
2019-01-25  8:06   ` Ilya Maximets
2019-01-25  9:53     ` David Marchand
2019-01-25 13:48       ` Burakov, Anatoly
2019-01-25 14:00         ` Ilya Maximets
2019-01-25 14:12           ` Burakov, Anatoly
2019-01-26 18:20             ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).