[dpdk-dev] [PATCH v2 03/12] virtio: reinitialize the device in configure callback

Yuanhan Liu yuanhan.liu at linux.intel.com
Thu Oct 13 09:54:15 CEST 2016


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

	--yliu


More information about the dev mailing list