Hi Ayuj,

I believe there are couple of issues with this patch.

Are these experimental APIs? I believe they were made stable this release and 
I'm not sure if it is a right practice to edit an API without deprecation 
notice after it is made stable. Especially now that RC2 is done. @Akhil, what 
is your take on this?

I think, the approach here is wrong. If the lifetime of the session is expected 
to be only few packets, then session-less (which I believe is in the pipeline) 
would make more sense. If the lifetime of the session is expected to be more 
than that, then having this feature/limitation would make application more 
complicated. Also, since one asymmetric session can hold both public & private 
keys, the implicit assumption would be, the session can be used for multiple 
kinds of operations. This change is in contradiction with that.

But my major concern is how this can lead to accidental errors. Making the 
argument as const will mean the API won't edit its contents. But if there is a 
pointer in that (key happens to be a pointer inside the xform), having const 
for xform will not help. This is my understanding. Please correct me if I'm 
wrong. Also, I could have the xform allocated from stack (non const, regular 
local variable) and then call the session_init. Would compiler throw an issue 
in that case? I doubt so.

void abc(const int t)
{
        printf("%d\n", t);
}

void main()
{
        int t = 0;
        abc(t);
        t = 2;
        abc(t);
}

To summarize, if this assumption is accepted, then compiler will not be able to 
ensure it. And to properly use it, application will have to be drafted 
differently. And when similar effect can be achieved by having session-less, 
this seems redundant.

So this change is NACK from my side.

Thanks,
Anoob

From: Ayuj Verma
Sent: Wednesday, July 24, 2019 2:23 PM
To: akhil.go...@nxp.com
Cc: arkadiuszx.kusz...@intel.com; Shally Verma <shal...@marvell.com>; Sunila 
Sahu <ss...@marvell.com>; Kanaka Durga Kotamarthy <kkotamar...@marvell.com>; 
Anoob Joseph <ano...@marvell.com>; dev@dpdk.org; Fiona Trahe 
<fiona.tr...@intel.com>
Subject: Re: [PATCH v1 0/2] declare crypto asym xform immutable


+Fiona.

________________________________
From: Ayuj Verma <ayve...@marvell.com<mailto:ayve...@marvell.com>>
Sent: 24 July 2019 14:21:55
To: akhil.go...@nxp.com<mailto:akhil.go...@nxp.com> 
<akhil.go...@nxp.com<mailto:akhil.go...@nxp.com>>
Cc: arkadiuszx.kusz...@intel.com<mailto:arkadiuszx.kusz...@intel.com> 
<arkadiuszx.kusz...@intel.com<mailto:arkadiuszx.kusz...@intel.com>>; Shally 
Verma <shal...@marvell.com<mailto:shal...@marvell.com>>; Sunila Sahu 
<ss...@marvell.com<mailto:ss...@marvell.com>>; Kanaka Durga Kotamarthy 
<kkotamar...@marvell.com<mailto:kkotamar...@marvell.com>>; Anoob Joseph 
<ano...@marvell.com<mailto:ano...@marvell.com>>; 
dev@dpdk.org<mailto:dev@dpdk.org> <dev@dpdk.org<mailto:dev@dpdk.org>>; Ayuj 
Verma <ayve...@marvell.com<mailto:ayve...@marvell.com>>
Subject: [PATCH v1 0/2] declare crypto asym xform immutable

Mark asym xform as immutable till lifetime
of session. It will save session setup time for
PMDs, which doesn't require any manipulation of
xform data, by directly using these buffers.

* Updated xform type in session init/configure
  API as constant.
* Updated doc with proper transform description.
* Updated openssl PMD with above changes.

Ayuj Verma (2):
  lib/crypto: declare crypto asym xform immutable
  crypto/openssl: mark asym xform constant

 doc/guides/prog_guide/cryptodev_lib.rst      | 10 ++++++++++
 drivers/crypto/openssl/rte_openssl_pmd_ops.c |  8 ++++----
 lib/librte_cryptodev/rte_cryptodev.c         |  2 +-
 lib/librte_cryptodev/rte_cryptodev.h         |  2 +-
 lib/librte_cryptodev/rte_cryptodev_pmd.h     |  2 +-
 5 files changed, 17 insertions(+), 7 deletions(-)

--
1.8.3.1

Reply via email to