Re: [dpdk-dev] [PATCH v1 1/7] compress/octeontx: add octeontx zip PMD support

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Tue,  5 Jun 2018 16:05:07 +0530
> From: Shally Verma <shally.verma@caviumnetworks.com>
> To: pablo.de.lara.guarch@intel.com
> CC: fiona.trahe@intel.com, dev@dpdk.org, pathreya@caviumnetworks.com,
>  mchalla@caviumnetworks.com, Ashish Gupta
>  <ashish.gupta@caviumnetworks.com>, Sunila Sahu
>  <sunila.sahu@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH v1 1/7] compress/octeontx: add octeontx zip PMD
>  support
> X-Mailer: git-send-email 1.9.1
> 
> Add octeontx zip pmd support in compressdev driver.
> Add device probe and remove support.
> Update makefile to build octeontx zip pmd
> 
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> ---
>  config/common_base                                 |   6 +
>  drivers/compress/Makefile                          |   2 +-
>  drivers/compress/octeontx/Makefile                 |  33 ++++++
>  drivers/compress/octeontx/meson.build              |  10 ++
>  .../octeontx/rte_pmd_octeontx_compress_version.map |   3 +
>  drivers/compress/octeontx/zip_pmd.c                | 129 +++++++++++++++++++++
>  drivers/compress/octeontx/zipvf.c                  |  48 ++++++++
>  drivers/compress/octeontx/zipvf.h                  |  47 ++++++++
>  usertools/dpdk-devbind.py                          |   9 ++
>  9 files changed, 286 insertions(+), 1 deletion(-)
> 
> diff --git a/config/common_base b/config/common_base
> index 6b0d1cbbb..e56d14b7f 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -584,6 +584,12 @@ CONFIG_RTE_COMPRESS_MAX_DEVS=64
>  #
>  CONFIG_RTE_COMPRESSDEV_TEST=n
>  
> +#
> +# Compile PMD for Octeontx ZIPVF compression device
> +#
> +CONFIG_RTE_LIBRTE_PMD_OCTEONTX_ZIPVF=n

Please enable this option  by default, If there are arm64 specific 
usage then please stub it out so that it can compile on 
all architectures. It will help to verify the API changes
across the architecture by author.(i.e author should not depend arm64
box to verify the compilation changes)

> +CONFIG_RTE_LIBRTE_PMD_OCTEONTX_ZIPVF_DEBUG=n

This config option is not required when we are using
dynamic debugging.

> +
>  #
>  # Compile PMD for ISA-L compression device
>  #
> diff --git a/drivers/compress/Makefile b/drivers/compress/Makefile
> index 592497f51..62b4e5abe 100644
> --- a/drivers/compress/Makefile
> +++ b/drivers/compress/Makefile
> @@ -4,5 +4,5 @@
>  include $(RTE_SDK)/mk/rte.vars.mk
>  
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_ISAL) += isal
> -
> +DIRS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_ZIPVF) += octeontx
>  include $(RTE_SDK)/mk/rte.subdir.mk
> diff --git a/drivers/compress/octeontx/Makefile b/drivers/compress/octeontx/Makefile
> new file mode 100644
> index 000000000..89078f085
> --- /dev/null
> +++ b/drivers/compress/octeontx/Makefile
> @@ -0,0 +1,33 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2017-2018 Cavium Network

We are using the following copyright header across DPDK,
Please use the same schematics new.

example:
Copyright(c) 2017 Cavium, Inc


> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +# library name
> +LIB = librte_pmd_octeontx_zip.a
> +
> +# library version
> +LIBABIVER := 1
> +
> +# build flags
> +CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -O3
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
> +CFLAGS += -I$(RTE_SDK)/drivers/compress/octeontx/include
> +
> +# external library include paths
> +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
> +LDLIBS += -lrte_compressdev
> +LDLIBS += -lrte_pci -lrte_bus_pci
> +
> +# library source files
> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_ZIPVF) += zip_pmd.c
> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_ZIPVF) += zipvf.c
> +
> +# export include files
> +SYMLINK-y-include +=
> +
> +# versioning export map
> +EXPORT_MAP := rte_pmd_octeontx_compress_version.map
> +
> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/compress/octeontx/meson.build b/drivers/compress/octeontx/meson.build
> new file mode 100644
> index 000000000..cce404337
> --- /dev/null
> +++ b/drivers/compress/octeontx/meson.build
> @@ -0,0 +1,10 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2018 Cavium Networks
> +
> +sources = files('zipvf.c', 'zip_pmd.c')
> +allow_experimental_apis = true
> +includes += include_directories('include')
> +deps += ['mempool_octeontx', 'bus_pci']
> +ext_deps += dep
> +
> +
> diff --git a/drivers/compress/octeontx/rte_pmd_octeontx_compress_version.map b/drivers/compress/octeontx/rte_pmd_octeontx_compress_version.map
> new file mode 100644
> index 000000000..33c1b976f
> --- /dev/null
> +++ b/drivers/compress/octeontx/rte_pmd_octeontx_compress_version.map
> @@ -0,0 +1,3 @@
> +EXPERIMENTAL {
> +	local: *;
> +};
> diff --git a/drivers/compress/octeontx/zip_pmd.c b/drivers/compress/octeontx/zip_pmd.c
> new file mode 100644
> index 000000000..1181bed19
> --- /dev/null
> +++ b/drivers/compress/octeontx/zip_pmd.c
> @@ -0,0 +1,129 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2017-2018 Cavium Networks
> + */
> +
> +#include <string.h>
> +#include <rte_common.h>
> +#include <rte_malloc.h>
> +#include <rte_cpuflags.h>
> +#include <rte_byteorder.h>

Use alphabetical order.

> +#include "zipvf.h"
> +
> +/* global structure to keep driver info */
> +struct zip_pmd_private zip_pmd_priv;

Remove global memory. Use name based memzone infrastructure to
allocate the memory.

> +
> +static int
> +zip_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> +	struct rte_pci_device *pci_dev)
> +{
> +	int ret = 0;
> +	char compressdev_name[RTE_COMPRESSDEV_NAME_MAX_LEN];
> +	struct rte_compressdev *compressdev;
> +	struct rte_compressdev_pmd_init_params init_params = {
> +		"",
> +		rte_socket_id(),
> +	};
> +
> +	ZIP_PMD_INFO("vendor_id=0x%x device_id=0x%x",
> +			(unsigned int)pci_dev->id.vendor_id,
> +			(unsigned int)pci_dev->id.device_id);
> +
> +	rte_pci_device_name(&pci_dev->addr, compressdev_name,
> +			    sizeof(compressdev_name));
> +
> +	compressdev = rte_compressdev_pmd_create(compressdev_name,
> +		&pci_dev->device, sizeof(struct zip_vf), &init_params);
> +	if (compressdev == NULL) {
> +		ZIP_PMD_ERR("driver %s: create failed", init_params.name);
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * create only if proc_type is primary.
> +	 */
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +		/*  create vf dev with given pmd dev id */
> +		ret = zipvf_create(compressdev, zip_pmd_priv.num_zipvfs);
> +		if (ret < 0) {
> +			ZIP_PMD_ERR("Device creation failed");
> +			rte_compressdev_pmd_destroy(compressdev);
> +			return ret;
> +		}
> +	}
> +
> +	/* TBD: check if driver global structure to be shared
> +	 * too among processes. if yes, then zip_pmd_priv
> +	 * had to be allocated from shared memzone. Currently
> +	 * it is in global data segment
> +	 */
> +	zip_pmd_priv.zipvf_table[zip_pmd_priv.num_zipvfs] =
> +		compressdev->data->dev_private;
> +	zip_pmd_priv.num_zipvfs++;
> +
> +	compressdev->dev_ops = &octtx_zip_pmd_ops;
> +	/* register rx/tx burst functions for data path */
> +	compressdev->dequeue_burst = zip_pmd_dequeue_burst_sync;
> +	compressdev->enqueue_burst = zip_pmd_enqueue_burst_sync;
> +	compressdev->feature_flags = RTE_COMPDEV_FF_HW_ACCELERATED;
> +	return ret;
> +}
> +
> +static int
> +zip_pci_remove(struct rte_pci_device *pci_dev)
> +{
> +	struct rte_compressdev *compressdev;
> +	char compressdev_name[RTE_COMPRESSDEV_NAME_MAX_LEN];
> +
> +	if (pci_dev == NULL) {
> +		ZIP_PMD_ERR(" Invalid PCI Device\n");
> +		return -EINVAL;
> +	}
> +	rte_pci_device_name(&pci_dev->addr, compressdev_name,
> +			sizeof(compressdev_name));
> +
> +	compressdev = rte_compressdev_pmd_get_named_dev(compressdev_name);
> +	if (compressdev == NULL)
> +		return -ENODEV;
> +
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +		if (zipvf_destroy(compressdev) < 0)
> +			return -ENODEV;
> +	}
> +	return rte_compressdev_pmd_destroy(compressdev);
> +}
> +
> +
> +
> +static struct rte_pci_id pci_id_octtx_zipvf_table[] = {
> +	{
> +		RTE_PCI_DEVICE(PCI_VENDOR_ID_CAVIUM,
> +			PCI_DEVICE_ID_OCTEONTX_ZIPVF),
> +	},
> +	{
> +		.device_id = 0
> +	},
> +};
> +
> +/**
> + * Structure that represents a PCI driver
> + */
> +static struct rte_pci_driver octtx_zip_pmd = {
> +	.id_table    = pci_id_octtx_zipvf_table,
> +	.drv_flags   = RTE_PCI_DRV_NEED_MAPPING,
> +	.probe       = zip_pci_probe,
> +	.remove      = zip_pci_remove,
> +};
> +
> +RTE_PMD_REGISTER_PCI(COMPRESSDEV_NAME_ZIP_PMD, octtx_zip_pmd);
> +RTE_PMD_REGISTER_PCI_TABLE(COMPRESSDEV_NAME_ZIP_PMD, pci_id_octtx_zipvf_table);
> +
> +RTE_INIT(octtx_zip_init_log);
> +
> +static void
> +octtx_zip_init_log(void)
> +{
> +	octtx_zip_logtype_driver = rte_log_register("comp_octeontx_zip");
> +	if (octtx_zip_logtype_driver >= 0)
> +		rte_log_set_level(octtx_zip_logtype_driver, RTE_LOG_INFO);
> +}
> +
> +
> +#endif /* _RTE_ZIP_VF_H_ */
> diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py

Addition to usertools/dpdk-devbind.py, please send it as separate patch as
it is an common code change