Re: [dpdk-dev] [PATCH 2/2] lib/security: add SA lifetime configuration

[Akhil Goyal <gakhil@marvell.com>]

Hi Konstantin,
> > There are two options that we considered,
> > 1. Extend the enum, rte_crypto_op_status,  to cover warnings [1]
> > 2. There are reserved fields in rte_cryto_op structure. So we can use bits in
> them to indicate various cases. [2]
> >
> > Both the submitted patches follow approach 1 (following how it's done
> currently), but we can switch to approach 2 if we think there can be
> > more such "warnings" that can occur simultaneously. Can you share your
> thoughts on how we should extend the library to handle such
> > cases?
> >
> > [1] https://doc.dpdk.org/api/rte__crypto_8h.html#afe16508b77c2a8dc5caf74a4e9850171
> > [2] https://doc.dpdk.org/api/rte__crypto_8h_source.html
> 
> My vote would probably be for option #2 (use one of the reserved fields for
> it).
> That way - existing code wouldn't need to be changed.

Adding a single enum or multiple enums is the same thing. Right wrt code changes?
However, if the check is something like 
If (status != RTE_CRYPTO_OP_STATUS_SUCCESS)
	Report appropriate error number
App code will need to be updated to take care the warnings in both options.
It will be something like
Option #1
If (status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
	If (status < RTE_CRYPTO_OP_STATUS_SUCCESS)
		Report appropriate error number.
	Else
		Report appropriate warning number probably in debug prints.
}
Option #2	
If (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
	If (op->status == RTE_CRYPTO_OP_STATUS_WARNING) {
		Report appropriate warning based on op->reserved[0]
	} else {
		Report appropriate error number
	}
}
Here both the options are same wrt performance.
But in option #2, driver and app need to fill and decode 2 separate variables
As against 1 variable in option #1

In both the options, there will be similar code changes.
Do you suspect any other code change?

> Again these warnings, it probably needs to be a bit-flags, correct?

We can deal with both bit flags as well as new enums in the status.
I believe both are same and in fact using enum in application is more convenient
for user, instead of decoding bit flags.
However, it is personal choice. People may differ on that.

Regards,
Akhil