Re: [dpdk-dev] [RFC PATCH v2 0/4] IPSec Inline and look aside crypto offload

[Narayana Prasad Athreya <pathreya@caviumnetworks.com>]

> This patchet showcases the definition and usage of the rte_security
> APIs described in the RFC v1 sent earlier.
>
> The data path and configuration path is similar to what was proposed in
> version 1. However, rte_security_configure API is removed, as it looked
> redundant.
>
> Also the rte_security.x files are placed inside the lib/librte_cryptodev/
> as the APIs are defined with the help of crypto APIs and it makes more sense
> to extend the cryptodev library instead of a separate library which perform
> similar actions.
>
> Some of the parameters of the APIs are also modified for better usability.
> The parameter ``dev_name`` is removed as the appropriate device(crypto/eth)
> can be obtained by using the action type.
>
> The patchset is still in work in progress state and there may be some changes
> and cleanup in the next version. This is just to enable others to work
> in parallel on the crypto offloading using ethernet devices.
>
> This patchset include the definition of rte_security APIs in patch 1,
> changes required in cryptodev in patch 2, sample driver implementation
> in patch 3 and ipsec-secgw application changes in patch 4.
>
> Akhil Goyal (4):
>    RFC2: rte_security: API definitions
>    cryptodev: entend cryptodev to support security APIs
>    crypto/dpaa2_sec: add support for protocol offload ipsec
>    example/ipsec-secgw: add support for offloading crypto op
>
>   drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 368 ++++++++++++++++++++++++-
>   drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h   |  33 +++
>   examples/ipsec-secgw/ipsec.c                | 125 ++++++---
>   examples/ipsec-secgw/ipsec.h                |  13 +-
>   examples/ipsec-secgw/sa.c                   | 142 +++++++---
>   lib/librte_cryptodev/Makefile               |   3 +-
>   lib/librte_cryptodev/rte_crypto_sym.h       |  15 +
>   lib/librte_cryptodev/rte_cryptodev.h        |  20 +-
>   lib/librte_cryptodev/rte_cryptodev_pmd.h    |  35 +++
>   lib/librte_cryptodev/rte_security.c         | 171 ++++++++++++
>   lib/librte_cryptodev/rte_security.h         | 409 ++++++++++++++++++++++++++++
>   11 files changed, 1243 insertions(+), 91 deletions(-)
>   create mode 100644 lib/librte_cryptodev/rte_security.c
>   create mode 100644 lib/librte_cryptodev/rte_security.h
>
> -- 
> 2.9.3
I have a few questions/comments on the v1 and v2 versions of this patch. 
I accumulated these from a few different cavium stakeholders.

1. conf_ipsec_sa::sa_dir and ipsec_xform::op seem to have same purpose.
2. Its unclear how the Crypto Device will be configured to use a 
specific Network device and vice-versa. The situation is when the same 
network port must process IPsec and regular traffic. Should regular 
traffic also use the singular device?
3. The spec seems to assume PMD Network device. Event driven model is 
also needed.
4. SA Options for expiry(byte/time) are lacking.
5. Error handling and Status notifications are not specified. These can 
be tricky in the inline mode of operation, particularly inbound.
6. SA expiry handling is another key aspect which hasn’t been accounted for.
7. No anti-replay window size SA param.
8. ESP TFC padding not addressed.
9. Incremental checksum computation in transport mode ESP doesnt appear 
to be addressed
10. Didnt spot details for tunnel mode header preservation
11. Selector checking, especially for the inner packet in tunnel mode 
appears to be missing
12. Dynamic offloading - selectively offload some packets in hardware is 
a feature we would like to support.
13. Destination queue for IPSEC events: Operations in asynchronous or 
inline mode enqueue resulting events into this queue. This helps with 
our 93xx inline ipsec design.
14. If event model (ASYNC) and inline are supported, there should be a 
“pipeline” classifier option for  inbound SAs.
15. Maximum number of destination CoSes is not supported. The same CoS 
may be used for many SAs.
16. Per protocol header parsing capability  after inbound processing is 
missing. Preferred options : None/L2/L3/L4/ALL protocols.
17. Per protocol outer header retention in inbound processing is 
missing. Preferred options : None/L2/L3/L4/ALL protocols.

Thanks
Prasad