From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Subject: Re: [PATCH v2 03/12] virtio: reinitialize the device in
 configure callback
Date: Wed, 12 Oct 2016 22:41:08 +0800
Message-ID: <20161012144108.GN16751@yliu-dev.sh.intel.com>
References: <1475485223-30566-1-git-send-email-olivier.matz@6wind.com>
 <1475485223-30566-4-git-send-email-olivier.matz@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
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
To: Olivier Matz <olivier.matz@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
 by dpdk.org (Postfix) with ESMTP id A5BE737AC
 for <dev@dpdk.org>; Wed, 12 Oct 2016 16:40:15 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <1475485223-30566-4-git-send-email-olivier.matz@6wind.com>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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?

	--yliu