crypto: recognize OP_TYPE_UNDEFINED in rte_crypto_op_pool_create

Message ID 201810022008.w92K8B2r031185@lectura.cs.arizona.edu (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series crypto: recognize OP_TYPE_UNDEFINED in rte_crypto_op_pool_create |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK

Commit Message

Junxiao Shi Oct. 2, 2018, 7:05 p.m. UTC
  Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
---
 lib/librte_cryptodev/rte_cryptodev.c | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Akhil Goyal Oct. 9, 2018, 10:50 a.m. UTC | #1
Hi  Junxiao,

On 10/3/2018 12:35 AM, Junxiao Shi wrote:
> Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
> ---
>   lib/librte_cryptodev/rte_cryptodev.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
> index 63ae23f..3d6f474 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -1477,6 +1477,8 @@ rte_crypto_op_pool_create(const char *name, enum rte_crypto_op_type type,
>   		elt_size += sizeof(struct rte_crypto_sym_op);
>   	} else if (type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC) {
>   		elt_size += sizeof(struct rte_crypto_asym_op);
> +	} else if (type == RTE_CRYPTO_OP_TYPE_UNDEFINED) {
> +		elt_size += RTE_MAX(sizeof(struct rte_crypto_sym_op), sizeof(struct rte_crypto_asym_op));
>   	} else {
>   		CDEV_LOG_ERR("Invalid op_type\n");
>   		return NULL;
Could you please explain the need for this change. If there is some type 
which is missing, we can add that.
defining the size of undefined op type does not make sense.

-Akhil
  
yoursunny Oct. 9, 2018, 10:58 a.m. UTC | #2
Hi Akhil

The documentation of rte_crypto_op_pool_create indicates that _UNDEFINED
creates a pool that supports both symmetric and assymetric crypto. This
change makes the code consistent with documentation.

Yours, Junxiao

On Tue, Oct 9, 2018 at 06:51 Akhil Goyal <akhil.goyal@nxp.com> wrote:

> Hi  Junxiao,
>
> On 10/3/2018 12:35 AM, Junxiao Shi wrote:
> > Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
> > ---
> >   lib/librte_cryptodev/rte_cryptodev.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/librte_cryptodev/rte_cryptodev.c
> b/lib/librte_cryptodev/rte_cryptodev.c
> > index 63ae23f..3d6f474 100644
> > --- a/lib/librte_cryptodev/rte_cryptodev.c
> > +++ b/lib/librte_cryptodev/rte_cryptodev.c
> > @@ -1477,6 +1477,8 @@ rte_crypto_op_pool_create(const char *name, enum
> rte_crypto_op_type type,
> >               elt_size += sizeof(struct rte_crypto_sym_op);
> >       } else if (type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC) {
> >               elt_size += sizeof(struct rte_crypto_asym_op);
> > +     } else if (type == RTE_CRYPTO_OP_TYPE_UNDEFINED) {
> > +             elt_size += RTE_MAX(sizeof(struct rte_crypto_sym_op),
> sizeof(struct rte_crypto_asym_op));
> >       } else {
> >               CDEV_LOG_ERR("Invalid op_type\n");
> >               return NULL;
> Could you please explain the need for this change. If there is some type
> which is missing, we can add that.
> defining the size of undefined op type does not make sense.
>
> -Akhil
>
  
Akhil Goyal Oct. 9, 2018, 11:11 a.m. UTC | #3
On 10/9/2018 4:28 PM, yoursunny wrote:
> Hi Akhil
>
> The documentation of rte_crypto_op_pool_create indicates that _UNDEFINED
> creates a pool that supports both symmetric and assymetric crypto. This
> change makes the code consistent with documentation.
>
> Yours, Junxiao
Please add this information in the patch description and this should be 
marked as a fix.

Please also add
Fixes: c0f87eb5252b ("cryptodev: change burst API to be crypto op oriented")

and cc stable@dpdk.org
> On Tue, Oct 9, 2018 at 06:51 Akhil Goyal <akhil.goyal@nxp.com> wrote:
>
>> Hi  Junxiao,
>>
>> On 10/3/2018 12:35 AM, Junxiao Shi wrote:
>>> Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
>>> ---
>>>    lib/librte_cryptodev/rte_cryptodev.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/lib/librte_cryptodev/rte_cryptodev.c
>> b/lib/librte_cryptodev/rte_cryptodev.c
>>> index 63ae23f..3d6f474 100644
>>> --- a/lib/librte_cryptodev/rte_cryptodev.c
>>> +++ b/lib/librte_cryptodev/rte_cryptodev.c
>>> @@ -1477,6 +1477,8 @@ rte_crypto_op_pool_create(const char *name, enum
>> rte_crypto_op_type type,
>>>                elt_size += sizeof(struct rte_crypto_sym_op);
>>>        } else if (type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC) {
>>>                elt_size += sizeof(struct rte_crypto_asym_op);
>>> +     } else if (type == RTE_CRYPTO_OP_TYPE_UNDEFINED) {
>>> +             elt_size += RTE_MAX(sizeof(struct rte_crypto_sym_op),
>> sizeof(struct rte_crypto_asym_op));
>>>        } else {
>>>                CDEV_LOG_ERR("Invalid op_type\n");
>>>                return NULL;
>> Could you please explain the need for this change. If there is some type
>> which is missing, we can add that.
>> defining the size of undefined op type does not make sense.
>>
>> -Akhil
>>
  
Akhil Goyal Oct. 9, 2018, 12:51 p.m. UTC | #4
Also change title to
crypto: fix element size for undefined crypto op

On 10/9/2018 4:41 PM, Akhil Goyal wrote:
>
>
> On 10/9/2018 4:28 PM, yoursunny wrote:
>> Hi Akhil
>>
>> The documentation of rte_crypto_op_pool_create indicates that _UNDEFINED
>> creates a pool that supports both symmetric and assymetric crypto. This
>> change makes the code consistent with documentation.
>>
>> Yours, Junxiao
> Please add this information in the patch description and this should 
> be marked as a fix.
>
> Please also add
> Fixes: c0f87eb5252b ("cryptodev: change burst API to be crypto op 
> oriented")
>
> and cc stable@dpdk.org
>> On Tue, Oct 9, 2018 at 06:51 Akhil Goyal <akhil.goyal@nxp.com> wrote:
>>
>>> Hi  Junxiao,
>>>
>>> On 10/3/2018 12:35 AM, Junxiao Shi wrote:
>>>> Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
>>>> ---
>>>>    lib/librte_cryptodev/rte_cryptodev.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/lib/librte_cryptodev/rte_cryptodev.c
>>> b/lib/librte_cryptodev/rte_cryptodev.c
>>>> index 63ae23f..3d6f474 100644
>>>> --- a/lib/librte_cryptodev/rte_cryptodev.c
>>>> +++ b/lib/librte_cryptodev/rte_cryptodev.c
>>>> @@ -1477,6 +1477,8 @@ rte_crypto_op_pool_create(const char *name, enum
>>> rte_crypto_op_type type,
>>>>                elt_size += sizeof(struct rte_crypto_sym_op);
>>>>        } else if (type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC) {
>>>>                elt_size += sizeof(struct rte_crypto_asym_op);
>>>> +     } else if (type == RTE_CRYPTO_OP_TYPE_UNDEFINED) {
>>>> +             elt_size += RTE_MAX(sizeof(struct rte_crypto_sym_op),
>>> sizeof(struct rte_crypto_asym_op));
one more nit..
The line is beyond 80 characters.
Before sending the patch you should run checkpatch script.
>>>>        } else {
>>>>                CDEV_LOG_ERR("Invalid op_type\n");
>>>>                return NULL;
>>> Could you please explain the need for this change. If there is some 
>>> type
>>> which is missing, we can add that.
>>> defining the size of undefined op type does not make sense.
>>>
>>> -Akhil
>>>
>
  

Patch

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 63ae23f..3d6f474 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -1477,6 +1477,8 @@  rte_crypto_op_pool_create(const char *name, enum rte_crypto_op_type type,
 		elt_size += sizeof(struct rte_crypto_sym_op);
 	} else if (type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC) {
 		elt_size += sizeof(struct rte_crypto_asym_op);
+	} else if (type == RTE_CRYPTO_OP_TYPE_UNDEFINED) {
+		elt_size += RTE_MAX(sizeof(struct rte_crypto_sym_op), sizeof(struct rte_crypto_asym_op));
 	} else {
 		CDEV_LOG_ERR("Invalid op_type\n");
 		return NULL;