>-Original Message-
>From: De Lara Guarch, Pablo [mailto:pablo.de.lara.gua...@intel.com]
>Sent: 11 July 2018 17:57
>To: Verma, Shally
>Cc: dev@dpdk.org; Athreya, Narayana Prasad
>; Challa, Mahipal
>; Sahu, Sunila ; Sahu,
>Sunila ; Gupta, Ashish
>
>Subject: RE: [PATCH v2 3/5] compress/zlib: add xform and stream create support
>
>External Email
>
>> -Original Message-
>> From: Shally Verma [mailto:shally.ve...@caviumnetworks.com]
>> Sent: Monday, July 2, 2018 5:57 PM
>> To: De Lara Guarch, Pablo
>> Cc: dev@dpdk.org; pathr...@caviumnetworks.com;
>> mcha...@caviumnetworks.com; Sunila Sahu ;
>> Sunila Sahu ; Ashish Gupta
>>
>> Subject: [PATCH v2 3/5] compress/zlib: add xform and stream create support
>
>Retitle to "compress/zlib: support xform and stream creation"
>
>About stateful, since it looks like it is not supported for the moment,
>I think stream_create/stream_free functions should fail or should not be
>implemented.
>
>More comments inline.
>
>>
>> From: Sunila Sahu
>>
>> Implement private xform and stream create ops
>>
>> Signed-off-by: Sunila Sahu
>> Signed-off-by: Shally Verma
>> Signed-off-by: Ashish Gupta
>> ---
>> drivers/compress/zlib/zlib_pmd.c | 93
>>
>> drivers/compress/zlib/zlib_pmd_ops.c | 83 ++--
>> drivers/compress/zlib/zlib_pmd_private.h | 4 ++
>> 3 files changed, 176 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/compress/zlib/zlib_pmd.c
>> b/drivers/compress/zlib/zlib_pmd.c
>> index c4f67bb..7c2614e 100644
>> --- a/drivers/compress/zlib/zlib_pmd.c
>> +++ b/drivers/compress/zlib/zlib_pmd.c
>
>...
>
>> + case RTE_COMP_HUFFMAN_DYNAMIC:
>> + strategy = Z_HUFFMAN_ONLY;
>
>I don't think this is the suitable value for dynamic compression.
>Z_HUFFMAN_ONLY does not do LZ77. I think the strategy for this case is
>Z_DEFAULT_STRATEGY.
Ya. you right. It will be changed to DEFAULT_STRATEGY.
Thanks
Shally
>
>> + break;
>> + default:
>> + ZLIB_PMD_ERR("Compression strategy not
>> supported\n");
>> + return -1;
>> + }
>
>...
>
>> +++ b/drivers/compress/zlib/zlib_pmd_ops.c
>
>...
>
>> + struct rte_mempool *mp = rte_mempool_lookup(internals->mp_name);
>
>As said in other email, you can directly get the mempool pointer from
>internals.
>
>> + if (rte_mempool_get(mp, zstream)) {
>> + ZLIB_PMD_ERR(
>> + "Couldn't get object from session mempool");
>
>This can be in the same line.
>
>...
>
>> +/** Configure private xform */
>> +static int
>> +zlib_pmd_private_xform_create(struct rte_compressdev *dev,
>> + const struct rte_comp_xform *xform,
>> + void **private_xform)
>> +{
>> + int ret = 0;
>> +
>> + ret = zlib_pmd_stream_create(dev, xform, private_xform);
>> + return ret;
>
>This can be written like "return zlib_pmd_stream_create(...);
>
>...
>
>> + .private_xform_create = zlib_pmd_private_xform_create,
>> + .private_xform_free =
>> zlib_pmd_private_xform_free,
>
>Fix indentation here.
>
>>
>> - .stream_create = NULL,
>> - .stream_free= NULL
>> + .stream_create = zlib_pmd_stream_create,
>> + .stream_free= zlib_pmd_stream_free
>> };
