From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 4870D47CD for ; Fri, 25 Nov 2016 11:17:59 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP; 25 Nov 2016 02:17:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,546,1473145200"; d="scan'208";a="1064026668" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.57]) ([10.237.220.57]) by orsmga001.jf.intel.com with ESMTP; 25 Nov 2016 02:17:58 -0800 To: Andrew Rybchenko , dev@dpdk.org References: <1479740470-6723-1-git-send-email-arybchenko@solarflare.com> <1479740470-6723-2-git-send-email-arybchenko@solarflare.com> From: Ferruh Yigit Message-ID: Date: Fri, 25 Nov 2016 10:17:57 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 01/56] net/sfc: libefx-based PMD stub sufficient to build and init X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Nov 2016 10:18:00 -0000 On 11/24/2016 3:59 PM, Andrew Rybchenko wrote: > On 11/23/2016 06:26 PM, Ferruh Yigit wrote: >> On 11/21/2016 3:00 PM, Andrew Rybchenko wrote: >>> The PMD is put into the sfc/efx subdirectory to have a place for >>> the second PMD and library shared by both. >>> >>> Enable the PMD by default on supported configuratons. >>> >>> Reviewed-by: Andy Moreton >>> Signed-off-by: Andrew Rybchenko >>> --- >>> MAINTAINERS | 6 ++ >>> config/common_base | 6 ++ >>> config/defconfig_arm-armv7a-linuxapp-gcc | 1 + >>> config/defconfig_arm64-armv8a-linuxapp-gcc | 1 + >>> config/defconfig_i686-native-linuxapp-gcc | 5 + >>> config/defconfig_i686-native-linuxapp-icc | 5 + >>> config/defconfig_ppc_64-power8-linuxapp-gcc | 1 + >>> config/defconfig_tile-tilegx-linuxapp-gcc | 1 + >>> config/defconfig_x86_64-native-linuxapp-icc | 5 + >>> config/defconfig_x86_x32-native-linuxapp-gcc | 5 + >>> doc/guides/nics/features/sfc_efx.ini | 10 ++ >>> doc/guides/nics/index.rst | 1 + >>> doc/guides/nics/sfc_efx.rst | 109 +++++++++++++++++++++ >> Can you also update release notes please, to announce new driver. > > Thanks, will do in v2. > >> <...> >> >>> diff --git a/drivers/net/sfc/efx/Makefile b/drivers/net/sfc/efx/Makefile >>> new file mode 100644 >>> index 0000000..71f07ca >>> --- /dev/null >>> +++ b/drivers/net/sfc/efx/Makefile >>> @@ -0,0 +1,81 @@ >> <...> >>> + >>> +include $(RTE_SDK)/mk/rte.vars.mk >>> + >>> +# >>> +# library name >>> +# >>> +LIB = librte_pmd_sfc_efx.a >>> + >>> +CFLAGS += -O3 >>> + >>> +# Enable basic warnings but disable some which are accepted >>> +CFLAGS += -Wall >> It is possible to use $(WERROR_FLAGS), which set automatically based on >> selected compiler. See mk/toolchain/* . > > Thanks, will do in v2. > >> And you can add extra options here, please keep in mind that there are >> three compiler supported right now: gcc, clang and icc. You may require >> to add compiler and version checks.. > > I've tried to disable the driver build on ICC since we've never tested it. I believe we don't support selective config per compiler. Currently if a code is enabled by default, it should support compilation with all three compilers. > I've failed to find list of compiler versions which must/should be checked. That list is not clear as far as I know. Mostly version related fixes added based on reported build errors. So you can leave as it is right now, or can test with default compiler versions of some common distributions. > I've tested versions which come with RHEL 7.2, Debian Jessie and Sid. > (In v1 I've lost my fixes for clang which produce warnings because of > unsupported -W option) > >>> +CFLAGS += -Wno-strict-aliasing >>> + >>> +# Enable extra warnings but disable some which are accepted >>> +CFLAGS += -Wextra >>> +CFLAGS += -Wno-empty-body >>> +CFLAGS += -Wno-sign-compare >>> +CFLAGS += -Wno-type-limits >>> +CFLAGS += -Wno-unused-parameter >> Is there a way to not disable these warnings but fix in the source code? >> Or move to CFLAGS_BASE_DRIVER, if the reason is the base driver? > > Will do in v2. > >>> + >>> +# More warnings not enabled by above aggregators >>> +CFLAGS += -Waggregate-return >>> +CFLAGS += -Wbad-function-cast >>> +CFLAGS += -Wcast-qual >>> +CFLAGS += -Wdisabled-optimization >>> +CFLAGS += -Wmissing-declarations >>> +CFLAGS += -Wmissing-prototypes >>> +CFLAGS += -Wnested-externs >>> +CFLAGS += -Wold-style-definition >>> +CFLAGS += -Wpointer-arith >>> +CFLAGS += -Wstrict-prototypes >>> +CFLAGS += -Wundef >>> +CFLAGS += -Wwrite-strings >> If you believe some can be useful for everybody, please feel free to add >> to mk/toolchain/* . > > I'll definitely remove duplicates which are already included in > $(WERROR_FLAGS). > I'd prefer to keep the rest just here for now. I think that adding it > world-wide > requires testing on really many compiler versions etc. > >>> + >>> +EXPORT_MAP := rte_pmd_sfc_efx_version.map >>> + >>> +LIBABIVER := 1 >>> + >>> +# >>> +# all source are stored in SRCS-y >>> +# >>> +SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_ethdev.c >>> +SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_kvargs.c >>> + >>> + >>> +# this lib depends upon: >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_eal >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_kvargs >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_ether >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_mempool >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_mbuf >>> + >>> +include $(RTE_SDK)/mk/rte.lib.mk >>> diff --git a/drivers/net/sfc/efx/rte_pmd_sfc_efx_version.map b/drivers/net/sfc/efx/rte_pmd_sfc_efx_version.map >>> new file mode 100644 >>> index 0000000..1901bcb >>> --- /dev/null >>> +++ b/drivers/net/sfc/efx/rte_pmd_sfc_efx_version.map >>> @@ -0,0 +1,4 @@ >>> +DPDK_16.07 { >> Now this become 17.02 > > Thanks, will fix in v2. > >>> + >>> + local: *; >>> +}; >>> diff --git a/drivers/net/sfc/efx/sfc.h b/drivers/net/sfc/efx/sfc.h >>> new file mode 100644 >>> index 0000000..16fd2bb >>> --- /dev/null >>> +++ b/drivers/net/sfc/efx/sfc.h >>> @@ -0,0 +1,53 @@ >> <..> >>> + >>> +#ifndef _SFC_H >>> +#define _SFC_H >> s/^I/ / >> This also exists in other locations and files.. > > Will fix in v2. > I thought that DPDK prefers TAB after #define as FreeBSD does, but > counting shows that space is really preferred. > I think that such things should be caught by checkpatch. > >> <...> > >