[v4,05/10] ipsec: add SA data-path API

  Introduce Security Association (SA-level) data-path API
Operates at SA level, provides functions to:
    - initialize/teardown SA object
    - process inbound/outbound ESP/AH packets associated with the given SA
      (decrypt/encrypt, authenticate, check integrity,
      add/remove ESP/AH related headers and data, etc.).

Signed-off-by: Mohammad Abdul Awal <mohammad.abdul.awal@intel.com>
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Declan Doherty <declan.doherty@intel.com>
---
 lib/librte_ipsec/Makefile              |   2 +
 lib/librte_ipsec/meson.build           |   4 +-
 lib/librte_ipsec/rte_ipsec.h           | 151 +++++++++++++++++++++++++
 lib/librte_ipsec/rte_ipsec_version.map |   3 +
 lib/librte_ipsec/sa.c                  |  21 +++-
 lib/librte_ipsec/sa.h                  |   4 +
 lib/librte_ipsec/ses.c                 |  45 ++++++++
 7 files changed, 227 insertions(+), 3 deletions(-)
 create mode 100644 lib/librte_ipsec/rte_ipsec.h
 create mode 100644 lib/librte_ipsec/ses.c
  

Hi Konstantin,

Sorry for a late review. I was on unplanned leave for more than 2 weeks.

On 12/14/2018 9:53 PM, Konstantin Ananyev wrote:
> Introduce Security Association (SA-level) data-path API
> Operates at SA level, provides functions to:
>      - initialize/teardown SA object
>      - process inbound/outbound ESP/AH packets associated with the given SA
>        (decrypt/encrypt, authenticate, check integrity,
>        add/remove ESP/AH related headers and data, etc.).
>
> Signed-off-by: Mohammad Abdul Awal <mohammad.abdul.awal@intel.com>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Acked-by: Declan Doherty <declan.doherty@intel.com>
> ---
>   lib/librte_ipsec/Makefile              |   2 +
>   lib/librte_ipsec/meson.build           |   4 +-
>   lib/librte_ipsec/rte_ipsec.h           | 151 +++++++++++++++++++++++++
>   lib/librte_ipsec/rte_ipsec_version.map |   3 +
>   lib/librte_ipsec/sa.c                  |  21 +++-
>   lib/librte_ipsec/sa.h                  |   4 +
>   lib/librte_ipsec/ses.c                 |  45 ++++++++
>   7 files changed, 227 insertions(+), 3 deletions(-)
>   create mode 100644 lib/librte_ipsec/rte_ipsec.h
>   create mode 100644 lib/librte_ipsec/ses.c
>
> diff --git a/lib/librte_ipsec/Makefile b/lib/librte_ipsec/Makefile
> index 7758dcc6d..79f187fae 100644
> --- a/lib/librte_ipsec/Makefile
> +++ b/lib/librte_ipsec/Makefile
> @@ -17,8 +17,10 @@ LIBABIVER := 1
>   
>   # all source are stored in SRCS-y
>   SRCS-$(CONFIG_RTE_LIBRTE_IPSEC) += sa.c
> +SRCS-$(CONFIG_RTE_LIBRTE_IPSEC) += ses.c
>   
>   # install header files
> +SYMLINK-$(CONFIG_RTE_LIBRTE_IPSEC)-include += rte_ipsec.h
>   SYMLINK-$(CONFIG_RTE_LIBRTE_IPSEC)-include += rte_ipsec_sa.h
>   
>   include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_ipsec/meson.build b/lib/librte_ipsec/meson.build
> index 52c78eaeb..6e8c6fabe 100644
> --- a/lib/librte_ipsec/meson.build
> +++ b/lib/librte_ipsec/meson.build
> @@ -3,8 +3,8 @@
>   
>   allow_experimental_apis = true
>   
> -sources=files('sa.c')
> +sources=files('sa.c', 'ses.c')
>   
> -install_headers = files('rte_ipsec_sa.h')
> +install_headers = files('rte_ipsec.h', 'rte_ipsec_sa.h')
>   
>   deps += ['mbuf', 'net', 'cryptodev', 'security']
> diff --git a/lib/librte_ipsec/rte_ipsec.h b/lib/librte_ipsec/rte_ipsec.h
> new file mode 100644
> index 000000000..cbcd861b5
> --- /dev/null
> +++ b/lib/librte_ipsec/rte_ipsec.h
> @@ -0,0 +1,151 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#ifndef _RTE_IPSEC_H_
> +#define _RTE_IPSEC_H_
> +
> +/**
> + * @file rte_ipsec.h
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * RTE IPsec support.
> + * librte_ipsec provides a framework for data-path IPsec protocol
> + * processing (ESP/AH).
> + */
> +
> +#include <rte_ipsec_sa.h>
> +#include <rte_mbuf.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +struct rte_ipsec_session;
> +
> +/**
> + * IPsec session specific functions that will be used to:
> + * - prepare - for input mbufs and given IPsec session prepare crypto ops
> + *   that can be enqueued into the cryptodev associated with given session
> + *   (see *rte_ipsec_pkt_crypto_prepare* below for more details).
> + * - process - finalize processing of packets after crypto-dev finished
> + *   with them or process packets that are subjects to inline IPsec offload
> + *   (see rte_ipsec_pkt_process for more details).
> + */
> +struct rte_ipsec_sa_pkt_func {
> +	uint16_t (*prepare)(const struct rte_ipsec_session *ss,
> +				struct rte_mbuf *mb[],
> +				struct rte_crypto_op *cop[],
> +				uint16_t num);
> +	uint16_t (*process)(const struct rte_ipsec_session *ss,
> +				struct rte_mbuf *mb[],
> +				uint16_t num);
> +};
> +
> +/**
> + * rte_ipsec_session is an aggregate structure that defines particular
> + * IPsec Security Association IPsec (SA) on given security/crypto device:
> + * - pointer to the SA object
> + * - security session action type
> + * - pointer to security/crypto session, plus other related data
> + * - session/device specific functions to prepare/process IPsec packets.
> + */
> +struct rte_ipsec_session {
> +
extra line
> +	/**
> +	 * SA that session belongs to.
> +	 * Note that multiple sessions can belong to the same SA.
> +	 */
> +	struct rte_ipsec_sa *sa;
> +	/** session action type */
> +	enum rte_security_session_action_type type;
> +	/** session and related data */
> +	union {
> +		struct {
> +			struct rte_cryptodev_sym_session *ses;
> +		} crypto;
> +		struct {
> +			struct rte_security_session *ses;
> +			struct rte_security_ctx *ctx;
> +			uint32_t ol_flags;
> +		} security;
> +	};
> +	/** functions to prepare/process IPsec packets */
> +	struct rte_ipsec_sa_pkt_func pkt_func;
> +} __rte_cache_aligned;
> +
> +/**
> + * Checks that inside given rte_ipsec_session crypto/security fields
> + * are filled correctly and setups function pointers based on these values.
it means user need not fill rte_ipsec_sa_pkt_fun, specify this in the 
comments.
> + * @param ss
> + *   Pointer to the *rte_ipsec_session* object
> + * @return
> + *   - Zero if operation completed successfully.
> + *   - -EINVAL if the parameters are invalid.
> + */
> +int __rte_experimental
> +rte_ipsec_session_prepare(struct rte_ipsec_session *ss);
> +
> +/**
> + * For input mbufs and given IPsec session prepare crypto ops that can be
> + * enqueued into the cryptodev associated with given session.
> + * expects that for each input packet:
> + *      - l2_len, l3_len are setup correctly
> + * Note that erroneous mbufs are not freed by the function,
> + * but are placed beyond last valid mbuf in the *mb* array.
> + * It is a user responsibility to handle them further.
How will the user know how many mbufs are correctly processed.
> + * @param ss
> + *   Pointer to the *rte_ipsec_session* object the packets belong to.
> + * @param mb
> + *   The address of an array of *num* pointers to *rte_mbuf* structures
> + *   which contain the input packets.
> + * @param cop
> + *   The address of an array of *num* pointers to the output *rte_crypto_op*
> + *   structures.
> + * @param num
> + *   The maximum number of packets to process.
> + * @return
> + *   Number of successfully processed packets, with error code set in rte_errno.
> + */
> +static inline uint16_t __rte_experimental
> +rte_ipsec_pkt_crypto_prepare(const struct rte_ipsec_session *ss,
> +	struct rte_mbuf *mb[], struct rte_crypto_op *cop[], uint16_t num)
> +{
> +	return ss->pkt_func.prepare(ss, mb, cop, num);
> +}
> +
> +/**
> + * Finalise processing of packets after crypto-dev finished with them or
> + * process packets that are subjects to inline IPsec offload.
> + * Expects that for each input packet:
> + *      - l2_len, l3_len are setup correctly
> + * Output mbufs will be:
> + * inbound - decrypted & authenticated, ESP(AH) related headers removed,
> + * *l2_len* and *l3_len* fields are updated.
> + * outbound - appropriate mbuf fields (ol_flags, tx_offloads, etc.)
> + * properly setup, if necessary - IP headers updated, ESP(AH) fields added,
> + * Note that erroneous mbufs are not freed by the function,
> + * but are placed beyond last valid mbuf in the *mb* array.
same question
> + * It is a user responsibility to handle them further.
> + * @param ss
> + *   Pointer to the *rte_ipsec_session* object the packets belong to.
> + * @param mb
> + *   The address of an array of *num* pointers to *rte_mbuf* structures
> + *   which contain the input packets.
> + * @param num
> + *   The maximum number of packets to process.
> + * @return
> + *   Number of successfully processed packets, with error code set in rte_errno.
> + */
> +static inline uint16_t __rte_experimental
> +rte_ipsec_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> +	uint16_t num)
> +{
> +	return ss->pkt_func.process(ss, mb, num);
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_IPSEC_H_ */
> diff --git a/lib/librte_ipsec/rte_ipsec_version.map b/lib/librte_ipsec/rte_ipsec_version.map
> index 1a66726b8..d1c52d7ca 100644
> --- a/lib/librte_ipsec/rte_ipsec_version.map
> +++ b/lib/librte_ipsec/rte_ipsec_version.map
> @@ -1,6 +1,9 @@
>   EXPERIMENTAL {
>   	global:
>   
> +	rte_ipsec_pkt_crypto_prepare;
> +	rte_ipsec_session_prepare;
> +	rte_ipsec_pkt_process;
alphabetical order incorrect
>   	rte_ipsec_sa_fini;
>   	rte_ipsec_sa_init;
>   	rte_ipsec_sa_size;
> diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
> index f927a82bf..e4c5361e7 100644
> --- a/lib/librte_ipsec/sa.c
> +++ b/lib/librte_ipsec/sa.c
> @@ -2,7 +2,7 @@
>    * Copyright(c) 2018 Intel Corporation
>    */
>   
> -#include <rte_ipsec_sa.h>
> +#include <rte_ipsec.h>
>   #include <rte_esp.h>
>   #include <rte_ip.h>
>   #include <rte_errno.h>
> @@ -325,3 +325,22 @@ rte_ipsec_sa_init(struct rte_ipsec_sa *sa, const struct rte_ipsec_sa_prm *prm,
>   
>   	return sz;
>   }
> +
> +int
> +ipsec_sa_pkt_func_select(const struct rte_ipsec_session *ss,
> +	const struct rte_ipsec_sa *sa, struct rte_ipsec_sa_pkt_func *pf)
> +{
> +	int32_t rc;
> +
> +	RTE_SET_USED(sa);
> +
> +	rc = 0;
> +	pf[0] = (struct rte_ipsec_sa_pkt_func) { 0 };
> +
> +	switch (ss->type) {
> +	default:
> +		rc = -ENOTSUP;
> +	}
> +
> +	return rc;
> +}
Is this a dummy function? Will it be updated later? I believe should 
have appropriate comments in that case.
> diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
> index 5d113891a..050a6d7ae 100644
> --- a/lib/librte_ipsec/sa.h
> +++ b/lib/librte_ipsec/sa.h
> @@ -74,4 +74,8 @@ struct rte_ipsec_sa {
>   
>   } __rte_cache_aligned;
>   
> +int
> +ipsec_sa_pkt_func_select(const struct rte_ipsec_session *ss,
> +	const struct rte_ipsec_sa *sa, struct rte_ipsec_sa_pkt_func *pf);
> +
>   #endif /* _SA_H_ */
> diff --git a/lib/librte_ipsec/ses.c b/lib/librte_ipsec/ses.c
> new file mode 100644
> index 000000000..562c1423e
> --- /dev/null
> +++ b/lib/librte_ipsec/ses.c
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#include <rte_ipsec.h>
> +#include "sa.h"
> +
> +static int
> +session_check(struct rte_ipsec_session *ss)
> +{
> +	if (ss == NULL || ss->sa == NULL)
> +		return -EINVAL;
> +
> +	if (ss->type == RTE_SECURITY_ACTION_TYPE_NONE) {
> +		if (ss->crypto.ses == NULL)
> +			return -EINVAL;
> +	} else if (ss->security.ses == NULL || ss->security.ctx == NULL)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +int __rte_experimental
> +rte_ipsec_session_prepare(struct rte_ipsec_session *ss)
> +{
> +	int32_t rc;
> +	struct rte_ipsec_sa_pkt_func fp;
> +
> +	rc = session_check(ss);
> +	if (rc != 0)
> +		return rc;
> +
> +	rc = ipsec_sa_pkt_func_select(ss, ss->sa, &fp);
> +	if (rc != 0)
> +		return rc;
> +
> +	ss->pkt_func = fp;
> +
> +	if (ss->type == RTE_SECURITY_ACTION_TYPE_NONE)
> +		ss->crypto.ses->opaque_data = (uintptr_t)ss;
> +	else
> +		ss->security.ses->opaque_data = (uintptr_t)ss;
> +
> +	return 0;
> +}
  

Hi Akhil,

> 
> Hi Konstantin,
> 
> Sorry for a late review. I was on unplanned leave for more than 2 weeks.

No worries, thanks for review.
Comments/answers inline.
Konstantin

> > +/**
> > + * Checks that inside given rte_ipsec_session crypto/security fields
> > + * are filled correctly and setups function pointers based on these values.
> it means user need not fill rte_ipsec_sa_pkt_fun, 

Yes.

> specify this in the comments.

Ok.

> > + * @param ss
> > + *   Pointer to the *rte_ipsec_session* object
> > + * @return
> > + *   - Zero if operation completed successfully.
> > + *   - -EINVAL if the parameters are invalid.
> > + */
> > +int __rte_experimental
> > +rte_ipsec_session_prepare(struct rte_ipsec_session *ss);
> > +
> > +/**
> > + * For input mbufs and given IPsec session prepare crypto ops that can be
> > + * enqueued into the cryptodev associated with given session.
> > + * expects that for each input packet:
> > + *      - l2_len, l3_len are setup correctly
> > + * Note that erroneous mbufs are not freed by the function,
> > + * but are placed beyond last valid mbuf in the *mb* array.
> > + * It is a user responsibility to handle them further.
> How will the user know how many mbufs are correctly processed.

Function return value contains number of successfully processed packets,
see comments below.
As an example, let say at input mb[]={A, B, C, D}, num=4, and prepare()
was able to successfully process A, B, D mbufs but failed to process C. 
Then return value will be 3, and mb[]={A, B, D, C}. 

> > + * @param ss
> > + *   Pointer to the *rte_ipsec_session* object the packets belong to.
> > + * @param mb
> > + *   The address of an array of *num* pointers to *rte_mbuf* structures
> > + *   which contain the input packets.
> > + * @param cop
> > + *   The address of an array of *num* pointers to the output *rte_crypto_op*
> > + *   structures.
> > + * @param num
> > + *   The maximum number of packets to process.
> > + * @return
> > + *   Number of successfully processed packets, with error code set in rte_errno.
> > + */
> > +static inline uint16_t __rte_experimental
> > +rte_ipsec_pkt_crypto_prepare(const struct rte_ipsec_session *ss,
> > +	struct rte_mbuf *mb[], struct rte_crypto_op *cop[], uint16_t num)
> > +{
> > +	return ss->pkt_func.prepare(ss, mb, cop, num);
> > +}
> > +
> > +/**
> > + * Finalise processing of packets after crypto-dev finished with them or
> > + * process packets that are subjects to inline IPsec offload.
> > + * Expects that for each input packet:
> > + *      - l2_len, l3_len are setup correctly
> > + * Output mbufs will be:
> > + * inbound - decrypted & authenticated, ESP(AH) related headers removed,
> > + * *l2_len* and *l3_len* fields are updated.
> > + * outbound - appropriate mbuf fields (ol_flags, tx_offloads, etc.)
> > + * properly setup, if necessary - IP headers updated, ESP(AH) fields added,
> > + * Note that erroneous mbufs are not freed by the function,
> > + * but are placed beyond last valid mbuf in the *mb* array.
> same question

Same answer as above.

> > + * It is a user responsibility to handle them further.
> > + * @param ss
> > + *   Pointer to the *rte_ipsec_session* object the packets belong to.
> > + * @param mb
> > + *   The address of an array of *num* pointers to *rte_mbuf* structures
> > + *   which contain the input packets.
> > + * @param num
> > + *   The maximum number of packets to process.
> > + * @return
> > + *   Number of successfully processed packets, with error code set in rte_errno.
> > + */
> > +static inline uint16_t __rte_experimental
> > +rte_ipsec_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> > +	uint16_t num)
> > +{
> > +	return ss->pkt_func.process(ss, mb, num);
> > +}
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* _RTE_IPSEC_H_ */


> > +
> > +int
> > +ipsec_sa_pkt_func_select(const struct rte_ipsec_session *ss,
> > +	const struct rte_ipsec_sa *sa, struct rte_ipsec_sa_pkt_func *pf)
> > +{
> > +	int32_t rc;
> > +
> > +	RTE_SET_USED(sa);
> > +
> > +	rc = 0;
> > +	pf[0] = (struct rte_ipsec_sa_pkt_func) { 0 };
> > +
> > +	switch (ss->type) {
> > +	default:
> > +		rc = -ENOTSUP;
> > +	}
> > +
> > +	return rc;
> > +}
> Is this a dummy function? Will it be updated later?

Yes it is a dummy function in that patch, will be filled in patch #6.

 I believe should
> have appropriate comments in that case.
  

On 12/20/2018 3:47 PM, Ananyev, Konstantin wrote:
>
>>> + * @param ss
>>> + *   Pointer to the *rte_ipsec_session* object
>>> + * @return
>>> + *   - Zero if operation completed successfully.
>>> + *   - -EINVAL if the parameters are invalid.
>>> + */
>>> +int __rte_experimental
>>> +rte_ipsec_session_prepare(struct rte_ipsec_session *ss);
>>> +
>>> +/**
>>> + * For input mbufs and given IPsec session prepare crypto ops that can be
>>> + * enqueued into the cryptodev associated with given session.
>>> + * expects that for each input packet:
>>> + *      - l2_len, l3_len are setup correctly
>>> + * Note that erroneous mbufs are not freed by the function,
>>> + * but are placed beyond last valid mbuf in the *mb* array.
>>> + * It is a user responsibility to handle them further.
>> How will the user know how many mbufs are correctly processed.
> Function return value contains number of successfully processed packets,
> see comments below.
> As an example, let say at input mb[]={A, B, C, D}, num=4, and prepare()
> was able to successfully process A, B, D mbufs but failed to process C.
> Then return value will be 3, and mb[]={A, B, D, C}.
Wouldn't that hit performance, movement of mbufs.
Can we leverage the crypto_op->status field. It is just a thought.