From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id C8C82A2EEB for ; Tue, 10 Sep 2019 13:44:10 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id CCC7E1EE59; Tue, 10 Sep 2019 13:44:09 +0200 (CEST) Received: from huawei.com (szxga07-in.huawei.com [45.249.212.35]) by dpdk.org (Postfix) with ESMTP id 1E7131C1BA for ; Tue, 10 Sep 2019 13:44:08 +0200 (CEST) Received: from DGGEMS409-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 7910BBE45DC8E06020F5; Tue, 10 Sep 2019 19:44:05 +0800 (CST) Received: from [127.0.0.1] (10.57.115.182) by DGGEMS409-HUB.china.huawei.com (10.3.19.209) with Microsoft SMTP Server id 14.3.439.0; Tue, 10 Sep 2019 19:43:58 +0800 To: Ferruh Yigit , References: <1566568031-45991-1-git-send-email-xavier.huwei@huawei.com> <1566568031-45991-23-git-send-email-xavier.huwei@huawei.com> CC: , , , From: "Wei Hu (Xavier)" Message-ID: <81bb00fb-1775-1632-52e4-991b1d788a97@huawei.com> Date: Tue, 10 Sep 2019 19:43:58 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.57.115.182] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH 22/22] net/hns3: add hns3 build files X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, Ferruh Yigit On 2019/8/30 22:58, Ferruh Yigit wrote: > 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) >> Signed-off-by: Min Hu (Connor) >> Signed-off-by: Chunsong Feng >> Signed-off-by: Hao Chen >> Signed-off-by: Huisong Li >> --- >> 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' Thanks for your suggestion. We will fix it in patch V2. > > <...> > >> 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? We will fix it in patch V2. > <...> > >> 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? > The official website link of kunpeng920 chip is not available yet, And we will paste the link to the servers product using kunpeng920 in patch V2. Thanks for your suggestion. > <...> > >> @@ -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? There are some APIs as follows in hns3 PMD driver, '-DALLOW_EXPERIMENTAL_API' can clear warning during compiling. # Experimantal APIs: # - rte_mp_action_register # - rte_mp_action_unregister # - rte_mp_reply # - rte_mp_request_sync >> + >> +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? We will fix it in patch V2. >> + >> +EXPORT_MAP := rte_pmd_hns3_version.map >> + >> +LIBABIVER := 2 > It should be 1. We will fix it in patch V2. > > <...> > >> +# 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. We will fix it in patch V2. > > <...> > >> @@ -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? I will update it as follows: allow_experimental_apis = true # Experimantal APIs: # - rte_mp_action_register # - rte_mp_action_unregister # - rte_mp_reply # - rte_mp_request_sync Thanks for your suggestion. >> 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 We will fix it in patch V2. Thanks for your suggestion. Regards Xavier > > . >