Re: [dpdk-dev] [PATCH v10 13/14] eal/pci: Add rte_eal_dev_attach/detach() functions

[Tetsuya Mukawa <mukawa@igel.co.jp>]

On 2015/02/21 0:20, Maxime Leroy wrote:
> Hi Tetsuya,
>
> Thanks for your comment.
>
> On Fri, Feb 20, 2015 at 11:32 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
>> On 2015/02/20 19:14, Maxime Leroy wrote:
>>> Hi Tetsuya,
> [...]
>> Hi Maxime,
>>
>> I appreciate for your comment.
>>
>> When rte_eal_init() is called, if we have "--vdev" options, these will
>> be stored in vdevargs as you describe above.
>> I agree this is the current behavior of DPDK.
>>
>> When we call hotplug functions, I guess doing same thing will be more
>> consistent design.
> The rte_eal_devargs_add is here to store a white list parameters for
> later creating the devices.
> It means that the devargs_list is only needed at the init to store the
> devargs parsed .
> I think we should not use the devargs_list after eal initialization.
>
> Why you want to add devargs in the devargs_list, if there are no needs
> to store this information ?

In eal initialization code, virtual device names stored in devargs are
checked not to register a same device name twice.
And each init function of PMD just trust a device name received by eal.
So there is no code in PMD to check whether device name is unique.

For example, according to your suggestion, how to prevent below case?
$ ./testpmd -c f -n 1 -- -i
testpmd> port attach eth_pcap0,iface=eth0
testpmd> port attach eth_pcap0,iface=eth1

Also, type below, after doing above.
testpmd> port detach 0

Probably port 0 will be "eth_pcap0,iface=eth0".
But uninit code of PMD only receives a device name like 'eth_pcap0'.
(We have 2 'eth_pcap0' devices in PMD.)

To prevent above case, probably we have 2 options at least.
One is changing init code of all virtual PMDs not to register same
device name.
The other is to use devargs_list in EAL, and call init code of PMD with
a unique device name.

But first case, eal initialization code can calls init of PMD with
unique name, but hotplug cannot.
It's not so consistent behavior.
Also, we need to have almost same code that assure unique name in each PMD.

> At the end, it adds extra codes for nothing.
>
>> For example, we can do like below.
>> 1. $ ./testpmd --vdev 'eth_pcap' -- -i
>> 2. testpmd>port detach
> It's exactly the same for physical device:
> 1. $./testpmd -w 0000:08:00:1 -- -i
> 2. testpmd> port detach
>
> But you don't call rte_eal_devargs_add with RTE_DEVTYPE_WHILISTED_PCI
> in  rte_eal_dev_attach_pdev ?

Yes, I don't.
Hotplug functions should not change BLACKLIST and WHITELIST.
So not to touch the list is correct behavior.

I guess you feel something strange about usage of devargs_list.
If so, I agree with it.
Probably the issue is that devargs_list is used for not only storing
command parameters, but also not to register same virtual device name twice.

> Thus it makes the hotplug implementation not coherent for me.
>
> What do you think ?

Anyway, here is my guess.

Current DPDK uses devargs not to register same virtual device name.
If we follow this, probably using devargs is straight forward.

But if we should change the design itself, could you check below.
http://dpdk.org/dev/patchwork/patch/3374/
Thomas and I agreed that I will try to change design in post-rc1.

In a future patch, probably it might be nice to have virtualdev_list,
then we can use devargs_list only for storing command arguments.

Regards,
Tetsuya

> Regards,
>
> Maxime