[dpdk-dev] [PATCH 01/56] net/sfc: libefx-based PMD stub sufficient to build and init

Andrew Rybchenko arybchenko at solarflare.com
Thu Nov 24 16:59:10 CET 2016


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 <amoreton at solarflare.com>
>> Signed-off-by: Andrew Rybchenko <arybchenko at solarflare.com>
>> ---
>>   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've failed to find list of compiler versions which must/should be checked.
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.

> <...>




More information about the dev mailing list