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

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma at cavium.com]
> Sent: Tuesday, July 24, 2018 8:45 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>
> Cc: dev at dpdk.org; Athreya, Narayana Prasad
> <NarayanaPrasad.Athreya at cavium.com>; Challa, Mahipal
> <Mahipal.Challa at cavium.com>; Sahu, Sunila <Sunila.Sahu at cavium.com>; Sahu,
> Sunila <Sunila.Sahu at cavium.com>; Gupta, Ashish <Ashish.Gupta at 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 at intel.com>
> >Sent: 24 July 2018 03:55
> >To: Verma, Shally <Shally.Verma at cavium.com>
> >Cc: dev at dpdk.org; Athreya, Narayana Prasad
> ><NarayanaPrasad.Athreya at cavium.com>; Challa, Mahipal
> ><Mahipal.Challa at cavium.com>; Sahu, Sunila <Sunila.Sahu at cavium.com>;
> >Sahu, Sunila <Sunila.Sahu at cavium.com>; Gupta, Ashish
> ><Ashish.Gupta at cavium.com>
> >Subject: RE: [PATCH v4 4/5] compress/zlib: support burst
> >enqueue/dequeue
> >
> >External Email
> >
> >> -----Original Message-----
> >> From: Shally Verma [mailto:shally.verma at caviumnetworks.com]
> >> Sent: Monday, July 23, 2018 3:51 PM
> >> To: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>
> >> Cc: dev at dpdk.org; pathreya at caviumnetworks.com;
> >> mchalla at caviumnetworks.com; Sunila Sahu <ssahu at caviumnetworks.com>;
> >> Sunila Sahu <sunila.sahu at caviumnetworks.com>; Ashish Gupta
> >> <ashish.gupta at caviumnetworks.com>
> >> Subject: [PATCH v4 4/5] compress/zlib: support burst enqueue/dequeue
> >>
> >> From: Sunila Sahu <ssahu at caviumnetworks.com>
> >>
> >> Signed-off-by: Sunila Sahu <sunila.sahu at caviumnetworks.com>
> >> Signed-off-by: Shally Verma <shally.verma at caviumnetworks.com>
> >> Signed-off-by: Ashish Gupta <ashish.gupta at 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.

Thanks,
Pablo

> Thanks
> Shally