diff mbox

[dpdk-dev,v4,6/6] testpmd: Set Rx VMDq RSS mode

Message ID 1420355937-18484-7-git-send-email-changchun.ouyang@intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Ouyang Changchun Jan. 4, 2015, 7:18 a.m. UTC
Set VMDq RSS mode if it has VF(VF number is more than 1) and has RSS information.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
 app/test-pmd/testpmd.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Vladislav Zolotarov Jan. 4, 2015, 8:49 a.m. UTC | #1
On 01/04/15 09:18, Ouyang Changchun wrote:
> Set VMDq RSS mode if it has VF(VF number is more than 1) and has RSS information.
>
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
>   app/test-pmd/testpmd.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 8c69756..6230f8b 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1708,6 +1708,16 @@ init_port_config(void)
>   				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
>   		}
>   
> +		if (port->dev_info.max_vfs != 0) {
> +			if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> +				port->dev_conf.rxmode.mq_mode =
> +					ETH_MQ_RX_VMDQ_RSS;
> +			else {
> +				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
> +				port->dev_conf.txmode.mq_mode = ETH_MQ_TX_NONE;

And what about the txmode.mq_mode when RSS is available (the :if" clause)?

> +			}
> +		}
> +
>   		port->rx_conf.rx_thresh = rx_thresh;
>   		port->rx_conf.rx_free_thresh = rx_free_thresh;
>   		port->rx_conf.rx_drop_en = rx_drop_en;
Ouyang Changchun Jan. 4, 2015, 9:01 a.m. UTC | #2
> -----Original Message-----
> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> Sent: Sunday, January 4, 2015 4:50 PM
> To: Ouyang, Changchun; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
> 
> 
> On 01/04/15 09:18, Ouyang Changchun wrote:
> > Set VMDq RSS mode if it has VF(VF number is more than 1) and has RSS
> information.
> >
> > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> > ---
> >   app/test-pmd/testpmd.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 8c69756..6230f8b 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -1708,6 +1708,16 @@ init_port_config(void)
> >   				port->dev_conf.rxmode.mq_mode =
> ETH_MQ_RX_NONE;
> >   		}
> >
> > +		if (port->dev_info.max_vfs != 0) {
> > +			if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> > +				port->dev_conf.rxmode.mq_mode =
> > +					ETH_MQ_RX_VMDQ_RSS;
> > +			else {
> > +				port->dev_conf.rxmode.mq_mode =
> ETH_MQ_RX_NONE;
> > +				port->dev_conf.txmode.mq_mode =
> ETH_MQ_TX_NONE;
> 
> And what about the txmode.mq_mode when RSS is available (the :if" clause)?

I think we can keep its original value for txmode.mq_mode, so don't change its value. How do you think of it?
Thanks
Changchun
Vladislav Zolotarov Jan. 4, 2015, 9:46 a.m. UTC | #3
On 01/04/15 11:01, Ouyang, Changchun wrote:
>> -----Original Message-----
>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>> Sent: Sunday, January 4, 2015 4:50 PM
>> To: Ouyang, Changchun; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
>>
>>
>> On 01/04/15 09:18, Ouyang Changchun wrote:
>>> Set VMDq RSS mode if it has VF(VF number is more than 1) and has RSS
>> information.
>>> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
>>> ---
>>>    app/test-pmd/testpmd.c | 10 ++++++++++
>>>    1 file changed, 10 insertions(+)
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>> 8c69756..6230f8b 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -1708,6 +1708,16 @@ init_port_config(void)
>>>    				port->dev_conf.rxmode.mq_mode =
>> ETH_MQ_RX_NONE;
>>>    		}
>>>
>>> +		if (port->dev_info.max_vfs != 0) {
>>> +			if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
>>> +				port->dev_conf.rxmode.mq_mode =
>>> +					ETH_MQ_RX_VMDQ_RSS;
>>> +			else {
>>> +				port->dev_conf.rxmode.mq_mode =
>> ETH_MQ_RX_NONE;
>>> +				port->dev_conf.txmode.mq_mode =
>> ETH_MQ_TX_NONE;
>>
>> And what about the txmode.mq_mode when RSS is available (the :if" clause)?
> I think we can keep its original value for txmode.mq_mode, so don't change its value. How do you think of it?

I agree that not changing a Tx mq_mode in both cases would be better.

> Thanks
> Changchun
>
>
Ouyang Changchun Jan. 5, 2015, 2:38 a.m. UTC | #4
> -----Original Message-----
> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> Sent: Sunday, January 4, 2015 5:47 PM
> To: Ouyang, Changchun; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
> 
> 
> On 01/04/15 11:01, Ouyang, Changchun wrote:
> >> -----Original Message-----
> >> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >> Sent: Sunday, January 4, 2015 4:50 PM
> >> To: Ouyang, Changchun; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
> >>
> >>
> >> On 01/04/15 09:18, Ouyang Changchun wrote:
> >>> Set VMDq RSS mode if it has VF(VF number is more than 1) and has RSS
> >> information.
> >>> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> >>> ---
> >>>    app/test-pmd/testpmd.c | 10 ++++++++++
> >>>    1 file changed, 10 insertions(+)
> >>>
> >>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> >>> 8c69756..6230f8b 100644
> >>> --- a/app/test-pmd/testpmd.c
> >>> +++ b/app/test-pmd/testpmd.c
> >>> @@ -1708,6 +1708,16 @@ init_port_config(void)
> >>>    				port->dev_conf.rxmode.mq_mode =
> >> ETH_MQ_RX_NONE;
> >>>    		}
> >>>
> >>> +		if (port->dev_info.max_vfs != 0) {
> >>> +			if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> >>> +				port->dev_conf.rxmode.mq_mode =
> >>> +					ETH_MQ_RX_VMDQ_RSS;
> >>> +			else {
> >>> +				port->dev_conf.rxmode.mq_mode =
> >> ETH_MQ_RX_NONE;
> >>> +				port->dev_conf.txmode.mq_mode =
> >> ETH_MQ_TX_NONE;
> >>
> >> And what about the txmode.mq_mode when RSS is available (the :if"
> clause)?
> > I think we can keep its original value for txmode.mq_mode, so don't
> change its value. How do you think of it?
> 
> I agree that not changing a Tx mq_mode in both cases would be better.

In the else clause, set txmode.mq_mode as ETH_MQ_TX_NONE explicitly to make sure it is neither ETH_MQ_TX_DCB,
ETH_MQ_TX_VMDQ_DCB, nor ETH_MQ_TX_VMDQ_ONLY.

> > Thanks
> > Changchun
> >
> >
Vladislav Zolotarov Jan. 5, 2015, 10:12 a.m. UTC | #5
On 01/05/15 04:38, Ouyang, Changchun wrote:
>
>> -----Original Message-----
>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>> Sent: Sunday, January 4, 2015 5:47 PM
>> To: Ouyang, Changchun; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
>>
>>
>> On 01/04/15 11:01, Ouyang, Changchun wrote:
>>>> -----Original Message-----
>>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>>>> Sent: Sunday, January 4, 2015 4:50 PM
>>>> To: Ouyang, Changchun; dev@dpdk.org
>>>> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
>>>>
>>>>
>>>> On 01/04/15 09:18, Ouyang Changchun wrote:
>>>>> Set VMDq RSS mode if it has VF(VF number is more than 1) and has RSS
>>>> information.
>>>>> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
>>>>> ---
>>>>>     app/test-pmd/testpmd.c | 10 ++++++++++
>>>>>     1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>>>> 8c69756..6230f8b 100644
>>>>> --- a/app/test-pmd/testpmd.c
>>>>> +++ b/app/test-pmd/testpmd.c
>>>>> @@ -1708,6 +1708,16 @@ init_port_config(void)
>>>>>     				port->dev_conf.rxmode.mq_mode =
>>>> ETH_MQ_RX_NONE;
>>>>>     		}
>>>>>
>>>>> +		if (port->dev_info.max_vfs != 0) {
>>>>> +			if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
>>>>> +				port->dev_conf.rxmode.mq_mode =
>>>>> +					ETH_MQ_RX_VMDQ_RSS;
>>>>> +			else {
>>>>> +				port->dev_conf.rxmode.mq_mode =
>>>> ETH_MQ_RX_NONE;
>>>>> +				port->dev_conf.txmode.mq_mode =
>>>> ETH_MQ_TX_NONE;
>>>>
>>>> And what about the txmode.mq_mode when RSS is available (the :if"
>> clause)?
>>> I think we can keep its original value for txmode.mq_mode, so don't
>> change its value. How do you think of it?
>>
>> I agree that not changing a Tx mq_mode in both cases would be better.
> In the else clause, set txmode.mq_mode as ETH_MQ_TX_NONE explicitly to make sure it is neither ETH_MQ_TX_DCB,
> ETH_MQ_TX_VMDQ_DCB, nor ETH_MQ_TX_VMDQ_ONLY.

It's not obvious to me why u should do that since AFAIK any of these 
modes requires RX_RSS. Do I miss anything?

>
>>> Thanks
>>> Changchun
>>>
>>>
Ouyang Changchun Jan. 6, 2015, 2:01 a.m. UTC | #6
> -----Original Message-----
> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> Sent: Monday, January 5, 2015 6:12 PM
> To: Ouyang, Changchun; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
> 
> 
> On 01/05/15 04:38, Ouyang, Changchun wrote:
> >
> >> -----Original Message-----
> >> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >> Sent: Sunday, January 4, 2015 5:47 PM
> >> To: Ouyang, Changchun; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
> >>
> >>
> >> On 01/04/15 11:01, Ouyang, Changchun wrote:
> >>>> -----Original Message-----
> >>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >>>> Sent: Sunday, January 4, 2015 4:50 PM
> >>>> To: Ouyang, Changchun; dev@dpdk.org
> >>>> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS
> >>>> mode
> >>>>
> >>>>
> >>>> On 01/04/15 09:18, Ouyang Changchun wrote:
> >>>>> Set VMDq RSS mode if it has VF(VF number is more than 1) and has
> >>>>> RSS
> >>>> information.
> >>>>> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> >>>>> ---
> >>>>>     app/test-pmd/testpmd.c | 10 ++++++++++
> >>>>>     1 file changed, 10 insertions(+)
> >>>>>
> >>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> >>>>> 8c69756..6230f8b 100644
> >>>>> --- a/app/test-pmd/testpmd.c
> >>>>> +++ b/app/test-pmd/testpmd.c
> >>>>> @@ -1708,6 +1708,16 @@ init_port_config(void)
> >>>>>     				port->dev_conf.rxmode.mq_mode =
> >>>> ETH_MQ_RX_NONE;
> >>>>>     		}
> >>>>>
> >>>>> +		if (port->dev_info.max_vfs != 0) {
> >>>>> +			if (port-
> >dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> >>>>> +				port->dev_conf.rxmode.mq_mode =
> >>>>> +					ETH_MQ_RX_VMDQ_RSS;
> >>>>> +			else {
> >>>>> +				port->dev_conf.rxmode.mq_mode =
> >>>> ETH_MQ_RX_NONE;
> >>>>> +				port->dev_conf.txmode.mq_mode =
> >>>> ETH_MQ_TX_NONE;
> >>>>
> >>>> And what about the txmode.mq_mode when RSS is available (the :if"
> >> clause)?
> >>> I think we can keep its original value for txmode.mq_mode, so don't
> >> change its value. How do you think of it?
> >>
> >> I agree that not changing a Tx mq_mode in both cases would be better.
> > In the else clause, set txmode.mq_mode as ETH_MQ_TX_NONE explicitly
> to
> > make sure it is neither ETH_MQ_TX_DCB, ETH_MQ_TX_VMDQ_DCB, nor
> ETH_MQ_TX_VMDQ_ONLY.
> 
> It's not obvious to me why u should do that since AFAIK any of these modes
> requires RX_RSS. Do I miss anything?

No, I don't think so, in the else clause, it doesn't need rx_rss, and no way to do it,
because the case is there is no rss configuration information(note: in the else clause, dev_conf.rx_adv_conf.rss_conf.rss_hf == 0). 

So ETH_MQ_RX_NONE for rx_mode, and ETH_MQ_TX_NONE for tx_mode.

> >
> >>> Thanks
> >>> Changchun
> >>>
> >>>
Vladislav Zolotarov Jan. 6, 2015, 12:53 p.m. UTC | #7
On 01/06/15 04:01, Ouyang, Changchun wrote:
>
>> -----Original Message-----
>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>> Sent: Monday, January 5, 2015 6:12 PM
>> To: Ouyang, Changchun; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
>>
>>
>> On 01/05/15 04:38, Ouyang, Changchun wrote:
>>>> -----Original Message-----
>>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>>>> Sent: Sunday, January 4, 2015 5:47 PM
>>>> To: Ouyang, Changchun; dev@dpdk.org
>>>> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
>>>>
>>>>
>>>> On 01/04/15 11:01, Ouyang, Changchun wrote:
>>>>>> -----Original Message-----
>>>>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>>>>>> Sent: Sunday, January 4, 2015 4:50 PM
>>>>>> To: Ouyang, Changchun; dev@dpdk.org
>>>>>> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS
>>>>>> mode
>>>>>>
>>>>>>
>>>>>> On 01/04/15 09:18, Ouyang Changchun wrote:
>>>>>>> Set VMDq RSS mode if it has VF(VF number is more than 1) and has
>>>>>>> RSS
>>>>>> information.
>>>>>>> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
>>>>>>> ---
>>>>>>>      app/test-pmd/testpmd.c | 10 ++++++++++
>>>>>>>      1 file changed, 10 insertions(+)
>>>>>>>
>>>>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>>>>>> 8c69756..6230f8b 100644
>>>>>>> --- a/app/test-pmd/testpmd.c
>>>>>>> +++ b/app/test-pmd/testpmd.c
>>>>>>> @@ -1708,6 +1708,16 @@ init_port_config(void)
>>>>>>>      				port->dev_conf.rxmode.mq_mode =
>>>>>> ETH_MQ_RX_NONE;
>>>>>>>      		}
>>>>>>>
>>>>>>> +		if (port->dev_info.max_vfs != 0) {
>>>>>>> +			if (port-
>>> dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
>>>>>>> +				port->dev_conf.rxmode.mq_mode =
>>>>>>> +					ETH_MQ_RX_VMDQ_RSS;
>>>>>>> +			else {
>>>>>>> +				port->dev_conf.rxmode.mq_mode =
>>>>>> ETH_MQ_RX_NONE;
>>>>>>> +				port->dev_conf.txmode.mq_mode =
>>>>>> ETH_MQ_TX_NONE;
>>>>>>
>>>>>> And what about the txmode.mq_mode when RSS is available (the :if"
>>>> clause)?
>>>>> I think we can keep its original value for txmode.mq_mode, so don't
>>>> change its value. How do you think of it?
>>>>
>>>> I agree that not changing a Tx mq_mode in both cases would be better.
>>> In the else clause, set txmode.mq_mode as ETH_MQ_TX_NONE explicitly
>> to
>>> make sure it is neither ETH_MQ_TX_DCB, ETH_MQ_TX_VMDQ_DCB, nor
>> ETH_MQ_TX_VMDQ_ONLY.
>>
>> It's not obvious to me why u should do that since AFAIK any of these modes
>> requires RX_RSS. Do I miss anything?
> No, I don't think so, in the else clause, it doesn't need rx_rss, and no way to do it,
> because the case is there is no rss configuration information(note: in the else clause, dev_conf.rx_adv_conf.rss_conf.rss_hf == 0).
>
> So ETH_MQ_RX_NONE for rx_mode, and ETH_MQ_TX_NONE for tx_mode.

Of course, however, in general, one may ask, why u configure TX MQ mode 
in "else" clause an don't do it in the "if" one. Possibly the "if" case 
in TX MQ context has been handled elsewhere but this is what makes this 
code confusing: to make it the most readable u'd rather configure the 
same feature set in both "if" and "else".
For instance:

if (bla-bla) {
   tx_mode = X1;
   rx_mode = X2;
} else {
  tx_mode = Y1;
  rx_mode = Y2;
}

Look at the non-SR-IOV clause right above the "if-else" block u've 
added. Why don't they configure tx_mode there? Is it a bug in their code?
By the way, u forgot to fix the remark below

/* In SR-IOV mode, RSS mode is not available */

which is located a few lines above the code u've added. ;)

>
>>>>> Thanks
>>>>> Changchun
>>>>>
>>>>>
Ouyang Changchun Jan. 7, 2015, 1:50 a.m. UTC | #8
> -----Original Message-----
> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> Sent: Tuesday, January 6, 2015 8:53 PM
> To: Ouyang, Changchun; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
> 
> 
> On 01/06/15 04:01, Ouyang, Changchun wrote:
> >
> >> -----Original Message-----
> >> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >> Sent: Monday, January 5, 2015 6:12 PM
> >> To: Ouyang, Changchun; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
> >>
> >>
> >> On 01/05/15 04:38, Ouyang, Changchun wrote:
> >>>> -----Original Message-----
> >>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >>>> Sent: Sunday, January 4, 2015 5:47 PM
> >>>> To: Ouyang, Changchun; dev@dpdk.org
> >>>> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS
> >>>> mode
> >>>>
> >>>>
> >>>> On 01/04/15 11:01, Ouyang, Changchun wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >>>>>> Sent: Sunday, January 4, 2015 4:50 PM
> >>>>>> To: Ouyang, Changchun; dev@dpdk.org
> >>>>>> Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS
> >>>>>> mode
> >>>>>>
> >>>>>>
> >>>>>> On 01/04/15 09:18, Ouyang Changchun wrote:
> >>>>>>> Set VMDq RSS mode if it has VF(VF number is more than 1) and has
> >>>>>>> RSS
> >>>>>> information.
> >>>>>>> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> >>>>>>> ---
> >>>>>>>      app/test-pmd/testpmd.c | 10 ++++++++++
> >>>>>>>      1 file changed, 10 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> >>>>>>> index 8c69756..6230f8b 100644
> >>>>>>> --- a/app/test-pmd/testpmd.c
> >>>>>>> +++ b/app/test-pmd/testpmd.c
> >>>>>>> @@ -1708,6 +1708,16 @@ init_port_config(void)
> >>>>>>>      				port->dev_conf.rxmode.mq_mode =
> >>>>>> ETH_MQ_RX_NONE;
> >>>>>>>      		}
> >>>>>>>
> >>>>>>> +		if (port->dev_info.max_vfs != 0) {
> >>>>>>> +			if (port-
> >>> dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> >>>>>>> +				port->dev_conf.rxmode.mq_mode =
> >>>>>>> +					ETH_MQ_RX_VMDQ_RSS;
> >>>>>>> +			else {
> >>>>>>> +				port->dev_conf.rxmode.mq_mode =
> >>>>>> ETH_MQ_RX_NONE;
> >>>>>>> +				port->dev_conf.txmode.mq_mode =
> >>>>>> ETH_MQ_TX_NONE;
> >>>>>>
> >>>>>> And what about the txmode.mq_mode when RSS is available
> (the :if"
> >>>> clause)?
> >>>>> I think we can keep its original value for txmode.mq_mode, so
> >>>>> don't
> >>>> change its value. How do you think of it?
> >>>>
> >>>> I agree that not changing a Tx mq_mode in both cases would be better.
> >>> In the else clause, set txmode.mq_mode as ETH_MQ_TX_NONE
> explicitly
> >> to
> >>> make sure it is neither ETH_MQ_TX_DCB, ETH_MQ_TX_VMDQ_DCB, nor
> >> ETH_MQ_TX_VMDQ_ONLY.
> >>
> >> It's not obvious to me why u should do that since AFAIK any of these
> >> modes requires RX_RSS. Do I miss anything?
> > No, I don't think so, in the else clause, it doesn't need rx_rss, and
> > no way to do it, because the case is there is no rss configuration
> information(note: in the else clause, dev_conf.rx_adv_conf.rss_conf.rss_hf
> == 0).
> >
> > So ETH_MQ_RX_NONE for rx_mode, and ETH_MQ_TX_NONE for tx_mode.
> 
> Of course, however, in general, one may ask, why u configure TX MQ mode
> in "else" clause an don't do it in the "if" one. Possibly the "if" case in TX MQ
> context has been handled elsewhere but this is what makes this code
> confusing: to make it the most readable u'd rather configure the same
> feature set in both "if" and "else".
> For instance:
> 
> if (bla-bla) {
>    tx_mode = X1;
>    rx_mode = X2;
> } else {
>   tx_mode = Y1;
>   rx_mode = Y2;
> }
> 
> Look at the non-SR-IOV clause right above the "if-else" block u've added.
> Why don't they configure tx_mode there? Is it a bug in their code?

It also makes sense,  I will add  tx_mode = ETH_MQ_TX_NONE as no rss for tx mode,
Rss only for rx mode.

> By the way, u forgot to fix the remark below
> 
> /* In SR-IOV mode, RSS mode is not available */
> 
> which is located a few lines above the code u've added. ;)

Sorry, I missed these few lines before, I will remove them in v5. 

Thanks
Changchun
diff mbox

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 8c69756..6230f8b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1708,6 +1708,16 @@  init_port_config(void)
 				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
 		}
 
+		if (port->dev_info.max_vfs != 0) {
+			if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
+				port->dev_conf.rxmode.mq_mode =
+					ETH_MQ_RX_VMDQ_RSS;
+			else {
+				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
+				port->dev_conf.txmode.mq_mode = ETH_MQ_TX_NONE;
+			}
+		}
+
 		port->rx_conf.rx_thresh = rx_thresh;
 		port->rx_conf.rx_free_thresh = rx_free_thresh;
 		port->rx_conf.rx_drop_en = rx_drop_en;