From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id E68D52A5E for ; Thu, 13 Oct 2016 15:57:57 +0200 (CEST) Received: from [10.16.0.195] (unknown [10.16.0.195]) by proxy.6wind.com (Postfix) with ESMTP id 4FEFA1FDF0; Thu, 13 Oct 2016 15:57:55 +0200 (CEST) To: Yuanhan Liu 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> <57FE5E55.4010809@6wind.com> <20161013075415.GS16751@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 Message-ID: <57FF92E1.7050705@6wind.com> Date: Thu, 13 Oct 2016 15:57:53 +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: <20161013075415.GS16751@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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Oct 2016 13:57:58 -0000 On 10/13/2016 09:54 AM, Yuanhan Liu wrote: > On Wed, Oct 12, 2016 at 06:01:25PM +0200, Olivier MATZ wrote: >> 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. > > I see. Thanks. > > Besides, I think you should return error when LRO is not negoiated > after the reset (say, when it's disabled through qemu command line)? Good idea, I now return an error if offload cannot be negotiated. Olivier