From: "Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>
To: Akhil Goyal <akhil.goyal@nxp.com>,
"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>,
"Doherty, Declan" <declan.doherty@intel.com>,
"Jacob, Jerin" <Jerin.JacobKollanukkaran@cavium.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Vangati, Narender" <narender.vangati@intel.com>,
"Rao, Nikhil" <nikhil.rao@intel.com>
Subject: Re: [dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data
Date: Thu, 18 Jan 2018 06:52:21 +0000 [thread overview]
Message-ID: <5612CB344B05EE4F95FC5B729939F780706011A0@PGSMSX102.gar.corp.intel.com> (raw)
In-Reply-To: <41967568-8501-b9da-95bc-6072cfa8a210@nxp.com>
Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Wednesday, January 17, 2018 4:23 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Jacob, Jerin
> <Jerin.JacobKollanukkaran@cavium.com>
> Cc: dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>; Rao,
> Nikhil <nikhil.rao@intel.com>
> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session private data
>
> Hi Abhinandan,
> On 1/17/2018 3:35 PM, Gujjar, Abhinandan S wrote:
> > Hi Akhil,
> >
> >> -----Original Message-----
> >> From: De Lara Guarch, Pablo
> >> Sent: Wednesday, January 17, 2018 3:16 PM
> >> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Akhil Goyal
> >> <akhil.goyal@nxp.com>; Doherty, Declan <declan.doherty@intel.com>;
> >> Jacob, Jerin <Jerin.JacobKollanukkaran@cavium.com>
> >> Cc: dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>;
> >> Rao, Nikhil <nikhil.rao@intel.com>
> >> Subject: RE: [PATCH 1/2] lib/cryptodev: add support to set session
> >> private data
> >>
> >> Hi Abhinandan,
> >>
> >>> -----Original Message-----
> >>> From: Gujjar, Abhinandan S
> >>> Sent: Wednesday, January 17, 2018 6:35 AM
> >>> To: Akhil Goyal <akhil.goyal@nxp.com>; Doherty, Declan
> >>> <declan.doherty@intel.com>; De Lara Guarch, Pablo
> >>> <pablo.de.lara.guarch@intel.com>; Jacob, Jerin
> >>> <Jerin.JacobKollanukkaran@cavium.com>
> >>> Cc: dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>;
> >>> Rao, Nikhil <nikhil.rao@intel.com>
> >>> Subject: RE: [PATCH 1/2] lib/cryptodev: add support to set session
> >>> private data
> >>>
> >>> Hi Akhil,
> >>>
> >>
> >> ...
> >>
> >>> I guess, you are suggesting below changes:
> >>> diff --git a/lib/librte_cryptodev/rte_cryptodev.h
> >>> b/lib/librte_cryptodev/rte_cryptodev.h
> >>> index 56958a6..057c39a 100644
> >>> --- a/lib/librte_cryptodev/rte_cryptodev.h
> >>> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> >>> @@ -892,6 +892,8 @@ struct rte_cryptodev_data {
> >>>
> >>> /** Cryptodev symmetric crypto session */ struct
> >>> rte_cryptodev_sym_session {
> >>> + uint16_t private_data_offset;
> >>> + /**< Private data offset */
> >>> __extension__ void *sess_private_data[0];
> >>> /**< Private session material */ }; I am ok with this.
> >>>
> >>> Declan/Pablo,
> >>> Is this ok? Do you see any impact on performance or anything else
> >>> has to be considered?
> >>
> >> This is breaking ABI, and since there is a zero length array, this
> >> latter has to be at the end of the structure.
> >> Therefore, this is not a valid option unless ABI deprecation is
> >> announced and then it could be merged in the next release.
> > What is your opinion on this?
> > Should we consider retaining the enum rte_crypto_op_private_data_type?
>
> As per our previous discussion we are anyway pushing crypto adapter to next
> release, then we do have time for the deprecation notice to be sent.
Not sure, it is really worth breaking ABI or have an enum.
> Or you can reserve the first byte of private data (internal to library) in the session
> to check whether the private data is valid or not.
Regarding reserving the first byte which validates the rest of the metadata data,
unless this byte is also included part of rte_cryptodev_sym_session_create()
i.e.
memset(sess, 0, (sizeof(void *) * nb_drivers + private_data_flag));
and in
rte_cryptodev_get_header_session_size(void)
{
/*
* Header contains pointers to the private data
* of all registered drivers
*/
return (sizeof(void *) * nb_drivers + private_data_flag);
}
Without above changes, the flag content can't be just trusted. Do you agree?
Pablo/Declan,
Hope the changes are ok? ABI breakage or anything has to be considered again?
>
> IMO, private data offset in session is a better approach instead of adding one
> more enum. Others can suggest.
@Others, please provide your inputs so that I can prepare the next patch.
-Abhinandan
>
> -Akhil
> >>
> >> Pablo
> > Abhinandan
> >
next prev parent reply other threads:[~2018-01-18 6:52 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-15 11:51 Abhinandan Gujjar
2018-01-15 12:18 ` Akhil Goyal
2018-01-16 6:09 ` Gujjar, Abhinandan S
2018-01-16 6:24 ` Akhil Goyal
2018-01-16 7:05 ` Gujjar, Abhinandan S
2018-01-16 7:26 ` Akhil Goyal
2018-01-16 9:03 ` Gujjar, Abhinandan S
2018-01-16 9:21 ` Akhil Goyal
2018-01-16 11:36 ` Gujjar, Abhinandan S
2018-01-16 12:00 ` Akhil Goyal
2018-01-16 12:29 ` Gujjar, Abhinandan S
2018-01-16 13:02 ` Akhil Goyal
2018-01-17 6:35 ` Gujjar, Abhinandan S
2018-01-17 9:46 ` De Lara Guarch, Pablo
2018-01-17 10:05 ` Gujjar, Abhinandan S
2018-01-17 10:52 ` Akhil Goyal
2018-01-18 6:52 ` Gujjar, Abhinandan S [this message]
2018-01-22 6:51 ` Gujjar, Abhinandan S
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=5612CB344B05EE4F95FC5B729939F780706011A0@PGSMSX102.gar.corp.intel.com \
--to=abhinandan.gujjar@intel.com \
--cc=Jerin.JacobKollanukkaran@cavium.com \
--cc=akhil.goyal@nxp.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=narender.vangati@intel.com \
--cc=nikhil.rao@intel.com \
--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).