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 <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 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.
>
>> <...>
>
>
