diff mbox series

examples/vhost: change the default value of NIC's max queues

Message ID 20210910135205.269651-1-wenwux.ma@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series examples/vhost: change the default value of NIC's max queues |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Wenwu Ma Sept. 10, 2021, 1:52 p.m. UTC 
vswitch can't launch with 40G FTV due to Device start fails
if NIC’s max queues > the default number of 128,
so, we changed the default value from 128 to 512.

Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
---
 examples/vhost/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jiang, Cheng1 Sept. 10, 2021, 2:12 a.m. UTC | #1 
Acked-by: Cheng Jiang <cheng1.jiang@intel.com>



> -----Original Message-----
> From: Ma, WenwuX <wenwux.ma@intel.com>
> Sent: Friday, September 10, 2021 9:52 PM
> To: dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> Jiang, Cheng1 <cheng1.jiang@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>;
> Wang, Yinan <yinan.wang@intel.com>; Ma, WenwuX
> <wenwux.ma@intel.com>
> Subject: [PATCH] examples/vhost: change the default value of NIC's max
> queues
> 
> vswitch can't launch with 40G FTV due to Device start fails if NIC’s max
> queues > the default number of 128, so, we changed the default value from
> 128 to 512.
> 
> Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
> ---
>  examples/vhost/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c index
> bc3d71c898..36969a4de5 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -29,7 +29,7 @@
>  #include "main.h"
> 
>  #ifndef MAX_QUEUES
> -#define MAX_QUEUES 128
> +#define MAX_QUEUES 512
>  #endif
> 
>  /* the maximum number of external ports supported */
> --
> 2.25.1
Chenbo Xia Sept. 10, 2021, 3:17 a.m. UTC | #2 
Hi Wenwu,

> -----Original Message-----
> From: Ma, WenwuX <wenwux.ma@intel.com>
> Sent: Friday, September 10, 2021 9:52 PM
> To: dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>; Jiang,
> Cheng1 <cheng1.jiang@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan
> <yinan.wang@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>
> Subject: [PATCH] examples/vhost: change the default value of NIC's max queues
> 
> vswitch can't launch with 40G FTV due to Device start fails

Not many people can understand what's FTV. So let's describe it with a driver
name. Example if it's 'i40e':

vswitch can't launch with a 40G i40e port...

And Device -> device

> if NIC’s max queues > the default number of 128,
> so, we changed the default value from 128 to 512.
>

I'd say it's not cool to still hard-code the MAX_QUEUES so that only 'some' NICs
can work with the example. The app should have a way to check this kind of info 
before init/start. But as I would like to see at some point, this example will
be removed and all our tests go to testpmd. Let's not waste too much effort on
this example.

Besides: it can be a fix. Let's backport it.

Thanks,
Chenbo
 
> Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
> ---
>  examples/vhost/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index bc3d71c898..36969a4de5 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -29,7 +29,7 @@
>  #include "main.h"
> 
>  #ifndef MAX_QUEUES
> -#define MAX_QUEUES 128
> +#define MAX_QUEUES 512
>  #endif
> 
>  /* the maximum number of external ports supported */
> --
> 2.25.1
Maxime Coquelin Sept. 13, 2021, 3:49 p.m. UTC | #3 
On 9/10/21 5:17 AM, Xia, Chenbo wrote:
> Hi Wenwu,
> 
>> -----Original Message-----
>> From: Ma, WenwuX <wenwux.ma@intel.com>
>> Sent: Friday, September 10, 2021 9:52 PM
>> To: dev@dpdk.org
>> Cc: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>; Jiang,
>> Cheng1 <cheng1.jiang@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan
>> <yinan.wang@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>
>> Subject: [PATCH] examples/vhost: change the default value of NIC's max queues
>>
>> vswitch can't launch with 40G FTV due to Device start fails
> 
> Not many people can understand what's FTV. So let's describe it with a driver
> name. Example if it's 'i40e':
> 
> vswitch can't launch with a 40G i40e port...
> 
> And Device -> device
> 
>> if NIC’s max queues > the default number of 128,
>> so, we changed the default value from 128 to 512.
>>
> 
> I'd say it's not cool to still hard-code the MAX_QUEUES so that only 'some' NICs
> can work with the example. The app should have a way to check this kind of info 
> before init/start. But as I would like to see at some point, this example will
> be removed and all our tests go to testpmd. Let's not waste too much effort on
> this example.

+1 on this, I agree with Chenbo it is better to invest time in porting
existing tests to testpmd.

> 
> Besides: it can be a fix. Let's backport it.
> 
> Thanks,
> Chenbo
>  
>> Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
>> ---
>>  examples/vhost/main.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
>> index bc3d71c898..36969a4de5 100644
>> --- a/examples/vhost/main.c
>> +++ b/examples/vhost/main.c
>> @@ -29,7 +29,7 @@
>>  #include "main.h"
>>
>>  #ifndef MAX_QUEUES
>> -#define MAX_QUEUES 128
>> +#define MAX_QUEUES 512
>>  #endif
>>
>>  /* the maximum number of external ports supported */
>> --
>> 2.25.1
>
Maxime Coquelin Oct. 15, 2021, 7:30 a.m. UTC | #4 
Hi,

On 9/10/21 05:17, Xia, Chenbo wrote:
> Hi Wenwu,
> 
>> -----Original Message-----
>> From: Ma, WenwuX <wenwux.ma@intel.com>
>> Sent: Friday, September 10, 2021 9:52 PM
>> To: dev@dpdk.org
>> Cc: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>; Jiang,
>> Cheng1 <cheng1.jiang@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan
>> <yinan.wang@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>
>> Subject: [PATCH] examples/vhost: change the default value of NIC's max queues
>>
>> vswitch can't launch with 40G FTV due to Device start fails
> 
> Not many people can understand what's FTV. So let's describe it with a driver
> name. Example if it's 'i40e':
> 
> vswitch can't launch with a 40G i40e port...
> 
> And Device -> device
> 
>> if NIC’s max queues > the default number of 128,
>> so, we changed the default value from 128 to 512.
>>
> 
> I'd say it's not cool to still hard-code the MAX_QUEUES so that only 'some' NICs
> can work with the example. The app should have a way to check this kind of info
> before init/start. But as I would like to see at some point, this example will
> be removed and all our tests go to testpmd. Let's not waste too much effort on
> this example.
> 
> Besides: it can be a fix. Let's backport it.
> 
> Thanks,
> Chenbo
>   
>> Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
>> ---
>>   examples/vhost/main.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
>> index bc3d71c898..36969a4de5 100644
>> --- a/examples/vhost/main.c
>> +++ b/examples/vhost/main.c
>> @@ -29,7 +29,7 @@
>>   #include "main.h"
>>
>>   #ifndef MAX_QUEUES
>> -#define MAX_QUEUES 128
>> +#define MAX_QUEUES 512
>>   #endif
>>
>>   /* the maximum number of external ports supported */
>> --
>> 2.25.1
> 

Are you planning to post a new revision handling Chenbo's comments?

Thanks,
Maxime
David Marchand Oct. 15, 2021, 8:52 a.m. UTC | #5 
On Fri, Sep 10, 2021 at 5:17 AM Xia, Chenbo <chenbo.xia@intel.com> wrote:
> > if NIC’s max queues > the default number of 128,
> > so, we changed the default value from 128 to 512.
> >
>
> I'd say it's not cool to still hard-code the MAX_QUEUES so that only 'some' NICs
> can work with the example. The app should have a way to check this kind of info

+1...

> before init/start. But as I would like to see at some point, this example will
> be removed and all our tests go to testpmd. Let's not waste too much effort on
> this example.

And +1, this example is a mess.
diff mbox series

Patch

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index bc3d71c898..36969a4de5 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -29,7 +29,7 @@ 
 #include "main.h"
 
 #ifndef MAX_QUEUES
-#define MAX_QUEUES 128
+#define MAX_QUEUES 512
 #endif
 
 /* the maximum number of external ports supported */