Re: [dpdk-dev] [PATCH] build: force pkg-config for dependency detection

2021-01-21 Thread Daly, Lee



> -Original Message-
> From: Richardson, Bruce 
> Sent: Monday, January 18, 2021 2:30 PM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh ; Richardson, Bruce
> ; sta...@dpdk.org; Matan Azrad
> ; Shahaf Shuler ; Viacheslav
> Ovsiienko ; Liron Himi ;
> Trahe, Fiona ; Griffin, John
> ; Jain, Deepak K ; Daly,
> Lee ; Ashish Gupta ;
> Sunila Sahu ; Ruifeng Wang
> ; Somalapuram Amaranath
> ; Michael Shamis ;
> Doherty, Declan ; Loftus, Ciara
> ; Zhang, Qi Z ; Rasesh Mody
> ; Shahed Shaikh ; Zyta
> Szpak ; Martin Spinler ; Hunt, David
> ; Ananyev, Konstantin
> 
> Subject: [PATCH] build: force pkg-config for dependency detection
> 
> Meson can use cmake as a fallback for detecting packages, and this can lead
> to picking up 64-libs for 32-bit builds. To work around this, force the use of
> pkg-config only for detecting libcrypto, zlib, jansson and other package
> dependencies.
> 
> CC: sta...@dpdk.org
> 
> Signed-off-by: Bruce Richardson 
> ---
> 
Tested ISA-L driver change. Thanks.
Tested-by: Lee Daly 


Re: [dpdk-dev] [PATCH v5 0/5] add initial version of compress-perf

2018-12-05 Thread Daly, Lee



> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Tomasz Jozwiak
> Sent: Wednesday, December 5, 2018 8:47 AM
> To: dev@dpdk.org; Trahe, Fiona ; Jozwiak, TomaszX
> ; shally.ve...@cavium.com;
> akhil.go...@nxp.com
> Subject: [dpdk-dev] [PATCH v5 0/5] add initial version of compress-perf
> 
> This patchset adds initial version of compression performance test.
> 
Series Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH v1 1/2] test/compress: add out of space test

2018-12-14 Thread Daly, Lee


> -Original Message-
> From: Kovacevic, Marko
> Sent: Friday, December 14, 2018 3:33 PM
> To: dev@dpdk.org
> Cc: akhil.go...@nxp.com; Daly, Lee ; Jozwiak, TomaszX
> ; O'Hare, Cathal ;
> Trahe, Fiona ; Kovacevic, Marko
> 
> Subject: [PATCH v1 1/2] test/compress: add out of space test
> 
> From: "Kovacevic, Marko" 
> 
> This patch adds new out of space testcase to check that the destination mbuf
> is smaller than required for the output of compression to ensure the driver
> doesn't crash and returns the valid error case.
> 
> Signed-off-by: Marko Kovacevic 
> ---
Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH v1 2/2] test/compress: add varied buffer input/outputs

2018-12-14 Thread Daly, Lee



> -Original Message-
> From: Kovacevic, Marko
> Sent: Friday, December 14, 2018 3:33 PM
> To: dev@dpdk.org
> Cc: akhil.go...@nxp.com; Daly, Lee ; Jozwiak, TomaszX
> ; O'Hare, Cathal ;
> Trahe, Fiona ; Kovacevic, Marko
> 
> Subject: [PATCH v1 2/2] test/compress: add varied buffer input/outputs
> 
> Added unit test to check if a SGL buffer was added as an input and a Linear
> Buffer as output and vice versa so we can test if the application would
> process the different buffers properly.
> 
> Signed-off-by: Marko Kovacevic 
> ---
Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH v4 1/3] compress/isal: enable checksum support in driver

2018-12-17 Thread Daly, Lee
Hi Vipin, thanks for the question.

> -Original Message-
> From: Varghese, Vipin
> Sent: Monday, December 17, 2018 8:20 AM
> To: Daly, Lee ; akhil.go...@nxp.com
> Cc: dev@dpdk.org; Daly, Lee 
> Subject: RE: [dpdk-dev] [PATCH v4 1/3] compress/isal: enable checksum
> support in driver
> 
> Hi Lee,
> 
> Apologies if the logic is already done for my query
> 
> snipped
> > +   /* Set private xform checksum */
> > +   switch (xform->compress.chksum) {
> > +   /* Raw deflate by default */
> Does the user have to choose RTE_COMP_CHECKSUM_NONE while creating
> compress/isal instance?

At the moment, yes the application must fill out the xform/instance. 
> 
> > +   case(RTE_COMP_CHECKSUM_NONE):
> > +   priv_xform->compress.chksum = IGZIP_DEFLATE;
> > +   break;
> > +   case(RTE_COMP_CHECKSUM_CRC32):
> > +   priv_xform->compress.chksum =
> IGZIP_GZIP_NO_HDR;
> > +   break;
> > +   case(RTE_COMP_CHECKSUM_ADLER32):
> > +   priv_xform->compress.chksum =
> IGZIP_ZLIB_NO_HDR;
> > +   break;
> > +   case(RTE_COMP_CHECKSUM_CRC32_ADLER32):
> > +   ISAL_PMD_LOG(ERR, "Combined CRC and ADLER
> > checksum not"
> > +   " supported\n");
> > +   return -ENOTSUP;
> > +   default:
> > +   ISAL_PMD_LOG(ERR, "Checksum type not
> > supported\n");
> > +   return -ENOTSUP;
> Do we not fall back to RTE_COMP_CHECKSUM_NONE if the configuration is
> wrong and report warning?
> 

Right now, the xform must be filled out correctly before compression can be 
executed. 
Perhaps we could do as you say and fall back to a default config if the 
non-essential params are incorrect.
> snipped


Re: [dpdk-dev] [PATCH v3 3/3] doc: update ISA-L guide to reflect checksum support

2019-01-07 Thread Daly, Lee
Hi Vipin, thanks for your suggestion. 
I will send another version with the change.
/Lee.

> -Original Message-
> From: Varghese, Vipin
> Sent: Friday, December 28, 2018 5:05 AM
> To: Daly, Lee ; akhil.go...@nxp.com
> Cc: dev@dpdk.org; Daly, Lee 
> Subject: RE: [dpdk-dev] [PATCH v3 3/3] doc: update ISA-L guide to reflect
> checksum support
> 
> Hi Lee,
> 
> A humble suggestion being shared for document update, Based on DPDK
> 19.02-rc1 release both code and associated document update is to be in
> single patch file.
> 
> Note: I am not fully sure how this is applicable to 'isal.ini and isal.rst'. 
> It will be
> good to have a check.
> 
> Thanks
> Vipin Varghese
> 
> > -Original Message-
> > From: dev  On Behalf Of Lee Daly
> > Sent: Thursday, December 13, 2018 4:13 PM
> > To: akhil.go...@nxp.com
> > Cc: dev@dpdk.org; Daly, Lee 
> > Subject: [dpdk-dev] [PATCH v3 3/3] doc: update ISA-L guide to reflect
> > checksum support
> >
> > This updates the ISA-L compression driver guide on how to enable and
> > use checksums.
> >
> > This also updates the compression drivers features matrix.
> >
> > V2:
> > Documentation Changes
> > V3:
> > Added Release note
> >
> > Signed-off-by: Lee Daly 
> > Acked-by: Fiona Trahe 
> > ---
> >  doc/guides/compressdevs/features/isal.ini |  2 ++
> >  doc/guides/compressdevs/isal.rst  | 30
> -
> > -
> >  doc/guides/rel_notes/release_19_02.rst|  5 +
> >  3 files changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/doc/guides/compressdevs/features/isal.ini
> > b/doc/guides/compressdevs/features/isal.ini
> > index 919cf70..e705031 100644
> > --- a/doc/guides/compressdevs/features/isal.ini
> > +++ b/doc/guides/compressdevs/features/isal.ini
> > @@ -12,5 +12,7 @@ OOP SGL In SGL Out = Y  OOP SGL In LB  Out = Y  OOP
> > LB  In SGL Out = Y
> >  Deflate= Y
> > +Adler32= Y
> > +Crc32  = Y
> >  Fixed  = Y
> >  Dynamic= Y
> > diff --git a/doc/guides/compressdevs/isal.rst
> > b/doc/guides/compressdevs/isal.rst
> > index 3bc3022..af1f41f 100644
> > --- a/doc/guides/compressdevs/isal.rst
> > +++ b/doc/guides/compressdevs/isal.rst
> > @@ -27,6 +27,33 @@ Window size support:
> >
> >  * 32K
> >
> > +Checksum:
> > +
> > +* CRC32
> > +* ADLER32
> > +
> > +To enable a checksum in the driver, the compression and/or
> > +decompression xform structure, rte_comp_xform, must be filled with
> > +either of the CompressDev checksum flags supported. ::
> > +
> > + compress_xform->compress.chksum = RTE_COMP_CHECKSUM_CRC32
> > +
> > + decompress_xform->decompress.chksum =
> RTE_COMP_CHECKSUM_CRC32
> > +
> > +::
> > +
> > + compress_xform->compress.chksum =
> RTE_COMP_CHECKSUM_ADLER32
> > +
> > + decompress_xform->decompress.chksum =
> RTE_COMP_CHECKSUM_ADLER32
> > +
> > +If you request a checksum for compression or decompression, the
> > +checksum field in the operation structure,  ``op->output_chksum``,
> > +will be filled with the checksum.
> > +
> > +.. Note::
> > +
> > + For the compression case above, your output buffer will need to be
> > + large
> > enough to hold the compressed data plus a scratchpad for the checksum
> > at the end, the scratchpad is 8 bytes for CRC32 and 4 bytes for Adler32.
> > +
> >  Level guide:
> >
> >  The ISA-L levels have been mapped to somewhat correspond to the same
> > ZLIB level, @@ -75,13 +102,12 @@ As a result the level mappings from
> > the API to the PMD are shown below.
> >   The above table only shows mapping when API calls for dynamic
> compression.
> >   For fixed compression, regardless of API level, internally ISA-L
> > level 0 is always used.
> >
> > +
> >  Limitations
> >  ---
> >
> >  * Compressdev level 0, no compression, is not supported.
> >
> > -* Checksums will not be supported until future release.
> > -
> >  Installation
> >  
> >
> > diff --git a/doc/guides/rel_notes/release_19_02.rst
> > b/doc/guides/rel_notes/release_19_02.rst
> > index a94fa86..3a4d264 100644
> > --- a/doc/guides/rel_notes/release_19_02.rst
> > +++ b/doc/guides/rel_notes/release_19_02.rst
> > @@ -54,6 +54,11 @@ New Features
> >   Also, make sure to start the actual text at the margin.
> >
> =
> >
> > +   * **Enabled checksum support in the ISA-L compressdev driver.**
> > +
> > + Added support for both adler and crc32 checksums in the ISA-L PMD.
> > + This aids data integrity across both compression and decompression.
> > +
> >
> >  Removed Items
> >  -
> > --
> > 2.7.4



Re: [dpdk-dev] [PATCH 2/3] app/compress-perf: add performance measurement

2018-10-15 Thread Daly, Lee
Thanks for your input Shally see comments below.


I will be reviewing these changes while Tomasz is out this week.

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Verma, Shally
> Sent: Friday, October 12, 2018 11:16 AM
> To: Jozwiak, TomaszX ; dev@dpdk.org; Trahe,
> Fiona ; akhil.go...@nxp.com; De Lara Guarch, Pablo
> 
> Cc: d...@dpdk.org; l...@dpdk.org; gua...@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/3] app/compress-perf: add performance
> measurement
> 
> HI TomaszX
> 
> Sorry for delay in response. Comments inline.
> 

<...>
> >+static int
> >+comp_perf_check_capabilities(struct comp_test_data *test_data) {
> >+   const struct rte_compressdev_capabilities *cap;
> >+
> >+   cap = rte_compressdev_capability_get(test_data->cdev_id,
> >+RTE_COMP_ALGO_DEFLATE);
> >+
> >+   if (cap == NULL) {
> >+   RTE_LOG(ERR, USER1,
> >+   "Compress device does not support DEFLATE\n");
> >+   return -1;
> >+   }
> >+
> >+   uint64_t comp_flags = cap->comp_feature_flags;
> >+
> >+   /* Huffman enconding */
> >+   if (test_data->huffman_enc == RTE_COMP_HUFFMAN_FIXED &&
> >+   (comp_flags & RTE_COMP_FF_HUFFMAN_FIXED) == 0) {
> >+   RTE_LOG(ERR, USER1,
> >+   "Compress device does not supported Fixed 
> >Huffman\n");
> >+   return -1;
> >+   }
> >+
> >+   if (test_data->huffman_enc == RTE_COMP_HUFFMAN_DYNAMIC &&
> >+   (comp_flags & RTE_COMP_FF_HUFFMAN_DYNAMIC) == 0) {
> >+   RTE_LOG(ERR, USER1,
> >+   "Compress device does not supported Dynamic 
> >Huffman\n");
> >+   return -1;
> >+   }
> >+
> >+   /* Window size */
> >+   if (test_data->window_sz != -1) {
> >+   if (param_range_check(test_data->window_sz,
> >+ &cap->window_size)
> What if cap->window_size is 0 i.e. implementation default?
What do you mean when you say cap->window_size = 0?
Cap->window_size is the range structure here, min, max and increment, which are 
filled out by the driver.
Our implementation default in the perf tool will set the window size to max the 
driver can support.

> 
> >+   < 0) {
> >+   RTE_LOG(ERR, USER1,
> >+   "Compress device does not support "
> >+   "this window size\n");
> >+   return -1;
> >+   }
> >+   } else
> >+   /* Set window size to PMD maximum if none was specified */
> >+   test_data->window_sz = cap->window_size.max;
> >+

<...>
> >+
> >+static int
> >+comp_perf_dump_input_data(struct comp_test_data *test_data) {
> >+   FILE *f = fopen(test_data->input_file, "r");
> >+
> >+   if (f == NULL) {
> >+   RTE_LOG(ERR, USER1, "Input file could not be opened\n");
> >+   return -1;
> >+   }
> >+
> >+   if (fseek(f, 0, SEEK_END) != 0) {
> >+   RTE_LOG(ERR, USER1, "Size of input could not be 
> >calculated\n");
> >+   goto err;
> >+   }
> >+   size_t actual_file_sz = ftell(f);
> >+   /* If extended input data size has not been set,
> >+* input data size = file size
> >+*/
> >+
> >+   if (test_data->input_data_sz == 0)
> >+   test_data->input_data_sz = actual_file_sz;
> >+
> >+   if (fseek(f, 0, SEEK_SET) != 0) {
> >+   RTE_LOG(ERR, USER1, "Size of input could not be 
> >calculated\n");
> >+   goto err;
> >+   }
> >+
> >+   test_data->input_data = rte_zmalloc_socket(NULL,
> >+   test_data->input_data_sz, 0,
> >+ rte_socket_id());
> >+
> >+   if (test_data->input_data == NULL) {
> >+   RTE_LOG(ERR, USER1, "Memory to hold the data from the input "
> >+   "file could not be allocated\n");
> >+   goto err;
> >+   }
> >+
> >+   size_t remaining_data = test_data->input_data_sz;
> >+   uint8_t *data = test_data->input_data;
> >+
> >+   while (remaining_data > 0) {
> >+   size_t data_to_read = RTE_MIN(remaining_data,
> >+ actual_file_sz);
> >+
> >+   if (fread(data, data_to_read, 1, f) != 1) {
> >+   RTE_LOG(ERR, USER1, "Input file could not be 
> >read\n");
> >+   goto err;
> >+   }
> >+   if (fseek(f, 0, SEEK_SET) != 0) {
> >+   RTE_LOG(ERR, USER1,
> >+   "Size of input could not be calculated\n");
> >+   goto err;
> >+   }
> >+   remaining_data -= data_to_read;
> >+   data += data_to_read;
> It looks like it will run 2nd time only if input file size < input data size 
> in which
> case it will just keep filling input buffer with repeated data.

Re: [dpdk-dev] [PATCH v1 1/6] compress/zlib: add ZLIB PMD support

2018-06-15 Thread Daly, Lee
Hi,
 thanks for the work, reviewed the PMD see comments below.

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shally Verma
> Sent: Tuesday, May 15, 2018 11:32 AM
> To: De Lara Guarch, Pablo 
> Cc: Trahe, Fiona ; dev@dpdk.org;
> pathr...@caviumnetworks.com; Sunila Sahu
> ; Ashish Gupta
> 
> Subject: [dpdk-dev] [PATCH v1 1/6] compress/zlib: add ZLIB PMD support

<...>

> diff --git a/config/common_base b/config/common_base index
> 28557ed..537e9e4 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -586,6 +586,12 @@ CONFIG_RTE_COMPRESSDEV_TEST=n
> CONFIG_RTE_LIBRTE_PMD_ISAL=n
> 
>  #
> +# Compile PMD for ZLIB compression device #
> +CONFIG_RTE_LIBRTE_PMD_ZLIB=n
> CONFIG_RTE_LIBRTE_PMD_ZLIB_DEBUG=n
[Lee] DPDK is moving from the static debugging, I see you have implemented 
dynamic logging but have not yet used it in the PMD.
i.e using ZLIB_LOG_ERR/INFO() rather than ZLIB_PMD_LOG(INFO, "").
When you have removed the static debugging use, could you remove this flag to 
enable it please.

> +
> +#
>  # Compile generic event device library
>  #
>  CONFIG_RTE_LIBRTE_EVENTDEV=y
> diff --git a/drivers/compress/Makefile b/drivers/compress/Makefile index
> 592497f..1f159a5 100644
> --- a/drivers/compress/Makefile
> +++ b/drivers/compress/Makefile
> @@ -4,5 +4,6 @@
>  include $(RTE_SDK)/mk/rte.vars.mk
> 
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_ISAL) += isal
> +DIRS-$(CONFIG_RTE_LIBRTE_PMD_ZLIB) += zlib
> 
>  include $(RTE_SDK)/mk/rte.subdir.mk
> diff --git a/drivers/compress/zlib/Makefile b/drivers/compress/zlib/Makefile
> new file mode 100644 index 000..e613960
> --- /dev/null
> +++ b/drivers/compress/zlib/Makefile
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2018 Cavium
> +Networks
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +# library name
> +LIB = librte_pmd_zlib.a
> +
> +# build flags
> +CFLAGS += -O3
> +CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
> +
> +# library version
> +LIBABIVER := 1
> +
> +# versioning export map
> +EXPORT_MAP := rte_pmd_zlib_version.map
> +
> +# external library dependencies
> +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring -lz LDLIBS +=
> +-lrte_compressdev LDLIBS += -lrte_bus_vdev
> +
> +# library source files
> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_ZLIB) += zlib_pmd.c
> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_ZLIB) += zlib_pmd_ops.c
> +
> +# export include files
> +SYMLINK-y-include +=
> +
[Lee] Do you have a need for this in the future, not being used now.

> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/compress/zlib/rte_pmd_zlib_version.map
> b/drivers/compress/zlib/rte_pmd_zlib_version.map
> new file mode 100644
> index 000..33c1b97
> --- /dev/null
> +++ b/drivers/compress/zlib/rte_pmd_zlib_version.map
> @@ -0,0 +1,3 @@
> +EXPERIMENTAL {
[Lee] Would be better to replace EXPERMENTAL with 18.08 in this case.

> + local: *;
> +};
> diff --git a/drivers/compress/zlib/zlib_pmd.c
> b/drivers/compress/zlib/zlib_pmd.c
> new file mode 100644
> index 000..bbf49f1
> --- /dev/null
> +++ b/drivers/compress/zlib/zlib_pmd.c
> @@ -0,0 +1,106 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2017-2018 Cavium Networks  */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include "zlib_pmd_private.h"
> +
> +static uint8_t compressdev_driver_id;
[Lee] The current API no longer has rte_compressdev->driver_id anymore therefor 
this variable will be unused.

> +int zlib_logtype_driver;
> +
> +static int zlib_remove(struct rte_vdev_device *vdev);
> +
> +static int
> +zlib_create(const char *name,
> + struct rte_vdev_device *vdev,
> + struct rte_compressdev_pmd_init_params *init_params) {
> + struct rte_compressdev *dev;
> + struct zlib_private *internals;
> +
> + dev = rte_compressdev_pmd_create(name, &vdev->device,
> + sizeof(struct zlib_private), init_params);
> + if (dev == NULL) {
> + ZLIB_LOG_ERR("driver %s: create failed", init_params-
> >name);
> + return -ENODEV;
> + }
> +
> + dev->driver_id = compressdev_driver_id;
[Lee] See above comment.

> + dev->dev_ops = rte_zlib_pmd_ops;
> +
> + dev->feature_flags = 0;
> + dev->feature_flags |= RTE_COMP_FF_SHAREABLE_PRIV_XFORM |
> + RTE_COMP_FF_NONCOMPRESSED_BLOCKS |
> + RTE_COMP_FF_ADLER32_CHECKSUM;

Thanks,
Lee.


Re: [dpdk-dev] [PATCH v1 3/6] compress/zlib: add xform and stream create support

2018-06-15 Thread Daly, Lee
Comment Inline.

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shally Verma
> Sent: Tuesday, May 15, 2018 11:32 AM
> To: De Lara Guarch, Pablo 
> Cc: Trahe, Fiona ; dev@dpdk.org;
> pathr...@caviumnetworks.com; Sunila Sahu
> ; Ashish Gupta
> 
> Subject: [dpdk-dev] [PATCH v1 3/6] compress/zlib: add xform and stream
> create support
> 
> Implement private xform and stream create ops
> 

<...>
>  struct rte_compressdev_ops zlib_pmd_ops = {
>   .dev_configure  = zlib_pmd_config,
>   .dev_start  = zlib_pmd_start,
> @@ -228,11 +304,11 @@ struct rte_compressdev_ops zlib_pmd_ops = {
>   .queue_pair_setup   = zlib_pmd_qp_setup,
>   .queue_pair_release = zlib_pmd_qp_release,
> 
> - .private_xform_create   = NULL,
> - .private_xform_free = NULL,
> + .private_xform_create   = zlib_pmd_private_xform_create,
> + .private_xform_free =
> zlib_pmd_private_xform_free,
[Lee] Quick fix of formatting here please.

> 
> - .stream_create  = NULL,
> - .stream_free= NULL
> + .stream_create  = zlib_pmd_stream_create,
> + .stream_free= zlib_pmd_stream_free
>  };
> 
>  struct rte_compressdev_ops *rte_zlib_pmd_ops = &zlib_pmd_ops;
> --
> 2.9.5
Thanks,
Lee.



Re: [dpdk-dev] [PATCH v1 6/6] doc: add ZLIB PMD documentation

2018-06-15 Thread Daly, Lee
Hi, Comments inline.

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shally Verma
> Sent: Tuesday, May 15, 2018 11:32 AM
> To: De Lara Guarch, Pablo 
> Cc: Trahe, Fiona ; dev@dpdk.org;
> pathr...@caviumnetworks.com; Sunila Sahu
> ; Ashish Gupta
> 
> Subject: [dpdk-dev] [PATCH v1 6/6] doc: add ZLIB PMD documentation
> 
> add zlib pmd feature specification and overview documentation
> 
> Signed-off-by: Sunila Sahu 
> Signed-off-by: Shally Verma 
> Signed-off-by: Ashish Gupta 
> ---
>  doc/guides/compressdevs/features/zlib.ini | 22 ++
>  doc/guides/compressdevs/zlib.rst  | 72
> +++
>  2 files changed, 94 insertions(+)
> 
> diff --git a/doc/guides/compressdevs/features/zlib.ini
> b/doc/guides/compressdevs/features/zlib.ini
> new file mode 100644
> index 000..10e758b
> --- /dev/null
> +++ b/doc/guides/compressdevs/features/zlib.ini
> @@ -0,0 +1,22 @@
> +;
> +; Refer to default.ini for the full list of available PMD features.
> +;
> +; Supported features of 'ZLIB' compression driver.
> +;
> +[Features]
> +HW Accelerated =
> +CPU SSE=
> +CPU AVX=
> +CPU AVX2   =
> +CPU AVX512 =
> +CPU NEON   =
> +Stateful   =
> +By-Pass=
> +Chained mbufs  =
> +Deflate= Y
> +LZS=
> +Adler32= Y
> +Crc32  = Y
[Lee] here you say you support both adler and crc checksums, in the feature 
flags of the PMD it is says only adler32, but I see no implementation of any 
checksums in the PMD? 
Note* the checksum of the data will need to be put in the op->input_chksum & 
op->output_chksum.

> +Adler32&Crc32  =
> +Fixed  = Y
> +Dynamic= Y
> diff --git a/doc/guides/compressdevs/zlib.rst
> b/doc/guides/compressdevs/zlib.rst
> new file mode 100644
> index 000..130750b
> --- /dev/null
> +++ b/doc/guides/compressdevs/zlib.rst
> @@ -0,0 +1,72 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +Copyright(c) 2018 Cavium Networks.
> +
> +ZLIB Compression Poll Mode Driver
> +==
> +
> +The ZLIB PMD (**librte_pmd_zlib**) provides poll mode compression &
> +decompression driver based on SW zlib library,
> +
> +Features
> +
> +
> +ZLIB PMD has support for:
> +
> +Compression/Decompression algorithm:
> +
> +* DEFLATE
> +
> +Huffman code type:
> +
> +* FIXED
> +* DYNAMIC
> +
> +Checksum support:
> +
> +* Adler32
> +* CRC32
[Lee] see comment above.

> +
> +Window size support:
> +
> +* 32K
[Lee] Say only 32K window size support here but in the capabilities of the PMD 
you have a window_size ranging from 8(256) to 15(32k). You also have increments 
of 2, which wouldn't allow 15 as max.

Thanks for the work,
Lee.


Re: [dpdk-dev] [PATCH v1 4/6] compress/zlib: add enq deq apis

2018-06-15 Thread Daly, Lee
Comments inline.

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shally Verma
> Sent: Tuesday, May 15, 2018 11:32 AM
> To: De Lara Guarch, Pablo 
> Cc: Trahe, Fiona ; dev@dpdk.org;
> pathr...@caviumnetworks.com; Sunila Sahu
> ; Ashish Gupta
> 
> Subject: [dpdk-dev] [PATCH v1 4/6] compress/zlib: add enq deq apis
> 
> implement enqueue and dequeue apis

<...>
> diff --git a/drivers/compress/zlib/meson.build
> b/drivers/compress/zlib/meson.build
> new file mode 100644
> index 000..d66de95
> --- /dev/null
> +++ b/drivers/compress/zlib/meson.build
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2018 Cavium
> +Networks
> +
> +dep = dependency('zlib', required: false) if not dep.found()
> + build = false
> +endif
> +deps += 'bus_vdev'
> +sources = files('zlib_pmd.c', 'zlib_pmd_ops.c') ext_deps += dep
> +pkgconfig_extra_libs += '-lz'
[Lee] You have added "allow_experimental_apis" tag to the zlib/Makefile, better 
to add to the meson as well.
With the tag; allow_experimental_apis = true.

> diff --git a/drivers/compress/zlib/zlib_pmd.c
> b/drivers/compress/zlib/zlib_pmd.c
> index 3dc71ec..e2681a7 100644
> --- a/drivers/compress/zlib/zlib_pmd.c
> +++ b/drivers/compress/zlib/zlib_pmd.c
> @@ -17,7 +17,239 @@
>  #include "zlib_pmd_private.h"
> 
>  static uint8_t compressdev_driver_id;
> -int zlib_logtype_driver;
> +
> +/** compute the next mbuf in list and assign dst buffer and dlen,
> + * set op->status to appropriate flag if we run out of mbuf  */
> +#define COMPUTE_DST_BUF(mbuf, dst, dlen) \
> + ((mbuf = mbuf->next) ?
>   \
> + (dst = rte_pktmbuf_mtod(mbuf, uint8_t *)),  \
> + dlen = rte_pktmbuf_data_len(mbuf) : \
> + !(op->status =  \
> + ((op->op_type == RTE_COMP_OP_STATELESS) ?
>   \
[Lee] Clever & useful macro.

> +
>   RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED :\
> +
>   RTE_COMP_OP_STATUS_OUT_OF_SPACE_RECOVERABLE)))
> +
> +static void
> +process_zlib_deflate(struct rte_comp_op *op, z_stream *strm) {
> + int ret, flush, fin_flush;
> + uint8_t *src, *dst;
> + uint32_t sl, dl, have;
> + struct rte_mbuf *mbuf_src = op->m_src;
> + struct rte_mbuf *mbuf_dst = op->m_dst;
> +
> + src = rte_pktmbuf_mtod_offset(mbuf_src, uint8_t *, op-
> >src.offset);
> +
> + sl = rte_pktmbuf_data_len(mbuf_src) - op->src.offset;
> +
> + dst = rte_pktmbuf_mtod_offset(mbuf_dst, unsigned char *,
> + op->dst.offset);
> +
> + dl = rte_pktmbuf_data_len(mbuf_dst) - op->dst.offset;
> +
> + if (unlikely(!src || !dst || !strm)) {
> + op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
> + ZLIB_LOG_ERR("\nInvalid source or destination buffers");
> + return;
> + }
> + switch (op->flush_flag) {
> + case RTE_COMP_FLUSH_NONE:
> + fin_flush = Z_NO_FLUSH;
> + break;
> + case RTE_COMP_FLUSH_SYNC:
> + fin_flush = Z_SYNC_FLUSH;
> + break;
> + case RTE_COMP_FLUSH_FULL:
> + fin_flush = Z_FULL_FLUSH;
> + break;
> + case RTE_COMP_FLUSH_FINAL:
> + fin_flush = Z_FINISH;
> + break;
> + default:
> + op->status = RTE_COMP_OP_STATUS_ERROR;
> + goto def_end;
> + }
> + if (op->src.length <= sl) {
> + sl = op->src.length;
> + flush = fin_flush;
> + } else {
> + /* if there're more than one mbufs in input,
> +  * process intermediate with NO_FLUSH
> +  */
> + flush = Z_NO_FLUSH;
> + }
> + /* initialize status to SUCCESS */
> + op->status = RTE_COMP_OP_STATUS_SUCCESS;
> +
> + do {
> + /* Update z_stream with the inputs provided by application
> */
> + strm->next_in = src;
> + strm->avail_in = sl;
> +
> + do {
> + strm->avail_out = dl;
> + strm->next_out = dst;
> + ret = deflate(strm, flush);
> + if (unlikely(ret == Z_STREAM_ERROR)) {
> + /* error return, do not process further */
> + op->status =
> RTE_COMP_OP_STATUS_ERROR;
> + goto def_end;
> + }
> + /* Update op stats */
> + op->produced += dl - strm->avail_out;
> + op->consumed += sl - strm->avail_in;
[Lee] strm struct has a total_in & total_out field which can be used here and 
will save these cycles doing the calculation each iteration.

> + /* Break if Z_STREAM_END is encountered or dst mbuf gets
> over */
> + } while (!(ret == Z_STREAM_END) && (strm->avail_out == 0)
> &&
> + COMPUTE_DST_BUF(mbuf_dst, dst, dl));

Re: [dpdk-dev] [PATCH v1 2/6] compress/zlib: add device setup PMD ops

2018-06-15 Thread Daly, Lee
Hi Shally,
Comments inline.


> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shally Verma
> Sent: Tuesday, May 15, 2018 11:32 AM
> To: De Lara Guarch, Pablo 
> Cc: Trahe, Fiona ; dev@dpdk.org;
> pathr...@caviumnetworks.com; Sunila Sahu
> ; Ashish Gupta
> 
> Subject: [dpdk-dev] [PATCH v1 2/6] compress/zlib: add device setup PMD
> ops
> 
>diff --git a/drivers/compress/zlib/zlib_pmd_ops.c 
>b/drivers/compress/zlib/zlib_pmd_ops.c
>new file mode 100644
>index 000..0bd42f3
>--- /dev/null
>+++ b/drivers/compress/zlib/zlib_pmd_ops.c
>@@ -0,0 +1,238 @@ 
>+/* SPDX-License-Identifier: BSD-3-Clause
>+ * Copyright(c) 2018 Cavium Networks
>+ */
>+
>+#include 
>+
>+#include 
>+#include 
>+#include 
>+
>+#include "zlib_pmd_private.h"
>+
>+static const struct rte_compressdev_capabilities zlib_pmd_capabilities[] = {
>+  {   /* Deflate */
>+  .algo = RTE_COMP_ALGO_DEFLATE,
>+  .comp_feature_flags = RTE_COMP_FF_SHAREABLE_PRIV_XFORM,
[Lee] The priv_xform structure in this case is not shareable, as it contains 
your zlib_stream structure, which contains zlibs own zstream struct. This is 
not read only, the contents of this zstream will be written to, which means it 
is not shareable across queue pairs or devices.

>+  .window_size = {
>+  .min = 8,
>+  .max = 15,
>+  .increment = 2
>+  },
>+  },
>+
>+  RTE_COMP_END_OF_CAPABILITIES_LIST()
>+
>+};
> +/** Configure device */
> +static int
> +zlib_pmd_config(struct rte_compressdev *dev,
> + struct rte_compressdev_config *config) {
> + struct rte_mempool *mp;
> +
> + struct zlib_private *internals = dev->data->dev_private;
> + snprintf(internals->mp_name, RTE_MEMPOOL_NAMESIZE,
> + "stream_mp_%u", dev->data->dev_id);
> + mp = rte_mempool_create(internals->mp_name,
> + config->max_nb_priv_xforms + config-
> >max_nb_streams,
> + sizeof(struct zlib_priv_xform),
> + 0, 0, NULL, NULL, NULL,
> + NULL, config->socket_id,
> + 0);
[Lee] Could you add a mempool_lookup here to ensure its not already created 
please.

> + if (mp == NULL) {
> + ZLIB_LOG_ERR("Cannot create private xform pool on socket
> %d\n",
> + config->socket_id);
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +
> +/** Start device */
> +static int
> +zlib_pmd_start(__rte_unused struct rte_compressdev *dev) {
> + return 0;
> +}
> +
> +/** Stop device */
> +static void
> +zlib_pmd_stop(struct rte_compressdev *dev) {
> + struct zlib_private *internals = dev->data->dev_private;
> + struct rte_mempool *mp = rte_mempool_lookup(internals-
> >mp_name);
> + rte_mempool_free(mp);
> +}
> +
[Lee] I believe it would be better to have the freeing functionality in the 
pmd_close function, as a user may want to stop a device, without freeing its 
memory, especially since the start function does nothing here. i.e. if the user 
stops device then starts again, memory needed has been free'd but not 
realloc'ed. Hope this makes sense.

> +/** Close device */
> +static int
> +zlib_pmd_close(__rte_unused struct rte_compressdev *dev) {
> + return 0;
> +}

<...>
> diff --git a/drivers/compress/zlib/zlib_pmd_private.h
> b/drivers/compress/zlib/zlib_pmd_private.h
> new file mode 100644
> index 000..d29dc59
> --- /dev/null
> +++ b/drivers/compress/zlib/zlib_pmd_private.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2017-2018 Cavium Networks  */
> +
> +#ifndef _RTE_ZLIB_PMD_PRIVATE_H_
> +#define _RTE_ZLIB_PMD_PRIVATE_H_
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define COMPRESSDEV_NAME_ZLIB_PMDcompress_zlib
> +/**< ZLIB PMD device name */
> +
> +#define ZLIB_PMD_MAX_NB_QUEUE_PAIRS  1
> +/**< ZLIB PMD specified queue pairs */
[Lee] Doesn't look like this macro is being used anywhere, may be better to 
remove this altogether as there is no limit in software for queue pairs.

> +
> +#define DEF_MEM_LEVEL8
> +
> +int zlib_logtype_driver;
> +#define ZLIB_LOG(level, fmt, args...) \
> + rte_log(RTE_LOG_ ## level, zlib_logtype_driver, "%s(): "fmt "\n", \
> + __func__, ##args)
> +
> +#define ZLIB_LOG_INFO(fmt, args...) \
> + ZLIB_LOG(INFO, fmt, ## args)
> +#define ZLIB_LOG_ERR(fmt, args...) \
> + ZLIB_LOG(ERR, fmt, ## args)
> +#define ZLIB_LOG_WARN(fmt, args...) \
> + ZLIB_LOG(WARNING, fmt, ## args)
[Lee] See previous comments re/ static logging.

> +
> +struct zlib_private {
> + uint32_t max_nb_queue_pairs;
> + char mp_name[RTE_MEMPOOL_NAMESIZE];
> +};
> +
Thanks,
Lee.


Re: [dpdk-dev] [PATCH v1 2/6] compress/zlib: add device setup PMD ops

2018-06-25 Thread Daly, Lee
> >
> >> -Original Message-
> >> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shally Verma
> >> Sent: Tuesday, May 15, 2018 11:32 AM
> >> To: De Lara Guarch, Pablo 
> >> Cc: Trahe, Fiona ; dev@dpdk.org;
> >> pathr...@caviumnetworks.com; Sunila Sahu
> >> ; Ashish Gupta
> >> 
> >> Subject: [dpdk-dev] [PATCH v1 2/6] compress/zlib: add device setup
> >> PMD ops
> >>
> >>diff --git a/drivers/compress/zlib/zlib_pmd_ops.c
> >>b/drivers/compress/zlib/zlib_pmd_ops.c
> >>new file mode 100644
> >>index 000..0bd42f3
> >>--- /dev/null
> >>+++ b/drivers/compress/zlib/zlib_pmd_ops.c
> >>@@ -0,0 +1,238 @@
> >>+/* SPDX-License-Identifier: BSD-3-Clause
> >>+ * Copyright(c) 2018 Cavium Networks
> >>+ */
> >>+
> >>+#include 
> >>+
> >>+#include 
> >>+#include 
> >>+#include 
> >>+
> >>+#include "zlib_pmd_private.h"
> >>+
> >>+static const struct rte_compressdev_capabilities zlib_pmd_capabilities[]
> = {
> >>+  {   /* Deflate */
> >>+  .algo = RTE_COMP_ALGO_DEFLATE,
> >>+  .comp_feature_flags =
> RTE_COMP_FF_SHAREABLE_PRIV_XFORM,
> >[Lee] The priv_xform structure in this case is not shareable, as it
> >contains your zlib_stream structure, which contains zlibs own zstream
> >struct. This is not read only, the contents of this zstream will be written 
> >to,
> which means it is not shareable across queue pairs or devices.
> >
> [Shally] Per my understanding, SHAREABLE_PRIV_XFORM here means xform
> is shareable by all ops in one single enqueue_burst() but not across devices
> or qps by multiple threads in parallel. Does your implementation support
> such usage of shareable priv_xforms?
> 
> Thanks for review.
> Shally
[Lee]
Hey Shally, I have just clarified this with Fiona and Pablo and the intended 
use of Shareable priv xforms is to allow a xform to be shared across devices & 
qps not just all the ops in a burst, yes the ISA-L PMD has a shareable private 
xform due to all its contents being read only. 

> 
> >>+  .window_size = {
> >>+  .min = 8,
> >>+  .max = 15,
> >>+  .increment = 2
> >>+  },
> >>+  },
> >>+
> >>+  RTE_COMP_END_OF_CAPABILITIES_LIST()
> >>+



Re: [dpdk-dev] [PATCH 2/2] compressdev: add huffman encoding flags

2018-06-28 Thread Daly, Lee
Hi Pablo,

> -Original Message-
> From: De Lara Guarch, Pablo
> Sent: Wednesday, June 27, 2018 6:51 AM
> To: Trahe, Fiona ;
> ashish.gu...@caviumnetworks.com; Daly, Lee 
> Cc: dev@dpdk.org; De Lara Guarch, Pablo 
> Subject: [PATCH 2/2] compressdev: add huffman encoding flags
> 
> Added Huffman fixed and dynamic encoding feature flags, so an application
> can query if a device supports these two types, when performing DEFLATE
> compression.
> 
> Signed-off-by: Pablo de Lara 
> ---
>  drivers/compress/isal/isal_compress_pmd_ops.c | 4 +++-
>  lib/librte_compressdev/rte_comp.c | 4 
>  lib/librte_compressdev/rte_comp.h | 4 
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/compress/isal/isal_compress_pmd_ops.c
> b/drivers/compress/isal/isal_compress_pmd_ops.c
> index 970a0413b..585f22802 100644
> --- a/drivers/compress/isal/isal_compress_pmd_ops.c
> +++ b/drivers/compress/isal/isal_compress_pmd_ops.c
> @@ -12,7 +12,9 @@
>  static const struct rte_compressdev_capabilities isal_pmd_capabilities[] = {
>   {
>   .algo = RTE_COMP_ALGO_DEFLATE,
> - .comp_feature_flags =
>   RTE_COMP_FF_SHAREABLE_PRIV_XFORM,
> + .comp_feature_flags =
>   RTE_COMP_FF_SHAREABLE_PRIV_XFORM |
> + RTE_COMP_FF_HUFFMAN_FIXED |
> +
>   RTE_COMP_FF_HUFFMAN_DYNAMIC,
>   .window_size = {
>   .min = 15,
>   .max = 15,
Acked-by: Lee Daly 
Thanks,
Lee.


Re: [dpdk-dev] [PATCH v3 1/4] doc: cleanup ISA-L PMD feature matrix

2018-07-05 Thread Daly, Lee
> -Original Message-
> From: De Lara Guarch, Pablo
> Sent: Wednesday, July 4, 2018 3:11 PM
> To: shally.ve...@caviumnetworks.com;
> ashish.gu...@caviumnetworks.com; Trahe, Fiona ;
> Daly, Lee 
> Cc: dev@dpdk.org; De Lara Guarch, Pablo 
> Subject: [PATCH v3 1/4] doc: cleanup ISA-L PMD feature matrix
> 
> In PMD feature matrices (.ini files), it is not required to have the list of
> features that are not supported, just the ones that are.
> 
> Signed-off-by: Pablo de Lara 
> ---
>  doc/guides/compressdevs/features/isal.ini | 8 
>  1 file changed, 8 deletions(-)
> 
> diff --git a/doc/guides/compressdevs/features/isal.ini
> b/doc/guides/compressdevs/features/isal.ini
> index ad2718df0..1d4ff1c41 100644
> --- a/doc/guides/compressdevs/features/isal.ini
> +++ b/doc/guides/compressdevs/features/isal.ini
> @@ -9,14 +9,6 @@ CPU SSE= Y
>  CPU AVX= Y
>  CPU AVX2   = Y
>  CPU AVX512 = Y
> -CPU NEON   =
> -Stateful   =
> -By-Pass=
> -Chained mbufs  =
>  Deflate= Y
> -LZS=
> -Adler32=
> -Crc32  =
> -Adler32&Crc32  =
>  Fixed  = Y
>  Dynamic= Y
> --
> 2.14.4
Acked-by: Lee Daly 



Re: [dpdk-dev] [PATCH] test/compress: improve trace

2018-10-31 Thread Daly, Lee



> -Original Message-
> From: Trahe, Fiona
> Sent: Wednesday, October 31, 2018 12:42 AM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; akhil.go...@nxp.com; Jozwiak, TomaszX
> ; Daly, Lee ; Trahe, Fiona
> 
> Subject: [PATCH] test/compress: improve trace
> 
> Make clear which engine is compressing and which is decompressing in
> debug output. Also add newline and print ratio = 0 if test fails.
> 
> Signed-off-by: Fiona Trahe 
> ---
>  test/test/test_compressdev.c | 26 --
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/test/test/test_compressdev.c b/test/test/test_compressdev.c
> index 8645388..5d5e519 100644
> --- a/test/test/test_compressdev.c
> +++ b/test/test/test_compressdev.c
<...>
Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH 2/2] test/compress: use bulk free operations api

2018-11-22 Thread Daly, Lee



> -Original Message-
> From: Trahe, Fiona
> Sent: Monday, November 19, 2018 10:10 PM
> To: dev@dpdk.org
> Cc: akhil.go...@nxp.com; Jozwiak, TomaszX ;
> shally.ve...@caviumnetworks.com; ashish.gu...@caviumnetworks.com;
> Daly, Lee ; Trahe, Fiona 
> Subject: [PATCH 2/2] test/compress: use bulk free operations api
> 
> Use the new rte_comp_op_bulk_free API.
> Add trace to catch any mempool elements not freed at test end.
> 
> Signed-off-by: Fiona Trahe 
> ---
>  test/test/test_compressdev.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/test/test/test_compressdev.c b/test/test/test_compressdev.c
> index 5d5e519..3ea726d 100644
> --- a/test/test/test_compressdev.c
> +++ b/test/test/test_compressdev.c
> @@ -69,6 +69,13 @@ testsuite_teardown(void)  {
>   struct comp_testsuite_params *ts_params = &testsuite_params;
> 
> + if (rte_mempool_in_use_count(ts_params->large_mbuf_pool))
> + RTE_LOG(ERR, USER1, "Large mbuf pool still has unfreed
> bufs\n");
> + if (rte_mempool_in_use_count(ts_params->small_mbuf_pool))
> + RTE_LOG(ERR, USER1, "Small mbuf pool still has unfreed
> bufs\n");
> + if (rte_mempool_in_use_count(ts_params->op_pool))
> + RTE_LOG(ERR, USER1, "op pool still has unfreed ops\n");
> +
>   rte_mempool_free(ts_params->large_mbuf_pool);
>   rte_mempool_free(ts_params->small_mbuf_pool);
>   rte_mempool_free(ts_params->op_pool);
> @@ -731,6 +738,7 @@ test_deflate_comp_decomp(const char * const
> test_bufs[],
>   goto exit;
>   }
> 
> +
>   for (i = 0; i < num_bufs; i++) {
>   ops[i]->m_src = uncomp_bufs[i];
>   ops[i]->m_dst = comp_bufs[i];
> @@ -961,12 +969,9 @@ test_deflate_comp_decomp(const char * const
> test_bufs[],
> 
>   /*
>* Free the previous compress operations,
> -  * as it is not needed anymore
> +  * as they are not needed anymore
>*/
> - for (i = 0; i < num_bufs; i++) {
> - rte_comp_op_free(ops_processed[i]);
> - ops_processed[i] = NULL;
> - }
> + rte_comp_op_bulk_free(ops_processed, num_bufs);
> 
>   /* Decompress data (either with Zlib API or compressdev API */
>   if (zlib_dir == ZLIB_DECOMPRESS || zlib_dir == ZLIB_ALL) {
> --
> 2.7.4

Series Acked-by: Lee Daly 



Re: [dpdk-dev] [PATCH 3/3] compress/isal: fix memory leak

2018-07-11 Thread Daly, Lee



> -Original Message-
> From: De Lara Guarch, Pablo
> Sent: Wednesday, July 11, 2018 7:39 AM
> To: Daly, Lee 
> Cc: dev@dpdk.org; De Lara Guarch, Pablo ;
> sta...@dpdk.org
> Subject: [PATCH 3/3] compress/isal: fix memory leak
> 
> Processed operations ring is created for each queue pair, but it was not being
> freed when the queue pair was released.
> 
> Fixes: b0e23c458a6f ("compress/isal: add queue pair related ops")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Pablo de Lara 
> ---
>  drivers/compress/isal/isal_compress_pmd_ops.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/compress/isal/isal_compress_pmd_ops.c
> b/drivers/compress/isal/isal_compress_pmd_ops.c
> index 0738fb9c4..c61acd40c 100644
> --- a/drivers/compress/isal/isal_compress_pmd_ops.c
> +++ b/drivers/compress/isal/isal_compress_pmd_ops.c
> @@ -167,6 +167,9 @@ isal_comp_pmd_qp_release(struct rte_compressdev
> *dev, uint16_t qp_id)
>   if (qp->state != NULL)
>   rte_free(qp->state);
> 
> + if (qp->processed_pkts != NULL)
> + rte_ring_free(qp->processed_pkts);
> +
>   rte_free(qp);
>   dev->data->queue_pairs[qp_id] = NULL;
> 
> --
> 2.14.4
Acked-by: Lee Daly 



Re: [dpdk-dev] [PATCH 2/3] compress/isal: set null pointer after freeing

2018-07-11 Thread Daly, Lee



> -Original Message-
> From: De Lara Guarch, Pablo
> Sent: Wednesday, July 11, 2018 7:39 AM
> To: Daly, Lee 
> Cc: dev@dpdk.org; De Lara Guarch, Pablo ;
> sta...@dpdk.org
> Subject: [PATCH 2/3] compress/isal: set null pointer after freeing
> 
> Fixes: b0e23c458a6f ("compress/isal: add queue pair related ops")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Pablo de Lara 
> ---
>  drivers/compress/isal/isal_compress_pmd_ops.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/compress/isal/isal_compress_pmd_ops.c
> b/drivers/compress/isal/isal_compress_pmd_ops.c
> index 585f22802..0738fb9c4 100644
> --- a/drivers/compress/isal/isal_compress_pmd_ops.c
> +++ b/drivers/compress/isal/isal_compress_pmd_ops.c
> @@ -167,8 +167,8 @@ isal_comp_pmd_qp_release(struct rte_compressdev
> *dev, uint16_t qp_id)
>   if (qp->state != NULL)
>   rte_free(qp->state);
> 
> - if (dev->data->queue_pairs[qp_id] != NULL)
> - rte_free(dev->data->queue_pairs[qp_id]);
> + rte_free(qp);
> + dev->data->queue_pairs[qp_id] = NULL;
> 
>   return 0;
>  }
> --
> 2.14.4
Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH 1/3] compress/isal: fix logtype name

2018-07-11 Thread Daly, Lee



> -Original Message-
> From: De Lara Guarch, Pablo
> Sent: Wednesday, July 11, 2018 7:39 AM
> To: Daly, Lee 
> Cc: dev@dpdk.org; De Lara Guarch, Pablo ;
> sta...@dpdk.org
> Subject: [PATCH 1/3] compress/isal: fix logtype name
> 
> There is a naming convention for logtypes of PMDs:
> "pmd.driverType.driverName".
> Therefore, the logtype for ISA-L PMD should be "pmd.compress.isal".
> 
> Fixes: 490e725b95b2 ("compress/isal: add device init and de-init")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Pablo de Lara 
> ---
>  drivers/compress/isal/isal_compress_pmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/compress/isal/isal_compress_pmd.c
> b/drivers/compress/isal/isal_compress_pmd.c
> index 0f025a3bf..a3b28f29b 100644
> --- a/drivers/compress/isal/isal_compress_pmd.c
> +++ b/drivers/compress/isal/isal_compress_pmd.c
> @@ -465,7 +465,7 @@ RTE_INIT(isal_init_log);  static void
>  isal_init_log(void)
>  {
> - isal_logtype_driver = rte_log_register("comp_isal");
> + isal_logtype_driver = rte_log_register("pmd.compress.isal");
>   if (isal_logtype_driver >= 0)
>   rte_log_set_level(isal_logtype_driver, RTE_LOG_INFO);  }
> --
> 2.14.4
Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH] test/compress: add scatter-gather tests

2018-07-16 Thread Daly, Lee


> -Original Message-
> From: De Lara Guarch, Pablo
> Sent: Tuesday, July 3, 2018 1:52 AM
> To: Trahe, Fiona ;
> ashish.gu...@caviumnetworks.com; Daly, Lee 
> Cc: dev@dpdk.org; De Lara Guarch, Pablo 
> Subject: [PATCH] test/compress: add scatter-gather tests
> 
> Added Scatter-Gather test, which split input data into multi-segment mbufs
> and compresses/decompresses the data into also a multi-segment mbuf.
> 
> Signed-off-by: Pablo de Lara 
> ---
>  test/test/test_compressdev.c | 390
> ++-
>  1 file changed, 351 insertions(+), 39 deletions(-)
> 
> diff --git a/test/test/test_compressdev.c b/test/test/test_compressdev.c
> --

<...>

> 2.14.4
Acked-by: Lee Daly 



[dpdk-dev] [PATCH v3] compress/isal: add chained mbuf support

2018-07-23 Thread Daly, Lee
This patch adds chained mbuf support for input or output buffers
during compression/decompression operations.

Signed-off-by: Lee Daly 
---
 doc/guides/compressdevs/features/isal.ini |  17 +-
 doc/guides/compressdevs/isal.rst  |   2 -
 drivers/compress/isal/isal_compress_pmd.c | 397 --
 drivers/compress/isal/isal_compress_pmd_ops.c |   5 +-
 4 files changed, 323 insertions(+), 98 deletions(-)

diff --git a/doc/guides/compressdevs/features/isal.ini 
b/doc/guides/compressdevs/features/isal.ini
index 7183d10..919cf70 100644
--- a/doc/guides/compressdevs/features/isal.ini
+++ b/doc/guides/compressdevs/features/isal.ini
@@ -4,10 +4,13 @@
 ; Supported features of 'ISA-L' compression driver.
 ;
 [Features]
-CPU SSE= Y
-CPU AVX= Y
-CPU AVX2   = Y
-CPU AVX512 = Y
-Deflate= Y
-Fixed  = Y
-Dynamic= Y
+CPU SSE= Y
+CPU AVX= Y
+CPU AVX2   = Y
+CPU AVX512 = Y
+OOP SGL In SGL Out = Y
+OOP SGL In LB  Out = Y
+OOP LB  In SGL Out = Y
+Deflate= Y
+Fixed  = Y
+Dynamic= Y
diff --git a/doc/guides/compressdevs/isal.rst b/doc/guides/compressdevs/isal.rst
index 276ff06..3bc3022 100644
--- a/doc/guides/compressdevs/isal.rst
+++ b/doc/guides/compressdevs/isal.rst
@@ -78,8 +78,6 @@ As a result the level mappings from the API to the PMD are 
shown below.
 Limitations
 ---
 
-* Chained mbufs will not be supported until a future release, meaning max data 
size being passed to PMD through a single mbuf is 64K - 1. If data is larger 
than this it will need to be split up and sent as multiple operations.
-
 * Compressdev level 0, no compression, is not supported.
 
 * Checksums will not be supported until future release.
diff --git a/drivers/compress/isal/isal_compress_pmd.c 
b/drivers/compress/isal/isal_compress_pmd.c
index 5966cc3..a776019 100644
--- a/drivers/compress/isal/isal_compress_pmd.c
+++ b/drivers/compress/isal/isal_compress_pmd.c
@@ -188,6 +188,206 @@ isal_comp_set_priv_xform_parameters(struct 
isal_priv_xform *priv_xform,
return 0;
 }
 
+/* Compression using chained mbufs for input/output data */
+static int
+chained_mbuf_compression(struct rte_comp_op *op, struct isal_comp_qp *qp)
+{
+   int ret;
+   uint32_t remaining_offset;
+   uint32_t remaining_data = op->src.length;
+   struct rte_mbuf *src = op->m_src;
+   struct rte_mbuf *dst = op->m_dst;
+
+   /* check for offset passing multiple segments
+* and point compression stream to input/output buffer
+*/
+   if (op->src.offset > src->data_len) {
+   remaining_offset = op->src.offset;
+   while (remaining_offset > src->data_len) {
+   remaining_offset -= src->data_len;
+   src = src->next;
+   }
+
+
+   qp->stream->avail_in = src->data_len - remaining_offset;
+   qp->stream->next_in = rte_pktmbuf_mtod_offset(src, uint8_t *,
+   (op->src.offset % src->data_len));
+   } else {
+   qp->stream->avail_in = src->data_len - op->src.offset;
+   qp->stream->next_in = rte_pktmbuf_mtod_offset(src, uint8_t *,
+   op->src.offset);
+   }
+
+   if (op->dst.offset > dst->data_len) {
+   remaining_offset = op->dst.offset;
+   while (remaining_offset > dst->data_len) {
+   remaining_offset -= dst->data_len;
+   dst = dst->next;
+   }
+
+   qp->stream->avail_out = dst->data_len - remaining_offset;
+   qp->stream->next_out = rte_pktmbuf_mtod_offset(dst, uint8_t *,
+   (op->dst.offset % dst->data_len));
+   } else {
+   qp->stream->avail_out = dst->data_len - op->dst.offset;
+   qp->stream->next_out = rte_pktmbuf_mtod_offset(dst, uint8_t *,
+   op->dst.offset);
+   }
+
+   if (unlikely(!qp->stream->next_in || !qp->stream->next_out)) {
+   ISAL_PMD_LOG(ERR, "Invalid source or destination buffer\n");
+   op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
+   return -1;
+   }
+
+   while (qp->stream->internal_state.state != ZSTATE_END) {
+   /* Last segment of data */
+   if (remaining_data <= src->data_len)
+   qp->stream->end_of_stream = 1;
+
+   /* Execute compression operation */
+   ret = isal_deflate(qp->stream);
+
+   remaining_data = op->src.length - qp->stream->total_in;
+
+   if (ret != COMP_OK) {
+   ISAL_PMD_LOG(ERR, "Compression operation failed\n");
+   op->status = RTE_COMP_OP_STATUS_ERROR;
+   return ret;
+   }
+
+   if (qp->stream->avail_in == 0 &&
+  

[dpdk-dev] [PATCH v4] compress/isal: add chained mbuf support

2018-07-24 Thread Daly, Lee
This patch adds chained mbuf support for input or output buffers
during compression/decompression operations.

V2 - split chained mbuf functionality into separate functions.
   - code optimizations.

V3 - bug fixes & optimizations.

v4 - bug fixes & optimizations.

Signed-off-by: Lee Daly 
---
 doc/guides/compressdevs/features/isal.ini |  17 +-
 doc/guides/compressdevs/isal.rst  |   2 -
 drivers/compress/isal/isal_compress_pmd.c | 350 --
 drivers/compress/isal/isal_compress_pmd_ops.c |   5 +-
 4 files changed, 286 insertions(+), 88 deletions(-)

diff --git a/doc/guides/compressdevs/features/isal.ini 
b/doc/guides/compressdevs/features/isal.ini
index 7183d10..919cf70 100644
--- a/doc/guides/compressdevs/features/isal.ini
+++ b/doc/guides/compressdevs/features/isal.ini
@@ -4,10 +4,13 @@
 ; Supported features of 'ISA-L' compression driver.
 ;
 [Features]
-CPU SSE= Y
-CPU AVX= Y
-CPU AVX2   = Y
-CPU AVX512 = Y
-Deflate= Y
-Fixed  = Y
-Dynamic= Y
+CPU SSE= Y
+CPU AVX= Y
+CPU AVX2   = Y
+CPU AVX512 = Y
+OOP SGL In SGL Out = Y
+OOP SGL In LB  Out = Y
+OOP LB  In SGL Out = Y
+Deflate= Y
+Fixed  = Y
+Dynamic= Y
diff --git a/doc/guides/compressdevs/isal.rst b/doc/guides/compressdevs/isal.rst
index 276ff06..3bc3022 100644
--- a/doc/guides/compressdevs/isal.rst
+++ b/doc/guides/compressdevs/isal.rst
@@ -78,8 +78,6 @@ As a result the level mappings from the API to the PMD are 
shown below.
 Limitations
 ---
 
-* Chained mbufs will not be supported until a future release, meaning max data 
size being passed to PMD through a single mbuf is 64K - 1. If data is larger 
than this it will need to be split up and sent as multiple operations.
-
 * Compressdev level 0, no compression, is not supported.
 
 * Checksums will not be supported until future release.
diff --git a/drivers/compress/isal/isal_compress_pmd.c 
b/drivers/compress/isal/isal_compress_pmd.c
index 5966cc3..c2bcd2d 100644
--- a/drivers/compress/isal/isal_compress_pmd.c
+++ b/drivers/compress/isal/isal_compress_pmd.c
@@ -188,6 +188,179 @@ isal_comp_set_priv_xform_parameters(struct 
isal_priv_xform *priv_xform,
return 0;
 }
 
+/* Compression using chained mbufs for input/output data */
+static int
+chained_mbuf_compression(struct rte_comp_op *op, struct isal_comp_qp *qp)
+{
+   int ret;
+   uint32_t remaining_offset;
+   uint32_t remaining_data = op->src.length;
+   struct rte_mbuf *src = op->m_src;
+   struct rte_mbuf *dst = op->m_dst;
+
+   /* check for source/destination offset passing multiple segments
+* and point compression stream to input/output buffer.
+*/
+   remaining_offset = op->src.offset;
+   while (remaining_offset >= src->data_len) {
+   remaining_offset -= src->data_len;
+   src = src->next;
+   }
+   qp->stream->avail_in = RTE_MIN(src->data_len - remaining_offset,
+   op->src.length);
+   qp->stream->next_in = rte_pktmbuf_mtod_offset(src, uint8_t *,
+   remaining_offset);
+
+   remaining_offset = op->dst.offset;
+   while (remaining_offset >= dst->data_len) {
+   remaining_offset -= dst->data_len;
+   dst = dst->next;
+   }
+   qp->stream->avail_out = dst->data_len - remaining_offset;
+   qp->stream->next_out = rte_pktmbuf_mtod_offset(dst, uint8_t *,
+   remaining_offset);
+
+   if (unlikely(!qp->stream->next_in || !qp->stream->next_out)) {
+   ISAL_PMD_LOG(ERR, "Invalid source or destination buffer\n");
+   op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
+   return -1;
+   }
+
+   while (qp->stream->internal_state.state != ZSTATE_END) {
+   /* Last segment of data */
+   if (remaining_data <= src->data_len)
+   qp->stream->end_of_stream = 1;
+
+   /* Execute compression operation */
+   ret = isal_deflate(qp->stream);
+
+   remaining_data = op->src.length - qp->stream->total_in;
+
+   if (ret != COMP_OK) {
+   ISAL_PMD_LOG(ERR, "Compression operation failed\n");
+   op->status = RTE_COMP_OP_STATUS_ERROR;
+   return ret;
+   }
+
+   if (qp->stream->avail_in == 0 &&
+   qp->stream->total_in != op->src.length) {
+   if (src->next != NULL) {
+   src = src->next;
+   qp->stream->next_in =
+   rte_pktmbuf_mtod(src, uint8_t 
*);
+   qp->stream->avail_in =
+   RTE_MIN(remaining_data, src->data_len);
+   } else {
+ 

Re: [dpdk-dev] [PATCH] test/compress: add offset tests

2018-07-31 Thread Daly, Lee
This test will not be up streamed in 18.08.
V2, containing a fix for this error, will be sent for 18.11.

> -Original Message-
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Tuesday, July 31, 2018 4:02 PM
> To: Daly, Lee 
> Cc: dev@dpdk.org; De Lara Guarch, Pablo ;
> Trahe, Fiona 
> Subject: Re: [dpdk-dev] [PATCH] test/compress: add offset tests
> 
> 24/07/2018 16:06, Lee Daly:
> > From: "Daly, Lee" 
> >
> > Added offset test, which tests compression/decompression with a buffer
> > containing an offset spanning across multiple segments.
> >
> > Signed-off-by: Lee Daly 
> 
> There is a compilation error (with GCC 8).
> 
> > +   if ((capab->comp_feature_flags &
> RTE_COMP_FF_OOP_SGL_IN_SGL_OUT) ==
> > +1) {
> 
> test_compressdev.c:1540:67: error:
> bitwise comparison always evaluates to false [-Werror=tautological-
> compare]
> 
> 



Re: [dpdk-dev] [PATCH v3 11/11] compress/isal: add ISA-L compression PMD docs

2018-04-23 Thread Daly, Lee
//snip
> Subject: RE: [dpdk-dev] [PATCH v3 11/11] compress/isal: add ISA-L compression
> PMD docs
> 
> 
> > Signed-off-by: Lee Daly 
> > ---
> >  MAINTAINERS   |  5 ++
> >  devtools/test-build.sh|  4 ++
> >  doc/guides/compressdevs/features/isal.ini | 40 +
> >  doc/guides/compressdevs/index.rst |  1 +
> >  doc/guides/compressdevs/isal.rst  | 94
> > +++
> 
> <...>
> 
> > +Limitations
> > +---
> > +
> > +* Chained mbufs are not supported.
> > +
> > +* Compressdev level 0, no compression, is not supported. ISA-L level
> > +0 used for
> > +
> > +fixed huffman codes.
> 

[Lee] Thanks for the feedback on patchset, V4 to be sent in coming days.

> For the bit above have the two lines together as it messes up the format and
> look of the sentance
> 
> * Compressdev level 0, no compression, is not supported. ISA-L level 0
> used for fixed huffman codes.
> 
> > +* Out of order operations are not supported
> > +
> > +Installation
> > +
//snip


Re: [dpdk-dev] [PATCH v3 10/11] compress/isal: add generic compression driver docs

2018-04-25 Thread Daly, Lee
Thanks for the feedback on the patch set Pablo, 
V4 in progress.

> -Original Message-
> From: De Lara Guarch, Pablo
> Sent: Tuesday, April 24, 2018 12:07 PM
> To: Daly, Lee ; dev@dpdk.org
> Cc: Tucker, Greg B ; Jain, Deepak K
> ; Trahe, Fiona 
> Subject: RE: [PATCH v3 10/11] compress/isal: add generic compression driver
> docs
> 
> > -Original Message-
> > From: Daly, Lee
> > Sent: Tuesday, April 17, 2018 2:36 PM
> > To: dev@dpdk.org
> > Cc: De Lara Guarch, Pablo ; Tucker,
> > Greg B ; Jain, Deepak K
> > ; Trahe, Fiona ; Daly,
> > Lee 
> > Subject: [PATCH v3 10/11] compress/isal: add generic compression
> > driver docs
> >
> 
> One last comment. Change title to: "doc: add compression feature guide", since
> this is not adding any information about ISA-L, but it is common for all
> compression PMDs.



Re: [dpdk-dev] [PATCH v3 05/11] compress/isal: add queue pair related ops

2018-04-26 Thread Daly, Lee

<...>
> > -Original Message-
> > From: Daly, Lee
> > Sent: Tuesday, April 17, 2018 2:35 PM
> > To: dev@dpdk.org
> > Cc: De Lara Guarch, Pablo ; Tucker,
> > Greg B ; Jain, Deepak K
> > ; Trahe, Fiona ; Daly,
> > Lee 
> > Subject: [PATCH v3 05/11] compress/isal: add queue pair related ops
> >
> > Signed-off-by: Lee Daly 
> > ---
> >  drivers/compress/isal/isal_compress_pmd_ops.c | 110
> > +-
> >  drivers/compress/isal/isal_compress_pmd_private.h |   2 +
> >  2 files changed, 110 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/compress/isal/isal_compress_pmd_ops.c
> > b/drivers/compress/isal/isal_compress_pmd_ops.c
> > index 2e9381d..73e4c84 100644
> > --- a/drivers/compress/isal/isal_compress_pmd_ops.c

//snip

> 
> > +qp_id */ static int isal_comp_pmd_qp_set_unique_name(struct
> > +rte_compressdev *dev, struct isal_comp_qp *qp) {
> > +   unsigned int n = snprintf(qp->name, sizeof(qp->name),
> > +   "isal_compression_pmd_%u_qp_%u",
> > +   dev->data->dev_id, qp->id);
> 
> Better to use strlcpy.

[Lee] I believe snprintf to be a better fit due to the fact strlcpy doesn't 
handle variable arguments as inputs, i.e 
"snprintf(dst, length, "%s", src)" 
"strlcpy(dst, src, length)"
To complete this action with strlcpy would cause more overhead, and snprintf() 
is still safe as it always null terminates.

> 
> > +
> > +   if (n >= sizeof(qp->name))
> > +   return -1;
> > +
> > +   return 0;
> > +}



Re: [dpdk-dev] [PATCH v3 0/5] Initial compressdev unit tests

2018-05-01 Thread Daly, Lee

<...> 
> Added initial tests for Compressdev library.
> The tests are performed compressing a test buffer (or multiple test buffers)
> with compressdev or Zlib, and decompressing it/them with the other library
> (if compression is done with compressdev, decompression is done with Zlib,
> and viceversa).
> 
> Tests added so far are based on the deflate algorithm,
> including:
> - Fixed huffman on single buffer
> - Dynamic huffman on single buffer
> - Multi compression level test on single buffer
> - Multi buffer
> - Multi xform using the same buffer
> 
> Due to a dependency on Zlib, the test is not enabled by default.
> Once the library is installed, the configuration option
> CONFIG_RTE_COMPRESSDEV_TEST must be set to Y.
> However, if building with Meson, the test will be built automatically, if 
> Zlib is
> installed.
> 
> The test requires a compressdev PMD to be initialized, when running the test
> app. For example:
> 
> ./build/app/test --vdev="compress_X"
> 
> RTE>>compressdev_autotest
> 
> This patchset depends on the Compressdev API patchset:
> http://dpdk.org/ml/archives/dev/2018-April/099580.html
> ("[PATCH v6 00/14] Implement compression API")
> 
<...>
Series-acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH v3 1/5] test/compress: add initial unit tests

2018-05-02 Thread Daly, Lee


> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Pablo de Lara
> Sent: Friday, April 27, 2018 3:15 PM
> To: dev@dpdk.org
> Cc: Trahe, Fiona ; shally.ve...@cavium.com;
> ahmed.mans...@nxp.com; ashish.gu...@cavium.com; De Lara Guarch,
> Pablo ; Ashish Gupta
> ; Shally Verma
> 
> Subject: [dpdk-dev] [PATCH v3 1/5] test/compress: add initial unit tests
> 
> This commit introduces the initial tests for compressdev, performing basic
> compression and decompression operations of sample test buffers, using
> the Zlib library in one direction and compressdev in another direction, to
> make sure that the library is compatible with Zlib.
> 
> Due to the use of Zlib API, the test is disabled by default, to avoid adding a
> new dependency on DPDK.
> 
> Signed-off-by: Pablo de Lara 
> Signed-off-by: Ashish Gupta 
> Signed-off-by: Shally Verma 


<...>

> +static int
> +testsuite_setup(void)
> +{
> + struct comp_testsuite_params *ts_params = &testsuite_params;
> + unsigned int i;
> +
> + if (rte_compressdev_count() == 0) {
> + RTE_LOG(ERR, USER1, "Need at least one compress
> device\n");
> + return -EINVAL;
> + }
> +
> + uint32_t max_buf_size = 0;
> + for (i = 0; i < RTE_DIM(compress_test_bufs); i++)
> + max_buf_size = RTE_MAX(max_buf_size,
> + strlen(compress_test_bufs[i]) + 1);
> +
> + max_buf_size *= COMPRESS_BUF_SIZE_RATIO;
> + /*
> +  * Buffers to be used in compression and decompression.
> +  * Since decompressed data might be larger than
> +  * compressed data (due to block header),
> +  * buffers should be big enough for both cases.
> +  */
> + ts_params->mbuf_pool = rte_pktmbuf_pool_create("mbuf_pool",
> + NUM_MBUFS,
> + CACHE_SIZE, 0,
> + max_buf_size + RTE_PKTMBUF_HEADROOM,
> + rte_socket_id());
> + if (ts_params->mbuf_pool == NULL) {
> + RTE_LOG(ERR, USER1, "Large mbuf pool could not be
> created\n");
> + goto exit;

[Lee]At this point there is nothing to be freed yet, therefore TEST_FAILED can 
be returned here without testsuite_teardown();

> + }
> +
> + ts_params->op_pool = rte_comp_op_pool_create("op_pool",
> NUM_OPS,
> + 0, 0, rte_socket_id());
> + if (ts_params->op_pool == NULL) {
> + RTE_LOG(ERR, USER1, "Operation pool could not be
> created\n");
> + goto exit;

[Lee]Similar point here, wasted cycles on freeing memory which has not be 
allocated yet.
May not be a major issue since this is only done on setup.

> + }
> +
> + /* Initializes default values for compress/decompress xforms */
> + ts_params->def_comp_xform.type = RTE_COMP_COMPRESS;
> + ts_params->def_comp_xform.compress.algo =
> RTE_COMP_ALGO_DEFLATE,
> + ts_params->def_comp_xform.compress.deflate.huffman =
> +
>   RTE_COMP_HUFFMAN_DEFAULT;
> + ts_params->def_comp_xform.compress.level =
> RTE_COMP_LEVEL_PMD_DEFAULT;
> + ts_params->def_comp_xform.compress.chksum =
> RTE_COMP_CHECKSUM_NONE;
> + ts_params->def_comp_xform.compress.window_size =
> DEFAULT_WINDOW_SIZE;
> +
> + ts_params->def_decomp_xform.type = RTE_COMP_DECOMPRESS;
> + ts_params->def_decomp_xform.decompress.algo =
> RTE_COMP_ALGO_DEFLATE,
> + ts_params->def_decomp_xform.decompress.chksum =
> RTE_COMP_CHECKSUM_NONE;
> + ts_params->def_decomp_xform.decompress.window_size =
> +DEFAULT_WINDOW_SIZE;
> +
> + return TEST_SUCCESS;
> +
> +exit:
> + testsuite_teardown();
> +
> + return TEST_FAILED;
> +}
> +
<...>


Re: [dpdk-dev] [PATCH v3 4/5] test/compress: add multi xform test

2018-05-02 Thread Daly, Lee
Hi Pablo,
Feedback for a small change below.

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Pablo de Lara
> Sent: Friday, April 27, 2018 3:15 PM
> To: dev@dpdk.org
> Cc: Trahe, Fiona ; shally.ve...@cavium.com;
> ahmed.mans...@nxp.com; ashish.gu...@cavium.com; De Lara Guarch,
> Pablo 
> Subject: [dpdk-dev] [PATCH v3 4/5] test/compress: add multi xform test
> 
> Add test that checks if multiple xforms can be handled on a single enqueue
> call.
> 
> Signed-off-by: Pablo de Lara 
> ---
>  test/test/test_compressdev.c | 257
> ---
>  1 file changed, 191 insertions(+), 66 deletions(-)
> 
> diff --git a/test/test/test_compressdev.c b/test/test/test_compressdev.c
> index bb026d74f..0253d12ea 100644
> --- a/test/test/test_compressdev.c
> +++ b/test/test/test_compressdev.c
> @@ -27,7 +27,7 @@
>  #define COMPRESS_BUF_SIZE_RATIO 1.3
>  #define NUM_MBUFS 16
>  #define NUM_OPS 16
> -#define NUM_MAX_XFORMS 1
> +#define NUM_MAX_XFORMS 16
>  #define NUM_MAX_INFLIGHT_OPS 128
>  #define CACHE_SIZE 0
> 
> @@ -52,8 +52,8 @@ struct priv_op_data {
>  struct comp_testsuite_params {
>   struct rte_mempool *mbuf_pool;
>   struct rte_mempool *op_pool;
> - struct rte_comp_xform def_comp_xform;
> - struct rte_comp_xform def_decomp_xform;
> + struct rte_comp_xform *def_comp_xform;
> + struct rte_comp_xform *def_decomp_xform;
>  };
> 
>  static struct comp_testsuite_params testsuite_params = { 0 }; @@ -65,6
> +65,8 @@ testsuite_teardown(void)
> 
>   rte_mempool_free(ts_params->mbuf_pool);
>   rte_mempool_free(ts_params->op_pool);
> + rte_free(ts_params->def_comp_xform);
> + rte_free(ts_params->def_decomp_xform);
>  }
> 
>  static int
> @@ -108,19 +110,24 @@ testsuite_setup(void)
>   goto exit;
>   }
> 
> + ts_params->def_comp_xform =
> + rte_malloc(NULL, sizeof(struct rte_comp_xform), 0);
> + ts_params->def_decomp_xform =
> + rte_malloc(NULL, sizeof(struct rte_comp_xform), 0);
> +


Perhaps add a check to ensure this memory has been successfully allocated, as 
you did you with the mempool malloc above this, in PATCH 1/5.

<...>


Re: [dpdk-dev] [PATCH] compress/isal: add ISA-L lib version display

2019-02-11 Thread Daly, Lee
Hi Tomasz,
Looks good to me.
> -Original Message-
> From: Cel, TomaszX
> Sent: Monday, February 11, 2019 9:19 AM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Trahe, Fiona ; Daly, Lee
> ; De Lara Guarch, Pablo
> ; Jozwiak, TomaszX
> ; Cel, TomaszX 
> Subject: [PATCH] compress/isal: add ISA-L lib version display
> 
> Display information about ISA-L lib version in UTs.
> 
> Signed-off-by: Tomasz Cel 
> ---
>  drivers/compress/isal/isal_compress_pmd.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/compress/isal/isal_compress_pmd.c
> b/drivers/compress/isal/isal_compress_pmd.c
> index 4748238..8879a42 100644
> --- a/drivers/compress/isal/isal_compress_pmd.c
> +++ b/drivers/compress/isal/isal_compress_pmd.c
<...>

> 2.7.4

Acked-by: Lee Daly 



Re: [dpdk-dev] [PATCH v3] test/compress: add mbuf offset unit test

2019-02-25 Thread Daly, Lee
Hi Thomas,
This patch was deferred to a later release, 
Patchwork did not reflect this so I have now changed it.

Rgds,
Lee.

> -Original Message-
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Sunday, February 24, 2019 10:59 PM
> To: Daly, Lee 
> Cc: dev@dpdk.org; akhil.go...@nxp.com; Jozwiak, TomaszX
> ; Trahe, Fiona 
> Subject: Re: [dpdk-dev] [PATCH v3] test/compress: add mbuf offset unit test
> 
> 05/12/2018 15:38, Lee Daly:
> > added mbuf offset test to compressdev test suite, which tests
> > compression/decompression with a mbuf containing an offset spanning
> > across mulitple segments.
> >
> > V2:
> >- Change how test checks capalilites structure.
> >
> > V3:
> >- Change commit message.
> 
> Better to move the changelog after --- so it is skipped by git.
> 
> > Signed-off-by: Lee Daly 
> 
> Please, could you rebase this patch?
> Thanks
> 
> 



Re: [dpdk-dev] [PATCH v3] test/compress: add mbuf offset unit test

2019-02-25 Thread Daly, Lee
Not in its current state.

-Fiona: Does the current team plan on sending another version of membuf_offset 
UT for 19.05?

> -Original Message-
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Monday, February 25, 2019 9:32 AM
> To: Daly, Lee 
> Cc: dev@dpdk.org; akhil.go...@nxp.com; Jozwiak, TomaszX
> ; Trahe, Fiona 
> Subject: Re: [dpdk-dev] [PATCH v3] test/compress: add mbuf offset unit test
> 
> 25/02/2019 09:56, Daly, Lee:
> > Hi Thomas,
> > This patch was deferred to a later release, Patchwork did not reflect
> > this so I have now changed it.
> 
> You mean you don't want this patch in DPDK 19.05?
> 
> 



Re: [dpdk-dev] [PATCH] examples: update copyrights and license

2018-02-08 Thread Daly, Lee

> -Original Message-
> From: Hemant Agrawal [mailto:hemant.agra...@nxp.com]
> Sent: Thursday, February 8, 2018 8:49 AM
> To: Daly, Lee ; Mcnamara, John
> ; De Lara Guarch, Pablo
> ; Richardson, Bruce
> ; Van Haaren, Harry
> ; hala...@gmail.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] examples: update copyrights and license
> 
> Hi Lee,
>   Hasan Alayli's license seems to be BSD-2-Clause, which is typically
> compatible with BSD-3-Clause.
>   It will be ideal to get ACK from Hasan Alayli to re-license it as BSD-3-
> clause and change it to SPDX tag.


Agree. This should be done for 18.05 release, 
This patch set it just updating the BSD-3 licenses.

Regards,
Lee.


Re: [dpdk-dev] [PATCH 1/2] lib: refactor basic stats code

2017-12-11 Thread Daly, Lee
> Subject: [dpdk-dev] [PATCH 1/2] lib: refactor basic stats code
> 
> Moved the code to get the basic stats names and values into static functions.
> 
> Signed-off-by: Elza Mathew 

Reviewed-by: Lee Daly 



Re: [dpdk-dev] [PATCH 2/2] lib: optimize _xstats_by_ids APIs

2017-12-11 Thread Daly, Lee
> Subject: [dpdk-dev] [PATCH 2/2] lib: optimize _xstats_by_ids APIs
> 
> Introduced a check to detect if the stats IDs being requested are all basic
> stats IDs. In that case, ensured that only the basic stats would be retrieved.
> Previously, both basic stats and xstats were being retrieved even if all the 
> IDs
> were basic stats IDs.
> 
> Signed-off-by: Elza Mathew 

Reviewed-by: Lee Daly 



Re: [dpdk-dev] [PATCH] compress/isal: fix compression stream initialization

2019-03-27 Thread Daly, Lee
Hi Tomasz,

> -Original Message-
> From: Cel, TomaszX
> Sent: Tuesday, March 26, 2019 9:42 AM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Trahe, Fiona ; Daly, Lee
> ; Tucker, Greg B ; Jozwiak,
> TomaszX 
> Subject: [PATCH] compress/isal: fix compression stream initialization
> 
> This patch fixes ISAL internal state fields initialization.
> 
> Fixes: dc49e6aa4879 ("compress/isal: add ISA-L compression functionality")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Tomasz Cel 
> ---
>  drivers/compress/isal/isal_compress_pmd.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/compress/isal/isal_compress_pmd.c
> b/drivers/compress/isal/isal_compress_pmd.c
> index 8879a42..1e518b3 100644
> --- a/drivers/compress/isal/isal_compress_pmd.c
> +++ b/drivers/compress/isal/isal_compress_pmd.c
> @@ -412,7 +412,7 @@ process_isal_deflate(struct rte_comp_op *op, struct
> isal_comp_qp *qp,
>   uint8_t *temp_level_buf = qp->stream->level_buf;
> 
>   /* Initialize compression stream */
> - isal_deflate_stateless_init(qp->stream);
> + isal_deflate_init(qp->stream);
> 
>   qp->stream->level_buf = temp_level_buf;
> 
> @@ -514,8 +514,6 @@ process_isal_deflate(struct rte_comp_op *op, struct
> isal_comp_qp *qp,
>   op->output_chksum = qp->stream->internal_state.crc;
>   }
> 
> - isal_deflate_reset(qp->stream);
I have tested this and it works, removing this reset function makes sense as 
everything cleared here is cleared in the init function anyway,
On that note, the same is to be said to isal_inflate_reset, everything cleared 
in reset is cleared, plus more, in isal_inflate_init.
I suggest we also remove isal_inflate_reset for commonality across both 
functions.

> -
>   return ret;
>  }
> 
> --
> 2.7.4



Re: [dpdk-dev] [PATCH] compress/isal: fix getting information about CPU

2019-03-27 Thread Daly, Lee
Hi Tomasz,

This patch makes sense to check does the CPU have the capability of 
certain instructions before
Adding it to the dev_info flags. I think one more addition should be made in 
isal_compress_pmd.c 
When checking what compression level should be used , decided by the AVX 
instructions available, it previously used the 
Rte_cpu_get_flag_enabled() function, however, I don’t believe that is needed 
any more since it will now be checked beforehand, it would be doing twice the 
work.
When the driver now needs to decide the compression level it should now just 
check the feature_flags set below. Perhaps a V2 could be sent with this 
addition.
Regards,
Lee.

> > This patch adds query about CPU features
> >
> > Fixes: 53a9baa98c36 ("compress/isal: add basic PMD ops")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Tomasz Cel 
> > ---
> >   drivers/compress/isal/isal_compress_pmd_ops.c | 16 
> >   1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/compress/isal/isal_compress_pmd_ops.c
> b/drivers/compress/isal/isal_compress_pmd_ops.c
> > index 7b91849..fe99959 100644
> > --- a/drivers/compress/isal/isal_compress_pmd_ops.c
> > +++ b/drivers/compress/isal/isal_compress_pmd_ops.c
> > @@ -135,10 +135,18 @@ isal_comp_pmd_info_get(struct
> rte_compressdev *dev __rte_unused,
> >   {
> > if (dev_info != NULL) {
> > dev_info->capabilities = isal_pmd_capabilities;
> > -   dev_info->feature_flags = RTE_COMPDEV_FF_CPU_AVX512 |
> > -   RTE_COMPDEV_FF_CPU_AVX2 |
> > -   RTE_COMPDEV_FF_CPU_AVX |
> > -   RTE_COMPDEV_FF_CPU_SSE;
> > +
> > +   /* Check CPU for supported vector instruction and set
> > +* feature_flags
> > +*/
> > +   if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F))
> > +   dev_info->feature_flags |=
> RTE_COMPDEV_FF_CPU_AVX512;
> > +   else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
> > +   dev_info->feature_flags |=
> RTE_COMPDEV_FF_CPU_AVX2;
> > +   else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX))
> > +   dev_info->feature_flags |=
> RTE_COMPDEV_FF_CPU_AVX;
> > +   else
> > +   dev_info->feature_flags |=
> RTE_COMPDEV_FF_CPU_SSE;
> > }
> >   }
> >



Re: [dpdk-dev] [PATCH v3 1/1] compress/isal: fix compression stream initialization

2019-03-28 Thread Daly, Lee



> -Original Message-
> From: Trahe, Fiona
> Sent: Thursday, March 28, 2019 12:59 PM
> To: Cel, TomaszX ; dev@dpdk.org
> Cc: sta...@dpdk.org; Daly, Lee ; Tucker, Greg B
> ; Jozwiak, TomaszX 
> Subject: RE: [PATCH v3 1/1] compress/isal: fix compression stream
> initialization
> 
> 
> 
> > -Original Message-
> > From: Cel, TomaszX
> > Sent: Thursday, March 28, 2019 9:06 AM
> > To: dev@dpdk.org
> > Cc: sta...@dpdk.org; Trahe, Fiona ; Daly, Lee
> > ; Tucker, Greg B ;
> > Jozwiak, TomaszX ; Cel, TomaszX
> > 
> > Subject: [PATCH v3 1/1] compress/isal: fix compression stream
> > initialization
> >
> > This patch fixes ISAL internal state fields initialization.
> >
> > Fixes: dc49e6aa4879 ("compress/isal: add ISA-L compression
> > functionality")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Tomasz Cel 
> Acked-by: Fiona Trahe 
Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH] compress/isal: fix getting information about CPU

2019-03-29 Thread Daly, Lee
Hi Tomasz,
After some testing I can now see the approach in your patch in the best 
method for getting the CPU info.
Rgds,
Lee.
> -Original Message-
> From: Daly, Lee
> Sent: Wednesday, March 27, 2019 3:01 PM
> To: Akhil Goyal ; Cel, TomaszX
> ; dev@dpdk.org
> Cc: sta...@dpdk.org; Trahe, Fiona 
> Subject: RE: [dpdk-dev] [PATCH] compress/isal: fix getting information about
> CPU
> 
> Hi Tomasz,
> 
>   This patch makes sense to check does the CPU have the capability of
> certain instructions before Adding it to the dev_info flags. I think one more
> addition should be made in isal_compress_pmd.c When checking what
> compression level should be used , decided by the AVX instructions available,
> it previously used the
> Rte_cpu_get_flag_enabled() function, however, I don’t believe that is needed
> any more since it will now be checked beforehand, it would be doing twice
> the work.
> When the driver now needs to decide the compression level it should now
> just check the feature_flags set below. Perhaps a V2 could be sent with this
> addition.
> Regards,
> Lee.
> 
> > > This patch adds query about CPU features
> > >
> > > Fixes: 53a9baa98c36 ("compress/isal: add basic PMD ops")
> > > Cc: sta...@dpdk.org
> > >
> > > Signed-off-by: Tomasz Cel 
> > > ---
Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH] compress/isal: create shorter qp name

2019-08-06 Thread Daly, Lee
Hi Adam,

Take care to add the maintainer of the code you are changing to your email, a 
list can be found in the MAINTAINERS file. 
This will hopefully ensure a response to the change. 
Thanks.
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Adam Dybkowski
> Sent: Tuesday, August 6, 2019 10:10 AM
> To: dev@dpdk.org; Trahe, Fiona ; Luse, Paul E
> 
> Cc: Dybkowski, AdamX 
> Subject: [dpdk-dev] [PATCH] compress/isal: create shorter qp name
> 
> This patch shortens the queue pair name created when initializing the queue
> pair of the ISAL PIM, based on the device and qp ids.
> The patch idea of shortening the queue pair name was proposed by Luse,
> Paul E <
> 
> Signed-off-by: Adam Dybkowski 
> ---
>  drivers/compress/isal/isal_compress_pmd_ops.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/compress/isal/isal_compress_pmd_ops.c
> b/drivers/compress/isal/isal_compress_pmd_ops.c
> index 77ac6fcf2..31c455991 100644
> --- a/drivers/compress/isal/isal_compress_pmd_ops.c
> +++ b/drivers/compress/isal/isal_compress_pmd_ops.c
> @@ -216,7 +216,7 @@ isal_comp_pmd_qp_set_unique_name(struct
> rte_compressdev *dev,  struct isal_comp_qp *qp)  {
>   unsigned int n = snprintf(qp->name, sizeof(qp->name),
> - "isal_compression_pmd_%u_qp_%u",
> + "isal_comp_pmd_%u_qp_%u",
>   dev->data->dev_id, qp->id);
> 
>   if (n >= sizeof(qp->name))
> --
> 2.17.1
Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH] compress/isal: fix use after free

2019-05-21 Thread Daly, Lee
Hi Stephen, 
Thanks for the patch.

> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Tuesday, May 21, 2019 3:47 PM
> To: Daly, Lee 
> Cc: dev@dpdk.org; Stephen Hemminger 
> Subject: [PATCH] compress/isal: fix use after free
> 
> The release function was using qp->stream after already releasing it and the
> null pointer checking was missing.
> 
> Also since rte_free(NULL) is a no-op, remove unnecessary checks for NULL.
> 
> Coverity issure: 340860
> Fixes: dc49e6aa4879 ("compress/isal: add ISA-L compression functionality")
> Signed-off-by: Stephen Hemminger 
> ---
>  drivers/compress/isal/isal_compress_pmd_ops.c | 14 --
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/compress/isal/isal_compress_pmd_ops.c

 Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH v5 4/4] power: send confirmation cmd to vm guest

2019-09-26 Thread Daly, Lee
> On 05/04/2019 14:24, Hajkowski wrote:
> > From: Marcin Hajkowski 
> >
> > Use new guest channel API to send confirmation message for received
> > power command.
> >
> > Signed-off-by: Marcin Hajkowski 
> > ---
> >   examples/vm_power_manager/channel_monitor.c | 68
> +++--
> >   1 file changed, 62 insertions(+), 6 deletions(-)
> >
> > diff --git a/examples/vm_power_manager/channel_monitor.c
> > b/examples/vm_power_manager/channel_monitor.c
> > index 7892d75de..ed580b36a 100644
> > --- a/examples/vm_power_manager/channel_monitor.c
> > +++ b/examples/vm_power_manager/channel_monitor.c


<...>

> 
> Using the guest_cli sample app in a VM, sending commands to the
> vm_power_manager app on the host, we now have Acks and Nacks coming
> from the host to the VM to conform the execution of the guest requests.
> 
> vmpower(guest)> set_cpu_freq 3 down
> ACK received for message sent to host.
> vmpower(guest)> set_cpu_freq 3 up
> ACK received for message sent to host.
> vmpower(guest)> set_cpu_freq 3 up
> NACK received for message sent to host.
> 
> (NACK because we're already at the maxumum frequency)
> 
> And in the host vm_power_manager command line, we can see when a
> guest request cannot be processed:
> POWER: Turbo is off, frequency can't be scaled up more 31
> 
> Patchset looks good functionally.
> 
> Tested-by: David Hunt 
 Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH v5 3/4] power: process incoming confirmation cmds

2019-09-26 Thread Daly, Lee
> On 05/04/2019 14:24, Hajkowski wrote:
> > From: Marcin Hajkowski 
> >
> > Extend vm_power_guest to check incoming confirmations of messages
> > previously sent to host.
> >
> > Signed-off-by: Marcin Hajkowski 
> > ---
> >   examples/vm_power_manager/guest_cli/Makefile  |  1 +
> >   .../guest_cli/vm_power_cli_guest.c| 73 +++
> >   2 files changed, 61 insertions(+), 13 deletions(-)
> >
> > diff --git a/examples/vm_power_manager/guest_cli/Makefile
> > b/examples/vm_power_manager/guest_cli/Makefile
> > index e35a68d0f..67cf08193 100644
> > --- a/examples/vm_power_manager/guest_cli/Makefile
> > +++ b/examples/vm_power_manager/guest_cli/Makefile
> > @@ -18,6 +18,7 @@ SRCS-y := main.c vm_power_cli_guest.c parse.c


<...>

> I tested this with the later patches in the set, and the acknowledges
> successfully get back to the guest from the host in the form of an Ack or Nack
> for various commands sent from the guest.
> 
> Tested-by: David Hunt 

Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH v5 2/4] power: extend guest channel API for reading

2019-09-26 Thread Daly, Lee
> On 05/04/2019 14:24, Hajkowski wrote:
> > From: Marcin Hajkowski 
> >
> > Added new experimental API rte_power_guest_channel_receive_msg
> > which gives possibility to receive messages send to guest.
> >
> > Signed-off-by: Marcin Hajkowski 
> > ---
> >   lib/librte_power/channel_commands.h|  5 +++
> >   lib/librte_power/guest_channel.c   | 60 ++
> >   lib/librte_power/guest_channel.h   | 35 +++
> >   lib/librte_power/rte_power_version.map |  1 +
> >   4 files changed, 101 insertions(+)
> >
> > diff --git a/lib/librte_power/channel_commands.h
> > b/lib/librte_power/channel_commands.h


<...>

> > diff --git a/lib/librte_power/rte_power_version.map
> > b/lib/librte_power/rte_power_version.map
> > index 042917360..69f5ea3f4 100644
> > --- a/lib/librte_power/rte_power_version.map
> > +++ b/lib/librte_power/rte_power_version.map
> > @@ -44,4 +44,5 @@ EXPERIMENTAL {
> > rte_power_empty_poll_stat_update;
> > rte_power_poll_stat_fetch;
> > rte_power_poll_stat_update;
> > +   rte_power_guest_channel_receive_msg;
> >   };
Nit Pick, new api should be added in alphabetical order, i.e after 
"rte_power_empty_poll_stat_update" .

> 
> 
> I tested this with the later patches in the set, and the acknowledges
> successfully get back to the guest from the host in the form of an Ack or Nack
> for various commands sent from the guest.
> 
> Tested-by: David Hunt 
> 

Feel free to add my ack after the above minor change. 
Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH 4/4] power: add cmd to query CPU freq.

2019-09-27 Thread Daly, Lee



> -Original Message-
> From: Hajkowski, MarcinX
> Sent: Wednesday, April 3, 2019 6:16 PM
> To: Hunt, David 
> Cc: dev@dpdk.org; Hajkowski, MarcinX 
> Subject: [PATCH 4/4] power: add cmd to query CPU freq.
> 
> From: Marcin Hajkowski 
> 
> Add command and related logic to query CPU frequencies either for specified
> CPU or all cores.
> 
> Signed-off-by: Marcin Hajkowski 
> ---
>  .../guest_cli/vm_power_cli_guest.c| 150 --
>  1 file changed, 138 insertions(+), 12 deletions(-)
> 
> diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
<...>

> +
> + pkt.command = CPU_POWER_QUERY_FREQ_LIST;
> + strcpy(pkt.vm_name, policy.vm_name);

Can you use the internal rte_strlcpy() functions for security.

Add my tag after small change above has been applied to all instances of 
strcpy() in patchset.
For all in series:
Acked-by: Lee Daly 



Re: [dpdk-dev] [PATCH 3/3] examples/power_guest: send request for specified core capabilities

2019-09-30 Thread Daly, Lee
Hi dave, 
See comments below. 
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Hajkowski
> Sent: Thursday, May 30, 2019 5:15 PM
> To: Hunt, David 
> Cc: dev@dpdk.org; Hajkowski, MarcinX 
> Subject: [dpdk-dev] [PATCH 3/3] examples/power_guest: send request for
> specified core capabilities
> 
> From: Marcin Hajkowski 
> 
> Send request to power manager for core id provided by user to get related
> capabilities.
> 
> Signed-off-by: Marcin Hajkowski 
> ---
>  .../guest_cli/vm_power_cli_guest.c| 119 +-
>  1 file changed, 117 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> index 848230248..de85c1406 100644
> --- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> +++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
));
> +
> + if (!strcmp(res->cpu_num, "all")) {
> +
> + /* Get first enabled lcore. */
> + lcore_id = rte_get_next_lcore(-1,
> + 0,
> + 0);
> + if (lcore_id == RTE_MAX_LCORE) {
> + cmdline_printf(cl, "Enabled core not found.\n");
> + return;
> + }
> +
> + pkt.command = CPU_POWER_QUERY_CAPS_LIST;
> + strcpy(pkt.vm_name, policy.vm_name);
2 uses of strcpy in this patch, could this be changed to strlcpy().
/Lee.



Re: [dpdk-dev] [PATCH 1/3] power: add new packet type for capabilities

2019-09-30 Thread Daly, Lee



> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Hajkowski
> Sent: Thursday, May 30, 2019 5:15 PM
> To: Hunt, David 
> Cc: dev@dpdk.org; Hajkowski, MarcinX 
> Subject: [dpdk-dev] [PATCH 1/3] power: add new packet type for capabilities
> 
> From: Marcin Hajkowski 
> 
> Add new packet type and commands for capabilities query.
> 
> Signed-off-by: Marcin Hajkowski 
> ---
>  lib/librte_power/channel_commands.h | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/lib/librte_power/channel_commands.h
> b/lib/librte_power/channel_commands.h
> index ce587283c..b1f5584a8 100644
> --- a/lib/librte_power/channel_commands.h
> +++ b/lib/librte_power/channel_commands.h
> @@ -34,6 +34,8 @@ extern "C" {
>  /* CPU Power Queries */
>  #define CPU_POWER_QUERY_FREQ_LIST  7
>  #define CPU_POWER_QUERY_FREQ   8
> +#define CPU_POWER_QUERY_CAPS_LIST  9
> +#define CPU_POWER_QUERY_CAPS   10
> 
>  /* --- Outgoing messages --- */
> 
> @@ -43,6 +45,7 @@ extern "C" {
> 
>  /* CPU Power Query Responses */
>  #define CPU_POWER_FREQ_LIST 3
> +#define CPU_POWER_CAPS_LIST 4
> 
>  #define HOURS 24
> 
> @@ -106,6 +109,17 @@ struct channel_packet_freq_list {
>   uint8_t num_vcpu;
>  };
> 
> +struct channel_packet_caps_list {
> + uint64_t resource_id; /**< core_num, device */
> + uint32_t unit;/**< scale down/up/min/max */
> + uint32_t command; /**< Power, IO, etc */
> + char vm_name[VM_MAX_NAME_SZ];
> +
> + uint64_t turbo[MAX_VCPU_PER_VM];
> + uint64_t priority[MAX_VCPU_PER_VM];
> + uint8_t num_vcpu;
> +};
> +
> 
>  #ifdef __cplusplus
>  }
> --
> 2.17.2
Acked-by: Lee Daly 



Re: [dpdk-dev] [PATCH 2/3] examples/power_manager: send cpu capabilities on vm request

2019-09-30 Thread Daly, Lee



> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Hajkowski
> Sent: Thursday, May 30, 2019 5:15 PM
> To: Hunt, David 
> Cc: dev@dpdk.org; Hajkowski, MarcinX 
> Subject: [dpdk-dev] [PATCH 2/3] examples/power_manager: send cpu
> capabilities on vm request
> 
> From: Marcin Hajkowski 
> 
> Send capabilities for requested cores.
> 
> Signed-off-by: Marcin Hajkowski 
> ---
>  examples/vm_power_manager/channel_monitor.c | 67
> +
>  1 file changed, 67 insertions(+)
> 
> diff --git a/examples/vm_power_manager/channel_monitor.c
> b/examples/vm_power_manager/channel_monitor.c
> index bfd9cc38d..731b3b480 100644
> --- a/examples/vm_power_manager/channel_monitor.c
> +++ b/examples/vm_power_manager/channel_monitor.c

Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH 3/3] examples/power_guest: send request for specified core capabilities

2019-09-30 Thread Daly, Lee



> -Original Message-
> From: Daly, Lee
> Sent: Monday, September 30, 2019 11:55 AM
> To: 'Hajkowski' ; Hunt, David
> 
> Cc: dev@dpdk.org; Hajkowski, MarcinX 
> Subject: RE: [dpdk-dev] [PATCH 3/3] examples/power_guest: send request
> for specified core capabilities
> 
> Hi dave,
> See comments below.
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Hajkowski
> > Sent: Thursday, May 30, 2019 5:15 PM
> > To: Hunt, David 
> > Cc: dev@dpdk.org; Hajkowski, MarcinX 
> > Subject: [dpdk-dev] [PATCH 3/3] examples/power_guest: send request for
> > specified core capabilities
> >
> > From: Marcin Hajkowski 
> >
> > Send request to power manager for core id provided by user to get
> > related capabilities.
> >
> > Signed-off-by: Marcin Hajkowski 
> > ---
> >  .../guest_cli/vm_power_cli_guest.c| 119 +-
> >  1 file changed, 117 insertions(+), 2 deletions(-)
> >
> > diff --git
> a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> > b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> > index 848230248..de85c1406 100644
> > --- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> > +++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> ));
> > +
> > +   if (!strcmp(res->cpu_num, "all")) {
> > +
> > +   /* Get first enabled lcore. */
> > +   lcore_id = rte_get_next_lcore(-1,
> > +   0,
> > +   0);
> > +   if (lcore_id == RTE_MAX_LCORE) {
> > +   cmdline_printf(cl, "Enabled core not found.\n");
> > +   return;
> > +   }
> > +
> > +   pkt.command = CPU_POWER_QUERY_CAPS_LIST;
> > +   strcpy(pkt.vm_name, policy.vm_name);
> 2 uses of strcpy in this patch, could this be changed to strlcpy().
> /Lee.
Feel free to add my ack after that small change.
Acked-by: Lee Daly 



Re: [dpdk-dev] [PATCH] compress/isal: check allocation in qp setup

2020-11-19 Thread Daly, Lee
> From: wangyunjian 
> Sent: Monday, November 2, 2020 11:36 AM
> To: dev@dpdk.org
> Cc: Daly, Lee ; jerry.lili...@huawei.com;
> xudin...@huawei.com; Yunjian Wang ;
> sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH] compress/isal: check allocation in qp setup
> 
> From: Yunjian Wang 
> 
> The function rte_zmalloc() could return NULL, the return value need to be
> checked.
> 
> Fixes: dc49e6aa4879 ("compress/isal: add ISA-L compression functionality")
> Fixes: 7bf4f0630af6 ("compress/isal: add ISA-L decomp functionality")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Yunjian Wang 
> ---
>  drivers/compress/isal/isal_compress_pmd_ops.c | 20 -
> --
Thanks for patch.
Acked-by: Lee Daly 


Re: [dpdk-dev] [PATCH v5 13/20] doc: remove references to make from compressdevs guides

2020-10-02 Thread Daly, Lee
> -Original Message-
> From: Power, Ciara 
> Sent: Monday, September 21, 2020 2:59 PM
> To: dev@dpdk.org
> Cc: Power, Ciara ; Daly, Lee ;
> Mcnamara, John ; Kovacevic, Marko
> ; Ashish Gupta ;
> Sunila Sahu 
> Subject: [PATCH v5 13/20] doc: remove references to make from compressdevs
> guides
> 
> Make is no longer supported for compiling DPDK, references are now removed
> in the documentation.
> 
> Signed-off-by: Ciara Power 
> Reviewed-by: Kevin Laatz 
> ---
>  doc/guides/compressdevs/isal.rst |  4 
>  doc/guides/compressdevs/octeontx.rst | 24 ++--
>  doc/guides/compressdevs/zlib.rst |  4 
>  3 files changed, 2 insertions(+), 30 deletions(-)
> 
> diff --git a/doc/guides/compressdevs/isal.rst
> b/doc/guides/compressdevs/isal.rst
> index af1f41f240..1d146fb4a6 100644
> --- a/doc/guides/compressdevs/isal.rst
> +++ b/doc/guides/compressdevs/isal.rst
> @@ -133,10 +133,6 @@ Installation
>  Initialization
>  --
> 
> -In order to enable this virtual compression PMD, user must:
> -
> -* Set ``CONFIG_RTE_LIBRTE_PMD_ISAL=y`` in config/common_base.
> -
>  To use the PMD in an application, user must:
> 
>  * Call ``rte_vdev_init("compress_isal")`` within the application.
<>

ISA-L change looks good, thanks for work.
Acked-by: Lee Daly