On 8/23/2019 2:47 PM, Wei Hu (Xavier) wrote:
> This patch add build related files for hns3 PMD driver.
>
> Signed-off-by: Wei Hu (Xavier) <xavier.hu...@huawei.com>
> Signed-off-by: Min Hu (Connor) <humi...@huawei.com>
> Signed-off-by: Chunsong Feng <fengchuns...@huawei.com>
> Signed-off-by: Hao Chen <chenhao...@huawei.com>
> Signed-off-by: Huisong Li <lihuis...@huawei.com>
> ---
> MAINTAINERS | 7 ++++
> config/common_armv8a_linux | 5 +++
> config/common_base | 5 +++
> config/defconfig_arm64-armv8a-linuxapp-clang | 2 +
> doc/guides/nics/features/hns3.ini | 38 +++++++++++++++++++
> doc/guides/nics/hns3.rst | 55
> ++++++++++++++++++++++++++++
This file needs to be added to the index file: 'doc/guides/nics/index.rst'
<...>
> diff --git a/config/defconfig_arm64-armv8a-linuxapp-clang
> b/config/defconfig_arm64-armv8a-linuxapp-clang
> index d3b4dad..c73f5fb 100644
> --- a/config/defconfig_arm64-armv8a-linuxapp-clang
> +++ b/config/defconfig_arm64-armv8a-linuxapp-clang
> @@ -6,3 +6,5 @@
>
> CONFIG_RTE_TOOLCHAIN="clang"
> CONFIG_RTE_TOOLCHAIN_CLANG=y
> +
> +CONFIG_RTE_LIBRTE_HNS3_PMD=n
I can understand the architecture ones, but why clang is not supported? Can you
please add this support?
<...>
> diff --git a/doc/guides/nics/hns3.rst b/doc/guides/nics/hns3.rst
> new file mode 100644
> index 0000000..c9d0253
> --- /dev/null
> +++ b/doc/guides/nics/hns3.rst
> @@ -0,0 +1,55 @@
> +.. SPDX-License-Identifier: BSD-3-Clause
> + Copyright(c) 2018-2019 Hisilicon Limited.
> +
> +HNS3 Poll Mode Driver
> +===============================
> +
> +The Hisilicon Network Subsystem is a long term evolution IP which is
> +supposed to be used in Hisilicon ICT SoCs such as Kunpeng 920.
Can you please add a official link/reference to the product?
<...>
> @@ -0,0 +1,43 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2018-2019 Hisilicon Limited.
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_pmd_hns3.a
> +
> +CFLAGS += -O3
> +CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -DALLOW_EXPERIMENTAL_API -fsigned-char
Why '-DALLOW_EXPERIMENTAL_API' is required? Can we remove it?
> +
> +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
> +LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs -lrte_hash
> +LDLIBS += -lrte_bus_pci
Are all these libraries really required, like kvargs? Can you please clean the
unused ones?
> +
> +EXPORT_MAP := rte_pmd_hns3_version.map
> +
> +LIBABIVER := 2
It should be 1.
<...>
> +# install this header file
> +SYMLINK-$(CONFIG_RTE_LIBRTE_HNS3_PMD)-include := hns3_ethdev.h
No need to expose the header file, it is not public header.
<...>
> @@ -0,0 +1,19 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2018-2019 Hisilicon Limited
> +
> +sources = files('hns3_cmd.c',
> + 'hns3_dcb.c',
> + 'hns3_intr.c',
> + 'hns3_ethdev.c',
> + 'hns3_ethdev_vf.c',
> + 'hns3_fdir.c',
> + 'hns3_flow.c',
> + 'hns3_mbx.c',
> + 'hns3_regs.c',
> + 'hns3_rss.c',
> + 'hns3_rxtx.c',
> + 'hns3_stats.c',
> + 'hns3_mp.c')
> +deps += ['hash']
> +
> +cflags += '-DALLOW_EXPERIMENTAL_API'
There is better way to do this in meson, please check other samples. But as the
makefile comment, does it really needed, if so can you please add the
experimental APIs used as a comment, to both meson and Makefile?
> diff --git a/drivers/net/hns3/rte_pmd_hns3_version.map
> b/drivers/net/hns3/rte_pmd_hns3_version.map
> new file mode 100644
> index 0000000..3aef967
> --- /dev/null
> +++ b/drivers/net/hns3/rte_pmd_hns3_version.map
> @@ -0,0 +1,3 @@
> +DPDK_19.08 {
DPDK_19.11
