Re: [dpdk-dev] [PATCH v4 4/5] compress/zlib: support burst enqueue/dequeue

["Verma, Shally" <Shally.Verma@cavium.com>]

>-----Original Message-----
>From: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>Sent: 24 July 2018 13:24
>To: Verma, Shally <Shally.Verma@cavium.com>
>Cc: dev@dpdk.org; Athreya, Narayana Prasad <NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
><Mahipal.Challa@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Gupta, Ashish
><Ashish.Gupta@cavium.com>
>Subject: RE: [PATCH v4 4/5] compress/zlib: support burst enqueue/dequeue
>
>External Email
>
>> -----Original Message-----
>> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
>> Sent: Tuesday, July 24, 2018 8:45 AM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>> Cc: dev@dpdk.org; Athreya, Narayana Prasad
>> <NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
>> <Mahipal.Challa@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Sahu,
>> Sunila <Sunila.Sahu@cavium.com>; Gupta, Ashish <Ashish.Gupta@cavium.com>
>> Subject: RE: [PATCH v4 4/5] compress/zlib: support burst enqueue/dequeue
>>
>>
>>
>> >-----Original Message-----
>> >From: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>> >Sent: 24 July 2018 03:55
>> >To: Verma, Shally <Shally.Verma@cavium.com>
>> >Cc: dev@dpdk.org; Athreya, Narayana Prasad
>> ><NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
>> ><Mahipal.Challa@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>;
>> >Sahu, Sunila <Sunila.Sahu@cavium.com>; Gupta, Ashish
>> ><Ashish.Gupta@cavium.com>
>> >Subject: RE: [PATCH v4 4/5] compress/zlib: support burst
>> >enqueue/dequeue
>> >
>> >External Email
>> >
>> >> -----Original Message-----
>> >> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
>> >> Sent: Monday, July 23, 2018 3:51 PM
>> >> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>> >> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
>> >> mchalla@caviumnetworks.com; Sunila Sahu <ssahu@caviumnetworks.com>;
>> >> Sunila Sahu <sunila.sahu@caviumnetworks.com>; Ashish Gupta
>> >> <ashish.gupta@caviumnetworks.com>
>> >> Subject: [PATCH v4 4/5] compress/zlib: support burst enqueue/dequeue
>> >>
>> >> From: Sunila Sahu <ssahu@caviumnetworks.com>
>> >>
>> >> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
>> >> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
>> >> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
>> >> ---
>> >>  drivers/compress/zlib/zlib_pmd.c | 255
>> >> ++++++++++++++++++++++++++++++++++++++-
>> >>  1 file changed, 254 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/compress/zlib/zlib_pmd.c
>> >> b/drivers/compress/zlib/zlib_pmd.c
>> >> index 47bc73d..dc1e230 100644
>> >> --- a/drivers/compress/zlib/zlib_pmd.c
>> >> +++ b/drivers/compress/zlib/zlib_pmd.c
>> >> @@ -7,7 +7,214 @@
>> >>
>> >>  #include "zlib_pmd_private.h"
>> >>
>> >> -/** Parse comp xform and set private xform/stream parameters */
>> >> +/** Compute next mbuf in the list, assign data buffer and length,
>> >> + *  returns 0 if mbuf is NULL
>> >> + */
>> >> +#define COMPUTE_BUF(mbuf, data, len)         \
>> >> +             ((mbuf = mbuf->next) ?          \
>> >> +             (data = rte_pktmbuf_mtod(mbuf, uint8_t *)),     \
>> >> +             (len = rte_pktmbuf_data_len(mbuf)) : 0)
>> >> +
>> >> +static void
>> >> +process_zlib_deflate(struct rte_comp_op *op, z_stream *strm) {
>> >
>> >...
>> >
>> >> +     /* Update z_stream with the inputs provided by application */
>> >> +     strm->next_in = rte_pktmbuf_mtod_offset(mbuf_src, uint8_t *,
>> >> +                     op->src.offset);
>> >
>> >This is assuming that src buffer is a linear buffer or that offset won't be large
>> enough to cross boundaries between segments.
>> >If an SGL is passed and offset is bigger than the first segment,
>> >next_in should point at a different segment, with the remaining part of
>> >the offset in that segment (look at ISA-L SGL patch:
>> http://patches.dpdk.org/patch/43283/). Same applies to avail_in, next_out and
>> avail_out.
>>
>> [Shally] as per my last knowledge, offset was expected to be belonging only to
>> the first segment in chained mbuf. Isn't that the case anymore? Did I miss any
>> update on its definition?
>> We had added the logic earlier that you're suggesting but removed that later, as
>> I understood clarification about offset falling into any segment is still pending.
>>
>
>According to the comments:
>
>                uint32_t offset;
>                /**< Starting point for compression or decompression,
>                 * specified as number of bytes from start of packet in
>                 * source buffer.
>                 * This offset starts from the first segment
>                 * of the buffer, in case the m_src is a chain of mbufs.
>
>It says that the offset starts from the first segment, but not that
>it is only applicable for the first segment.
>From my point of view, an SGL should be seen like a contiguous (linear) buffer,
>so if the offset crosses multiple segments, it is still valid, as it is still part of the buffer.
>
Also believe doc need an update, it's kind of misleading "start from first segment", should replace by "in case of chained mbuf, it can fall into any segments".

Thanks
Shally
>Thanks,
>Pablo
>
>> Thanks
>> Shally