From: Akhil Goyal <email@example.com> To: Akhil Goyal <firstname.lastname@example.org>, "De Lara Guarch, Pablo" <email@example.com>, "Coyle, David" <firstname.lastname@example.org>, "Doherty, Declan" <email@example.com>, "Trahe, Fiona" <firstname.lastname@example.org> Cc: "email@example.com" <firstname.lastname@example.org>, "Ryan, Brendan" <email@example.com>, "O'loingsigh, Mairtin" <firstname.lastname@example.org> Subject: Re: [dpdk-dev] [PATCH v1] app/crypto-perf: set mbuf lengths correctly for DOCSIS tests Date: Wed, 29 Jul 2020 16:18:58 +0000 Message-ID: <VI1PR04MB3168E8ADAC260E2C4AF50604E6700@VI1PR04MB3168.eurprd04.prod.outlook.com> (raw) In-Reply-To: <AM5PR04MB3153965F5D793A61975759E6E6730@AM5PR04MB3153.eurprd04.prod.outlook.com> > Hi Pablo/David, > > > > Hi David, > > > > > Hi Pablo, > > > > > > > -----Original Message----- > > > > From: De Lara Guarch, Pablo <email@example.com> > > > > Sent: Friday, July 17, 2020 8:04 PM > > > > > @@ -48,6 +48,10 @@ cperf_set_ops_security(struct rte_crypto_op > **ops, > > > > > } else > > > > > buf_sz = options->test_buffer_size; > > > > > > > > > > + sym_op->m_src->buf_len = options- > >segment_sz; > > > > > + sym_op->m_src->data_len = buf_sz; > > > > > + sym_op->m_src->pkt_len = buf_sz; > > > > > + > > > > > > > > Actually, I am wondering why this is needed at all (for DOCSIS and > > > > PDCP). This is already set in " fill_multi_seg_mbuf" or " > > > > fill_single_seg_mbuf" (and this was already working without this patch, > > right?). > > > > > > [DC] I have found that if a number of buffer sizes are specified like this on > the > > > cmd line "--buffer-sz 64,256,1024", then the pkt_len and data_len filled in > > > "fill_multi_seg_mbuf" or " fill_single_seg_mbuf" is always the largest of the > > sizes > > > specified. The cipher/auth lengths are then set based on the --buffer-sz > option. > > > > > > For DOCSIS, I tried to be more accurate and set the correct pkt_len and > > data_len > > > in the mbuf. This followed what PDCP did too, even though I'm not sure of > the > > > background why PDCP did it - possibly spotted the same issue. I have also > > found > > > that DOCSIS performance figures can be better if the correct pkt_len and > > > data_len are set in the mbuf - I don't have any proper explanation for this > > > though as the cipher/ auth lengths are always the same. > > > > > > I've dug around a bit more on this now though and this is actually a problem > > > across the perf tool. Some of the crypto PMDs have logic based on the mbuf > > > pkt_len and data_len, but because the perf tool isn't always setting these > fields > > > correctly, that logic may not work as expected. > > > > > > > > Right, thanks for checking this. If I remember correctly, it was fine to have this > > set to the maximum size as the important field for crypto PMDs to check is the > > cipher/auth lengths, as you said. If there is more logic that depends on > data_len > > on other PMDs, I agree it might be a problem. The only usage I knew for it was > > the multi segment case (in AES-GCM PMD), where data_len is checked in each > > segment size to see if all the cipher/auth length resides within these segments, > > but in the tool we set data_len for each segment when "going multi-segment". > I > > see that other PMDs like DPAA2_SEC use these fields for something which I am > > not sure what's for. It would be good if the maintainers check if this is a > problem > > for them, and in that case, this should be fixed for the other functions (for > > "normal" crypto). > > > > In case of test-crypto-perf, the buffers are flat and there is no case of multi > segment. > So this is not because of that. > In case of PDCP and probably all the protocol offload cases would need the > buf_len/ > data_len/pkt_len to be set properly. As the complete buffer is given to hardware > and depending on the headers added, HW/PMD will adjust these lengths when > the > packet is dequeued, provided it has room available to expand. > > We may not need this in cases of pure crypto which have fixed lengths and PMD > does > not control them. > > So in my opinion this patch is fine. > > Acked-by: Akhil Goyal <firstname.lastname@example.org> > > This patch was applied yesterday to dpdk-next-crypto And is now pulled to master as well.
prev parent reply other threads:[~2020-07-29 16:19 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-16 15:31 David Coyle 2020-07-17 19:03 ` De Lara Guarch, Pablo 2020-07-20 12:59 ` Coyle, David 2020-07-22 8:52 ` De Lara Guarch, Pablo 2020-07-28 19:44 ` Akhil Goyal 2020-07-29 16:18 ` Akhil Goyal [this message]
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=VI1PR04MB3168E8ADAC260E2C4AF50604E6700@VI1PR04MB3168.eurprd04.prod.outlook.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /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
DPDK patches and discussions This inbox may be cloned and mirrored by anyone: git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \ email@example.com public-inbox-index dev Example config snippet for mirrors. Newsgroup available over NNTP: nntp://inbox.dpdk.org/inbox.dpdk.dev AGPL code for this site: git clone https://public-inbox.org/public-inbox.git