>-----Original Message-----
>From: De Lara Guarch, Pablo [mailto:[email protected]]
>Sent: 11 July 2018 17:57
>To: Verma, Shally <[email protected]>
>Cc: [email protected]; Athreya, Narayana Prasad
><[email protected]>; Challa, Mahipal
><[email protected]>; Sahu, Sunila <[email protected]>; Sahu,
>Sunila <[email protected]>; Gupta, Ashish
><[email protected]>
>Subject: RE: [PATCH v2 3/5] compress/zlib: add xform and stream create support
>
>External Email
>
>> -----Original Message-----
>> From: Shally Verma [mailto:[email protected]]
>> Sent: Monday, July 2, 2018 5:57 PM
>> To: De Lara Guarch, Pablo <[email protected]>
>> Cc: [email protected]; [email protected];
>> [email protected]; Sunila Sahu <[email protected]>;
>> Sunila Sahu <[email protected]>; Ashish Gupta
>> <[email protected]>
>> 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 <[email protected]>
>>
>> Implement private xform and stream create ops
>>
>> Signed-off-by: Sunila Sahu <[email protected]>
>> Signed-off-by: Shally Verma <[email protected]>
>> Signed-off-by: Ashish Gupta <[email protected]>
>> ---
>> 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
>> };
