Re: [dpdk-dev] [PATCH 03/22] net/hns3: register hns3 PMD driver

["Wei Hu (Xavier)" <xavier.huwei@huawei.com>]

Hi, Ferruh Yigit


On 2019/8/30 23:01, Ferruh Yigit wrote:
> On 8/23/2019 2:46 PM, Wei Hu (Xavier) wrote:
>> This patch registers hns3 PMD driver and adds the definition for log
>> interfaces.
>>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> Signed-off-by: Chunsong Feng <fengchunsong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Signed-off-by: Hao Chen <chenhao164@huawei.com>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> <...>
>
>> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
>> new file mode 100644
>> index 0000000..0587a9c
>> --- /dev/null
>> +++ b/drivers/net/hns3/hns3_ethdev.c
>> @@ -0,0 +1,141 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright(c) 2018-2019 Hisilicon Limited.
>> + */
>> +
>> +#include <errno.h>
>> +#include <stdarg.h>
>> +#include <stdbool.h>
>> +#include <stdio.h>
>> +#include <stdint.h>
>> +#include <string.h>
>> +#include <sys/queue.h>
>> +#include <inttypes.h>
>> +#include <unistd.h>
>> +#include <arpa/inet.h>
>> +#include <rte_alarm.h>
>> +#include <rte_atomic.h>
>> +#include <rte_bus_pci.h>
>> +#include <rte_byteorder.h>
>> +#include <rte_common.h>
>> +#include <rte_cycles.h>
>> +#include <rte_debug.h>
>> +#include <rte_dev.h>
>> +#include <rte_eal.h>
>> +#include <rte_ether.h>
>> +#include <rte_ethdev_driver.h>
>> +#include <rte_ethdev_pci.h>
>> +#include <rte_interrupts.h>
>> +#include <rte_io.h>
>> +#include <rte_log.h>
>> +#include <rte_pci.h>
> Are all these headers really used at this stage? Can you please clean them and
> add later patches when they are required?
>
> <...>
>
>> +static int
>> +hns3_dev_init(struct rte_eth_dev *eth_dev)
>> +{
>> +	struct rte_device *dev = eth_dev->device;
>> +	struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev);
>> +	struct hns3_adapter *hns = eth_dev->data->dev_private;
>> +	struct hns3_hw *hw = &hns->hw;
>> +	uint16_t device_id = pci_dev->id.device_id;
>> +	int ret;
>> +
>> +	PMD_INIT_FUNC_TRACE();
>> +
>> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>> +		return 0;
>> +
>> +	eth_dev->dev_ops = &hns3_eth_dev_ops;
>> +	rte_eth_copy_pci_info(eth_dev, pci_dev);
> I think no need to call 'rte_eth_copy_pci_info()', it is called by
> 'rte_eth_dev_pci_generic_probe()' before 'hns3_dev_init()' called.
I will fix it in patch V2.
>
>> +
>> +	hns->is_vf = false;
> There is a separate VF driver, is this field still needed?
hns3 PMD driver includes PF and VF drivers.
In the struct hns3_adapter data, the memeber named is_vf is used to
distinguish PF from VF devices.
>> +	hw->data = eth_dev->data;
>> +	hw->adapter_state = HNS3_NIC_INITIALIZED;
>> +
>> +	return 0;
> Init should set 'RTE_ETH_DEV_CLOSE_REMOVE' flag, and '.dev_close' should free
> the driver allocated resources, which there is not up until this patch:
>
>  +eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
>
I will fix it in patch V2.
Thanks for your suggestion.

    Regards
Xavier