From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com
 [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id 6FE348D9C
 for <dev@dpdk.org>; Wed, 12 Oct 2016 18:01:30 +0200 (CEST)
Received: from [10.16.0.195] (unknown [10.16.0.195])
 by proxy.6wind.com (Postfix) with ESMTP id 4FFCD5B657;
 Wed, 12 Oct 2016 18:01:30 +0200 (CEST)
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
References: <1475485223-30566-1-git-send-email-olivier.matz@6wind.com>
 <1475485223-30566-4-git-send-email-olivier.matz@6wind.com>
 <20161012144108.GN16751@yliu-dev.sh.intel.com>
Cc: dev@dpdk.org, konstantin.ananyev@intel.com, sugesh.chandran@intel.com,
 bruce.richardson@intel.com, jianfeng.tan@intel.com, helin.zhang@intel.com,
 adrien.mazarguil@6wind.com, stephen@networkplumber.org, dprovan@bivio.net,
 xiao.w.wang@intel.com
From: Olivier MATZ <olivier.matz@6wind.com>
Message-ID: <57FE5E55.4010809@6wind.com>
Date: Wed, 12 Oct 2016 18:01:25 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Icedove/38.6.0
MIME-Version: 1.0
In-Reply-To: <20161012144108.GN16751@yliu-dev.sh.intel.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH v2 03/12] virtio: reinitialize the device in
 configure callback
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: Wed, 12 Oct 2016 16:01:30 -0000

Hello Yuanhan,

On 10/12/2016 04:41 PM, Yuanhan Liu wrote:
> On Mon, Oct 03, 2016 at 11:00:14AM +0200, Olivier Matz wrote:
>> @@ -1344,6 +1347,7 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>>   {
>>   	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>>   	struct virtio_hw *hw = dev->data->dev_private;
>> +	uint64_t req_features;
>>   	int ret;
>>
>>   	PMD_INIT_LOG(DEBUG, "configure");
>> @@ -1353,6 +1357,14 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>>   		return -EINVAL;
>>   	}
>>
>> +	req_features = VIRTIO_PMD_GUEST_FEATURES;
>> +	/* if request features changed, reinit the device */
>> +	if (req_features != hw->req_guest_features) {
>> +		ret = virtio_init_device(dev, req_features);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>
> Why do you have to reset virtio here? This doesn't make too much sense
> to me.
>
> IIUC, you want to make sure those TSO related features being unset at
> init time, and enable it (by doing reset) when it's asked to be enabled
> (by rte_eth_dev_configure)?
>
> Why not always setting those features? We could do the actual offloads
> when:
>
> - those features have been negoiated
>
> - they are enabled through rte_eth_dev_configure
>
> With that, I think we could avoid the reset here?

It would work for TX, since you decide to use or not the feature. But I 
think this won't work for RX: if you negociate LRO at init, the host may 
send you large packets, even if LRO is disabled in dev_configure.

Regards,
Olivier