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