From: "Verma, Shally" <Shally.Verma@cavium.com>
To: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
Cc: "dev@dpdk.org" <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: [dpdk-dev] [PATCH v4 4/5] compress/zlib: support burst enqueue/dequeue
Date: Tue, 24 Jul 2018 07:44:59 +0000 [thread overview]
Message-ID: <CY4PR0701MB3634967469199E30E43E3CCCF0550@CY4PR0701MB3634.namprd07.prod.outlook.com> (raw)
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D8977F8FFCB5@IRSMSX107.ger.corp.intel.com>
>-----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.
Thanks
Shally
>
>> +
>> + strm->avail_in = rte_pktmbuf_data_len(mbuf_src) - op->src.offset;
>> +
>> + strm->next_out = rte_pktmbuf_mtod_offset(mbuf_dst, uint8_t *,
>> + op->dst.offset);
>> +
>> + strm->avail_out = rte_pktmbuf_data_len(mbuf_dst) - op->dst.offset;
>> +
>
>...
>
>> + strm->next_in = rte_pktmbuf_mtod_offset(mbuf_src, uint8_t *,
>> + op->src.offset);
>> +
>> + strm->avail_in = rte_pktmbuf_data_len(mbuf_src) - op->src.offset;
>> +
>> + strm->next_out = rte_pktmbuf_mtod_offset(mbuf_dst, uint8_t *,
>> + op->dst.offset);
>> +
>> + strm->avail_out = rte_pktmbuf_data_len(mbuf_dst) - op->dst.offset;
>
>Same comments as above (compression).
>
>...
>
>> +static inline int
>> +process_zlib_op(struct zlib_qp *qp, struct rte_comp_op *op) {
>> + struct zlib_stream *stream;
>> + struct zlib_priv_xform *private_xform;
>> +
>> + if ((op->op_type == RTE_COMP_OP_STATEFUL) ||
>> + (op->src.offset > rte_pktmbuf_data_len(op->m_src)) ||
>> + (op->dst.offset > rte_pktmbuf_data_len(op->m_dst))) {
>
>Since m_src and m_dst could be SGLs, pkt_len should be checked, instead of data_len (which would be only for single segment).
>Also, you should check the length too, in case of source buffers (src.offset + src.length > m_src->pkt_len).
>
>Lastly, the two lines after the first if line should have double indentation to distinguish clearly against the body of the if.
>If line is too long, consider storing the length of the buffers in variables.
>
>
>> + op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
>> + ZLIB_PMD_ERR("Invalid source or destination buffers or "
>> + "invalid Operation requested\n");
next prev parent reply other threads:[~2018-07-24 7:45 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-23 14:51 [dpdk-dev] [PATCH v4 0/5] compress: add ZLIB compression PMD Shally Verma
2018-07-23 14:51 ` [dpdk-dev] [PATCH v4 1/5] compress/zlib: add ZLIB PMD Shally Verma
2018-07-23 21:58 ` De Lara Guarch, Pablo
2018-07-24 7:20 ` Verma, Shally
2018-07-23 14:51 ` [dpdk-dev] [PATCH v4 2/5] compress/zlib: add device PMD ops Shally Verma
2018-07-23 22:02 ` De Lara Guarch, Pablo
2018-07-24 7:27 ` Verma, Shally
2018-07-23 14:51 ` [dpdk-dev] [PATCH v4 3/5] compress/zlib: create private xform Shally Verma
2018-07-23 14:51 ` [dpdk-dev] [PATCH v4 4/5] compress/zlib: support burst enqueue/dequeue Shally Verma
2018-07-23 22:25 ` De Lara Guarch, Pablo
2018-07-24 7:44 ` Verma, Shally [this message]
2018-07-24 7:53 ` De Lara Guarch, Pablo
2018-07-24 8:14 ` Verma, Shally
2018-07-24 8:19 ` Verma, Shally
2018-07-23 14:51 ` [dpdk-dev] [PATCH v4 5/5] doc: add ZLIB PMD guide Shally Verma
2018-07-23 17:58 ` De Lara Guarch, Pablo
2018-07-23 18:00 ` Verma, Shally
2018-07-23 21:18 ` De Lara Guarch, Pablo
2018-07-24 5:32 ` Verma, Shally
2018-07-24 7:47 ` De Lara Guarch, Pablo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CY4PR0701MB3634967469199E30E43E3CCCF0550@CY4PR0701MB3634.namprd07.prod.outlook.com \
--to=shally.verma@cavium.com \
--cc=Ashish.Gupta@cavium.com \
--cc=Mahipal.Challa@cavium.com \
--cc=NarayanaPrasad.Athreya@cavium.com \
--cc=Sunila.Sahu@cavium.com \
--cc=dev@dpdk.org \
--cc=pablo.de.lara.guarch@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).