[dpdk-dev] [PATCH v13 12/13] eal/pci: Add rte_eal_dev_attach/detach() functions

[Tetsuya Mukawa]

2015-02-24 22:25 GMT+09:00 Maxime Leroy <maxime.leroy at 6wind.com>:
> Hi Tetsuya,
>
> On Tue, Feb 24, 2015 at 5:49 AM, Tetsuya Mukawa <mukawa at igel.co.jp> wrote:
>> These functions are used for attaching or detaching a port.
> [...]
>> +static int
>> +rte_eal_vdev_init(const char *name, const char *args)
>> +{
>> +       struct rte_driver *driver;
>> +
>> +       if (name == NULL)
>> +               return -EINVAL;
>> +
>> +       TAILQ_FOREACH(driver, &dev_driver_list, next) {
>> +               if (driver->type != PMD_VDEV)
>> +                       continue;
>> +
>> +               /*
>> +                * search a driver prefix in virtual device name.
>> +                * For example, if the driver is pcap PMD, driver->name
>> +                * will be "eth_pcap", but "name" will be "eth_pcapN".
>> +                * So use strncmp to compare.
>> +                */
>> +               if (!strncmp(driver->name, name, strlen(driver->name)))
>> +                       return driver->init(name, args);
>> +       }
>> +
>> +       if (driver == NULL) {
>
> This test is not needed anymore. You should remove it.
>

Sure I will.

>> +               RTE_LOG(ERR, EAL, "no driver found for %s\n", name);
>> +               return -EINVAL;
>> +       }
>> +       return 0;
>> +}
> [...]
>>  }
>> +
>> +/* So far, DPDK hotplug function only supports linux */
>> +#ifdef RTE_LIBRTE_EAL_HOTPLUG
>> +static int
>> +rte_eal_vdev_uninit(const char *name)
>> +{
>> +       struct rte_driver *driver;
>> +
>> +       if (name == NULL)
>> +               return -EINVAL;
>> +
>> +       TAILQ_FOREACH(driver, &dev_driver_list, next) {
>> +               if (driver->type != PMD_VDEV)
>> +                       continue;
>> +
>> +               /*
>> +                * search a driver prefix in virtual device name.
>> +                * For example, if the driver is pcap PMD, driver->name
>> +                * will be "eth_pcap", but "name" will be "eth_pcapN".
>> +                * So use strncmp to compare.
>> +                */
>> +               if (!strncmp(driver->name, name, strlen(driver->name)))
>> +                       return driver->uninit(name);
>> +       }
>> +
>> +       if (driver == NULL) {
>
> This test is not needed anymore . You should remove it.
>

I will.

>> +               RTE_LOG(ERR, EAL, "no driver found for %s\n", name);
>> +               return -EINVAL;
>> +       }
>> +       return 0;
>> +}
>> +
> [...]
>> diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
>> index e117cec..b59b201 100644
>> --- a/lib/librte_eal/linuxapp/eal/Makefile
>> +++ b/lib/librte_eal/linuxapp/eal/Makefile
>> @@ -45,6 +45,7 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
>>  CFLAGS += -I$(RTE_SDK)/lib/librte_ring
>>  CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
>>  CFLAGS += -I$(RTE_SDK)/lib/librte_malloc
>> +CFLAGS += -I$(RTE_SDK)/lib/librte_mbuf
>
> Why do you need to add librte_mbuf into EAL Makefile ?
>

I need to call some ethdev library APIs like rte_eth_dev_is_detachable().
So rte_ethdev.h is needed to be included.
Also, rte_mbuf.h is included in rte_ethdev.h.
This is because, I need to refer librte_mbuf library also.

I am out of office now, so I will submit new patches tomorrow.

Regards,
Tetsuya


>>  CFLAGS += -I$(RTE_SDK)/lib/librte_ether
>>  CFLAGS += -I$(RTE_SDK)/lib/librte_ivshmem
>>  CFLAGS += -I$(RTE_SDK)/lib/librte_pmd_ring
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> [...]
>> --
>> 1.9.1
>>
>
> Except these 3 points, ack for this patch.
>
> Regards,
>
> Maxime