Re: [dpdk-dev] [PATCH v1 01/58] net/octeontx2: add build infrastructure

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 6/2/2019 4:23 PM, jerinj@marvell.com wrote:
> From: Jerin Jacob <jerinj@marvell.com>
> 
> Adding bare minimum PMD library and doc build infrastructure.
> 
> Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>  config/common_base                            |  5 +++
>  doc/guides/nics/features/octeontx2.ini        |  8 ++++
>  doc/guides/nics/features/octeontx2_vec.ini    |  8 ++++
>  doc/guides/nics/features/octeontx2_vf.ini     |  8 ++++
>  drivers/net/Makefile                          |  1 +
>  drivers/net/meson.build                       |  2 +-
>  drivers/net/octeontx2/Makefile                | 38 +++++++++++++++++++
>  drivers/net/octeontx2/meson.build             | 24 ++++++++++++
>  drivers/net/octeontx2/otx2_ethdev.c           |  3 ++
>  .../octeontx2/rte_pmd_octeontx2_version.map   |  4 ++
>  mk/rte.app.mk                                 |  2 +

It can be good to include MAINTAINERS file in this patch, of course with the
content that introduced in this patch.

>  11 files changed, 102 insertions(+), 1 deletion(-)
>  create mode 100644 doc/guides/nics/features/octeontx2.ini
>  create mode 100644 doc/guides/nics/features/octeontx2_vec.ini
>  create mode 100644 doc/guides/nics/features/octeontx2_vf.ini
>  create mode 100644 drivers/net/octeontx2/Makefile
>  create mode 100644 drivers/net/octeontx2/meson.build
>  create mode 100644 drivers/net/octeontx2/otx2_ethdev.c
>  create mode 100644 drivers/net/octeontx2/rte_pmd_octeontx2_version.map
> 
> diff --git a/config/common_base b/config/common_base
> index 4a3de0360..38edad355 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -405,6 +405,11 @@ CONFIG_RTE_LIBRTE_LIO_DEBUG_REGS=n
>  #
>  CONFIG_RTE_LIBRTE_OCTEONTX_PMD=y
>  
> +#
> +# Compile burst-oriented Cavium OCTEONTX2 network PMD driver
> +#
> +CONFIG_RTE_LIBRTE_OCTEONTX2_PMD=y
> +

Since .ini files only has "ARMv8", should the PMD disabled in other config files?
Or is the support coming for those architectures in next patches?
If this is only for Armv8 & Linux, better to keep disabled it in the base config
and enable only in that specific config file.

>  #
>  # Compile WRS accelerated virtual port (AVP) guest PMD driver
>  #
> diff --git a/doc/guides/nics/features/octeontx2.ini b/doc/guides/nics/features/octeontx2.ini
> new file mode 100644
> index 000000000..0ec3b6983
> --- /dev/null
> +++ b/doc/guides/nics/features/octeontx2.ini
> @@ -0,0 +1,8 @@
> +;
> +; Supported features of the 'octeontx2' network poll mode driver.
> +;
> +; Refer to default.ini for the full list of available PMD features.
> +;
> +[Features]
> +Linux VFIO           = Y
> +ARMv8                = Y
> diff --git a/doc/guides/nics/features/octeontx2_vec.ini b/doc/guides/nics/features/octeontx2_vec.ini
> new file mode 100644
> index 000000000..774f136c1
> --- /dev/null
> +++ b/doc/guides/nics/features/octeontx2_vec.ini
> @@ -0,0 +1,8 @@
> +;
> +; Supported features of the 'octeontx2_vec' network poll mode driver.
> +;
> +; Refer to default.ini for the full list of available PMD features.
> +;
> +[Features]
> +Linux VFIO           = Y
> +ARMv8                = Y

I think it is good to introduce vector .ini file with the patch that enables
vector path, same with below vf one.

> diff --git a/doc/guides/nics/features/octeontx2_vf.ini b/doc/guides/nics/features/octeontx2_vf.ini
> new file mode 100644
> index 000000000..36642354e
> --- /dev/null
> +++ b/doc/guides/nics/features/octeontx2_vf.ini
> @@ -0,0 +1,8 @@
> +;
> +; Supported features of the 'octeontx2_vf' network poll mode driver.
> +;
> +; Refer to default.ini for the full list of available PMD features.
> +;
> +[Features]
> +Linux VFIO           = Y
> +ARMv8                = Y
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 3a72cf38c..5bb618b21 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -45,6 +45,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_NFP_PMD) += nfp
>  DIRS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += bnxt
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += null
>  DIRS-$(CONFIG_RTE_LIBRTE_OCTEONTX_PMD) += octeontx
> +DIRS-$(CONFIG_RTE_LIBRTE_OCTEONTX2_PMD) += octeontx2
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += pcap
>  DIRS-$(CONFIG_RTE_LIBRTE_QEDE_PMD) += qede
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_RING) += ring
> diff --git a/drivers/net/meson.build b/drivers/net/meson.build
> index ed99896c3..086a2f4cd 100644
> --- a/drivers/net/meson.build
> +++ b/drivers/net/meson.build
> @@ -31,7 +31,7 @@ drivers = ['af_packet',
>  	'netvsc',
>  	'nfb',
>  	'nfp',
> -	'null', 'octeontx', 'pcap', 'qede', 'ring',
> +	'null', 'octeontx', 'octeontx2', 'pcap', 'ring',

Multiline is causing conflicts, can you please break the line while adding new
one, like:
'null', 'octeontx',
'octeontx2',
'qede', 'ring',

>  	'sfc',
>  	'softnic',
>  	'szedata2',
> diff --git a/drivers/net/octeontx2/Makefile b/drivers/net/octeontx2/Makefile
> new file mode 100644
> index 000000000..0a606d27b
> --- /dev/null
> +++ b/drivers/net/octeontx2/Makefile
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(C) 2019 Marvell International Ltd.
> +#
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_pmd_octeontx2.a
> +
> +CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -I$(RTE_SDK)/drivers/common/octeontx2
> +CFLAGS += -I$(RTE_SDK)/drivers/mempool/octeontx2
> +CFLAGS += -I$(RTE_SDK)/drivers/net/octeontx2
> +CFLAGS += -O3
> +CFLAGS += -DALLOW_EXPERIMENTAL_API

Can you please add this flag when really an experimental API is called?
And for that case add a comment here the name of that experimental function,
this will help us to remove unnecessary flags when APIs become non experimental.

> +CFLAGS += -flax-vector-conversions

Same for this one, please add when needed.

> +
> +ifneq ($(CONFIG_RTE_ARCH_64),y)
> +CFLAGS += -Wno-int-to-pointer-cast
> +CFLAGS += -Wno-pointer-to-int-cast

Is there a way to get rid of these? Why need to ignore these warnings?

> +endif
> +
> +EXPORT_MAP := rte_pmd_octeontx2_version.map
> +
> +LIBABIVER := 1
> +
> +#
> +# all source are stored in SRCS-y
> +#
> +SRCS-$(CONFIG_RTE_LIBRTE_OCTEONTX2_PMD) += \
> +	otx2_ethdev.c
> +
> +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_common_octeontx2 -lm
> +LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs -lrte_bus_pci -lrte_mempool_octeontx2

Can you please just keep minimum required dependencies?

> +
> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/net/octeontx2/meson.build b/drivers/net/octeontx2/meson.build
> new file mode 100644
> index 000000000..0bd32446b
> --- /dev/null
> +++ b/drivers/net/octeontx2/meson.build
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(C) 2019 Marvell International Ltd.
> +#
> +
> +sources = files(
> +		'otx2_ethdev.c',
> +		)
> +
> +allow_experimental_apis = true

All comments for makefile valid for meson too, can you please check?

> +deps += ['bus_pci', 'common_octeontx2', 'mempool_octeontx2']
> +
> +cflags += ['-flax-vector-conversions','-DALLOW_EXPERIMENTAL_API']
> +
> +extra_flags = []
> +# This integrated controller runs only on a arm64 machine, remove 32bit warnings
> +if not dpdk_conf.get('RTE_ARCH_64')
> +	extra_flags += ['-Wno-int-to-pointer-cast', '-Wno-pointer-to-int-cast']
> +endif
> +
> +foreach flag: extra_flags
> +	if cc.has_argument(flag)
> +		cflags += flag
> +	endif
> +endforeach
> diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c
> new file mode 100644
> index 000000000..d26535dee
> --- /dev/null
> +++ b/drivers/net/octeontx2/otx2_ethdev.c
> @@ -0,0 +1,3 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2019 Marvell International Ltd.
> + */
> diff --git a/drivers/net/octeontx2/rte_pmd_octeontx2_version.map b/drivers/net/octeontx2/rte_pmd_octeontx2_version.map
> new file mode 100644
> index 000000000..fc8c95e91
> --- /dev/null
> +++ b/drivers/net/octeontx2/rte_pmd_octeontx2_version.map
> @@ -0,0 +1,4 @@
> +DPDK_19.05 {

DPDK_19.08 now.

> +
> +	local: *;
> +};
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index cd89ccfd5..3dff91190 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -127,6 +127,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_COMMON_DPAAX)   += -lrte_common_dpaax
>  endif
>  
>  OCTEONTX2-y := $(CONFIG_RTE_LIBRTE_OCTEONTX2_MEMPOOL)
> +OCTEONTX2-y += $(CONFIG_RTE_LIBRTE_OCTEONTX2_PMD)
>  ifeq ($(findstring y,$(OCTEONTX2-y)),y)
>  _LDLIBS-y += -lrte_common_octeontx2
>  endif
> @@ -197,6 +198,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_MVPP2_PMD)      += -lrte_pmd_mvpp2
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_MVNETA_PMD)     += -lrte_pmd_mvneta
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_NFP_PMD)        += -lrte_pmd_nfp
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL)       += -lrte_pmd_null
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_OCTEONTX2_PMD)  += -lrte_pmd_octeontx2 -lm
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)       += -lrte_pmd_pcap -lpcap
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_QEDE_PMD)       += -lrte_pmd_qede
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING)       += -lrte_pmd_ring
>