From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vladz@cloudius-systems.com>
Received: from mail-we0-f177.google.com (mail-we0-f177.google.com
 [74.125.82.177]) by dpdk.org (Postfix) with ESMTP id 9F029569A
 for <dev@dpdk.org>; Tue,  6 Jan 2015 13:53:32 +0100 (CET)
Received: by mail-we0-f177.google.com with SMTP id q59so9554220wes.36
 for <dev@dpdk.org>; Tue, 06 Jan 2015 04:53:32 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to
 :subject:references:in-reply-to:content-type
 :content-transfer-encoding;
 bh=+0TEMWfz7ylM1vot29dICRi6auLjqiLpzpJGNv2df9I=;
 b=C4P6b1eXoiUkj+4QsDs9EKYO+gMJwfy6iGhE8MsILi9+bBqiECUYV5uslsdmZQWNac
 d50lnruTV2nIWlw0DtbyjyfS9KelhdoxeC9A7uWabDV029rEAVgc2Y2GOOz8VofPp3JE
 mCFwPAe7S3c7qu15843/LueqqM1AzZH8tuOv+WKx583644ELbnJcubVBn6dLViECyqHe
 AYmoyKrUkL3CN24plKM9oNDSY4A5/BMO/XlOjhgIT9hyK1fe8xgPjj9Y1eKWAvsia5KQ
 6Qpx/MYJPx4qyYBMLbA+r6X0r98uP4RFxAFQ7pLsKaVZybaX44ZMgoxO4zvSvUHceIUc
 V2WQ==
X-Gm-Message-State: ALoCoQkx36wROpQoZAnqhAnozbz5UeICxJuPpAD/HzuQiJpnxOwA/mTJJQ9azDP9UCUx3Skda/hc
X-Received: by 10.180.75.237 with SMTP id f13mr35990468wiw.69.1420548812199;
 Tue, 06 Jan 2015 04:53:32 -0800 (PST)
Received: from [10.0.0.165] (system.cloudius-systems.com. [84.94.198.183])
 by mx.google.com with ESMTPSA id f7sm13855366wiz.13.2015.01.06.04.53.29
 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Tue, 06 Jan 2015 04:53:31 -0800 (PST)
Message-ID: <54ABDAC4.40506@cloudius-systems.com>
Date: Tue, 06 Jan 2015 14:53:24 +0200
From: Vlad Zolotarov <vladz@cloudius-systems.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:31.0) Gecko/20100101 Thunderbird/31.3.0
MIME-Version: 1.0
To: "Ouyang, Changchun" <changchun.ouyang@intel.com>, 
 "dev@dpdk.org" <dev@dpdk.org>
References: <1419398584-19520-1-git-send-email-changchun.ouyang@intel.com>
 <1420355937-18484-1-git-send-email-changchun.ouyang@intel.com>
 <1420355937-18484-7-git-send-email-changchun.ouyang@intel.com>
 <54A8FE9A.60606@cloudius-systems.com>
 <F52918179C57134FAEC9EA62FA2F96251194DA33@shsmsx102.ccr.corp.intel.com>
 <54A90C07.5090507@cloudius-systems.com>
 <F52918179C57134FAEC9EA62FA2F96251194E254@shsmsx102.ccr.corp.intel.com>
 <54AA6385.7020009@cloudius-systems.com>
 <F52918179C57134FAEC9EA62FA2F96251194E971@shsmsx102.ccr.corp.intel.com>
In-Reply-To: <F52918179C57134FAEC9EA62FA2F96251194E971@shsmsx102.ccr.corp.intel.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH v4 6/6] testpmd: Set Rx VMDq RSS mode
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 06 Jan 2015 12:53:32 -0000


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
>>>>>
>>>>>