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?
