2016-03-01 10:08, Xie, Huawei: > 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.
Yes it is. Currently we have 3 cases: - probe auto (best effort) - whitelist (implicitly blacklist other devices) - blacklist (implicitly whitelist other devices) If you want to mix blacklist and best effort (auto probing), we must set these requirements as per device flags instead of the current lists. > Even that, the upper layer should explicitly put the non-blacklisted > device into whitelist, i mean here we should only deal with whitelist. It is not the way it is done currently. But some per-device flags could be introduced later to make it explicit and deal only with the whitelist flag (let's call it PROBE_FORCE).