On 3/1/2016 6:09 PM, Xie, Huawei wrote: > On 3/1/2016 5:57 PM, Thomas Monjalon wrote: >> 2016-03-01 08:39, Xie, Huawei: >>> On 3/1/2016 4:24 PM, Thomas Monjalon wrote: >>>> 2016-03-01 07:53, Xie, Huawei: >>>>> On 3/1/2016 3:18 PM, Thomas Monjalon wrote: >>>>>> 2016-02-26 09:53, Huawei Xie: >>>>>>> @@ -1037,8 +1039,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) >>>>>>> >>>>>>> pci_dev = eth_dev->pci_dev; >>>>>>> >>>>>>> - if (vtpci_init(pci_dev, hw) < 0) >>>>>>> - return -1; >>>>>>> + ret = vtpci_init(pci_dev, hw); >>>>>>> + if (ret) { >>>>>>> + rte_free(eth_dev->data->mac_addrs); >>>>>> The freeing seems not related to this patch. >>>>> I can send a separate patch, ok within this patchset? >>>> Yes >>>> >>>>>> [...] >>>>>>> PMD_INIT_LOG(INFO, "trying with legacy virtio pci."); >>>>>>> - if (legacy_virtio_resource_init(dev, hw) < 0) >>>>>>> + if (legacy_virtio_resource_init(dev, hw) < 0) { >>>>>>> + if (dev->kdrv == RTE_KDRV_UNKNOWN) { >>>>>>> + PMD_INIT_LOG(INFO, >>>>>>> + "skip kernel managed virtio device."); >>>>>>> + return 1; >>>>>>> + } >>>>>>> return -1; >>>>>>> + } >>>>>> You cannot skip a device if it was whitelisted. >>>>>> I think you should check RTE_DEVTYPE_WHITELISTED_PCI and throw an error >>>>>> in this case. >>>>> I feel there is a subtle difference on the understanding of -w args. To >>>>> me, without it, probe all devices; with it, only probe whiltelisted API. >>>>> That is all. >>>> I don't know if it is clearly documented indeed. >>>> >>>>> Do you mean that -w implies that devices whitelisted must be probed >>>>> successfully otherwise we throw an error? If i get it right, then what >>>>> about the devices whitelisted but without PMD driver? >>>> Yes we should probably consider the whitelist as a "forced" init. >>>> Later, we could introduce some device flags for probing/discovery: >>>> PROBE_AUTO, PROBE_FORCE, PROBE_IGNORE. It would make white/black list >>>> more precise. >>>> >>>>> I will fix, :). >>>>> if (dev->kdrv == RTE_KDRV_UNKNOWN && dev->devargs->type != >>>>> RTE_DEVTYPE_WHITELISTED_PCI) { >>>>> .... >>>>> return 1; >>>>> } >>>> You should also consider the blacklist case: if there is a blacklist, >>>> the not blacklisted devices must be initialised or throw an error. >>>> >>> Don't we already skip probing the blacklisted device in >>> rte_eal_pci_probe_one_driver? >> Yes, I'm talking about the devices which are not blacklisted. >> Having some blacklisted devices imply that others are implicitly whitelisted. > For blacklist, it only means the blacklisted device should be excluded > from being probed. It doesn't mean all other devices should be probed > either successfully or otherwise throw an error which cause DPDK exit. > Even that, the upper layer should explicitly put the non-blacklisted > device into whitelist, i mean here we should only deal with whitelist.
Thomas, comments? Btw, i have reworked the patch, but git am always complains patch format detection failed. Investigating. Will send it later. >