[dpdk-dev] [PATCH v4 4/4] virtio: return 1 to tell the upper layer we don't take over this device

[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.
Even that, the upper layer should explicitly put the non-blacklisted
device into whitelist, i mean here we should only deal with whitelist.
>