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

[Jerin Jacob Kollanukkaran <jerinj@marvell.com>]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, June 6, 2019 9:03 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dev@dpdk.org; Thomas
> Monjalon <thomas@monjalon.net>; John McNamara
> <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>
> Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH v1 01/58] net/octeontx2: add build
> infrastructure
> 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.

OK

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

It does build for x86.  I have added in the default config so that
It will build for x86 as well so that ethdev changes will not be opted
Out for this driver as not everyone have arm64 platform to compile this driver.

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

I have added only slowpath stuff that’s common for vector and scalar.

> 
> > 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',

Makes sense. I will fix it.

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

I will fix it.

> 
> > +CFLAGS += -flax-vector-conversions
> 
> Same for this one, please add when needed.

I will fix it.

> 
> > +
> > +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?

Those things are from base code. I would keep as it is.

> 
> > +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?

Sure.

> 
> > +
> > +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?


Sure.

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

Good catch. I will fix it.