| Message ID | 1437553563-6709-1-git-send-email-alexandru.badicioiu@linaro.org |
|---|---|
| State | New |
| Headers | show |
Minor fixes On 22 July 2015 at 04:26, <alexandru.badicioiu@linaro.org> wrote: > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > This patch adds IPSec protocol processing capabilities to crypto > sesssions. Implementations which have these capabilities in hardware > crypto engines can use the extension to offload the application from > IPSec protocol processing. > > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > --- > include/odp/api/crypto_ipsec.h | 110 > ++++++++++++++++++++ > platform/linux-generic/include/odp/crypto.h | 2 + > .../include/odp/plat/crypto_ipsec_types.h | 53 ++++++++++ > 3 files changed, 165 insertions(+), 0 deletions(-) > create mode 100644 include/odp/api/crypto_ipsec.h > create mode 100644 > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > > diff --git a/include/odp/api/crypto_ipsec.h > b/include/odp/api/crypto_ipsec.h > new file mode 100644 > index 0000000..e59fea4 > --- /dev/null > +++ b/include/odp/api/crypto_ipsec.h > @@ -0,0 +1,110 @@ > +/* Copyright (c) 2014, Linaro Limited > 2015 > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP crypto IPSec extension > + */ > + > +#ifndef ODP_API_CRYPTO_IPSEC_H_ > +#define ODP_API_CRYPTO_IPSEC_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +/** > + * @enum odp_ipsec_outhdr_type > + * IPSec tunnel outer header type > + * > + * @enum odp_ipsec_ar_ws > + * IPSec Anti-replay window size > + * > + */ > + > +typedef struct odp_ipsec_params { > + uint32_t spi; /** SPI value */ > + uint32_t seq; /** Initial SEQ number */ > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - > + inbound session with > authentication */ > + odp_bool_t esn; /** Use extended sequence numbers */ > + odp_bool_t auto_iv; /** Auto IV generation for each operation. > */ > + uint16_t out_hdr_size; /** outer header size - tunnel mode */ > + uint8_t *out_hdr; /** outer header - tunnel mode */ > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header type - > + tunnel mode */ > + odp_bool_t ip_csum; /** update/verify ip header checksum */ > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode encap & > decap */ > + odp_bool_t remove_outer_hdr; /** remove outer header - tunnel mode > decap */ > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 TOS or > + IPv6 Traffic Class byte from the > inner/outer > + IP header to the outer/inner IP header > - > + tunnel mode encap & decap */ > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from > + the inner IP header to the > + outer IP header - tunnel mode encap */ > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - tunnel > mode */ > + odp_bool_t udp_csum; /** Update/verify UDP csum when NAT-T > enabled */ > + > +} odp_ipsec_params_t; > + > +/** > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL > + * IPSec tunnel mode > + * > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT > + * IPSec transport mode > + * > + * @enum odp_ipsec_proto > + * IPSec protocol > + */ > + > +/** > + * Configure crypto session for IPsec processing > + * > + * Configures a crypto session for IPSec protocol processing. > + * Packets submitted to an IPSec enabled session will have > + * relevant IPSec headers/trailers and tunnel headers > + * added/removed by the crypto implementation. > + * For example, the input packet for an IPSec ESP transport > + * enabled session should be the clear text packet with > + * no ESP headers/trailers prepared in advance for crypto operation. > + * The output packet will have ESP header, IV, trailer and the ESP ICV > + * added by crypto implementation. > + * Depending on the particular capabilities of an implementation and > + * the parameters enabled by application, the application may be > + * partially or completely offloaded from IPSec protocol processing. > + * For example, if an implementation does not support checksum > + * update for IP header after adding ESP header the application > + * should update after crypto IPSec operation. > + * > + * If an implementation does not support a particular set of > + * arguments it should return error. > + * > + * @param session Session handle > + * @param ipsec_mode IPSec protocol mode > + * @param ipsec_proto IPSec protocol > + * @param ipsec_params IPSec parameters. Parameters which are not > + * relevant for selected protocol & mode are > ignored - > + * e.g. outer_hdr/size set for ESP transport mode. > + * @retval 0 on success > + * @retval <0 on failure > + */ > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, > + enum odp_ipsec_mode ipsec_mode, > + enum odp_ipsec_proto ipsec_proto, > + odp_ipsec_params_t ipsec_params); > + > +/** > + * @} > + */ > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > diff --git a/platform/linux-generic/include/odp/crypto.h > b/platform/linux-generic/include/odp/crypto.h > index 7684c1e..718ab7d 100644 > --- a/platform/linux-generic/include/odp/crypto.h > +++ b/platform/linux-generic/include/odp/crypto.h > @@ -20,6 +20,7 @@ extern "C" { > #include <odp/std_types.h> > #include <odp/plat/packet_types.h> > #include <odp/plat/crypto_types.h> > +#include <odp/plat/crypto_ipsec_types.h> > #include <odp/plat/buffer_types.h> > #include <odp/plat/pool_types.h> > #include <odp/queue.h> > @@ -33,6 +34,7 @@ extern "C" { > */ > > #include <odp/api/crypto.h> > +#include <odp/api/crypto_ipsec.h> > > #ifdef __cplusplus > } > diff --git a/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > new file mode 100644 > index 0000000..74521da > --- /dev/null > +++ b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > @@ -0,0 +1,53 @@ > +/* Copyright (c) 2015, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP crypto > more descriptive file description would be: ODP crypto ipsec types > + */ > + > +#ifndef ODP_CRYPTO_IPSEC_TYPES_H_ > +#define ODP_CRYPTO_IPSEC_TYPES_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +/** @addtogroup odp_crypto > + * @{ > + */ > + > +enum odp_ipsec_mode { > + ODP_IPSEC_MODE_TUNNEL, /**< IPSec tunnel mode */ > + ODP_IPSEC_MODE_TRANSPORT, /**< IPSec transport mode */ > +}; > + > +enum odp_ipsec_proto { > + ODP_IPSEC_ESP, /**< ESP protocol */ > +}; > + > +enum odp_ipsec_outhdr_type { > + ODP_IPSEC_OUTHDR_IPV4, /**< Outer header is IPv4 */ > + ODP_IPSEC_OUTHDR_IPV6, /**< Outer header is IPv6 */ > +}; > + > +enum odp_ipsec_ar_ws { > + ODP_IPSEC_AR_WS_NONE, /**< Anti-replay is not enabled */ > + ODP_IPSEC_AR_WS_32, /**< Anti-replay window size 32 */ > + ODP_IPSEC_AR_WS_64, /**< Anti-replay window size 64 */ > + ODP_IPSEC_AR_WS_128, /**< Anti-replay window size 128 */ > +}; > + > +/** > + * @} > + */ > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > -- > 1.7.3.4 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
Ping. On 22 July 2015 at 11:26, <alexandru.badicioiu@linaro.org> wrote: > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > This patch adds IPSec protocol processing capabilities to crypto > sesssions. Implementations which have these capabilities in hardware > crypto engines can use the extension to offload the application from > IPSec protocol processing. > > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > --- > include/odp/api/crypto_ipsec.h | 110 > ++++++++++++++++++++ > platform/linux-generic/include/odp/crypto.h | 2 + > .../include/odp/plat/crypto_ipsec_types.h | 53 ++++++++++ > 3 files changed, 165 insertions(+), 0 deletions(-) > create mode 100644 include/odp/api/crypto_ipsec.h > create mode 100644 > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > > diff --git a/include/odp/api/crypto_ipsec.h > b/include/odp/api/crypto_ipsec.h > new file mode 100644 > index 0000000..e59fea4 > --- /dev/null > +++ b/include/odp/api/crypto_ipsec.h > @@ -0,0 +1,110 @@ > +/* Copyright (c) 2014, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP crypto IPSec extension > + */ > + > +#ifndef ODP_API_CRYPTO_IPSEC_H_ > +#define ODP_API_CRYPTO_IPSEC_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +/** > + * @enum odp_ipsec_outhdr_type > + * IPSec tunnel outer header type > + * > + * @enum odp_ipsec_ar_ws > + * IPSec Anti-replay window size > + * > + */ > + > +typedef struct odp_ipsec_params { > + uint32_t spi; /** SPI value */ > + uint32_t seq; /** Initial SEQ number */ > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - > + inbound session with > authentication */ > + odp_bool_t esn; /** Use extended sequence numbers */ > + odp_bool_t auto_iv; /** Auto IV generation for each operation. > */ > + uint16_t out_hdr_size; /** outer header size - tunnel mode */ > + uint8_t *out_hdr; /** outer header - tunnel mode */ > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header type - > + tunnel mode */ > + odp_bool_t ip_csum; /** update/verify ip header checksum */ > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode encap & > decap */ > + odp_bool_t remove_outer_hdr; /** remove outer header - tunnel mode > decap */ > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 TOS or > + IPv6 Traffic Class byte from the > inner/outer > + IP header to the outer/inner IP header > - > + tunnel mode encap & decap */ > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from > + the inner IP header to the > + outer IP header - tunnel mode encap */ > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - tunnel > mode */ > + odp_bool_t udp_csum; /** Update/verify UDP csum when NAT-T > enabled */ > + > +} odp_ipsec_params_t; > + > +/** > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL > + * IPSec tunnel mode > + * > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT > + * IPSec transport mode > + * > + * @enum odp_ipsec_proto > + * IPSec protocol > + */ > + > +/** > + * Configure crypto session for IPsec processing > + * > + * Configures a crypto session for IPSec protocol processing. > + * Packets submitted to an IPSec enabled session will have > + * relevant IPSec headers/trailers and tunnel headers > + * added/removed by the crypto implementation. > + * For example, the input packet for an IPSec ESP transport > + * enabled session should be the clear text packet with > + * no ESP headers/trailers prepared in advance for crypto operation. > + * The output packet will have ESP header, IV, trailer and the ESP ICV > + * added by crypto implementation. > + * Depending on the particular capabilities of an implementation and > + * the parameters enabled by application, the application may be > + * partially or completely offloaded from IPSec protocol processing. > + * For example, if an implementation does not support checksum > + * update for IP header after adding ESP header the application > + * should update after crypto IPSec operation. > + * > + * If an implementation does not support a particular set of > + * arguments it should return error. > + * > + * @param session Session handle > + * @param ipsec_mode IPSec protocol mode > + * @param ipsec_proto IPSec protocol > + * @param ipsec_params IPSec parameters. Parameters which are not > + * relevant for selected protocol & mode are > ignored - > + * e.g. outer_hdr/size set for ESP transport mode. > + * @retval 0 on success > + * @retval <0 on failure > + */ > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, > + enum odp_ipsec_mode ipsec_mode, > + enum odp_ipsec_proto ipsec_proto, > + odp_ipsec_params_t ipsec_params); > + > +/** > + * @} > + */ > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > diff --git a/platform/linux-generic/include/odp/crypto.h > b/platform/linux-generic/include/odp/crypto.h > index 7684c1e..718ab7d 100644 > --- a/platform/linux-generic/include/odp/crypto.h > +++ b/platform/linux-generic/include/odp/crypto.h > @@ -20,6 +20,7 @@ extern "C" { > #include <odp/std_types.h> > #include <odp/plat/packet_types.h> > #include <odp/plat/crypto_types.h> > +#include <odp/plat/crypto_ipsec_types.h> > #include <odp/plat/buffer_types.h> > #include <odp/plat/pool_types.h> > #include <odp/queue.h> > @@ -33,6 +34,7 @@ extern "C" { > */ > > #include <odp/api/crypto.h> > +#include <odp/api/crypto_ipsec.h> > > #ifdef __cplusplus > } > diff --git a/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > new file mode 100644 > index 0000000..74521da > --- /dev/null > +++ b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > @@ -0,0 +1,53 @@ > +/* Copyright (c) 2015, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP crypto > + */ > + > +#ifndef ODP_CRYPTO_IPSEC_TYPES_H_ > +#define ODP_CRYPTO_IPSEC_TYPES_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +/** @addtogroup odp_crypto > + * @{ > + */ > + > +enum odp_ipsec_mode { > + ODP_IPSEC_MODE_TUNNEL, /**< IPSec tunnel mode */ > + ODP_IPSEC_MODE_TRANSPORT, /**< IPSec transport mode */ > +}; > + > +enum odp_ipsec_proto { > + ODP_IPSEC_ESP, /**< ESP protocol */ > +}; > + > +enum odp_ipsec_outhdr_type { > + ODP_IPSEC_OUTHDR_IPV4, /**< Outer header is IPv4 */ > + ODP_IPSEC_OUTHDR_IPV6, /**< Outer header is IPv6 */ > +}; > + > +enum odp_ipsec_ar_ws { > + ODP_IPSEC_AR_WS_NONE, /**< Anti-replay is not enabled */ > + ODP_IPSEC_AR_WS_32, /**< Anti-replay window size 32 */ > + ODP_IPSEC_AR_WS_64, /**< Anti-replay window size 64 */ > + ODP_IPSEC_AR_WS_128, /**< Anti-replay window size 128 */ > +}; > + > +/** > + * @} > + */ > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > -- > 1.7.3.4 > >
On 07/30/15 12:04, Alexandru Badicioiu wrote: > Ping. Alex, Mike replayed to you to fix minor fixes. Maxim. > > On 22 July 2015 at 11:26, <alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>> wrote: > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>> > > This patch adds IPSec protocol processing capabilities to crypto > sesssions. Implementations which have these capabilities in hardware > crypto engines can use the extension to offload the application from > IPSec protocol processing. > > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>> > --- > include/odp/api/crypto_ipsec.h | 110 > ++++++++++++++++++++ > platform/linux-generic/include/odp/crypto.h | 2 + > .../include/odp/plat/crypto_ipsec_types.h | 53 ++++++++++ > 3 files changed, 165 insertions(+), 0 deletions(-) > create mode 100644 include/odp/api/crypto_ipsec.h > create mode 100644 > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > > diff --git a/include/odp/api/crypto_ipsec.h > b/include/odp/api/crypto_ipsec.h > new file mode 100644 > index 0000000..e59fea4 > --- /dev/null > +++ b/include/odp/api/crypto_ipsec.h > @@ -0,0 +1,110 @@ > +/* Copyright (c) 2014, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP crypto IPSec extension > + */ > + > +#ifndef ODP_API_CRYPTO_IPSEC_H_ > +#define ODP_API_CRYPTO_IPSEC_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +/** > + * @enum odp_ipsec_outhdr_type > + * IPSec tunnel outer header type > + * > + * @enum odp_ipsec_ar_ws > + * IPSec Anti-replay window size > + * > + */ > + > +typedef struct odp_ipsec_params { > + uint32_t spi; /** SPI value */ > + uint32_t seq; /** Initial SEQ number */ > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - > + inbound session with > authentication */ > + odp_bool_t esn; /** Use extended sequence numbers */ > + odp_bool_t auto_iv; /** Auto IV generation for each > operation. */ > + uint16_t out_hdr_size; /** outer header size - tunnel > mode */ > + uint8_t *out_hdr; /** outer header - tunnel mode */ > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header > type - > + tunnel mode */ > + odp_bool_t ip_csum; /** update/verify ip header > checksum */ > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode > encap & decap */ > + odp_bool_t remove_outer_hdr; /** remove outer header - > tunnel mode decap */ > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 > TOS or > + IPv6 Traffic Class byte from > the inner/outer > + IP header to the outer/inner > IP header - > + tunnel mode encap & decap */ > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from > + the inner IP header to the > + outer IP header - tunnel mode > encap */ > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - > tunnel mode */ > + odp_bool_t udp_csum; /** Update/verify UDP csum when > NAT-T enabled */ > + > +} odp_ipsec_params_t; > + > +/** > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL > + * IPSec tunnel mode > + * > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT > + * IPSec transport mode > + * > + * @enum odp_ipsec_proto > + * IPSec protocol > + */ > + > +/** > + * Configure crypto session for IPsec processing > + * > + * Configures a crypto session for IPSec protocol processing. > + * Packets submitted to an IPSec enabled session will have > + * relevant IPSec headers/trailers and tunnel headers > + * added/removed by the crypto implementation. > + * For example, the input packet for an IPSec ESP transport > + * enabled session should be the clear text packet with > + * no ESP headers/trailers prepared in advance for crypto operation. > + * The output packet will have ESP header, IV, trailer and the > ESP ICV > + * added by crypto implementation. > + * Depending on the particular capabilities of an implementation and > + * the parameters enabled by application, the application may be > + * partially or completely offloaded from IPSec protocol processing. > + * For example, if an implementation does not support checksum > + * update for IP header after adding ESP header the application > + * should update after crypto IPSec operation. > + * > + * If an implementation does not support a particular set of > + * arguments it should return error. > + * > + * @param session Session handle > + * @param ipsec_mode IPSec protocol mode > + * @param ipsec_proto IPSec protocol > + * @param ipsec_params IPSec parameters. Parameters which are not > + * relevant for selected protocol & mode > are ignored - > + * e.g. outer_hdr/size set for ESP > transport mode. > + * @retval 0 on success > + * @retval <0 on failure > + */ > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, > + enum odp_ipsec_mode ipsec_mode, > + enum odp_ipsec_proto ipsec_proto, > + odp_ipsec_params_t ipsec_params); > + > +/** > + * @} > + */ > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > diff --git a/platform/linux-generic/include/odp/crypto.h > b/platform/linux-generic/include/odp/crypto.h > index 7684c1e..718ab7d 100644 > --- a/platform/linux-generic/include/odp/crypto.h > +++ b/platform/linux-generic/include/odp/crypto.h > @@ -20,6 +20,7 @@ extern "C" { > #include <odp/std_types.h> > #include <odp/plat/packet_types.h> > #include <odp/plat/crypto_types.h> > +#include <odp/plat/crypto_ipsec_types.h> > #include <odp/plat/buffer_types.h> > #include <odp/plat/pool_types.h> > #include <odp/queue.h> > @@ -33,6 +34,7 @@ extern "C" { > */ > > #include <odp/api/crypto.h> > +#include <odp/api/crypto_ipsec.h> > > #ifdef __cplusplus > } > diff --git > a/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > new file mode 100644 > index 0000000..74521da > --- /dev/null > +++ b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > @@ -0,0 +1,53 @@ > +/* Copyright (c) 2015, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP crypto > + */ > + > +#ifndef ODP_CRYPTO_IPSEC_TYPES_H_ > +#define ODP_CRYPTO_IPSEC_TYPES_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +/** @addtogroup odp_crypto > + * @{ > + */ > + > +enum odp_ipsec_mode { > + ODP_IPSEC_MODE_TUNNEL, /**< IPSec tunnel mode */ > + ODP_IPSEC_MODE_TRANSPORT, /**< IPSec transport mode */ > +}; > + > +enum odp_ipsec_proto { > + ODP_IPSEC_ESP, /**< ESP protocol */ > +}; > + > +enum odp_ipsec_outhdr_type { > + ODP_IPSEC_OUTHDR_IPV4, /**< Outer header is IPv4 */ > + ODP_IPSEC_OUTHDR_IPV6, /**< Outer header is IPv6 */ > +}; > + > +enum odp_ipsec_ar_ws { > + ODP_IPSEC_AR_WS_NONE, /**< Anti-replay is not enabled */ > + ODP_IPSEC_AR_WS_32, /**< Anti-replay window size 32 */ > + ODP_IPSEC_AR_WS_64, /**< Anti-replay window size 64 */ > + ODP_IPSEC_AR_WS_128, /**< Anti-replay window size 128 */ > +}; > + > +/** > + * @} > + */ > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > -- > 1.7.3.4 > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
Hi Maxim, I noticed that but I wanted to know if there are some other comments more oriented to the functionality this patch proposes. Thanks, Alex On 30 July 2015 at 12:21, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 07/30/15 12:04, Alexandru Badicioiu wrote: > >> Ping. >> > > Alex, Mike replayed to you to fix minor fixes. > > Maxim. > >> >> On 22 July 2015 at 11:26, <alexandru.badicioiu@linaro.org <mailto: >> alexandru.badicioiu@linaro.org>> wrote: >> >> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org >> <mailto:alexandru.badicioiu@linaro.org>> >> >> This patch adds IPSec protocol processing capabilities to crypto >> sesssions. Implementations which have these capabilities in hardware >> crypto engines can use the extension to offload the application from >> IPSec protocol processing. >> >> Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org >> <mailto:alexandru.badicioiu@linaro.org>> >> >> --- >> include/odp/api/crypto_ipsec.h | 110 >> ++++++++++++++++++++ >> platform/linux-generic/include/odp/crypto.h | 2 + >> .../include/odp/plat/crypto_ipsec_types.h | 53 ++++++++++ >> 3 files changed, 165 insertions(+), 0 deletions(-) >> create mode 100644 include/odp/api/crypto_ipsec.h >> create mode 100644 >> platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >> >> diff --git a/include/odp/api/crypto_ipsec.h >> b/include/odp/api/crypto_ipsec.h >> new file mode 100644 >> index 0000000..e59fea4 >> --- /dev/null >> +++ b/include/odp/api/crypto_ipsec.h >> @@ -0,0 +1,110 @@ >> +/* Copyright (c) 2014, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +/** >> + * @file >> + * >> + * ODP crypto IPSec extension >> + */ >> + >> +#ifndef ODP_API_CRYPTO_IPSEC_H_ >> +#define ODP_API_CRYPTO_IPSEC_H_ >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> +/** >> + * @enum odp_ipsec_outhdr_type >> + * IPSec tunnel outer header type >> + * >> + * @enum odp_ipsec_ar_ws >> + * IPSec Anti-replay window size >> + * >> + */ >> + >> +typedef struct odp_ipsec_params { >> + uint32_t spi; /** SPI value */ >> + uint32_t seq; /** Initial SEQ number */ >> + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - >> + inbound session with >> authentication */ >> + odp_bool_t esn; /** Use extended sequence numbers */ >> + odp_bool_t auto_iv; /** Auto IV generation for each >> operation. */ >> + uint16_t out_hdr_size; /** outer header size - tunnel >> mode */ >> + uint8_t *out_hdr; /** outer header - tunnel mode */ >> + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header >> type - >> + tunnel mode */ >> + odp_bool_t ip_csum; /** update/verify ip header >> checksum */ >> + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode >> encap & decap */ >> + odp_bool_t remove_outer_hdr; /** remove outer header - >> tunnel mode decap */ >> + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 >> TOS or >> + IPv6 Traffic Class byte from >> the inner/outer >> + IP header to the outer/inner >> IP header - >> + tunnel mode encap & decap */ >> + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from >> + the inner IP header to the >> + outer IP header - tunnel mode >> encap */ >> + odp_bool_t nat_t; /** NAT-T encapsulation enabled - >> tunnel mode */ >> + odp_bool_t udp_csum; /** Update/verify UDP csum when >> NAT-T enabled */ >> + >> +} odp_ipsec_params_t; >> + >> +/** >> + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL >> + * IPSec tunnel mode >> + * >> + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT >> + * IPSec transport mode >> + * >> + * @enum odp_ipsec_proto >> + * IPSec protocol >> + */ >> + >> +/** >> + * Configure crypto session for IPsec processing >> + * >> + * Configures a crypto session for IPSec protocol processing. >> + * Packets submitted to an IPSec enabled session will have >> + * relevant IPSec headers/trailers and tunnel headers >> + * added/removed by the crypto implementation. >> + * For example, the input packet for an IPSec ESP transport >> + * enabled session should be the clear text packet with >> + * no ESP headers/trailers prepared in advance for crypto operation. >> + * The output packet will have ESP header, IV, trailer and the >> ESP ICV >> + * added by crypto implementation. >> + * Depending on the particular capabilities of an implementation and >> + * the parameters enabled by application, the application may be >> + * partially or completely offloaded from IPSec protocol processing. >> + * For example, if an implementation does not support checksum >> + * update for IP header after adding ESP header the application >> + * should update after crypto IPSec operation. >> + * >> + * If an implementation does not support a particular set of >> + * arguments it should return error. >> + * >> + * @param session Session handle >> + * @param ipsec_mode IPSec protocol mode >> + * @param ipsec_proto IPSec protocol >> + * @param ipsec_params IPSec parameters. Parameters which are not >> + * relevant for selected protocol & mode >> are ignored - >> + * e.g. outer_hdr/size set for ESP >> transport mode. >> + * @retval 0 on success >> + * @retval <0 on failure >> + */ >> +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, >> + enum odp_ipsec_mode ipsec_mode, >> + enum odp_ipsec_proto ipsec_proto, >> + odp_ipsec_params_t ipsec_params); >> + >> +/** >> + * @} >> + */ >> + >> +#ifdef __cplusplus >> +} >> +#endif >> + >> +#endif >> diff --git a/platform/linux-generic/include/odp/crypto.h >> b/platform/linux-generic/include/odp/crypto.h >> index 7684c1e..718ab7d 100644 >> --- a/platform/linux-generic/include/odp/crypto.h >> +++ b/platform/linux-generic/include/odp/crypto.h >> @@ -20,6 +20,7 @@ extern "C" { >> #include <odp/std_types.h> >> #include <odp/plat/packet_types.h> >> #include <odp/plat/crypto_types.h> >> +#include <odp/plat/crypto_ipsec_types.h> >> #include <odp/plat/buffer_types.h> >> #include <odp/plat/pool_types.h> >> #include <odp/queue.h> >> @@ -33,6 +34,7 @@ extern "C" { >> */ >> >> #include <odp/api/crypto.h> >> +#include <odp/api/crypto_ipsec.h> >> >> #ifdef __cplusplus >> } >> diff --git >> a/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >> b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >> new file mode 100644 >> index 0000000..74521da >> --- /dev/null >> +++ b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >> @@ -0,0 +1,53 @@ >> +/* Copyright (c) 2015, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +/** >> + * @file >> + * >> + * ODP crypto >> + */ >> + >> +#ifndef ODP_CRYPTO_IPSEC_TYPES_H_ >> +#define ODP_CRYPTO_IPSEC_TYPES_H_ >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> +/** @addtogroup odp_crypto >> + * @{ >> + */ >> + >> +enum odp_ipsec_mode { >> + ODP_IPSEC_MODE_TUNNEL, /**< IPSec tunnel mode */ >> + ODP_IPSEC_MODE_TRANSPORT, /**< IPSec transport mode */ >> +}; >> + >> +enum odp_ipsec_proto { >> + ODP_IPSEC_ESP, /**< ESP protocol */ >> +}; >> + >> +enum odp_ipsec_outhdr_type { >> + ODP_IPSEC_OUTHDR_IPV4, /**< Outer header is IPv4 */ >> + ODP_IPSEC_OUTHDR_IPV6, /**< Outer header is IPv6 */ >> +}; >> + >> +enum odp_ipsec_ar_ws { >> + ODP_IPSEC_AR_WS_NONE, /**< Anti-replay is not enabled */ >> + ODP_IPSEC_AR_WS_32, /**< Anti-replay window size 32 */ >> + ODP_IPSEC_AR_WS_64, /**< Anti-replay window size 64 */ >> + ODP_IPSEC_AR_WS_128, /**< Anti-replay window size 128 */ >> +}; >> + >> +/** >> + * @} >> + */ >> + >> +#ifdef __cplusplus >> +} >> +#endif >> + >> +#endif >> -- >> 1.7.3.4 >> >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On Wed, Jul 22, 2015 at 11:26:03AM +0300, alexandru.badicioiu@linaro.org wrote: > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > This patch adds IPSec protocol processing capabilities to crypto > sesssions. Implementations which have these capabilities in hardware > crypto engines can use the extension to offload the application from > IPSec protocol processing. > > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > --- > include/odp/api/crypto_ipsec.h | 110 ++++++++++++++++++++ > platform/linux-generic/include/odp/crypto.h | 2 + > .../include/odp/plat/crypto_ipsec_types.h | 53 ++++++++++ > 3 files changed, 165 insertions(+), 0 deletions(-) > create mode 100644 include/odp/api/crypto_ipsec.h > create mode 100644 platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > > diff --git a/include/odp/api/crypto_ipsec.h b/include/odp/api/crypto_ipsec.h > new file mode 100644 > index 0000000..e59fea4 > --- /dev/null > +++ b/include/odp/api/crypto_ipsec.h > @@ -0,0 +1,110 @@ > +/* Copyright (c) 2014, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP crypto IPSec extension > + */ > + > +#ifndef ODP_API_CRYPTO_IPSEC_H_ > +#define ODP_API_CRYPTO_IPSEC_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +/** > + * @enum odp_ipsec_outhdr_type > + * IPSec tunnel outer header type > + * > + * @enum odp_ipsec_ar_ws > + * IPSec Anti-replay window size > + * > + */ > + > +typedef struct odp_ipsec_params { > + uint32_t spi; /** SPI value */ > + uint32_t seq; /** Initial SEQ number */ > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - > + inbound session with authentication */ This name is pretty cryptic, how about just replay_window? > + odp_bool_t esn; /** Use extended sequence numbers */ > + odp_bool_t auto_iv; /** Auto IV generation for each operation. */ > + uint16_t out_hdr_size; /** outer header size - tunnel mode */ > + uint8_t *out_hdr; /** outer header - tunnel mode */ Can these be 0 and NULL if the application wants tunnel mode but wants to handle the outer header itself? (i.e. add ESP head/tail and include inner IP header in encap data) > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header type - > + tunnel mode */ > + odp_bool_t ip_csum; /** update/verify ip header checksum */ > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode encap & decap */ > + odp_bool_t remove_outer_hdr; /** remove outer header - tunnel mode decap */ > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 TOS or > + IPv6 Traffic Class byte from the inner/outer > + IP header to the outer/inner IP header - > + tunnel mode encap & decap */ > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from > + the inner IP header to the > + outer IP header - tunnel mode encap */ > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - tunnel mode */ > + odp_bool_t udp_csum; /** Update/verify UDP csum when NAT-T enabled */ > + > +} odp_ipsec_params_t; > + > +/** > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL > + * IPSec tunnel mode > + * > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT > + * IPSec transport mode > + * > + * @enum odp_ipsec_proto > + * IPSec protocol > + */ > + > +/** > + * Configure crypto session for IPsec processing > + * > + * Configures a crypto session for IPSec protocol processing. > + * Packets submitted to an IPSec enabled session will have > + * relevant IPSec headers/trailers and tunnel headers > + * added/removed by the crypto implementation. > + * For example, the input packet for an IPSec ESP transport > + * enabled session should be the clear text packet with > + * no ESP headers/trailers prepared in advance for crypto operation. > + * The output packet will have ESP header, IV, trailer and the ESP ICV > + * added by crypto implementation. If a packet fails a check on decap (e.g. out of window), presumably the application gets an odp_crypto_op_result_t with ok=false, but is there any way for it to tell what failed? > + * Depending on the particular capabilities of an implementation and > + * the parameters enabled by application, the application may be > + * partially or completely offloaded from IPSec protocol processing. > + * For example, if an implementation does not support checksum > + * update for IP header after adding ESP header the application > + * should update after crypto IPSec operation. > + * > + * If an implementation does not support a particular set of > + * arguments it should return error. > + * > + * @param session Session handle > + * @param ipsec_mode IPSec protocol mode > + * @param ipsec_proto IPSec protocol > + * @param ipsec_params IPSec parameters. Parameters which are not > + * relevant for selected protocol & mode are ignored - > + * e.g. outer_hdr/size set for ESP transport mode. > + * @retval 0 on success > + * @retval <0 on failure > + */ > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, > + enum odp_ipsec_mode ipsec_mode, > + enum odp_ipsec_proto ipsec_proto, > + odp_ipsec_params_t ipsec_params); Can this be called multiple times on the same session to update the parameters, or would the session need to be destroyed and recreated? Is it valid to create a session, issue a few operations, then later add the IPsec protocol config for that session? I'm wondering why the params here weren't just made an extension of the odp_crypto_session_params_t in the initial session create. -- Stuart.
On 30 July 2015 at 13:50, Stuart Haslam <stuart.haslam@linaro.org> wrote: > On Wed, Jul 22, 2015 at 11:26:03AM +0300, alexandru.badicioiu@linaro.org > wrote: > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > > > This patch adds IPSec protocol processing capabilities to crypto > > sesssions. Implementations which have these capabilities in hardware > > crypto engines can use the extension to offload the application from > > IPSec protocol processing. > > > > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > --- > > include/odp/api/crypto_ipsec.h | 110 > ++++++++++++++++++++ > > platform/linux-generic/include/odp/crypto.h | 2 + > > .../include/odp/plat/crypto_ipsec_types.h | 53 ++++++++++ > > 3 files changed, 165 insertions(+), 0 deletions(-) > > create mode 100644 include/odp/api/crypto_ipsec.h > > create mode 100644 > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > > > > diff --git a/include/odp/api/crypto_ipsec.h > b/include/odp/api/crypto_ipsec.h > > new file mode 100644 > > index 0000000..e59fea4 > > --- /dev/null > > +++ b/include/odp/api/crypto_ipsec.h > > @@ -0,0 +1,110 @@ > > +/* Copyright (c) 2014, Linaro Limited > > + * All rights reserved. > > + * > > + * SPDX-License-Identifier: BSD-3-Clause > > + */ > > + > > +/** > > + * @file > > + * > > + * ODP crypto IPSec extension > > + */ > > + > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ > > +#define ODP_API_CRYPTO_IPSEC_H_ > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +/** > > + * @enum odp_ipsec_outhdr_type > > + * IPSec tunnel outer header type > > + * > > + * @enum odp_ipsec_ar_ws > > + * IPSec Anti-replay window size > > + * > > + */ > > + > > +typedef struct odp_ipsec_params { > > + uint32_t spi; /** SPI value */ > > + uint32_t seq; /** Initial SEQ number */ > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - > > + inbound session with > authentication */ > > This name is pretty cryptic, how about just replay_window? > [Alex] The standard name for this parameter is anti-replay window. In the context of IPSec this should not be cryptic (odp_ipsec_arw vs odp_arw). If your suggestion is to replace the name of the struct member - ar_ws with replay_window I'm fine with it but not the name of the enum (odp_ipsec_ar_ws). Or maybe change it to enum odp_ipsec_arw. > > > + odp_bool_t esn; /** Use extended sequence numbers */ > > + odp_bool_t auto_iv; /** Auto IV generation for each operation. > */ > > + uint16_t out_hdr_size; /** outer header size - tunnel mode */ > > + uint8_t *out_hdr; /** outer header - tunnel mode */ > > Can these be 0 and NULL if the application wants tunnel mode but wants > to handle the outer header itself? (i.e. add ESP head/tail and include > inner IP header in encap data) > [Alex] Yes, that is the intended usage. If requested mode is tunnel and there's no out_hdr specified, the application has to add it. > > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header type - > > + tunnel mode */ > > + odp_bool_t ip_csum; /** update/verify ip header checksum */ > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode encap & > decap */ > > + odp_bool_t remove_outer_hdr; /** remove outer header - tunnel mode > decap */ > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 TOS or > > + IPv6 Traffic Class byte from the > inner/outer > > + IP header to the outer/inner IP header > - > > + tunnel mode encap & decap */ > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from > > + the inner IP header to the > > + outer IP header - tunnel mode encap */ > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - tunnel > mode */ > > + odp_bool_t udp_csum; /** Update/verify UDP csum when NAT-T > enabled */ > > + > > +} odp_ipsec_params_t; > > + > > +/** > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL > > + * IPSec tunnel mode > > + * > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT > > + * IPSec transport mode > > + * > > + * @enum odp_ipsec_proto > > + * IPSec protocol > > + */ > > + > > +/** > > + * Configure crypto session for IPsec processing > > + * > > + * Configures a crypto session for IPSec protocol processing. > > + * Packets submitted to an IPSec enabled session will have > > + * relevant IPSec headers/trailers and tunnel headers > > + * added/removed by the crypto implementation. > > + * For example, the input packet for an IPSec ESP transport > > + * enabled session should be the clear text packet with > > + * no ESP headers/trailers prepared in advance for crypto operation. > > + * The output packet will have ESP header, IV, trailer and the ESP ICV > > + * added by crypto implementation. > > If a packet fails a check on decap (e.g. out of window), presumably the > application gets an odp_crypto_op_result_t with ok=false, but is there > any way for it to tell what failed? > [Alex] Crypto operation error status should be extended with a code for > out of window condition. > > + * Depending on the particular capabilities of an implementation and > > + * the parameters enabled by application, the application may be > > + * partially or completely offloaded from IPSec protocol processing. > > + * For example, if an implementation does not support checksum > > + * update for IP header after adding ESP header the application > > + * should update after crypto IPSec operation. > > + * > > + * If an implementation does not support a particular set of > > + * arguments it should return error. > > + * > > + * @param session Session handle > > + * @param ipsec_mode IPSec protocol mode > > + * @param ipsec_proto IPSec protocol > > + * @param ipsec_params IPSec parameters. Parameters which are > not > > + * relevant for selected protocol & mode are > ignored - > > + * e.g. outer_hdr/size set for ESP transport mode. > > + * @retval 0 on success > > + * @retval <0 on failure > > + */ > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, > > + enum odp_ipsec_mode ipsec_mode, > > + enum odp_ipsec_proto ipsec_proto, > > + odp_ipsec_params_t ipsec_params); > > Can this be called multiple times on the same session to update the > parameters, or would the session need to be destroyed and recreated? > [Alex] No, this is not the intended use of this function. This is to be called on a raw session (algorithm only). To change a crypto session additional functions should be used. > > Is it valid to create a session, issue a few operations, then later add > the IPsec protocol config for that session? > [Alex] While this might work for a particular implementation/platform, I'm wondering if there's an use case for it? > > I'm wondering why the params here weren't just made an extension of the > odp_crypto_session_params_t in the initial session create. > [Alex] Do you mean to add these parameters as members of odp_crypto_session_params_t or to extend session_create call to take IPSec params? It wouldn't be a good idea to embed the IPSec parameters into the odp_crypto_session_params_t as IPSec is just a protocol among others supported by crypto engines (SSL/TLS, MACSEC, SRTP, etc). I think having a separate call odp_crypto_session_config_ipsec() makes the source code more readable regarding the intent of the application rather than filling in lots of IPSec parameters and calling then session_create(). > -- > Stuart. >
On 30 July 2015 at 16:00, Jerin Jacob <jerin.jacob@caviumnetworks.com> wrote: > On Wed, Jul 22, 2015 at 11:26:03AM +0300, alexandru.badicioiu@linaro.org > wrote: > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > > > This patch adds IPSec protocol processing capabilities to crypto > > sesssions. Implementations which have these capabilities in hardware > > crypto engines can use the extension to offload the application from > > IPSec protocol processing. > > > > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > --- > > include/odp/api/crypto_ipsec.h | 110 > ++++++++++++++++++++ > > platform/linux-generic/include/odp/crypto.h | 2 + > > .../include/odp/plat/crypto_ipsec_types.h | 53 ++++++++++ > > 3 files changed, 165 insertions(+), 0 deletions(-) > > create mode 100644 include/odp/api/crypto_ipsec.h > > create mode 100644 > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > > > > diff --git a/include/odp/api/crypto_ipsec.h > b/include/odp/api/crypto_ipsec.h > > new file mode 100644 > > index 0000000..e59fea4 > > --- /dev/null > > +++ b/include/odp/api/crypto_ipsec.h > > @@ -0,0 +1,110 @@ > > +/* Copyright (c) 2014, Linaro Limited > > + * All rights reserved. > > + * > > + * SPDX-License-Identifier: BSD-3-Clause > > + */ > > + > > +/** > > + * @file > > + * > > + * ODP crypto IPSec extension > > + */ > > + > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ > > +#define ODP_API_CRYPTO_IPSEC_H_ > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +/** > > + * @enum odp_ipsec_outhdr_type > > + * IPSec tunnel outer header type > > + * > > + * @enum odp_ipsec_ar_ws > > + * IPSec Anti-replay window size > > + * > > + */ > > + > > +typedef struct odp_ipsec_params { > > + uint32_t spi; /** SPI value */ > > + uint32_t seq; /** Initial SEQ number */ > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - > > + inbound session with > authentication */ > > + odp_bool_t esn; /** Use extended sequence numbers */ > > + odp_bool_t auto_iv; /** Auto IV generation for each operation. > */ > > + uint16_t out_hdr_size; /** outer header size - tunnel mode */ > > + uint8_t *out_hdr; /** outer header - tunnel mode */ > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header type - > > + tunnel mode */ > > + odp_bool_t ip_csum; /** update/verify ip header checksum */ > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode encap & > decap */ > > + odp_bool_t remove_outer_hdr; /** remove outer header - tunnel mode > decap */ > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 TOS or > > + IPv6 Traffic Class byte from the > inner/outer > > + IP header to the outer/inner IP header > - > > + tunnel mode encap & decap */ > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from > > + the inner IP header to the > > + outer IP header - tunnel mode encap */ > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - tunnel > mode */ > > + odp_bool_t udp_csum; /** Update/verify UDP csum when NAT-T > enabled */ > > + > > +} odp_ipsec_params_t; > > + > > +/** > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL > > + * IPSec tunnel mode > > + * > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT > > + * IPSec transport mode > > + * > > + * @enum odp_ipsec_proto > > + * IPSec protocol > > + */ > > + > > +/** > > + * Configure crypto session for IPsec processing > > + * > > + * Configures a crypto session for IPSec protocol processing. > > + * Packets submitted to an IPSec enabled session will have > > + * relevant IPSec headers/trailers and tunnel headers > > + * added/removed by the crypto implementation. > > + * For example, the input packet for an IPSec ESP transport > > + * enabled session should be the clear text packet with > > + * no ESP headers/trailers prepared in advance for crypto operation. > > + * The output packet will have ESP header, IV, trailer and the ESP ICV > > + * added by crypto implementation. > > + * Depending on the particular capabilities of an implementation and > > + * the parameters enabled by application, the application may be > > + * partially or completely offloaded from IPSec protocol processing. > > + * For example, if an implementation does not support checksum > > + * update for IP header after adding ESP header the application > > + * should update after crypto IPSec operation. > > How a portable application knows what are the pending operations ? > [Alex] I assume your question is related to asynchronous mode. A crypto operation can have an associated user context which can be retrieved from the results when operation completed. Such context can contain information about what the application is supposed to do next on the packets depending on the type of the session (raw, ipsec, etc). > > > > + * > > + * If an implementation does not support a particular set of > > + * arguments it should return error. > > + * > > + * @param session Session handle > > + * @param ipsec_mode IPSec protocol mode > > + * @param ipsec_proto IPSec protocol > > + * @param ipsec_params IPSec parameters. Parameters which are > not > > + * relevant for selected protocol & mode are > ignored - > > + * e.g. outer_hdr/size set for ESP transport mode. > > + * @retval 0 on success > > + * @retval <0 on failure > > + */ > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, > > + enum odp_ipsec_mode ipsec_mode, > > + enum odp_ipsec_proto ipsec_proto, > > + odp_ipsec_params_t ipsec_params); > > + > > IMO, We should have reference implementation of ipsec protocol offload > implementation with normal crypto operations so that it can be re used in > the > platform which don't have platform offload > [Alex] Do you mean a version of odp_ipsec with ipsec offloading running and a linux-generic implementation of IPSec protocol? > > > > +/** > > + * @} > > + */ > > + > > +#ifdef __cplusplus > > +} > > +#endif > > + > > +#endif > > diff --git a/platform/linux-generic/include/odp/crypto.h > b/platform/linux-generic/include/odp/crypto.h > > index 7684c1e..718ab7d 100644 > > --- a/platform/linux-generic/include/odp/crypto.h > > +++ b/platform/linux-generic/include/odp/crypto.h > > @@ -20,6 +20,7 @@ extern "C" { > > #include <odp/std_types.h> > > #include <odp/plat/packet_types.h> > > #include <odp/plat/crypto_types.h> > > +#include <odp/plat/crypto_ipsec_types.h> > > #include <odp/plat/buffer_types.h> > > #include <odp/plat/pool_types.h> > > #include <odp/queue.h> > > @@ -33,6 +34,7 @@ extern "C" { > > */ > > > > #include <odp/api/crypto.h> > > +#include <odp/api/crypto_ipsec.h> > > > > #ifdef __cplusplus > > } > > diff --git > a/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > > new file mode 100644 > > index 0000000..74521da > > --- /dev/null > > +++ b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > > @@ -0,0 +1,53 @@ > > +/* Copyright (c) 2015, Linaro Limited > > + * All rights reserved. > > + * > > + * SPDX-License-Identifier: BSD-3-Clause > > + */ > > + > > +/** > > + * @file > > + * > > + * ODP crypto > > + */ > > + > > +#ifndef ODP_CRYPTO_IPSEC_TYPES_H_ > > +#define ODP_CRYPTO_IPSEC_TYPES_H_ > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +/** @addtogroup odp_crypto > > + * @{ > > + */ > > + > > +enum odp_ipsec_mode { > > + ODP_IPSEC_MODE_TUNNEL, /**< IPSec tunnel mode */ > > + ODP_IPSEC_MODE_TRANSPORT, /**< IPSec transport mode */ > > +}; > > + > > +enum odp_ipsec_proto { > > + ODP_IPSEC_ESP, /**< ESP protocol */ > > +}; > > + > > +enum odp_ipsec_outhdr_type { > > + ODP_IPSEC_OUTHDR_IPV4, /**< Outer header is IPv4 */ > > + ODP_IPSEC_OUTHDR_IPV6, /**< Outer header is IPv6 */ > > +}; > > + > > +enum odp_ipsec_ar_ws { > > + ODP_IPSEC_AR_WS_NONE, /**< Anti-replay is not enabled */ > > + ODP_IPSEC_AR_WS_32, /**< Anti-replay window size 32 */ > > + ODP_IPSEC_AR_WS_64, /**< Anti-replay window size 64 */ > > + ODP_IPSEC_AR_WS_128, /**< Anti-replay window size 128 */ > > +}; > > + > > +/** > > + * @} > > + */ > > + > > +#ifdef __cplusplus > > +} > > +#endif > > + > > +#endif > > -- > > 1.7.3.4 > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > https://lists.linaro.org/mailman/listinfo/lng-odp >
On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote: > On 30 July 2015 at 13:50, Stuart Haslam <stuart.haslam@linaro.org> wrote: > > > On Wed, Jul 22, 2015 at 11:26:03AM +0300, alexandru.badicioiu@linaro.org > > wrote: > > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > > > > > This patch adds IPSec protocol processing capabilities to crypto > > > sesssions. Implementations which have these capabilities in hardware > > > crypto engines can use the extension to offload the application from > > > IPSec protocol processing. > > > > > > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > > --- > > > include/odp/api/crypto_ipsec.h | 110 > > ++++++++++++++++++++ > > > platform/linux-generic/include/odp/crypto.h | 2 + > > > .../include/odp/plat/crypto_ipsec_types.h | 53 ++++++++++ > > > 3 files changed, 165 insertions(+), 0 deletions(-) > > > create mode 100644 include/odp/api/crypto_ipsec.h > > > create mode 100644 > > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > > > > > > diff --git a/include/odp/api/crypto_ipsec.h > > b/include/odp/api/crypto_ipsec.h > > > new file mode 100644 > > > index 0000000..e59fea4 > > > --- /dev/null > > > +++ b/include/odp/api/crypto_ipsec.h > > > @@ -0,0 +1,110 @@ > > > +/* Copyright (c) 2014, Linaro Limited > > > + * All rights reserved. > > > + * > > > + * SPDX-License-Identifier: BSD-3-Clause > > > + */ > > > + > > > +/** > > > + * @file > > > + * > > > + * ODP crypto IPSec extension > > > + */ > > > + > > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ > > > +#define ODP_API_CRYPTO_IPSEC_H_ > > > + > > > +#ifdef __cplusplus > > > +extern "C" { > > > +#endif > > > + > > > +/** > > > + * @enum odp_ipsec_outhdr_type > > > + * IPSec tunnel outer header type > > > + * > > > + * @enum odp_ipsec_ar_ws > > > + * IPSec Anti-replay window size > > > + * > > > + */ > > > + > > > +typedef struct odp_ipsec_params { > > > + uint32_t spi; /** SPI value */ > > > + uint32_t seq; /** Initial SEQ number */ > > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - > > > + inbound session with > > authentication */ > > > > This name is pretty cryptic, how about just replay_window? > > > [Alex] The standard name for this parameter is anti-replay window. In the > context of IPSec this should not be cryptic (odp_ipsec_arw vs odp_arw). If > your suggestion is to replace the name of the struct member - ar_ws with > replay_window I'm fine with it but not the name of the enum > (odp_ipsec_ar_ws). > Or maybe change it to enum odp_ipsec_arw. I meant the variable name, don't mind about the enum name. > > > > > + odp_bool_t esn; /** Use extended sequence numbers */ > > > + odp_bool_t auto_iv; /** Auto IV generation for each operation. > > */ > > > + uint16_t out_hdr_size; /** outer header size - tunnel mode */ > > > + uint8_t *out_hdr; /** outer header - tunnel mode */ > > > > Can these be 0 and NULL if the application wants tunnel mode but wants > > to handle the outer header itself? (i.e. add ESP head/tail and include > > inner IP header in encap data) > > > [Alex] Yes, that is the intended usage. If requested mode is tunnel and > there's no out_hdr specified, the application has to add it. > > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header type - > > > + tunnel mode */ > > > + odp_bool_t ip_csum; /** update/verify ip header checksum */ > > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode encap & > > decap */ > > > + odp_bool_t remove_outer_hdr; /** remove outer header - tunnel mode > > decap */ > > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 TOS or > > > + IPv6 Traffic Class byte from the > > inner/outer > > > + IP header to the outer/inner IP header > > - > > > + tunnel mode encap & decap */ > > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from > > > + the inner IP header to the > > > + outer IP header - tunnel mode encap */ > > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - tunnel > > mode */ > > > + odp_bool_t udp_csum; /** Update/verify UDP csum when NAT-T > > enabled */ > > > + > > > +} odp_ipsec_params_t; > > > + > > > +/** > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL > > > + * IPSec tunnel mode > > > + * > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT > > > + * IPSec transport mode > > > + * > > > + * @enum odp_ipsec_proto > > > + * IPSec protocol > > > + */ > > > + > > > +/** > > > + * Configure crypto session for IPsec processing > > > + * > > > + * Configures a crypto session for IPSec protocol processing. > > > + * Packets submitted to an IPSec enabled session will have > > > + * relevant IPSec headers/trailers and tunnel headers > > > + * added/removed by the crypto implementation. > > > + * For example, the input packet for an IPSec ESP transport > > > + * enabled session should be the clear text packet with > > > + * no ESP headers/trailers prepared in advance for crypto operation. > > > + * The output packet will have ESP header, IV, trailer and the ESP ICV > > > + * added by crypto implementation. > > > > If a packet fails a check on decap (e.g. out of window), presumably the > > application gets an odp_crypto_op_result_t with ok=false, but is there > > any way for it to tell what failed? > > [Alex] Crypto operation error status should be extended with a code for > > out of window condition. > > > + * Depending on the particular capabilities of an implementation and > > > + * the parameters enabled by application, the application may be > > > + * partially or completely offloaded from IPSec protocol processing. > > > + * For example, if an implementation does not support checksum > > > + * update for IP header after adding ESP header the application > > > + * should update after crypto IPSec operation. > > > + * > > > + * If an implementation does not support a particular set of > > > + * arguments it should return error. > > > + * > > > + * @param session Session handle > > > + * @param ipsec_mode IPSec protocol mode > > > + * @param ipsec_proto IPSec protocol > > > + * @param ipsec_params IPSec parameters. Parameters which are > > not > > > + * relevant for selected protocol & mode are > > ignored - > > > + * e.g. outer_hdr/size set for ESP transport mode. > > > + * @retval 0 on success > > > + * @retval <0 on failure > > > + */ > > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, > > > + enum odp_ipsec_mode ipsec_mode, > > > + enum odp_ipsec_proto ipsec_proto, > > > + odp_ipsec_params_t ipsec_params); > > > > Can this be called multiple times on the same session to update the > > parameters, or would the session need to be destroyed and recreated? > > > [Alex] No, this is not the intended use of this function. > This is to be called on a raw session (algorithm only). To change a crypto > session additional functions should be used. > > > > > Is it valid to create a session, issue a few operations, then later add > > the IPsec protocol config for that session? > > > [Alex] While this might work for a particular implementation/platform, I'm > wondering if there's an use case for it? > I wasn't thinking about a particular use case, there likely isn't one, just that how it's currently defined is open to misinterpretation/misuse. > > > > I'm wondering why the params here weren't just made an extension of the > > odp_crypto_session_params_t in the initial session create. > > > [Alex] Do you mean to add these parameters as members of > odp_crypto_session_params_t or to extend session_create call to take IPSec > params? The first. > It wouldn't be a good idea to embed the IPSec parameters into the > odp_crypto_session_params_t as IPSec is just a protocol among others > supported by crypto engines (SSL/TLS, MACSEC, SRTP, etc). It could be done with a union + enum for each protocol (or NONE) and if we add odp_crypto_session_params_init(), which we should have anyway, you wouldn't need to know those fields existed when creating non-protocol sessions. > I think having a separate call odp_crypto_session_config_ipsec() makes the > source code more readable regarding the intent of the application rather > than filling in lots of IPSec parameters and calling then session_create(). > If there's only one way of doing it right - protocol config/params are set at session create time and not modified - and a number of ways of getting it wrong then it makes sense to define the API such that it can only be done the right way.
On 30 July 2015 at 17:44, Stuart Haslam <stuart.haslam@linaro.org> wrote: > On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote: > > On 30 July 2015 at 13:50, Stuart Haslam <stuart.haslam@linaro.org> > wrote: > > > > > On Wed, Jul 22, 2015 at 11:26:03AM +0300, > alexandru.badicioiu@linaro.org > > > wrote: > > > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > > > > > > > This patch adds IPSec protocol processing capabilities to crypto > > > > sesssions. Implementations which have these capabilities in hardware > > > > crypto engines can use the extension to offload the application from > > > > IPSec protocol processing. > > > > > > > > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > > > --- > > > > include/odp/api/crypto_ipsec.h | 110 > > > ++++++++++++++++++++ > > > > platform/linux-generic/include/odp/crypto.h | 2 + > > > > .../include/odp/plat/crypto_ipsec_types.h | 53 ++++++++++ > > > > 3 files changed, 165 insertions(+), 0 deletions(-) > > > > create mode 100644 include/odp/api/crypto_ipsec.h > > > > create mode 100644 > > > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h > > > > > > > > diff --git a/include/odp/api/crypto_ipsec.h > > > b/include/odp/api/crypto_ipsec.h > > > > new file mode 100644 > > > > index 0000000..e59fea4 > > > > --- /dev/null > > > > +++ b/include/odp/api/crypto_ipsec.h > > > > @@ -0,0 +1,110 @@ > > > > +/* Copyright (c) 2014, Linaro Limited > > > > + * All rights reserved. > > > > + * > > > > + * SPDX-License-Identifier: BSD-3-Clause > > > > + */ > > > > + > > > > +/** > > > > + * @file > > > > + * > > > > + * ODP crypto IPSec extension > > > > + */ > > > > + > > > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ > > > > +#define ODP_API_CRYPTO_IPSEC_H_ > > > > + > > > > +#ifdef __cplusplus > > > > +extern "C" { > > > > +#endif > > > > + > > > > +/** > > > > + * @enum odp_ipsec_outhdr_type > > > > + * IPSec tunnel outer header type > > > > + * > > > > + * @enum odp_ipsec_ar_ws > > > > + * IPSec Anti-replay window size > > > > + * > > > > + */ > > > > + > > > > +typedef struct odp_ipsec_params { > > > > + uint32_t spi; /** SPI value */ > > > > + uint32_t seq; /** Initial SEQ number */ > > > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - > > > > + inbound session with > > > authentication */ > > > > > > This name is pretty cryptic, how about just replay_window? > > > > > [Alex] The standard name for this parameter is anti-replay window. In the > > context of IPSec this should not be cryptic (odp_ipsec_arw vs odp_arw). > If > > your suggestion is to replace the name of the struct member - ar_ws with > > replay_window I'm fine with it but not the name of the enum > > (odp_ipsec_ar_ws). > > Or maybe change it to enum odp_ipsec_arw. > > I meant the variable name, don't mind about the enum name. > [Alex] I'm OK with the change. > > > > > > > + odp_bool_t esn; /** Use extended sequence numbers */ > > > > + odp_bool_t auto_iv; /** Auto IV generation for each > operation. > > > */ > > > > + uint16_t out_hdr_size; /** outer header size - tunnel mode */ > > > > + uint8_t *out_hdr; /** outer header - tunnel mode */ > > > > > > Can these be 0 and NULL if the application wants tunnel mode but wants > > > to handle the outer header itself? (i.e. add ESP head/tail and include > > > inner IP header in encap data) > > > > > [Alex] Yes, that is the intended usage. If requested mode is tunnel and > > there's no out_hdr specified, the application has to add it. > > > > > > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header type - > > > > + tunnel mode */ > > > > + odp_bool_t ip_csum; /** update/verify ip header checksum */ > > > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode encap & > > > decap */ > > > > + odp_bool_t remove_outer_hdr; /** remove outer header - tunnel > mode > > > decap */ > > > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 TOS > or > > > > + IPv6 Traffic Class byte from the > > > inner/outer > > > > + IP header to the outer/inner IP > header > > > - > > > > + tunnel mode encap & decap */ > > > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from > > > > + the inner IP header to the > > > > + outer IP header - tunnel mode > encap */ > > > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - > tunnel > > > mode */ > > > > + odp_bool_t udp_csum; /** Update/verify UDP csum when NAT-T > > > enabled */ > > > > + > > > > +} odp_ipsec_params_t; > > > > + > > > > +/** > > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL > > > > + * IPSec tunnel mode > > > > + * > > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT > > > > + * IPSec transport mode > > > > + * > > > > + * @enum odp_ipsec_proto > > > > + * IPSec protocol > > > > + */ > > > > + > > > > +/** > > > > + * Configure crypto session for IPsec processing > > > > + * > > > > + * Configures a crypto session for IPSec protocol processing. > > > > + * Packets submitted to an IPSec enabled session will have > > > > + * relevant IPSec headers/trailers and tunnel headers > > > > + * added/removed by the crypto implementation. > > > > + * For example, the input packet for an IPSec ESP transport > > > > + * enabled session should be the clear text packet with > > > > + * no ESP headers/trailers prepared in advance for crypto operation. > > > > + * The output packet will have ESP header, IV, trailer and the ESP > ICV > > > > + * added by crypto implementation. > > > > > > If a packet fails a check on decap (e.g. out of window), presumably the > > > application gets an odp_crypto_op_result_t with ok=false, but is there > > > any way for it to tell what failed? > > > [Alex] Crypto operation error status should be extended with a code for > > > out of window condition. > > > > + * Depending on the particular capabilities of an implementation and > > > > + * the parameters enabled by application, the application may be > > > > + * partially or completely offloaded from IPSec protocol processing. > > > > + * For example, if an implementation does not support checksum > > > > + * update for IP header after adding ESP header the application > > > > + * should update after crypto IPSec operation. > > > > + * > > > > + * If an implementation does not support a particular set of > > > > + * arguments it should return error. > > > > + * > > > > + * @param session Session handle > > > > + * @param ipsec_mode IPSec protocol mode > > > > + * @param ipsec_proto IPSec protocol > > > > + * @param ipsec_params IPSec parameters. Parameters which > are > > > not > > > > + * relevant for selected protocol & mode are > > > ignored - > > > > + * e.g. outer_hdr/size set for ESP transport > mode. > > > > + * @retval 0 on success > > > > + * @retval <0 on failure > > > > + */ > > > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, > > > > + enum odp_ipsec_mode ipsec_mode, > > > > + enum odp_ipsec_proto ipsec_proto, > > > > + odp_ipsec_params_t ipsec_params); > > > > > > Can this be called multiple times on the same session to update the > > > parameters, or would the session need to be destroyed and recreated? > > > > > [Alex] No, this is not the intended use of this function. > > This is to be called on a raw session (algorithm only). To change a > crypto > > session additional functions should be used. > > > > > > > > Is it valid to create a session, issue a few operations, then later add > > > the IPsec protocol config for that session? > > > > > [Alex] While this might work for a particular implementation/platform, > I'm > > wondering if there's an use case for it? > > > > I wasn't thinking about a particular use case, there likely isn't one, > just that how it's currently defined is open to misinterpretation/misuse. > [Alex] As we have only one session create call the session must be usable right after creation. Being possible to extend the session for protocol processing after some traffic passed it's up to implementation - platform guys may give some inputs here. Other question can be - can we configure a session for protocol processing while traffic is passing? Again it may be possible for some and not for others. If there's no use-case for it then applications should not rely on this and config can return an error. Alternatively, we may need enable/disable calls for crypto sessions, similarly with pktio start/stop functions. Pktio start/stop functions are meant to clearly mark configuration phase - e.g. open pktio, config classification, start pktio. However, enable/disable calls could be required by IPSec rekeying process. > > > > > > I'm wondering why the params here weren't just made an extension of the > > > odp_crypto_session_params_t in the initial session create. > > > > > [Alex] Do you mean to add these parameters as members of > > odp_crypto_session_params_t or to extend session_create call to take > IPSec > > params? > > The first. > > > It wouldn't be a good idea to embed the IPSec parameters into the > > odp_crypto_session_params_t as IPSec is just a protocol among others > > supported by crypto engines (SSL/TLS, MACSEC, SRTP, etc). > > It could be done with a union + enum for each protocol (or NONE) and if > we add odp_crypto_session_params_init(), which we should have anyway, > you wouldn't need to know those fields existed when creating non-protocol > sessions. > [Alex] I still don't think is a good idea. An application using only raw sessions will waste memory with fields it actually doesn't use. Making a structure larger is not cache/performance friendly, software running on the cores access these structures on a per-packet basis. Crypto sessions can be potentially millions. > > > I think having a separate call odp_crypto_session_config_ipsec() makes > the > > source code more readable regarding the intent of the application rather > > than filling in lots of IPSec parameters and calling then > session_create(). > > > > If there's only one way of doing it right - protocol config/params are > set at session create time and not modified - and a number of ways of > getting it wrong then it makes sense to define the API such that it can > only be done the right way. > [Alex] I'm not sure it is possible to design an API __impossible__ to misuse - i.e. not possible to compile a program which misuses the API. I think that misuse rather should fail gracefully and with no side effects. There are other ODP API areas that can be easily misused - for example pktio - can we remove the default input queue after start function or while traffic is passing ? Can we remove the queue after sending a few packets but without stopping? Scheduling API requires calling pause before exiting a schedule loop - what happens if pause is not called? I think these are rather runtime aspects that should be handled at runtime. More than that, different platforms/implementation may behave differently, but a portable application should not be based on a particular behavior. -- > Stuart. >
Can this proposal be extended to handle inline IPsec processing, e.g. encrypt and encapsulate packet (include Ethernet framing) and then send to (enqueue) to some user-defined queue (which might be a pktio output queue)? Need some way to report errors and other conditions back to SW so might need some kind of ipsec notification event. Something for the ingress side as well, e.g. connect user-defined queues to IPsec input queue(s) and then specify corresponding output queues. On 31 July 2015 at 11:30, Alexandru Badicioiu < alexandru.badicioiu@linaro.org> wrote: > > > On 30 July 2015 at 17:44, Stuart Haslam <stuart.haslam@linaro.org> wrote: > >> On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote: >> > On 30 July 2015 at 13:50, Stuart Haslam <stuart.haslam@linaro.org> >> wrote: >> > >> > > On Wed, Jul 22, 2015 at 11:26:03AM +0300, >> alexandru.badicioiu@linaro.org >> > > wrote: >> > > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >> > > > >> > > > This patch adds IPSec protocol processing capabilities to crypto >> > > > sesssions. Implementations which have these capabilities in hardware >> > > > crypto engines can use the extension to offload the application from >> > > > IPSec protocol processing. >> > > > >> > > > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >> > > > --- >> > > > include/odp/api/crypto_ipsec.h | 110 >> > > ++++++++++++++++++++ >> > > > platform/linux-generic/include/odp/crypto.h | 2 + >> > > > .../include/odp/plat/crypto_ipsec_types.h | 53 >> ++++++++++ >> > > > 3 files changed, 165 insertions(+), 0 deletions(-) >> > > > create mode 100644 include/odp/api/crypto_ipsec.h >> > > > create mode 100644 >> > > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >> > > > >> > > > diff --git a/include/odp/api/crypto_ipsec.h >> > > b/include/odp/api/crypto_ipsec.h >> > > > new file mode 100644 >> > > > index 0000000..e59fea4 >> > > > --- /dev/null >> > > > +++ b/include/odp/api/crypto_ipsec.h >> > > > @@ -0,0 +1,110 @@ >> > > > +/* Copyright (c) 2014, Linaro Limited >> > > > + * All rights reserved. >> > > > + * >> > > > + * SPDX-License-Identifier: BSD-3-Clause >> > > > + */ >> > > > + >> > > > +/** >> > > > + * @file >> > > > + * >> > > > + * ODP crypto IPSec extension >> > > > + */ >> > > > + >> > > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ >> > > > +#define ODP_API_CRYPTO_IPSEC_H_ >> > > > + >> > > > +#ifdef __cplusplus >> > > > +extern "C" { >> > > > +#endif >> > > > + >> > > > +/** >> > > > + * @enum odp_ipsec_outhdr_type >> > > > + * IPSec tunnel outer header type >> > > > + * >> > > > + * @enum odp_ipsec_ar_ws >> > > > + * IPSec Anti-replay window size >> > > > + * >> > > > + */ >> > > > + >> > > > +typedef struct odp_ipsec_params { >> > > > + uint32_t spi; /** SPI value */ >> > > > + uint32_t seq; /** Initial SEQ number */ >> > > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - >> > > > + inbound session with >> > > authentication */ >> > > >> > > This name is pretty cryptic, how about just replay_window? >> > > >> > [Alex] The standard name for this parameter is anti-replay window. In >> the >> > context of IPSec this should not be cryptic (odp_ipsec_arw vs odp_arw). >> If >> > your suggestion is to replace the name of the struct member - ar_ws with >> > replay_window I'm fine with it but not the name of the enum >> > (odp_ipsec_ar_ws). >> > Or maybe change it to enum odp_ipsec_arw. >> >> I meant the variable name, don't mind about the enum name. >> [Alex] I'm OK with the change. >> >> > > >> > > > + odp_bool_t esn; /** Use extended sequence numbers */ >> > > > + odp_bool_t auto_iv; /** Auto IV generation for each >> operation. >> > > */ >> > > > + uint16_t out_hdr_size; /** outer header size - tunnel mode >> */ >> > > > + uint8_t *out_hdr; /** outer header - tunnel mode */ >> > > >> > > Can these be 0 and NULL if the application wants tunnel mode but wants >> > > to handle the outer header itself? (i.e. add ESP head/tail and include >> > > inner IP header in encap data) >> > > >> > [Alex] Yes, that is the intended usage. If requested mode is tunnel and >> > there's no out_hdr specified, the application has to add it. >> >> >> >> > > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header type >> - >> > > > + tunnel mode */ >> > > > + odp_bool_t ip_csum; /** update/verify ip header checksum >> */ >> > > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode encap >> & >> > > decap */ >> > > > + odp_bool_t remove_outer_hdr; /** remove outer header - tunnel >> mode >> > > decap */ >> > > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 TOS >> or >> > > > + IPv6 Traffic Class byte from the >> > > inner/outer >> > > > + IP header to the outer/inner IP >> header >> > > - >> > > > + tunnel mode encap & decap */ >> > > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from >> > > > + the inner IP header to the >> > > > + outer IP header - tunnel mode >> encap */ >> > > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - >> tunnel >> > > mode */ >> > > > + odp_bool_t udp_csum; /** Update/verify UDP csum when NAT-T >> > > enabled */ >> > > > + >> > > > +} odp_ipsec_params_t; >> > > > + >> > > > +/** >> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL >> > > > + * IPSec tunnel mode >> > > > + * >> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT >> > > > + * IPSec transport mode >> > > > + * >> > > > + * @enum odp_ipsec_proto >> > > > + * IPSec protocol >> > > > + */ >> > > > + >> > > > +/** >> > > > + * Configure crypto session for IPsec processing >> > > > + * >> > > > + * Configures a crypto session for IPSec protocol processing. >> > > > + * Packets submitted to an IPSec enabled session will have >> > > > + * relevant IPSec headers/trailers and tunnel headers >> > > > + * added/removed by the crypto implementation. >> > > > + * For example, the input packet for an IPSec ESP transport >> > > > + * enabled session should be the clear text packet with >> > > > + * no ESP headers/trailers prepared in advance for crypto >> operation. >> > > > + * The output packet will have ESP header, IV, trailer and the ESP >> ICV >> > > > + * added by crypto implementation. >> > > >> > > If a packet fails a check on decap (e.g. out of window), presumably >> the >> > > application gets an odp_crypto_op_result_t with ok=false, but is there >> > > any way for it to tell what failed? >> > > [Alex] Crypto operation error status should be extended with a code >> for >> > > out of window condition. >> > > > + * Depending on the particular capabilities of an implementation >> and >> > > > + * the parameters enabled by application, the application may be >> > > > + * partially or completely offloaded from IPSec protocol >> processing. >> > > > + * For example, if an implementation does not support checksum >> > > > + * update for IP header after adding ESP header the application >> > > > + * should update after crypto IPSec operation. >> > > > + * >> > > > + * If an implementation does not support a particular set of >> > > > + * arguments it should return error. >> > > > + * >> > > > + * @param session Session handle >> > > > + * @param ipsec_mode IPSec protocol mode >> > > > + * @param ipsec_proto IPSec protocol >> > > > + * @param ipsec_params IPSec parameters. Parameters >> which are >> > > not >> > > > + * relevant for selected protocol & mode are >> > > ignored - >> > > > + * e.g. outer_hdr/size set for ESP transport >> mode. >> > > > + * @retval 0 on success >> > > > + * @retval <0 on failure >> > > > + */ >> > > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, >> > > > + enum odp_ipsec_mode ipsec_mode, >> > > > + enum odp_ipsec_proto ipsec_proto, >> > > > + odp_ipsec_params_t ipsec_params); >> > > >> > > Can this be called multiple times on the same session to update the >> > > parameters, or would the session need to be destroyed and recreated? >> > > >> > [Alex] No, this is not the intended use of this function. >> > This is to be called on a raw session (algorithm only). To change a >> crypto >> > session additional functions should be used. >> > >> > > >> > > Is it valid to create a session, issue a few operations, then later >> add >> > > the IPsec protocol config for that session? >> > > >> > [Alex] While this might work for a particular implementation/platform, >> I'm >> > wondering if there's an use case for it? >> > >> >> I wasn't thinking about a particular use case, there likely isn't one, >> just that how it's currently defined is open to misinterpretation/misuse. >> > [Alex] As we have only one session create call the session must be usable > right after creation. Being possible to extend the session for protocol > processing after some traffic passed it's up to implementation - platform > guys may give some inputs here. Other question can be - can we configure a > session for protocol processing while traffic is passing? Again it may be > possible for some and not for others. If there's no use-case for it then > applications should not rely on this and config can return an error. > Alternatively, we may need enable/disable calls for crypto sessions, > similarly with pktio start/stop functions. Pktio start/stop functions are > meant to clearly mark configuration phase - e.g. open pktio, config > classification, start pktio. However, enable/disable calls could be > required by IPSec rekeying process. > > >> > > >> > > I'm wondering why the params here weren't just made an extension of >> the >> > > odp_crypto_session_params_t in the initial session create. >> > > >> > [Alex] Do you mean to add these parameters as members of >> > odp_crypto_session_params_t or to extend session_create call to take >> IPSec >> > params? >> >> The first. >> >> > It wouldn't be a good idea to embed the IPSec parameters into the >> > odp_crypto_session_params_t as IPSec is just a protocol among others >> > supported by crypto engines (SSL/TLS, MACSEC, SRTP, etc). >> >> It could be done with a union + enum for each protocol (or NONE) and if >> we add odp_crypto_session_params_init(), which we should have anyway, >> you wouldn't need to know those fields existed when creating non-protocol >> sessions. >> > [Alex] I still don't think is a good idea. An application using only raw > sessions will waste memory with fields it actually doesn't use. Making a > structure larger is not cache/performance friendly, software running on the > cores access these structures on a per-packet basis. Crypto sessions can > be potentially millions. > >> >> > I think having a separate call odp_crypto_session_config_ipsec() makes >> the >> > source code more readable regarding the intent of the application rather >> > than filling in lots of IPSec parameters and calling then >> session_create(). >> > >> >> If there's only one way of doing it right - protocol config/params are >> set at session create time and not modified - and a number of ways of >> getting it wrong then it makes sense to define the API such that it can >> only be done the right way. >> > [Alex] I'm not sure it is possible to design an API __impossible__ to > misuse - i.e. not possible to compile a program which misuses the API. I > think that misuse rather should fail gracefully and with no side effects. > There are other ODP API areas that can be easily misused - for example > pktio - can we remove the default input queue after start function or while > traffic is passing ? Can we remove the queue after sending a few packets > but without stopping? Scheduling API requires calling pause before exiting > a schedule loop - what happens if pause is not called? > I think these are rather runtime aspects that should be handled at > runtime. More than that, different platforms/implementation may behave > differently, but a portable application should not be based on a particular > behavior. > > > > > > > > > > -- >> Stuart. >> > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp > >
The problem you raised is not strictly related to this patch. A crypto session has an output queue (for async mode) where the results , including the operation status, will be delivered. There is nothing in the API to prevent using a pktio output queue for crypto completion events but the pktio should be able to process the event as the application would do. In this case an extension of pktio functionality would be required. Alex On 8 October 2015 at 20:13, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > Can this proposal be extended to handle inline IPsec processing, e.g. > encrypt and encapsulate packet (include Ethernet framing) and then send to > (enqueue) to some user-defined queue (which might be a pktio output queue)? > Need some way to report errors and other conditions back to SW so might > need some kind of ipsec notification event. > Something for the ingress side as well, e.g. connect user-defined queues > to IPsec input queue(s) and then specify corresponding output queues. > > > On 31 July 2015 at 11:30, Alexandru Badicioiu < > alexandru.badicioiu@linaro.org> wrote: > >> >> >> On 30 July 2015 at 17:44, Stuart Haslam <stuart.haslam@linaro.org> wrote: >> >>> On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote: >>> > On 30 July 2015 at 13:50, Stuart Haslam <stuart.haslam@linaro.org> >>> wrote: >>> > >>> > > On Wed, Jul 22, 2015 at 11:26:03AM +0300, >>> alexandru.badicioiu@linaro.org >>> > > wrote: >>> > > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >>> > > > >>> > > > This patch adds IPSec protocol processing capabilities to crypto >>> > > > sesssions. Implementations which have these capabilities in >>> hardware >>> > > > crypto engines can use the extension to offload the application >>> from >>> > > > IPSec protocol processing. >>> > > > >>> > > > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org >>> > >>> > > > --- >>> > > > include/odp/api/crypto_ipsec.h | 110 >>> > > ++++++++++++++++++++ >>> > > > platform/linux-generic/include/odp/crypto.h | 2 + >>> > > > .../include/odp/plat/crypto_ipsec_types.h | 53 >>> ++++++++++ >>> > > > 3 files changed, 165 insertions(+), 0 deletions(-) >>> > > > create mode 100644 include/odp/api/crypto_ipsec.h >>> > > > create mode 100644 >>> > > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >>> > > > >>> > > > diff --git a/include/odp/api/crypto_ipsec.h >>> > > b/include/odp/api/crypto_ipsec.h >>> > > > new file mode 100644 >>> > > > index 0000000..e59fea4 >>> > > > --- /dev/null >>> > > > +++ b/include/odp/api/crypto_ipsec.h >>> > > > @@ -0,0 +1,110 @@ >>> > > > +/* Copyright (c) 2014, Linaro Limited >>> > > > + * All rights reserved. >>> > > > + * >>> > > > + * SPDX-License-Identifier: BSD-3-Clause >>> > > > + */ >>> > > > + >>> > > > +/** >>> > > > + * @file >>> > > > + * >>> > > > + * ODP crypto IPSec extension >>> > > > + */ >>> > > > + >>> > > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ >>> > > > +#define ODP_API_CRYPTO_IPSEC_H_ >>> > > > + >>> > > > +#ifdef __cplusplus >>> > > > +extern "C" { >>> > > > +#endif >>> > > > + >>> > > > +/** >>> > > > + * @enum odp_ipsec_outhdr_type >>> > > > + * IPSec tunnel outer header type >>> > > > + * >>> > > > + * @enum odp_ipsec_ar_ws >>> > > > + * IPSec Anti-replay window size >>> > > > + * >>> > > > + */ >>> > > > + >>> > > > +typedef struct odp_ipsec_params { >>> > > > + uint32_t spi; /** SPI value */ >>> > > > + uint32_t seq; /** Initial SEQ number */ >>> > > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - >>> > > > + inbound session with >>> > > authentication */ >>> > > >>> > > This name is pretty cryptic, how about just replay_window? >>> > > >>> > [Alex] The standard name for this parameter is anti-replay window. In >>> the >>> > context of IPSec this should not be cryptic (odp_ipsec_arw vs >>> odp_arw). If >>> > your suggestion is to replace the name of the struct member - ar_ws >>> with >>> > replay_window I'm fine with it but not the name of the enum >>> > (odp_ipsec_ar_ws). >>> > Or maybe change it to enum odp_ipsec_arw. >>> >>> I meant the variable name, don't mind about the enum name. >>> [Alex] I'm OK with the change. >>> >>> > > >>> > > > + odp_bool_t esn; /** Use extended sequence numbers */ >>> > > > + odp_bool_t auto_iv; /** Auto IV generation for each >>> operation. >>> > > */ >>> > > > + uint16_t out_hdr_size; /** outer header size - tunnel mode >>> */ >>> > > > + uint8_t *out_hdr; /** outer header - tunnel mode */ >>> > > >>> > > Can these be 0 and NULL if the application wants tunnel mode but >>> wants >>> > > to handle the outer header itself? (i.e. add ESP head/tail and >>> include >>> > > inner IP header in encap data) >>> > > >>> > [Alex] Yes, that is the intended usage. If requested mode is tunnel and >>> > there's no out_hdr specified, the application has to add it. >>> >>> >>> >>> > > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header >>> type - >>> > > > + tunnel mode */ >>> > > > + odp_bool_t ip_csum; /** update/verify ip header checksum >>> */ >>> > > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode >>> encap & >>> > > decap */ >>> > > > + odp_bool_t remove_outer_hdr; /** remove outer header - >>> tunnel mode >>> > > decap */ >>> > > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 >>> TOS or >>> > > > + IPv6 Traffic Class byte from the >>> > > inner/outer >>> > > > + IP header to the outer/inner IP >>> header >>> > > - >>> > > > + tunnel mode encap & decap */ >>> > > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit >>> from >>> > > > + the inner IP header to the >>> > > > + outer IP header - tunnel mode >>> encap */ >>> > > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - >>> tunnel >>> > > mode */ >>> > > > + odp_bool_t udp_csum; /** Update/verify UDP csum when NAT-T >>> > > enabled */ >>> > > > + >>> > > > +} odp_ipsec_params_t; >>> > > > + >>> > > > +/** >>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL >>> > > > + * IPSec tunnel mode >>> > > > + * >>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT >>> > > > + * IPSec transport mode >>> > > > + * >>> > > > + * @enum odp_ipsec_proto >>> > > > + * IPSec protocol >>> > > > + */ >>> > > > + >>> > > > +/** >>> > > > + * Configure crypto session for IPsec processing >>> > > > + * >>> > > > + * Configures a crypto session for IPSec protocol processing. >>> > > > + * Packets submitted to an IPSec enabled session will have >>> > > > + * relevant IPSec headers/trailers and tunnel headers >>> > > > + * added/removed by the crypto implementation. >>> > > > + * For example, the input packet for an IPSec ESP transport >>> > > > + * enabled session should be the clear text packet with >>> > > > + * no ESP headers/trailers prepared in advance for crypto >>> operation. >>> > > > + * The output packet will have ESP header, IV, trailer and the >>> ESP ICV >>> > > > + * added by crypto implementation. >>> > > >>> > > If a packet fails a check on decap (e.g. out of window), presumably >>> the >>> > > application gets an odp_crypto_op_result_t with ok=false, but is >>> there >>> > > any way for it to tell what failed? >>> > > [Alex] Crypto operation error status should be extended with a code >>> for >>> > > out of window condition. >>> > > > + * Depending on the particular capabilities of an implementation >>> and >>> > > > + * the parameters enabled by application, the application may be >>> > > > + * partially or completely offloaded from IPSec protocol >>> processing. >>> > > > + * For example, if an implementation does not support checksum >>> > > > + * update for IP header after adding ESP header the application >>> > > > + * should update after crypto IPSec operation. >>> > > > + * >>> > > > + * If an implementation does not support a particular set of >>> > > > + * arguments it should return error. >>> > > > + * >>> > > > + * @param session Session handle >>> > > > + * @param ipsec_mode IPSec protocol mode >>> > > > + * @param ipsec_proto IPSec protocol >>> > > > + * @param ipsec_params IPSec parameters. Parameters >>> which are >>> > > not >>> > > > + * relevant for selected protocol & mode are >>> > > ignored - >>> > > > + * e.g. outer_hdr/size set for ESP >>> transport mode. >>> > > > + * @retval 0 on success >>> > > > + * @retval <0 on failure >>> > > > + */ >>> > > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, >>> > > > + enum odp_ipsec_mode ipsec_mode, >>> > > > + enum odp_ipsec_proto ipsec_proto, >>> > > > + odp_ipsec_params_t ipsec_params); >>> > > >>> > > Can this be called multiple times on the same session to update the >>> > > parameters, or would the session need to be destroyed and recreated? >>> > > >>> > [Alex] No, this is not the intended use of this function. >>> > This is to be called on a raw session (algorithm only). To change a >>> crypto >>> > session additional functions should be used. >>> > >>> > > >>> > > Is it valid to create a session, issue a few operations, then later >>> add >>> > > the IPsec protocol config for that session? >>> > > >>> > [Alex] While this might work for a particular >>> implementation/platform, I'm >>> > wondering if there's an use case for it? >>> > >>> >>> I wasn't thinking about a particular use case, there likely isn't one, >>> just that how it's currently defined is open to misinterpretation/misuse. >>> >> [Alex] As we have only one session create call the session must be usable >> right after creation. Being possible to extend the session for protocol >> processing after some traffic passed it's up to implementation - platform >> guys may give some inputs here. Other question can be - can we configure a >> session for protocol processing while traffic is passing? Again it may be >> possible for some and not for others. If there's no use-case for it then >> applications should not rely on this and config can return an error. >> Alternatively, we may need enable/disable calls for crypto sessions, >> similarly with pktio start/stop functions. Pktio start/stop functions are >> meant to clearly mark configuration phase - e.g. open pktio, config >> classification, start pktio. However, enable/disable calls could be >> required by IPSec rekeying process. >> >> >>> > > >>> > > I'm wondering why the params here weren't just made an extension of >>> the >>> > > odp_crypto_session_params_t in the initial session create. >>> > > >>> > [Alex] Do you mean to add these parameters as members of >>> > odp_crypto_session_params_t or to extend session_create call to take >>> IPSec >>> > params? >>> >>> The first. >>> >>> > It wouldn't be a good idea to embed the IPSec parameters into the >>> > odp_crypto_session_params_t as IPSec is just a protocol among others >>> > supported by crypto engines (SSL/TLS, MACSEC, SRTP, etc). >>> >>> It could be done with a union + enum for each protocol (or NONE) and if >>> we add odp_crypto_session_params_init(), which we should have anyway, >>> you wouldn't need to know those fields existed when creating non-protocol >>> sessions. >>> >> [Alex] I still don't think is a good idea. An application using only raw >> sessions will waste memory with fields it actually doesn't use. Making a >> structure larger is not cache/performance friendly, software running on the >> cores access these structures on a per-packet basis. Crypto sessions can >> be potentially millions. >> >>> >>> > I think having a separate call odp_crypto_session_config_ipsec() >>> makes the >>> > source code more readable regarding the intent of the application >>> rather >>> > than filling in lots of IPSec parameters and calling then >>> session_create(). >>> > >>> >>> If there's only one way of doing it right - protocol config/params are >>> set at session create time and not modified - and a number of ways of >>> getting it wrong then it makes sense to define the API such that it can >>> only be done the right way. >>> >> [Alex] I'm not sure it is possible to design an API __impossible__ to >> misuse - i.e. not possible to compile a program which misuses the API. I >> think that misuse rather should fail gracefully and with no side effects. >> There are other ODP API areas that can be easily misused - for example >> pktio - can we remove the default input queue after start function or while >> traffic is passing ? Can we remove the queue after sending a few packets >> but without stopping? Scheduling API requires calling pause before exiting >> a schedule loop - what happens if pause is not called? >> I think these are rather runtime aspects that should be handled at >> runtime. More than that, different platforms/implementation may behave >> differently, but a portable application should not be based on a particular >> behavior. >> >> >> >> >> >> >> >> >> >> -- >>> Stuart. >>> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >
On 9 October 2015 at 09:34, Alexandru Badicioiu < alexandru.badicioiu@linaro.org> wrote: > The problem you raised is not strictly related to this patch. > A crypto session has an output queue (for async mode) where the results , > including the operation status, will be delivered. There is nothing in the > API to prevent using a pktio output queue for crypto completion events but > the pktio should be able to process the event as the application would do. > In this case an extension of pktio functionality would be required. > Yes but I was thinking of some change (in API and implementation) where the output of the crypto operations is the actual packet (with L2/L3 encapsulation), not a crypto completion event. > > Alex > > > On 8 October 2015 at 20:13, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > >> Can this proposal be extended to handle inline IPsec processing, e.g. >> encrypt and encapsulate packet (include Ethernet framing) and then send to >> (enqueue) to some user-defined queue (which might be a pktio output queue)? >> Need some way to report errors and other conditions back to SW so might >> need some kind of ipsec notification event. >> Something for the ingress side as well, e.g. connect user-defined queues >> to IPsec input queue(s) and then specify corresponding output queues. >> >> >> On 31 July 2015 at 11:30, Alexandru Badicioiu < >> alexandru.badicioiu@linaro.org> wrote: >> >>> >>> >>> On 30 July 2015 at 17:44, Stuart Haslam <stuart.haslam@linaro.org> >>> wrote: >>> >>>> On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote: >>>> > On 30 July 2015 at 13:50, Stuart Haslam <stuart.haslam@linaro.org> >>>> wrote: >>>> > >>>> > > On Wed, Jul 22, 2015 at 11:26:03AM +0300, >>>> alexandru.badicioiu@linaro.org >>>> > > wrote: >>>> > > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >>>> > > > >>>> > > > This patch adds IPSec protocol processing capabilities to crypto >>>> > > > sesssions. Implementations which have these capabilities in >>>> hardware >>>> > > > crypto engines can use the extension to offload the application >>>> from >>>> > > > IPSec protocol processing. >>>> > > > >>>> > > > Signed-off-by: Alexandru Badicioiu < >>>> alexandru.badicioiu@linaro.org> >>>> > > > --- >>>> > > > include/odp/api/crypto_ipsec.h | 110 >>>> > > ++++++++++++++++++++ >>>> > > > platform/linux-generic/include/odp/crypto.h | 2 + >>>> > > > .../include/odp/plat/crypto_ipsec_types.h | 53 >>>> ++++++++++ >>>> > > > 3 files changed, 165 insertions(+), 0 deletions(-) >>>> > > > create mode 100644 include/odp/api/crypto_ipsec.h >>>> > > > create mode 100644 >>>> > > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >>>> > > > >>>> > > > diff --git a/include/odp/api/crypto_ipsec.h >>>> > > b/include/odp/api/crypto_ipsec.h >>>> > > > new file mode 100644 >>>> > > > index 0000000..e59fea4 >>>> > > > --- /dev/null >>>> > > > +++ b/include/odp/api/crypto_ipsec.h >>>> > > > @@ -0,0 +1,110 @@ >>>> > > > +/* Copyright (c) 2014, Linaro Limited >>>> > > > + * All rights reserved. >>>> > > > + * >>>> > > > + * SPDX-License-Identifier: BSD-3-Clause >>>> > > > + */ >>>> > > > + >>>> > > > +/** >>>> > > > + * @file >>>> > > > + * >>>> > > > + * ODP crypto IPSec extension >>>> > > > + */ >>>> > > > + >>>> > > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ >>>> > > > +#define ODP_API_CRYPTO_IPSEC_H_ >>>> > > > + >>>> > > > +#ifdef __cplusplus >>>> > > > +extern "C" { >>>> > > > +#endif >>>> > > > + >>>> > > > +/** >>>> > > > + * @enum odp_ipsec_outhdr_type >>>> > > > + * IPSec tunnel outer header type >>>> > > > + * >>>> > > > + * @enum odp_ipsec_ar_ws >>>> > > > + * IPSec Anti-replay window size >>>> > > > + * >>>> > > > + */ >>>> > > > + >>>> > > > +typedef struct odp_ipsec_params { >>>> > > > + uint32_t spi; /** SPI value */ >>>> > > > + uint32_t seq; /** Initial SEQ number */ >>>> > > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - >>>> > > > + inbound session with >>>> > > authentication */ >>>> > > >>>> > > This name is pretty cryptic, how about just replay_window? >>>> > > >>>> > [Alex] The standard name for this parameter is anti-replay window. In >>>> the >>>> > context of IPSec this should not be cryptic (odp_ipsec_arw vs >>>> odp_arw). If >>>> > your suggestion is to replace the name of the struct member - ar_ws >>>> with >>>> > replay_window I'm fine with it but not the name of the enum >>>> > (odp_ipsec_ar_ws). >>>> > Or maybe change it to enum odp_ipsec_arw. >>>> >>>> I meant the variable name, don't mind about the enum name. >>>> [Alex] I'm OK with the change. >>>> >>>> > > >>>> > > > + odp_bool_t esn; /** Use extended sequence numbers */ >>>> > > > + odp_bool_t auto_iv; /** Auto IV generation for each >>>> operation. >>>> > > */ >>>> > > > + uint16_t out_hdr_size; /** outer header size - tunnel >>>> mode */ >>>> > > > + uint8_t *out_hdr; /** outer header - tunnel mode */ >>>> > > >>>> > > Can these be 0 and NULL if the application wants tunnel mode but >>>> wants >>>> > > to handle the outer header itself? (i.e. add ESP head/tail and >>>> include >>>> > > inner IP header in encap data) >>>> > > >>>> > [Alex] Yes, that is the intended usage. If requested mode is tunnel >>>> and >>>> > there's no out_hdr specified, the application has to add it. >>>> >>>> >>>> >>>> > > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header >>>> type - >>>> > > > + tunnel mode */ >>>> > > > + odp_bool_t ip_csum; /** update/verify ip header >>>> checksum */ >>>> > > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode >>>> encap & >>>> > > decap */ >>>> > > > + odp_bool_t remove_outer_hdr; /** remove outer header - >>>> tunnel mode >>>> > > decap */ >>>> > > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 >>>> TOS or >>>> > > > + IPv6 Traffic Class byte from the >>>> > > inner/outer >>>> > > > + IP header to the outer/inner IP >>>> header >>>> > > - >>>> > > > + tunnel mode encap & decap */ >>>> > > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit >>>> from >>>> > > > + the inner IP header to the >>>> > > > + outer IP header - tunnel mode >>>> encap */ >>>> > > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - >>>> tunnel >>>> > > mode */ >>>> > > > + odp_bool_t udp_csum; /** Update/verify UDP csum when >>>> NAT-T >>>> > > enabled */ >>>> > > > + >>>> > > > +} odp_ipsec_params_t; >>>> > > > + >>>> > > > +/** >>>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL >>>> > > > + * IPSec tunnel mode >>>> > > > + * >>>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT >>>> > > > + * IPSec transport mode >>>> > > > + * >>>> > > > + * @enum odp_ipsec_proto >>>> > > > + * IPSec protocol >>>> > > > + */ >>>> > > > + >>>> > > > +/** >>>> > > > + * Configure crypto session for IPsec processing >>>> > > > + * >>>> > > > + * Configures a crypto session for IPSec protocol processing. >>>> > > > + * Packets submitted to an IPSec enabled session will have >>>> > > > + * relevant IPSec headers/trailers and tunnel headers >>>> > > > + * added/removed by the crypto implementation. >>>> > > > + * For example, the input packet for an IPSec ESP transport >>>> > > > + * enabled session should be the clear text packet with >>>> > > > + * no ESP headers/trailers prepared in advance for crypto >>>> operation. >>>> > > > + * The output packet will have ESP header, IV, trailer and the >>>> ESP ICV >>>> > > > + * added by crypto implementation. >>>> > > >>>> > > If a packet fails a check on decap (e.g. out of window), presumably >>>> the >>>> > > application gets an odp_crypto_op_result_t with ok=false, but is >>>> there >>>> > > any way for it to tell what failed? >>>> > > [Alex] Crypto operation error status should be extended with a code >>>> for >>>> > > out of window condition. >>>> > > > + * Depending on the particular capabilities of an implementation >>>> and >>>> > > > + * the parameters enabled by application, the application may be >>>> > > > + * partially or completely offloaded from IPSec protocol >>>> processing. >>>> > > > + * For example, if an implementation does not support checksum >>>> > > > + * update for IP header after adding ESP header the application >>>> > > > + * should update after crypto IPSec operation. >>>> > > > + * >>>> > > > + * If an implementation does not support a particular set of >>>> > > > + * arguments it should return error. >>>> > > > + * >>>> > > > + * @param session Session handle >>>> > > > + * @param ipsec_mode IPSec protocol mode >>>> > > > + * @param ipsec_proto IPSec protocol >>>> > > > + * @param ipsec_params IPSec parameters. Parameters >>>> which are >>>> > > not >>>> > > > + * relevant for selected protocol & mode >>>> are >>>> > > ignored - >>>> > > > + * e.g. outer_hdr/size set for ESP >>>> transport mode. >>>> > > > + * @retval 0 on success >>>> > > > + * @retval <0 on failure >>>> > > > + */ >>>> > > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, >>>> > > > + enum odp_ipsec_mode ipsec_mode, >>>> > > > + enum odp_ipsec_proto >>>> ipsec_proto, >>>> > > > + odp_ipsec_params_t >>>> ipsec_params); >>>> > > >>>> > > Can this be called multiple times on the same session to update the >>>> > > parameters, or would the session need to be destroyed and recreated? >>>> > > >>>> > [Alex] No, this is not the intended use of this function. >>>> > This is to be called on a raw session (algorithm only). To change a >>>> crypto >>>> > session additional functions should be used. >>>> > >>>> > > >>>> > > Is it valid to create a session, issue a few operations, then later >>>> add >>>> > > the IPsec protocol config for that session? >>>> > > >>>> > [Alex] While this might work for a particular >>>> implementation/platform, I'm >>>> > wondering if there's an use case for it? >>>> > >>>> >>>> I wasn't thinking about a particular use case, there likely isn't one, >>>> just that how it's currently defined is open to >>>> misinterpretation/misuse. >>>> >>> [Alex] As we have only one session create call the session must be >>> usable right after creation. Being possible to extend the session for >>> protocol processing after some traffic passed it's up to implementation - >>> platform guys may give some inputs here. Other question can be - can we >>> configure a session for protocol processing while traffic is passing? Again >>> it may be possible for some and not for others. If there's no use-case for >>> it then applications should not rely on this and config can return an error. >>> Alternatively, we may need enable/disable calls for crypto sessions, >>> similarly with pktio start/stop functions. Pktio start/stop functions are >>> meant to clearly mark configuration phase - e.g. open pktio, config >>> classification, start pktio. However, enable/disable calls could be >>> required by IPSec rekeying process. >>> >>> >>>> > > >>>> > > I'm wondering why the params here weren't just made an extension of >>>> the >>>> > > odp_crypto_session_params_t in the initial session create. >>>> > > >>>> > [Alex] Do you mean to add these parameters as members of >>>> > odp_crypto_session_params_t or to extend session_create call to take >>>> IPSec >>>> > params? >>>> >>>> The first. >>>> >>>> > It wouldn't be a good idea to embed the IPSec parameters into the >>>> > odp_crypto_session_params_t as IPSec is just a protocol among others >>>> > supported by crypto engines (SSL/TLS, MACSEC, SRTP, etc). >>>> >>>> It could be done with a union + enum for each protocol (or NONE) and if >>>> we add odp_crypto_session_params_init(), which we should have anyway, >>>> you wouldn't need to know those fields existed when creating >>>> non-protocol >>>> sessions. >>>> >>> [Alex] I still don't think is a good idea. An application using only raw >>> sessions will waste memory with fields it actually doesn't use. Making a >>> structure larger is not cache/performance friendly, software running on the >>> cores access these structures on a per-packet basis. Crypto sessions can >>> be potentially millions. >>> >>>> >>>> > I think having a separate call odp_crypto_session_config_ipsec() >>>> makes the >>>> > source code more readable regarding the intent of the application >>>> rather >>>> > than filling in lots of IPSec parameters and calling then >>>> session_create(). >>>> > >>>> >>>> If there's only one way of doing it right - protocol config/params are >>>> set at session create time and not modified - and a number of ways of >>>> getting it wrong then it makes sense to define the API such that it can >>>> only be done the right way. >>>> >>> [Alex] I'm not sure it is possible to design an API __impossible__ to >>> misuse - i.e. not possible to compile a program which misuses the API. I >>> think that misuse rather should fail gracefully and with no side effects. >>> There are other ODP API areas that can be easily misused - for example >>> pktio - can we remove the default input queue after start function or while >>> traffic is passing ? Can we remove the queue after sending a few packets >>> but without stopping? Scheduling API requires calling pause before exiting >>> a schedule loop - what happens if pause is not called? >>> I think these are rather runtime aspects that should be handled at >>> runtime. More than that, different platforms/implementation may behave >>> differently, but a portable application should not be based on a particular >>> behavior. >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> -- >>>> Stuart. >>>> >>> >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> https://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >> >
This was a long discussion some time ago and the result was that crypto output should be abstracted in the form of the completion event and access functions would retrieve status and output packets. Also the implementation is in charge with crypto completion event allocation and not the application even if this is not really supported in HW. The previous approach was that application supplied the completion event as being the output packet but this was regarded as a hack. Alex On 9 October 2015 at 11:28, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > On 9 October 2015 at 09:34, Alexandru Badicioiu < > alexandru.badicioiu@linaro.org> wrote: > >> The problem you raised is not strictly related to this patch. >> A crypto session has an output queue (for async mode) where the results , >> including the operation status, will be delivered. There is nothing in the >> API to prevent using a pktio output queue for crypto completion events but >> the pktio should be able to process the event as the application would do. >> In this case an extension of pktio functionality would be required. >> > Yes but I was thinking of some change (in API and implementation) where > the output of the crypto operations is the actual packet (with L2/L3 > encapsulation), not a crypto completion event. > > >> >> Alex >> >> >> On 8 October 2015 at 20:13, Ola Liljedahl <ola.liljedahl@linaro.org> >> wrote: >> >>> Can this proposal be extended to handle inline IPsec processing, e.g. >>> encrypt and encapsulate packet (include Ethernet framing) and then send to >>> (enqueue) to some user-defined queue (which might be a pktio output queue)? >>> Need some way to report errors and other conditions back to SW so might >>> need some kind of ipsec notification event. >>> Something for the ingress side as well, e.g. connect user-defined queues >>> to IPsec input queue(s) and then specify corresponding output queues. >>> >>> >>> On 31 July 2015 at 11:30, Alexandru Badicioiu < >>> alexandru.badicioiu@linaro.org> wrote: >>> >>>> >>>> >>>> On 30 July 2015 at 17:44, Stuart Haslam <stuart.haslam@linaro.org> >>>> wrote: >>>> >>>>> On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote: >>>>> > On 30 July 2015 at 13:50, Stuart Haslam <stuart.haslam@linaro.org> >>>>> wrote: >>>>> > >>>>> > > On Wed, Jul 22, 2015 at 11:26:03AM +0300, >>>>> alexandru.badicioiu@linaro.org >>>>> > > wrote: >>>>> > > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >>>>> > > > >>>>> > > > This patch adds IPSec protocol processing capabilities to crypto >>>>> > > > sesssions. Implementations which have these capabilities in >>>>> hardware >>>>> > > > crypto engines can use the extension to offload the application >>>>> from >>>>> > > > IPSec protocol processing. >>>>> > > > >>>>> > > > Signed-off-by: Alexandru Badicioiu < >>>>> alexandru.badicioiu@linaro.org> >>>>> > > > --- >>>>> > > > include/odp/api/crypto_ipsec.h | 110 >>>>> > > ++++++++++++++++++++ >>>>> > > > platform/linux-generic/include/odp/crypto.h | 2 + >>>>> > > > .../include/odp/plat/crypto_ipsec_types.h | 53 >>>>> ++++++++++ >>>>> > > > 3 files changed, 165 insertions(+), 0 deletions(-) >>>>> > > > create mode 100644 include/odp/api/crypto_ipsec.h >>>>> > > > create mode 100644 >>>>> > > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >>>>> > > > >>>>> > > > diff --git a/include/odp/api/crypto_ipsec.h >>>>> > > b/include/odp/api/crypto_ipsec.h >>>>> > > > new file mode 100644 >>>>> > > > index 0000000..e59fea4 >>>>> > > > --- /dev/null >>>>> > > > +++ b/include/odp/api/crypto_ipsec.h >>>>> > > > @@ -0,0 +1,110 @@ >>>>> > > > +/* Copyright (c) 2014, Linaro Limited >>>>> > > > + * All rights reserved. >>>>> > > > + * >>>>> > > > + * SPDX-License-Identifier: BSD-3-Clause >>>>> > > > + */ >>>>> > > > + >>>>> > > > +/** >>>>> > > > + * @file >>>>> > > > + * >>>>> > > > + * ODP crypto IPSec extension >>>>> > > > + */ >>>>> > > > + >>>>> > > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ >>>>> > > > +#define ODP_API_CRYPTO_IPSEC_H_ >>>>> > > > + >>>>> > > > +#ifdef __cplusplus >>>>> > > > +extern "C" { >>>>> > > > +#endif >>>>> > > > + >>>>> > > > +/** >>>>> > > > + * @enum odp_ipsec_outhdr_type >>>>> > > > + * IPSec tunnel outer header type >>>>> > > > + * >>>>> > > > + * @enum odp_ipsec_ar_ws >>>>> > > > + * IPSec Anti-replay window size >>>>> > > > + * >>>>> > > > + */ >>>>> > > > + >>>>> > > > +typedef struct odp_ipsec_params { >>>>> > > > + uint32_t spi; /** SPI value */ >>>>> > > > + uint32_t seq; /** Initial SEQ number */ >>>>> > > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - >>>>> > > > + inbound session with >>>>> > > authentication */ >>>>> > > >>>>> > > This name is pretty cryptic, how about just replay_window? >>>>> > > >>>>> > [Alex] The standard name for this parameter is anti-replay window. >>>>> In the >>>>> > context of IPSec this should not be cryptic (odp_ipsec_arw vs >>>>> odp_arw). If >>>>> > your suggestion is to replace the name of the struct member - ar_ws >>>>> with >>>>> > replay_window I'm fine with it but not the name of the enum >>>>> > (odp_ipsec_ar_ws). >>>>> > Or maybe change it to enum odp_ipsec_arw. >>>>> >>>>> I meant the variable name, don't mind about the enum name. >>>>> [Alex] I'm OK with the change. >>>>> >>>>> > > >>>>> > > > + odp_bool_t esn; /** Use extended sequence numbers >>>>> */ >>>>> > > > + odp_bool_t auto_iv; /** Auto IV generation for each >>>>> operation. >>>>> > > */ >>>>> > > > + uint16_t out_hdr_size; /** outer header size - tunnel >>>>> mode */ >>>>> > > > + uint8_t *out_hdr; /** outer header - tunnel mode */ >>>>> > > >>>>> > > Can these be 0 and NULL if the application wants tunnel mode but >>>>> wants >>>>> > > to handle the outer header itself? (i.e. add ESP head/tail and >>>>> include >>>>> > > inner IP header in encap data) >>>>> > > >>>>> > [Alex] Yes, that is the intended usage. If requested mode is tunnel >>>>> and >>>>> > there's no out_hdr specified, the application has to add it. >>>>> >>>>> >>>>> >>>>> > > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header >>>>> type - >>>>> > > > + tunnel mode */ >>>>> > > > + odp_bool_t ip_csum; /** update/verify ip header >>>>> checksum */ >>>>> > > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode >>>>> encap & >>>>> > > decap */ >>>>> > > > + odp_bool_t remove_outer_hdr; /** remove outer header - >>>>> tunnel mode >>>>> > > decap */ >>>>> > > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 >>>>> TOS or >>>>> > > > + IPv6 Traffic Class byte from >>>>> the >>>>> > > inner/outer >>>>> > > > + IP header to the outer/inner >>>>> IP header >>>>> > > - >>>>> > > > + tunnel mode encap & decap */ >>>>> > > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit >>>>> from >>>>> > > > + the inner IP header to the >>>>> > > > + outer IP header - tunnel mode >>>>> encap */ >>>>> > > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - >>>>> tunnel >>>>> > > mode */ >>>>> > > > + odp_bool_t udp_csum; /** Update/verify UDP csum when >>>>> NAT-T >>>>> > > enabled */ >>>>> > > > + >>>>> > > > +} odp_ipsec_params_t; >>>>> > > > + >>>>> > > > +/** >>>>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL >>>>> > > > + * IPSec tunnel mode >>>>> > > > + * >>>>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT >>>>> > > > + * IPSec transport mode >>>>> > > > + * >>>>> > > > + * @enum odp_ipsec_proto >>>>> > > > + * IPSec protocol >>>>> > > > + */ >>>>> > > > + >>>>> > > > +/** >>>>> > > > + * Configure crypto session for IPsec processing >>>>> > > > + * >>>>> > > > + * Configures a crypto session for IPSec protocol processing. >>>>> > > > + * Packets submitted to an IPSec enabled session will have >>>>> > > > + * relevant IPSec headers/trailers and tunnel headers >>>>> > > > + * added/removed by the crypto implementation. >>>>> > > > + * For example, the input packet for an IPSec ESP transport >>>>> > > > + * enabled session should be the clear text packet with >>>>> > > > + * no ESP headers/trailers prepared in advance for crypto >>>>> operation. >>>>> > > > + * The output packet will have ESP header, IV, trailer and the >>>>> ESP ICV >>>>> > > > + * added by crypto implementation. >>>>> > > >>>>> > > If a packet fails a check on decap (e.g. out of window), >>>>> presumably the >>>>> > > application gets an odp_crypto_op_result_t with ok=false, but is >>>>> there >>>>> > > any way for it to tell what failed? >>>>> > > [Alex] Crypto operation error status should be extended with a >>>>> code for >>>>> > > out of window condition. >>>>> > > > + * Depending on the particular capabilities of an >>>>> implementation and >>>>> > > > + * the parameters enabled by application, the application may be >>>>> > > > + * partially or completely offloaded from IPSec protocol >>>>> processing. >>>>> > > > + * For example, if an implementation does not support checksum >>>>> > > > + * update for IP header after adding ESP header the application >>>>> > > > + * should update after crypto IPSec operation. >>>>> > > > + * >>>>> > > > + * If an implementation does not support a particular set of >>>>> > > > + * arguments it should return error. >>>>> > > > + * >>>>> > > > + * @param session Session handle >>>>> > > > + * @param ipsec_mode IPSec protocol mode >>>>> > > > + * @param ipsec_proto IPSec protocol >>>>> > > > + * @param ipsec_params IPSec parameters. Parameters >>>>> which are >>>>> > > not >>>>> > > > + * relevant for selected protocol & mode >>>>> are >>>>> > > ignored - >>>>> > > > + * e.g. outer_hdr/size set for ESP >>>>> transport mode. >>>>> > > > + * @retval 0 on success >>>>> > > > + * @retval <0 on failure >>>>> > > > + */ >>>>> > > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t >>>>> session, >>>>> > > > + enum odp_ipsec_mode ipsec_mode, >>>>> > > > + enum odp_ipsec_proto >>>>> ipsec_proto, >>>>> > > > + odp_ipsec_params_t >>>>> ipsec_params); >>>>> > > >>>>> > > Can this be called multiple times on the same session to update the >>>>> > > parameters, or would the session need to be destroyed and >>>>> recreated? >>>>> > > >>>>> > [Alex] No, this is not the intended use of this function. >>>>> > This is to be called on a raw session (algorithm only). To change a >>>>> crypto >>>>> > session additional functions should be used. >>>>> > >>>>> > > >>>>> > > Is it valid to create a session, issue a few operations, then >>>>> later add >>>>> > > the IPsec protocol config for that session? >>>>> > > >>>>> > [Alex] While this might work for a particular >>>>> implementation/platform, I'm >>>>> > wondering if there's an use case for it? >>>>> > >>>>> >>>>> I wasn't thinking about a particular use case, there likely isn't one, >>>>> just that how it's currently defined is open to >>>>> misinterpretation/misuse. >>>>> >>>> [Alex] As we have only one session create call the session must be >>>> usable right after creation. Being possible to extend the session for >>>> protocol processing after some traffic passed it's up to implementation - >>>> platform guys may give some inputs here. Other question can be - can we >>>> configure a session for protocol processing while traffic is passing? Again >>>> it may be possible for some and not for others. If there's no use-case for >>>> it then applications should not rely on this and config can return an error. >>>> Alternatively, we may need enable/disable calls for crypto sessions, >>>> similarly with pktio start/stop functions. Pktio start/stop functions are >>>> meant to clearly mark configuration phase - e.g. open pktio, config >>>> classification, start pktio. However, enable/disable calls could be >>>> required by IPSec rekeying process. >>>> >>>> >>>>> > > >>>>> > > I'm wondering why the params here weren't just made an extension >>>>> of the >>>>> > > odp_crypto_session_params_t in the initial session create. >>>>> > > >>>>> > [Alex] Do you mean to add these parameters as members of >>>>> > odp_crypto_session_params_t or to extend session_create call to take >>>>> IPSec >>>>> > params? >>>>> >>>>> The first. >>>>> >>>>> > It wouldn't be a good idea to embed the IPSec parameters into the >>>>> > odp_crypto_session_params_t as IPSec is just a protocol among others >>>>> > supported by crypto engines (SSL/TLS, MACSEC, SRTP, etc). >>>>> >>>>> It could be done with a union + enum for each protocol (or NONE) and if >>>>> we add odp_crypto_session_params_init(), which we should have anyway, >>>>> you wouldn't need to know those fields existed when creating >>>>> non-protocol >>>>> sessions. >>>>> >>>> [Alex] I still don't think is a good idea. An application using only >>>> raw sessions will waste memory with fields it actually doesn't use. Making >>>> a structure larger is not cache/performance friendly, software running on >>>> the cores access these structures on a per-packet basis. Crypto sessions >>>> can be potentially millions. >>>> >>>>> >>>>> > I think having a separate call odp_crypto_session_config_ipsec() >>>>> makes the >>>>> > source code more readable regarding the intent of the application >>>>> rather >>>>> > than filling in lots of IPSec parameters and calling then >>>>> session_create(). >>>>> > >>>>> >>>>> If there's only one way of doing it right - protocol config/params are >>>>> set at session create time and not modified - and a number of ways of >>>>> getting it wrong then it makes sense to define the API such that it can >>>>> only be done the right way. >>>>> >>>> [Alex] I'm not sure it is possible to design an API __impossible__ to >>>> misuse - i.e. not possible to compile a program which misuses the API. I >>>> think that misuse rather should fail gracefully and with no side effects. >>>> There are other ODP API areas that can be easily misused - for example >>>> pktio - can we remove the default input queue after start function or while >>>> traffic is passing ? Can we remove the queue after sending a few packets >>>> but without stopping? Scheduling API requires calling pause before exiting >>>> a schedule loop - what happens if pause is not called? >>>> I think these are rather runtime aspects that should be handled at >>>> runtime. More than that, different platforms/implementation may behave >>>> differently, but a portable application should not be based on a particular >>>> behavior. >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> -- >>>>> Stuart. >>>>> >>>> >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> https://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>>> >>> >> >
On 9 October 2015 at 10:37, Alexandru Badicioiu < alexandru.badicioiu@linaro.org> wrote: > This was a long discussion some time ago and the result was that crypto > output should be abstracted in the form of the completion event and access > functions would retrieve status and output packets. Also the implementation > is in charge with crypto completion event allocation and not the > application even if this is not really supported in HW. > Yes that is useful for lookaside crypto/IPsec operations and my intention is not to change that. But this model is not useful for inline IPsec acceleration. We need a new or (preferably?) extended API for that. That's what I am asking about. I know not all silicon vendors would be able to support inline IPsec acceleration in HW. But with an Event Machine-like programming model where the application uses per-queue call-backs, inline IPsec acceleration can be implemented/emulated in SW. Even if this model is not exposed to the user, an ODP implementation should be able to do something like it "under the hood". The previous approach was that application supplied the completion event as > being the output packet but this was regarded as a hack. > > Alex > > On 9 October 2015 at 11:28, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > >> On 9 October 2015 at 09:34, Alexandru Badicioiu < >> alexandru.badicioiu@linaro.org> wrote: >> >>> The problem you raised is not strictly related to this patch. >>> A crypto session has an output queue (for async mode) where the results >>> , including the operation status, will be delivered. There is nothing in >>> the API to prevent using a pktio output queue for crypto completion events >>> but the pktio should be able to process the event as the application would >>> do. >>> In this case an extension of pktio functionality would be required. >>> >> Yes but I was thinking of some change (in API and implementation) where >> the output of the crypto operations is the actual packet (with L2/L3 >> encapsulation), not a crypto completion event. >> >> >>> >>> Alex >>> >>> >>> On 8 October 2015 at 20:13, Ola Liljedahl <ola.liljedahl@linaro.org> >>> wrote: >>> >>>> Can this proposal be extended to handle inline IPsec processing, e.g. >>>> encrypt and encapsulate packet (include Ethernet framing) and then send to >>>> (enqueue) to some user-defined queue (which might be a pktio output queue)? >>>> Need some way to report errors and other conditions back to SW so might >>>> need some kind of ipsec notification event. >>>> Something for the ingress side as well, e.g. connect user-defined >>>> queues to IPsec input queue(s) and then specify corresponding output queues. >>>> >>>> >>>> On 31 July 2015 at 11:30, Alexandru Badicioiu < >>>> alexandru.badicioiu@linaro.org> wrote: >>>> >>>>> >>>>> >>>>> On 30 July 2015 at 17:44, Stuart Haslam <stuart.haslam@linaro.org> >>>>> wrote: >>>>> >>>>>> On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote: >>>>>> > On 30 July 2015 at 13:50, Stuart Haslam <stuart.haslam@linaro.org> >>>>>> wrote: >>>>>> > >>>>>> > > On Wed, Jul 22, 2015 at 11:26:03AM +0300, >>>>>> alexandru.badicioiu@linaro.org >>>>>> > > wrote: >>>>>> > > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >>>>>> > > > >>>>>> > > > This patch adds IPSec protocol processing capabilities to crypto >>>>>> > > > sesssions. Implementations which have these capabilities in >>>>>> hardware >>>>>> > > > crypto engines can use the extension to offload the application >>>>>> from >>>>>> > > > IPSec protocol processing. >>>>>> > > > >>>>>> > > > Signed-off-by: Alexandru Badicioiu < >>>>>> alexandru.badicioiu@linaro.org> >>>>>> > > > --- >>>>>> > > > include/odp/api/crypto_ipsec.h | 110 >>>>>> > > ++++++++++++++++++++ >>>>>> > > > platform/linux-generic/include/odp/crypto.h | 2 + >>>>>> > > > .../include/odp/plat/crypto_ipsec_types.h | 53 >>>>>> ++++++++++ >>>>>> > > > 3 files changed, 165 insertions(+), 0 deletions(-) >>>>>> > > > create mode 100644 include/odp/api/crypto_ipsec.h >>>>>> > > > create mode 100644 >>>>>> > > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >>>>>> > > > >>>>>> > > > diff --git a/include/odp/api/crypto_ipsec.h >>>>>> > > b/include/odp/api/crypto_ipsec.h >>>>>> > > > new file mode 100644 >>>>>> > > > index 0000000..e59fea4 >>>>>> > > > --- /dev/null >>>>>> > > > +++ b/include/odp/api/crypto_ipsec.h >>>>>> > > > @@ -0,0 +1,110 @@ >>>>>> > > > +/* Copyright (c) 2014, Linaro Limited >>>>>> > > > + * All rights reserved. >>>>>> > > > + * >>>>>> > > > + * SPDX-License-Identifier: BSD-3-Clause >>>>>> > > > + */ >>>>>> > > > + >>>>>> > > > +/** >>>>>> > > > + * @file >>>>>> > > > + * >>>>>> > > > + * ODP crypto IPSec extension >>>>>> > > > + */ >>>>>> > > > + >>>>>> > > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ >>>>>> > > > +#define ODP_API_CRYPTO_IPSEC_H_ >>>>>> > > > + >>>>>> > > > +#ifdef __cplusplus >>>>>> > > > +extern "C" { >>>>>> > > > +#endif >>>>>> > > > + >>>>>> > > > +/** >>>>>> > > > + * @enum odp_ipsec_outhdr_type >>>>>> > > > + * IPSec tunnel outer header type >>>>>> > > > + * >>>>>> > > > + * @enum odp_ipsec_ar_ws >>>>>> > > > + * IPSec Anti-replay window size >>>>>> > > > + * >>>>>> > > > + */ >>>>>> > > > + >>>>>> > > > +typedef struct odp_ipsec_params { >>>>>> > > > + uint32_t spi; /** SPI value */ >>>>>> > > > + uint32_t seq; /** Initial SEQ number */ >>>>>> > > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - >>>>>> > > > + inbound session with >>>>>> > > authentication */ >>>>>> > > >>>>>> > > This name is pretty cryptic, how about just replay_window? >>>>>> > > >>>>>> > [Alex] The standard name for this parameter is anti-replay window. >>>>>> In the >>>>>> > context of IPSec this should not be cryptic (odp_ipsec_arw vs >>>>>> odp_arw). If >>>>>> > your suggestion is to replace the name of the struct member - ar_ws >>>>>> with >>>>>> > replay_window I'm fine with it but not the name of the enum >>>>>> > (odp_ipsec_ar_ws). >>>>>> > Or maybe change it to enum odp_ipsec_arw. >>>>>> >>>>>> I meant the variable name, don't mind about the enum name. >>>>>> [Alex] I'm OK with the change. >>>>>> >>>>>> > > >>>>>> > > > + odp_bool_t esn; /** Use extended sequence numbers >>>>>> */ >>>>>> > > > + odp_bool_t auto_iv; /** Auto IV generation for each >>>>>> operation. >>>>>> > > */ >>>>>> > > > + uint16_t out_hdr_size; /** outer header size - tunnel >>>>>> mode */ >>>>>> > > > + uint8_t *out_hdr; /** outer header - tunnel mode */ >>>>>> > > >>>>>> > > Can these be 0 and NULL if the application wants tunnel mode but >>>>>> wants >>>>>> > > to handle the outer header itself? (i.e. add ESP head/tail and >>>>>> include >>>>>> > > inner IP header in encap data) >>>>>> > > >>>>>> > [Alex] Yes, that is the intended usage. If requested mode is tunnel >>>>>> and >>>>>> > there's no out_hdr specified, the application has to add it. >>>>>> >>>>>> >>>>>> >>>>>> > > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header >>>>>> type - >>>>>> > > > + tunnel mode */ >>>>>> > > > + odp_bool_t ip_csum; /** update/verify ip header >>>>>> checksum */ >>>>>> > > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode >>>>>> encap & >>>>>> > > decap */ >>>>>> > > > + odp_bool_t remove_outer_hdr; /** remove outer header - >>>>>> tunnel mode >>>>>> > > decap */ >>>>>> > > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 >>>>>> TOS or >>>>>> > > > + IPv6 Traffic Class byte from >>>>>> the >>>>>> > > inner/outer >>>>>> > > > + IP header to the outer/inner >>>>>> IP header >>>>>> > > - >>>>>> > > > + tunnel mode encap & decap */ >>>>>> > > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit >>>>>> from >>>>>> > > > + the inner IP header to the >>>>>> > > > + outer IP header - tunnel mode >>>>>> encap */ >>>>>> > > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled - >>>>>> tunnel >>>>>> > > mode */ >>>>>> > > > + odp_bool_t udp_csum; /** Update/verify UDP csum when >>>>>> NAT-T >>>>>> > > enabled */ >>>>>> > > > + >>>>>> > > > +} odp_ipsec_params_t; >>>>>> > > > + >>>>>> > > > +/** >>>>>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL >>>>>> > > > + * IPSec tunnel mode >>>>>> > > > + * >>>>>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT >>>>>> > > > + * IPSec transport mode >>>>>> > > > + * >>>>>> > > > + * @enum odp_ipsec_proto >>>>>> > > > + * IPSec protocol >>>>>> > > > + */ >>>>>> > > > + >>>>>> > > > +/** >>>>>> > > > + * Configure crypto session for IPsec processing >>>>>> > > > + * >>>>>> > > > + * Configures a crypto session for IPSec protocol processing. >>>>>> > > > + * Packets submitted to an IPSec enabled session will have >>>>>> > > > + * relevant IPSec headers/trailers and tunnel headers >>>>>> > > > + * added/removed by the crypto implementation. >>>>>> > > > + * For example, the input packet for an IPSec ESP transport >>>>>> > > > + * enabled session should be the clear text packet with >>>>>> > > > + * no ESP headers/trailers prepared in advance for crypto >>>>>> operation. >>>>>> > > > + * The output packet will have ESP header, IV, trailer and the >>>>>> ESP ICV >>>>>> > > > + * added by crypto implementation. >>>>>> > > >>>>>> > > If a packet fails a check on decap (e.g. out of window), >>>>>> presumably the >>>>>> > > application gets an odp_crypto_op_result_t with ok=false, but is >>>>>> there >>>>>> > > any way for it to tell what failed? >>>>>> > > [Alex] Crypto operation error status should be extended with a >>>>>> code for >>>>>> > > out of window condition. >>>>>> > > > + * Depending on the particular capabilities of an >>>>>> implementation and >>>>>> > > > + * the parameters enabled by application, the application may >>>>>> be >>>>>> > > > + * partially or completely offloaded from IPSec protocol >>>>>> processing. >>>>>> > > > + * For example, if an implementation does not support checksum >>>>>> > > > + * update for IP header after adding ESP header the application >>>>>> > > > + * should update after crypto IPSec operation. >>>>>> > > > + * >>>>>> > > > + * If an implementation does not support a particular set of >>>>>> > > > + * arguments it should return error. >>>>>> > > > + * >>>>>> > > > + * @param session Session handle >>>>>> > > > + * @param ipsec_mode IPSec protocol mode >>>>>> > > > + * @param ipsec_proto IPSec protocol >>>>>> > > > + * @param ipsec_params IPSec parameters. Parameters >>>>>> which are >>>>>> > > not >>>>>> > > > + * relevant for selected protocol & mode >>>>>> are >>>>>> > > ignored - >>>>>> > > > + * e.g. outer_hdr/size set for ESP >>>>>> transport mode. >>>>>> > > > + * @retval 0 on success >>>>>> > > > + * @retval <0 on failure >>>>>> > > > + */ >>>>>> > > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t >>>>>> session, >>>>>> > > > + enum odp_ipsec_mode >>>>>> ipsec_mode, >>>>>> > > > + enum odp_ipsec_proto >>>>>> ipsec_proto, >>>>>> > > > + odp_ipsec_params_t >>>>>> ipsec_params); >>>>>> > > >>>>>> > > Can this be called multiple times on the same session to update >>>>>> the >>>>>> > > parameters, or would the session need to be destroyed and >>>>>> recreated? >>>>>> > > >>>>>> > [Alex] No, this is not the intended use of this function. >>>>>> > This is to be called on a raw session (algorithm only). To change a >>>>>> crypto >>>>>> > session additional functions should be used. >>>>>> > >>>>>> > > >>>>>> > > Is it valid to create a session, issue a few operations, then >>>>>> later add >>>>>> > > the IPsec protocol config for that session? >>>>>> > > >>>>>> > [Alex] While this might work for a particular >>>>>> implementation/platform, I'm >>>>>> > wondering if there's an use case for it? >>>>>> > >>>>>> >>>>>> I wasn't thinking about a particular use case, there likely isn't one, >>>>>> just that how it's currently defined is open to >>>>>> misinterpretation/misuse. >>>>>> >>>>> [Alex] As we have only one session create call the session must be >>>>> usable right after creation. Being possible to extend the session for >>>>> protocol processing after some traffic passed it's up to implementation - >>>>> platform guys may give some inputs here. Other question can be - can we >>>>> configure a session for protocol processing while traffic is passing? Again >>>>> it may be possible for some and not for others. If there's no use-case for >>>>> it then applications should not rely on this and config can return an error. >>>>> Alternatively, we may need enable/disable calls for crypto sessions, >>>>> similarly with pktio start/stop functions. Pktio start/stop functions are >>>>> meant to clearly mark configuration phase - e.g. open pktio, config >>>>> classification, start pktio. However, enable/disable calls could be >>>>> required by IPSec rekeying process. >>>>> >>>>> >>>>>> > > >>>>>> > > I'm wondering why the params here weren't just made an extension >>>>>> of the >>>>>> > > odp_crypto_session_params_t in the initial session create. >>>>>> > > >>>>>> > [Alex] Do you mean to add these parameters as members of >>>>>> > odp_crypto_session_params_t or to extend session_create call to >>>>>> take IPSec >>>>>> > params? >>>>>> >>>>>> The first. >>>>>> >>>>>> > It wouldn't be a good idea to embed the IPSec parameters into the >>>>>> > odp_crypto_session_params_t as IPSec is just a protocol among >>>>>> others >>>>>> > supported by crypto engines (SSL/TLS, MACSEC, SRTP, etc). >>>>>> >>>>>> It could be done with a union + enum for each protocol (or NONE) and >>>>>> if >>>>>> we add odp_crypto_session_params_init(), which we should have anyway, >>>>>> you wouldn't need to know those fields existed when creating >>>>>> non-protocol >>>>>> sessions. >>>>>> >>>>> [Alex] I still don't think is a good idea. An application using only >>>>> raw sessions will waste memory with fields it actually doesn't use. Making >>>>> a structure larger is not cache/performance friendly, software running on >>>>> the cores access these structures on a per-packet basis. Crypto sessions >>>>> can be potentially millions. >>>>> >>>>>> >>>>>> > I think having a separate call odp_crypto_session_config_ipsec() >>>>>> makes the >>>>>> > source code more readable regarding the intent of the application >>>>>> rather >>>>>> > than filling in lots of IPSec parameters and calling then >>>>>> session_create(). >>>>>> > >>>>>> >>>>>> If there's only one way of doing it right - protocol config/params are >>>>>> set at session create time and not modified - and a number of ways of >>>>>> getting it wrong then it makes sense to define the API such that it >>>>>> can >>>>>> only be done the right way. >>>>>> >>>>> [Alex] I'm not sure it is possible to design an API __impossible__ to >>>>> misuse - i.e. not possible to compile a program which misuses the API. I >>>>> think that misuse rather should fail gracefully and with no side effects. >>>>> There are other ODP API areas that can be easily misused - for example >>>>> pktio - can we remove the default input queue after start function or while >>>>> traffic is passing ? Can we remove the queue after sending a few packets >>>>> but without stopping? Scheduling API requires calling pause before exiting >>>>> a schedule loop - what happens if pause is not called? >>>>> I think these are rather runtime aspects that should be handled at >>>>> runtime. More than that, different platforms/implementation may behave >>>>> differently, but a portable application should not be based on a particular >>>>> behavior. >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>>> Stuart. >>>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> lng-odp mailing list >>>>> lng-odp@lists.linaro.org >>>>> https://lists.linaro.org/mailman/listinfo/lng-odp >>>>> >>>>> >>>> >>> >> >
So you would like inline/synchronous API mode to have a completion queue too? What would be the use for it? On 9 October 2015 at 13:01, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > On 9 October 2015 at 10:37, Alexandru Badicioiu < > alexandru.badicioiu@linaro.org> wrote: > >> This was a long discussion some time ago and the result was that crypto >> output should be abstracted in the form of the completion event and access >> functions would retrieve status and output packets. Also the implementation >> is in charge with crypto completion event allocation and not the >> application even if this is not really supported in HW. >> > Yes that is useful for lookaside crypto/IPsec operations and my intention > is not to change that. But this model is not useful for inline IPsec > acceleration. We need a new or (preferably?) extended API for that. That's > what I am asking about. > > I know not all silicon vendors would be able to support inline IPsec > acceleration in HW. But with an Event Machine-like programming model where > the application uses per-queue call-backs, inline IPsec acceleration can be > implemented/emulated in SW. Even if this model is not exposed to the user, > an ODP implementation should be able to do something like it "under the > hood". > > > The previous approach was that application supplied the completion event >> as being the output packet but this was regarded as a hack. >> >> Alex >> >> On 9 October 2015 at 11:28, Ola Liljedahl <ola.liljedahl@linaro.org> >> wrote: >> >>> On 9 October 2015 at 09:34, Alexandru Badicioiu < >>> alexandru.badicioiu@linaro.org> wrote: >>> >>>> The problem you raised is not strictly related to this patch. >>>> A crypto session has an output queue (for async mode) where the results >>>> , including the operation status, will be delivered. There is nothing in >>>> the API to prevent using a pktio output queue for crypto completion events >>>> but the pktio should be able to process the event as the application would >>>> do. >>>> In this case an extension of pktio functionality would be required. >>>> >>> Yes but I was thinking of some change (in API and implementation) where >>> the output of the crypto operations is the actual packet (with L2/L3 >>> encapsulation), not a crypto completion event. >>> >>> >>>> >>>> Alex >>>> >>>> >>>> On 8 October 2015 at 20:13, Ola Liljedahl <ola.liljedahl@linaro.org> >>>> wrote: >>>> >>>>> Can this proposal be extended to handle inline IPsec processing, e.g. >>>>> encrypt and encapsulate packet (include Ethernet framing) and then send to >>>>> (enqueue) to some user-defined queue (which might be a pktio output queue)? >>>>> Need some way to report errors and other conditions back to SW so >>>>> might need some kind of ipsec notification event. >>>>> Something for the ingress side as well, e.g. connect user-defined >>>>> queues to IPsec input queue(s) and then specify corresponding output queues. >>>>> >>>>> >>>>> On 31 July 2015 at 11:30, Alexandru Badicioiu < >>>>> alexandru.badicioiu@linaro.org> wrote: >>>>> >>>>>> >>>>>> >>>>>> On 30 July 2015 at 17:44, Stuart Haslam <stuart.haslam@linaro.org> >>>>>> wrote: >>>>>> >>>>>>> On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote: >>>>>>> > On 30 July 2015 at 13:50, Stuart Haslam <stuart.haslam@linaro.org> >>>>>>> wrote: >>>>>>> > >>>>>>> > > On Wed, Jul 22, 2015 at 11:26:03AM +0300, >>>>>>> alexandru.badicioiu@linaro.org >>>>>>> > > wrote: >>>>>>> > > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >>>>>>> > > > >>>>>>> > > > This patch adds IPSec protocol processing capabilities to >>>>>>> crypto >>>>>>> > > > sesssions. Implementations which have these capabilities in >>>>>>> hardware >>>>>>> > > > crypto engines can use the extension to offload the >>>>>>> application from >>>>>>> > > > IPSec protocol processing. >>>>>>> > > > >>>>>>> > > > Signed-off-by: Alexandru Badicioiu < >>>>>>> alexandru.badicioiu@linaro.org> >>>>>>> > > > --- >>>>>>> > > > include/odp/api/crypto_ipsec.h | 110 >>>>>>> > > ++++++++++++++++++++ >>>>>>> > > > platform/linux-generic/include/odp/crypto.h | 2 + >>>>>>> > > > .../include/odp/plat/crypto_ipsec_types.h | 53 >>>>>>> ++++++++++ >>>>>>> > > > 3 files changed, 165 insertions(+), 0 deletions(-) >>>>>>> > > > create mode 100644 include/odp/api/crypto_ipsec.h >>>>>>> > > > create mode 100644 >>>>>>> > > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >>>>>>> > > > >>>>>>> > > > diff --git a/include/odp/api/crypto_ipsec.h >>>>>>> > > b/include/odp/api/crypto_ipsec.h >>>>>>> > > > new file mode 100644 >>>>>>> > > > index 0000000..e59fea4 >>>>>>> > > > --- /dev/null >>>>>>> > > > +++ b/include/odp/api/crypto_ipsec.h >>>>>>> > > > @@ -0,0 +1,110 @@ >>>>>>> > > > +/* Copyright (c) 2014, Linaro Limited >>>>>>> > > > + * All rights reserved. >>>>>>> > > > + * >>>>>>> > > > + * SPDX-License-Identifier: BSD-3-Clause >>>>>>> > > > + */ >>>>>>> > > > + >>>>>>> > > > +/** >>>>>>> > > > + * @file >>>>>>> > > > + * >>>>>>> > > > + * ODP crypto IPSec extension >>>>>>> > > > + */ >>>>>>> > > > + >>>>>>> > > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ >>>>>>> > > > +#define ODP_API_CRYPTO_IPSEC_H_ >>>>>>> > > > + >>>>>>> > > > +#ifdef __cplusplus >>>>>>> > > > +extern "C" { >>>>>>> > > > +#endif >>>>>>> > > > + >>>>>>> > > > +/** >>>>>>> > > > + * @enum odp_ipsec_outhdr_type >>>>>>> > > > + * IPSec tunnel outer header type >>>>>>> > > > + * >>>>>>> > > > + * @enum odp_ipsec_ar_ws >>>>>>> > > > + * IPSec Anti-replay window size >>>>>>> > > > + * >>>>>>> > > > + */ >>>>>>> > > > + >>>>>>> > > > +typedef struct odp_ipsec_params { >>>>>>> > > > + uint32_t spi; /** SPI value */ >>>>>>> > > > + uint32_t seq; /** Initial SEQ number */ >>>>>>> > > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - >>>>>>> > > > + inbound session with >>>>>>> > > authentication */ >>>>>>> > > >>>>>>> > > This name is pretty cryptic, how about just replay_window? >>>>>>> > > >>>>>>> > [Alex] The standard name for this parameter is anti-replay window. >>>>>>> In the >>>>>>> > context of IPSec this should not be cryptic (odp_ipsec_arw vs >>>>>>> odp_arw). If >>>>>>> > your suggestion is to replace the name of the struct member - >>>>>>> ar_ws with >>>>>>> > replay_window I'm fine with it but not the name of the enum >>>>>>> > (odp_ipsec_ar_ws). >>>>>>> > Or maybe change it to enum odp_ipsec_arw. >>>>>>> >>>>>>> I meant the variable name, don't mind about the enum name. >>>>>>> [Alex] I'm OK with the change. >>>>>>> >>>>>>> > > >>>>>>> > > > + odp_bool_t esn; /** Use extended sequence >>>>>>> numbers */ >>>>>>> > > > + odp_bool_t auto_iv; /** Auto IV generation for each >>>>>>> operation. >>>>>>> > > */ >>>>>>> > > > + uint16_t out_hdr_size; /** outer header size - tunnel >>>>>>> mode */ >>>>>>> > > > + uint8_t *out_hdr; /** outer header - tunnel mode >>>>>>> */ >>>>>>> > > >>>>>>> > > Can these be 0 and NULL if the application wants tunnel mode but >>>>>>> wants >>>>>>> > > to handle the outer header itself? (i.e. add ESP head/tail and >>>>>>> include >>>>>>> > > inner IP header in encap data) >>>>>>> > > >>>>>>> > [Alex] Yes, that is the intended usage. If requested mode is >>>>>>> tunnel and >>>>>>> > there's no out_hdr specified, the application has to add it. >>>>>>> >>>>>>> >>>>>>> >>>>>>> > > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header >>>>>>> type - >>>>>>> > > > + tunnel mode >>>>>>> */ >>>>>>> > > > + odp_bool_t ip_csum; /** update/verify ip header >>>>>>> checksum */ >>>>>>> > > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode >>>>>>> encap & >>>>>>> > > decap */ >>>>>>> > > > + odp_bool_t remove_outer_hdr; /** remove outer header - >>>>>>> tunnel mode >>>>>>> > > decap */ >>>>>>> > > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the >>>>>>> IPv4 TOS or >>>>>>> > > > + IPv6 Traffic Class byte from >>>>>>> the >>>>>>> > > inner/outer >>>>>>> > > > + IP header to the outer/inner >>>>>>> IP header >>>>>>> > > - >>>>>>> > > > + tunnel mode encap & decap */ >>>>>>> > > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF >>>>>>> bit from >>>>>>> > > > + the inner IP header to the >>>>>>> > > > + outer IP header - tunnel >>>>>>> mode encap */ >>>>>>> > > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled >>>>>>> - tunnel >>>>>>> > > mode */ >>>>>>> > > > + odp_bool_t udp_csum; /** Update/verify UDP csum when >>>>>>> NAT-T >>>>>>> > > enabled */ >>>>>>> > > > + >>>>>>> > > > +} odp_ipsec_params_t; >>>>>>> > > > + >>>>>>> > > > +/** >>>>>>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL >>>>>>> > > > + * IPSec tunnel mode >>>>>>> > > > + * >>>>>>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT >>>>>>> > > > + * IPSec transport mode >>>>>>> > > > + * >>>>>>> > > > + * @enum odp_ipsec_proto >>>>>>> > > > + * IPSec protocol >>>>>>> > > > + */ >>>>>>> > > > + >>>>>>> > > > +/** >>>>>>> > > > + * Configure crypto session for IPsec processing >>>>>>> > > > + * >>>>>>> > > > + * Configures a crypto session for IPSec protocol processing. >>>>>>> > > > + * Packets submitted to an IPSec enabled session will have >>>>>>> > > > + * relevant IPSec headers/trailers and tunnel headers >>>>>>> > > > + * added/removed by the crypto implementation. >>>>>>> > > > + * For example, the input packet for an IPSec ESP transport >>>>>>> > > > + * enabled session should be the clear text packet with >>>>>>> > > > + * no ESP headers/trailers prepared in advance for crypto >>>>>>> operation. >>>>>>> > > > + * The output packet will have ESP header, IV, trailer and >>>>>>> the ESP ICV >>>>>>> > > > + * added by crypto implementation. >>>>>>> > > >>>>>>> > > If a packet fails a check on decap (e.g. out of window), >>>>>>> presumably the >>>>>>> > > application gets an odp_crypto_op_result_t with ok=false, but is >>>>>>> there >>>>>>> > > any way for it to tell what failed? >>>>>>> > > [Alex] Crypto operation error status should be extended with a >>>>>>> code for >>>>>>> > > out of window condition. >>>>>>> > > > + * Depending on the particular capabilities of an >>>>>>> implementation and >>>>>>> > > > + * the parameters enabled by application, the application may >>>>>>> be >>>>>>> > > > + * partially or completely offloaded from IPSec protocol >>>>>>> processing. >>>>>>> > > > + * For example, if an implementation does not support checksum >>>>>>> > > > + * update for IP header after adding ESP header the >>>>>>> application >>>>>>> > > > + * should update after crypto IPSec operation. >>>>>>> > > > + * >>>>>>> > > > + * If an implementation does not support a particular set of >>>>>>> > > > + * arguments it should return error. >>>>>>> > > > + * >>>>>>> > > > + * @param session Session handle >>>>>>> > > > + * @param ipsec_mode IPSec protocol mode >>>>>>> > > > + * @param ipsec_proto IPSec protocol >>>>>>> > > > + * @param ipsec_params IPSec parameters. Parameters >>>>>>> which are >>>>>>> > > not >>>>>>> > > > + * relevant for selected protocol & >>>>>>> mode are >>>>>>> > > ignored - >>>>>>> > > > + * e.g. outer_hdr/size set for ESP >>>>>>> transport mode. >>>>>>> > > > + * @retval 0 on success >>>>>>> > > > + * @retval <0 on failure >>>>>>> > > > + */ >>>>>>> > > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t >>>>>>> session, >>>>>>> > > > + enum odp_ipsec_mode >>>>>>> ipsec_mode, >>>>>>> > > > + enum odp_ipsec_proto >>>>>>> ipsec_proto, >>>>>>> > > > + odp_ipsec_params_t >>>>>>> ipsec_params); >>>>>>> > > >>>>>>> > > Can this be called multiple times on the same session to update >>>>>>> the >>>>>>> > > parameters, or would the session need to be destroyed and >>>>>>> recreated? >>>>>>> > > >>>>>>> > [Alex] No, this is not the intended use of this function. >>>>>>> > This is to be called on a raw session (algorithm only). To change >>>>>>> a crypto >>>>>>> > session additional functions should be used. >>>>>>> > >>>>>>> > > >>>>>>> > > Is it valid to create a session, issue a few operations, then >>>>>>> later add >>>>>>> > > the IPsec protocol config for that session? >>>>>>> > > >>>>>>> > [Alex] While this might work for a particular >>>>>>> implementation/platform, I'm >>>>>>> > wondering if there's an use case for it? >>>>>>> > >>>>>>> >>>>>>> I wasn't thinking about a particular use case, there likely isn't >>>>>>> one, >>>>>>> just that how it's currently defined is open to >>>>>>> misinterpretation/misuse. >>>>>>> >>>>>> [Alex] As we have only one session create call the session must be >>>>>> usable right after creation. Being possible to extend the session for >>>>>> protocol processing after some traffic passed it's up to implementation - >>>>>> platform guys may give some inputs here. Other question can be - can we >>>>>> configure a session for protocol processing while traffic is passing? Again >>>>>> it may be possible for some and not for others. If there's no use-case for >>>>>> it then applications should not rely on this and config can return an error. >>>>>> Alternatively, we may need enable/disable calls for crypto sessions, >>>>>> similarly with pktio start/stop functions. Pktio start/stop functions are >>>>>> meant to clearly mark configuration phase - e.g. open pktio, config >>>>>> classification, start pktio. However, enable/disable calls could be >>>>>> required by IPSec rekeying process. >>>>>> >>>>>> >>>>>>> > > >>>>>>> > > I'm wondering why the params here weren't just made an extension >>>>>>> of the >>>>>>> > > odp_crypto_session_params_t in the initial session create. >>>>>>> > > >>>>>>> > [Alex] Do you mean to add these parameters as members of >>>>>>> > odp_crypto_session_params_t or to extend session_create call to >>>>>>> take IPSec >>>>>>> > params? >>>>>>> >>>>>>> The first. >>>>>>> >>>>>>> > It wouldn't be a good idea to embed the IPSec parameters into the >>>>>>> > odp_crypto_session_params_t as IPSec is just a protocol among >>>>>>> others >>>>>>> > supported by crypto engines (SSL/TLS, MACSEC, SRTP, etc). >>>>>>> >>>>>>> It could be done with a union + enum for each protocol (or NONE) and >>>>>>> if >>>>>>> we add odp_crypto_session_params_init(), which we should have anyway, >>>>>>> you wouldn't need to know those fields existed when creating >>>>>>> non-protocol >>>>>>> sessions. >>>>>>> >>>>>> [Alex] I still don't think is a good idea. An application using only >>>>>> raw sessions will waste memory with fields it actually doesn't use. Making >>>>>> a structure larger is not cache/performance friendly, software running on >>>>>> the cores access these structures on a per-packet basis. Crypto sessions >>>>>> can be potentially millions. >>>>>> >>>>>>> >>>>>>> > I think having a separate call odp_crypto_session_config_ipsec() >>>>>>> makes the >>>>>>> > source code more readable regarding the intent of the application >>>>>>> rather >>>>>>> > than filling in lots of IPSec parameters and calling then >>>>>>> session_create(). >>>>>>> > >>>>>>> >>>>>>> If there's only one way of doing it right - protocol config/params >>>>>>> are >>>>>>> set at session create time and not modified - and a number of ways of >>>>>>> getting it wrong then it makes sense to define the API such that it >>>>>>> can >>>>>>> only be done the right way. >>>>>>> >>>>>> [Alex] I'm not sure it is possible to design an API __impossible__ to >>>>>> misuse - i.e. not possible to compile a program which misuses the API. I >>>>>> think that misuse rather should fail gracefully and with no side effects. >>>>>> There are other ODP API areas that can be easily misused - for example >>>>>> pktio - can we remove the default input queue after start function or while >>>>>> traffic is passing ? Can we remove the queue after sending a few packets >>>>>> but without stopping? Scheduling API requires calling pause before exiting >>>>>> a schedule loop - what happens if pause is not called? >>>>>> I think these are rather runtime aspects that should be handled at >>>>>> runtime. More than that, different platforms/implementation may behave >>>>>> differently, but a portable application should not be based on a particular >>>>>> behavior. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>>> Stuart. >>>>>>> >>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> lng-odp mailing list >>>>>> lng-odp@lists.linaro.org >>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp >>>>>> >>>>>> >>>>> >>>> >>> >> >
On 9 October 2015 at 12:48, Alexandru Badicioiu < alexandru.badicioiu@linaro.org> wrote: > So you would like inline/synchronous API mode to have a completion queue > too? What would be the use for it? > I think we are misunderstanding each other and I haven't been very detailed. The output of IPsec egress processing must be a queue (e.g. pktio queue) where complete packets (L2/L3 encapsulation specified by the user) are enqueued. But there probably also needs to be a queue for sending notifications of errors or other exceptional events (e.g. SA not found, seqno nears the end etc). I haven't tried to understand exactly what is needed for inline IPsec on ingress. > On 9 October 2015 at 13:01, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > >> On 9 October 2015 at 10:37, Alexandru Badicioiu < >> alexandru.badicioiu@linaro.org> wrote: >> >>> This was a long discussion some time ago and the result was that crypto >>> output should be abstracted in the form of the completion event and access >>> functions would retrieve status and output packets. Also the implementation >>> is in charge with crypto completion event allocation and not the >>> application even if this is not really supported in HW. >>> >> Yes that is useful for lookaside crypto/IPsec operations and my intention >> is not to change that. But this model is not useful for inline IPsec >> acceleration. We need a new or (preferably?) extended API for that. That's >> what I am asking about. >> >> I know not all silicon vendors would be able to support inline IPsec >> acceleration in HW. But with an Event Machine-like programming model where >> the application uses per-queue call-backs, inline IPsec acceleration can be >> implemented/emulated in SW. Even if this model is not exposed to the user, >> an ODP implementation should be able to do something like it "under the >> hood". >> >> >> The previous approach was that application supplied the completion event >>> as being the output packet but this was regarded as a hack. >>> >>> Alex >>> >>> On 9 October 2015 at 11:28, Ola Liljedahl <ola.liljedahl@linaro.org> >>> wrote: >>> >>>> On 9 October 2015 at 09:34, Alexandru Badicioiu < >>>> alexandru.badicioiu@linaro.org> wrote: >>>> >>>>> The problem you raised is not strictly related to this patch. >>>>> A crypto session has an output queue (for async mode) where the >>>>> results , including the operation status, will be delivered. There is >>>>> nothing in the API to prevent using a pktio output queue for crypto >>>>> completion events but the pktio should be able to process the event as the >>>>> application would do. >>>>> In this case an extension of pktio functionality would be required. >>>>> >>>> Yes but I was thinking of some change (in API and implementation) where >>>> the output of the crypto operations is the actual packet (with L2/L3 >>>> encapsulation), not a crypto completion event. >>>> >>>> >>>>> >>>>> Alex >>>>> >>>>> >>>>> On 8 October 2015 at 20:13, Ola Liljedahl <ola.liljedahl@linaro.org> >>>>> wrote: >>>>> >>>>>> Can this proposal be extended to handle inline IPsec processing, e.g. >>>>>> encrypt and encapsulate packet (include Ethernet framing) and then send to >>>>>> (enqueue) to some user-defined queue (which might be a pktio output queue)? >>>>>> Need some way to report errors and other conditions back to SW so >>>>>> might need some kind of ipsec notification event. >>>>>> Something for the ingress side as well, e.g. connect user-defined >>>>>> queues to IPsec input queue(s) and then specify corresponding output queues. >>>>>> >>>>>> >>>>>> On 31 July 2015 at 11:30, Alexandru Badicioiu < >>>>>> alexandru.badicioiu@linaro.org> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On 30 July 2015 at 17:44, Stuart Haslam <stuart.haslam@linaro.org> >>>>>>> wrote: >>>>>>> >>>>>>>> On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote: >>>>>>>> > On 30 July 2015 at 13:50, Stuart Haslam <stuart.haslam@linaro.org> >>>>>>>> wrote: >>>>>>>> > >>>>>>>> > > On Wed, Jul 22, 2015 at 11:26:03AM +0300, >>>>>>>> alexandru.badicioiu@linaro.org >>>>>>>> > > wrote: >>>>>>>> > > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >>>>>>>> > > > >>>>>>>> > > > This patch adds IPSec protocol processing capabilities to >>>>>>>> crypto >>>>>>>> > > > sesssions. Implementations which have these capabilities in >>>>>>>> hardware >>>>>>>> > > > crypto engines can use the extension to offload the >>>>>>>> application from >>>>>>>> > > > IPSec protocol processing. >>>>>>>> > > > >>>>>>>> > > > Signed-off-by: Alexandru Badicioiu < >>>>>>>> alexandru.badicioiu@linaro.org> >>>>>>>> > > > --- >>>>>>>> > > > include/odp/api/crypto_ipsec.h | 110 >>>>>>>> > > ++++++++++++++++++++ >>>>>>>> > > > platform/linux-generic/include/odp/crypto.h | 2 + >>>>>>>> > > > .../include/odp/plat/crypto_ipsec_types.h | 53 >>>>>>>> ++++++++++ >>>>>>>> > > > 3 files changed, 165 insertions(+), 0 deletions(-) >>>>>>>> > > > create mode 100644 include/odp/api/crypto_ipsec.h >>>>>>>> > > > create mode 100644 >>>>>>>> > > platform/linux-generic/include/odp/plat/crypto_ipsec_types.h >>>>>>>> > > > >>>>>>>> > > > diff --git a/include/odp/api/crypto_ipsec.h >>>>>>>> > > b/include/odp/api/crypto_ipsec.h >>>>>>>> > > > new file mode 100644 >>>>>>>> > > > index 0000000..e59fea4 >>>>>>>> > > > --- /dev/null >>>>>>>> > > > +++ b/include/odp/api/crypto_ipsec.h >>>>>>>> > > > @@ -0,0 +1,110 @@ >>>>>>>> > > > +/* Copyright (c) 2014, Linaro Limited >>>>>>>> > > > + * All rights reserved. >>>>>>>> > > > + * >>>>>>>> > > > + * SPDX-License-Identifier: BSD-3-Clause >>>>>>>> > > > + */ >>>>>>>> > > > + >>>>>>>> > > > +/** >>>>>>>> > > > + * @file >>>>>>>> > > > + * >>>>>>>> > > > + * ODP crypto IPSec extension >>>>>>>> > > > + */ >>>>>>>> > > > + >>>>>>>> > > > +#ifndef ODP_API_CRYPTO_IPSEC_H_ >>>>>>>> > > > +#define ODP_API_CRYPTO_IPSEC_H_ >>>>>>>> > > > + >>>>>>>> > > > +#ifdef __cplusplus >>>>>>>> > > > +extern "C" { >>>>>>>> > > > +#endif >>>>>>>> > > > + >>>>>>>> > > > +/** >>>>>>>> > > > + * @enum odp_ipsec_outhdr_type >>>>>>>> > > > + * IPSec tunnel outer header type >>>>>>>> > > > + * >>>>>>>> > > > + * @enum odp_ipsec_ar_ws >>>>>>>> > > > + * IPSec Anti-replay window size >>>>>>>> > > > + * >>>>>>>> > > > + */ >>>>>>>> > > > + >>>>>>>> > > > +typedef struct odp_ipsec_params { >>>>>>>> > > > + uint32_t spi; /** SPI value */ >>>>>>>> > > > + uint32_t seq; /** Initial SEQ number */ >>>>>>>> > > > + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size >>>>>>>> - >>>>>>>> > > > + inbound session with >>>>>>>> > > authentication */ >>>>>>>> > > >>>>>>>> > > This name is pretty cryptic, how about just replay_window? >>>>>>>> > > >>>>>>>> > [Alex] The standard name for this parameter is anti-replay >>>>>>>> window. In the >>>>>>>> > context of IPSec this should not be cryptic (odp_ipsec_arw vs >>>>>>>> odp_arw). If >>>>>>>> > your suggestion is to replace the name of the struct member - >>>>>>>> ar_ws with >>>>>>>> > replay_window I'm fine with it but not the name of the enum >>>>>>>> > (odp_ipsec_ar_ws). >>>>>>>> > Or maybe change it to enum odp_ipsec_arw. >>>>>>>> >>>>>>>> I meant the variable name, don't mind about the enum name. >>>>>>>> [Alex] I'm OK with the change. >>>>>>>> >>>>>>>> > > >>>>>>>> > > > + odp_bool_t esn; /** Use extended sequence >>>>>>>> numbers */ >>>>>>>> > > > + odp_bool_t auto_iv; /** Auto IV generation for each >>>>>>>> operation. >>>>>>>> > > */ >>>>>>>> > > > + uint16_t out_hdr_size; /** outer header size - tunnel >>>>>>>> mode */ >>>>>>>> > > > + uint8_t *out_hdr; /** outer header - tunnel mode >>>>>>>> */ >>>>>>>> > > >>>>>>>> > > Can these be 0 and NULL if the application wants tunnel mode >>>>>>>> but wants >>>>>>>> > > to handle the outer header itself? (i.e. add ESP head/tail and >>>>>>>> include >>>>>>>> > > inner IP header in encap data) >>>>>>>> > > >>>>>>>> > [Alex] Yes, that is the intended usage. If requested mode is >>>>>>>> tunnel and >>>>>>>> > there's no out_hdr specified, the application has to add it. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> > > > + enum odp_ipsec_outhdr_type out_hdr_type; /* outer >>>>>>>> header type - >>>>>>>> > > > + tunnel mode >>>>>>>> */ >>>>>>>> > > > + odp_bool_t ip_csum; /** update/verify ip header >>>>>>>> checksum */ >>>>>>>> > > > + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode >>>>>>>> encap & >>>>>>>> > > decap */ >>>>>>>> > > > + odp_bool_t remove_outer_hdr; /** remove outer header - >>>>>>>> tunnel mode >>>>>>>> > > decap */ >>>>>>>> > > > + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the >>>>>>>> IPv4 TOS or >>>>>>>> > > > + IPv6 Traffic Class byte >>>>>>>> from the >>>>>>>> > > inner/outer >>>>>>>> > > > + IP header to the >>>>>>>> outer/inner IP header >>>>>>>> > > - >>>>>>>> > > > + tunnel mode encap & decap */ >>>>>>>> > > > + odp_bool_t copy_df; /** Copy DF bit - copy the DF >>>>>>>> bit from >>>>>>>> > > > + the inner IP header to the >>>>>>>> > > > + outer IP header - tunnel >>>>>>>> mode encap */ >>>>>>>> > > > + odp_bool_t nat_t; /** NAT-T encapsulation enabled >>>>>>>> - tunnel >>>>>>>> > > mode */ >>>>>>>> > > > + odp_bool_t udp_csum; /** Update/verify UDP csum when >>>>>>>> NAT-T >>>>>>>> > > enabled */ >>>>>>>> > > > + >>>>>>>> > > > +} odp_ipsec_params_t; >>>>>>>> > > > + >>>>>>>> > > > +/** >>>>>>>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL >>>>>>>> > > > + * IPSec tunnel mode >>>>>>>> > > > + * >>>>>>>> > > > + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT >>>>>>>> > > > + * IPSec transport mode >>>>>>>> > > > + * >>>>>>>> > > > + * @enum odp_ipsec_proto >>>>>>>> > > > + * IPSec protocol >>>>>>>> > > > + */ >>>>>>>> > > > + >>>>>>>> > > > +/** >>>>>>>> > > > + * Configure crypto session for IPsec processing >>>>>>>> > > > + * >>>>>>>> > > > + * Configures a crypto session for IPSec protocol processing. >>>>>>>> > > > + * Packets submitted to an IPSec enabled session will have >>>>>>>> > > > + * relevant IPSec headers/trailers and tunnel headers >>>>>>>> > > > + * added/removed by the crypto implementation. >>>>>>>> > > > + * For example, the input packet for an IPSec ESP transport >>>>>>>> > > > + * enabled session should be the clear text packet with >>>>>>>> > > > + * no ESP headers/trailers prepared in advance for crypto >>>>>>>> operation. >>>>>>>> > > > + * The output packet will have ESP header, IV, trailer and >>>>>>>> the ESP ICV >>>>>>>> > > > + * added by crypto implementation. >>>>>>>> > > >>>>>>>> > > If a packet fails a check on decap (e.g. out of window), >>>>>>>> presumably the >>>>>>>> > > application gets an odp_crypto_op_result_t with ok=false, but >>>>>>>> is there >>>>>>>> > > any way for it to tell what failed? >>>>>>>> > > [Alex] Crypto operation error status should be extended with a >>>>>>>> code for >>>>>>>> > > out of window condition. >>>>>>>> > > > + * Depending on the particular capabilities of an >>>>>>>> implementation and >>>>>>>> > > > + * the parameters enabled by application, the application >>>>>>>> may be >>>>>>>> > > > + * partially or completely offloaded from IPSec protocol >>>>>>>> processing. >>>>>>>> > > > + * For example, if an implementation does not support >>>>>>>> checksum >>>>>>>> > > > + * update for IP header after adding ESP header the >>>>>>>> application >>>>>>>> > > > + * should update after crypto IPSec operation. >>>>>>>> > > > + * >>>>>>>> > > > + * If an implementation does not support a particular set of >>>>>>>> > > > + * arguments it should return error. >>>>>>>> > > > + * >>>>>>>> > > > + * @param session Session handle >>>>>>>> > > > + * @param ipsec_mode IPSec protocol mode >>>>>>>> > > > + * @param ipsec_proto IPSec protocol >>>>>>>> > > > + * @param ipsec_params IPSec parameters. >>>>>>>> Parameters which are >>>>>>>> > > not >>>>>>>> > > > + * relevant for selected protocol & >>>>>>>> mode are >>>>>>>> > > ignored - >>>>>>>> > > > + * e.g. outer_hdr/size set for ESP >>>>>>>> transport mode. >>>>>>>> > > > + * @retval 0 on success >>>>>>>> > > > + * @retval <0 on failure >>>>>>>> > > > + */ >>>>>>>> > > > +int odp_crypto_session_config_ipsec(odp_crypto_session_t >>>>>>>> session, >>>>>>>> > > > + enum odp_ipsec_mode >>>>>>>> ipsec_mode, >>>>>>>> > > > + enum odp_ipsec_proto >>>>>>>> ipsec_proto, >>>>>>>> > > > + odp_ipsec_params_t >>>>>>>> ipsec_params); >>>>>>>> > > >>>>>>>> > > Can this be called multiple times on the same session to update >>>>>>>> the >>>>>>>> > > parameters, or would the session need to be destroyed and >>>>>>>> recreated? >>>>>>>> > > >>>>>>>> > [Alex] No, this is not the intended use of this function. >>>>>>>> > This is to be called on a raw session (algorithm only). To change >>>>>>>> a crypto >>>>>>>> > session additional functions should be used. >>>>>>>> > >>>>>>>> > > >>>>>>>> > > Is it valid to create a session, issue a few operations, then >>>>>>>> later add >>>>>>>> > > the IPsec protocol config for that session? >>>>>>>> > > >>>>>>>> > [Alex] While this might work for a particular >>>>>>>> implementation/platform, I'm >>>>>>>> > wondering if there's an use case for it? >>>>>>>> > >>>>>>>> >>>>>>>> I wasn't thinking about a particular use case, there likely isn't >>>>>>>> one, >>>>>>>> just that how it's currently defined is open to >>>>>>>> misinterpretation/misuse. >>>>>>>> >>>>>>> [Alex] As we have only one session create call the session must be >>>>>>> usable right after creation. Being possible to extend the session for >>>>>>> protocol processing after some traffic passed it's up to implementation - >>>>>>> platform guys may give some inputs here. Other question can be - can we >>>>>>> configure a session for protocol processing while traffic is passing? Again >>>>>>> it may be possible for some and not for others. If there's no use-case for >>>>>>> it then applications should not rely on this and config can return an error. >>>>>>> Alternatively, we may need enable/disable calls for crypto sessions, >>>>>>> similarly with pktio start/stop functions. Pktio start/stop functions are >>>>>>> meant to clearly mark configuration phase - e.g. open pktio, config >>>>>>> classification, start pktio. However, enable/disable calls could be >>>>>>> required by IPSec rekeying process. >>>>>>> >>>>>>> >>>>>>>> > > >>>>>>>> > > I'm wondering why the params here weren't just made an >>>>>>>> extension of the >>>>>>>> > > odp_crypto_session_params_t in the initial session create. >>>>>>>> > > >>>>>>>> > [Alex] Do you mean to add these parameters as members of >>>>>>>> > odp_crypto_session_params_t or to extend session_create call to >>>>>>>> take IPSec >>>>>>>> > params? >>>>>>>> >>>>>>>> The first. >>>>>>>> >>>>>>>> > It wouldn't be a good idea to embed the IPSec parameters into the >>>>>>>> > odp_crypto_session_params_t as IPSec is just a protocol among >>>>>>>> others >>>>>>>> > supported by crypto engines (SSL/TLS, MACSEC, SRTP, etc). >>>>>>>> >>>>>>>> It could be done with a union + enum for each protocol (or NONE) >>>>>>>> and if >>>>>>>> we add odp_crypto_session_params_init(), which we should have >>>>>>>> anyway, >>>>>>>> you wouldn't need to know those fields existed when creating >>>>>>>> non-protocol >>>>>>>> sessions. >>>>>>>> >>>>>>> [Alex] I still don't think is a good idea. An application using only >>>>>>> raw sessions will waste memory with fields it actually doesn't use. Making >>>>>>> a structure larger is not cache/performance friendly, software running on >>>>>>> the cores access these structures on a per-packet basis. Crypto sessions >>>>>>> can be potentially millions. >>>>>>> >>>>>>>> >>>>>>>> > I think having a separate call odp_crypto_session_config_ipsec() >>>>>>>> makes the >>>>>>>> > source code more readable regarding the intent of the application >>>>>>>> rather >>>>>>>> > than filling in lots of IPSec parameters and calling then >>>>>>>> session_create(). >>>>>>>> > >>>>>>>> >>>>>>>> If there's only one way of doing it right - protocol config/params >>>>>>>> are >>>>>>>> set at session create time and not modified - and a number of ways >>>>>>>> of >>>>>>>> getting it wrong then it makes sense to define the API such that it >>>>>>>> can >>>>>>>> only be done the right way. >>>>>>>> >>>>>>> [Alex] I'm not sure it is possible to design an API __impossible__ >>>>>>> to misuse - i.e. not possible to compile a program which misuses the API. I >>>>>>> think that misuse rather should fail gracefully and with no side effects. >>>>>>> There are other ODP API areas that can be easily misused - for example >>>>>>> pktio - can we remove the default input queue after start function or while >>>>>>> traffic is passing ? Can we remove the queue after sending a few packets >>>>>>> but without stopping? Scheduling API requires calling pause before exiting >>>>>>> a schedule loop - what happens if pause is not called? >>>>>>> I think these are rather runtime aspects that should be handled at >>>>>>> runtime. More than that, different platforms/implementation may behave >>>>>>> differently, but a portable application should not be based on a particular >>>>>>> behavior. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>>> Stuart. >>>>>>>> >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> lng-odp mailing list >>>>>>> lng-odp@lists.linaro.org >>>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
diff --git a/include/odp/api/crypto_ipsec.h b/include/odp/api/crypto_ipsec.h new file mode 100644 index 0000000..e59fea4 --- /dev/null +++ b/include/odp/api/crypto_ipsec.h @@ -0,0 +1,110 @@ +/* Copyright (c) 2014, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + * + * ODP crypto IPSec extension + */ + +#ifndef ODP_API_CRYPTO_IPSEC_H_ +#define ODP_API_CRYPTO_IPSEC_H_ + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @enum odp_ipsec_outhdr_type + * IPSec tunnel outer header type + * + * @enum odp_ipsec_ar_ws + * IPSec Anti-replay window size + * + */ + +typedef struct odp_ipsec_params { + uint32_t spi; /** SPI value */ + uint32_t seq; /** Initial SEQ number */ + enum odp_ipsec_ar_ws ar_ws; /** Anti-replay window size - + inbound session with authentication */ + odp_bool_t esn; /** Use extended sequence numbers */ + odp_bool_t auto_iv; /** Auto IV generation for each operation. */ + uint16_t out_hdr_size; /** outer header size - tunnel mode */ + uint8_t *out_hdr; /** outer header - tunnel mode */ + enum odp_ipsec_outhdr_type out_hdr_type; /* outer header type - + tunnel mode */ + odp_bool_t ip_csum; /** update/verify ip header checksum */ + odp_bool_t ip_dttl; /** decrement ttl - tunnel mode encap & decap */ + odp_bool_t remove_outer_hdr; /** remove outer header - tunnel mode decap */ + odp_bool_t copy_dscp; /** DiffServ Copy - Copy the IPv4 TOS or + IPv6 Traffic Class byte from the inner/outer + IP header to the outer/inner IP header - + tunnel mode encap & decap */ + odp_bool_t copy_df; /** Copy DF bit - copy the DF bit from + the inner IP header to the + outer IP header - tunnel mode encap */ + odp_bool_t nat_t; /** NAT-T encapsulation enabled - tunnel mode */ + odp_bool_t udp_csum; /** Update/verify UDP csum when NAT-T enabled */ + +} odp_ipsec_params_t; + +/** + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TUNNEL + * IPSec tunnel mode + * + * @enum odp_ipsec_mode:ODP_IPSEC_MODE_TRANSPORT + * IPSec transport mode + * + * @enum odp_ipsec_proto + * IPSec protocol + */ + +/** + * Configure crypto session for IPsec processing + * + * Configures a crypto session for IPSec protocol processing. + * Packets submitted to an IPSec enabled session will have + * relevant IPSec headers/trailers and tunnel headers + * added/removed by the crypto implementation. + * For example, the input packet for an IPSec ESP transport + * enabled session should be the clear text packet with + * no ESP headers/trailers prepared in advance for crypto operation. + * The output packet will have ESP header, IV, trailer and the ESP ICV + * added by crypto implementation. + * Depending on the particular capabilities of an implementation and + * the parameters enabled by application, the application may be + * partially or completely offloaded from IPSec protocol processing. + * For example, if an implementation does not support checksum + * update for IP header after adding ESP header the application + * should update after crypto IPSec operation. + * + * If an implementation does not support a particular set of + * arguments it should return error. + * + * @param session Session handle + * @param ipsec_mode IPSec protocol mode + * @param ipsec_proto IPSec protocol + * @param ipsec_params IPSec parameters. Parameters which are not + * relevant for selected protocol & mode are ignored - + * e.g. outer_hdr/size set for ESP transport mode. + * @retval 0 on success + * @retval <0 on failure + */ +int odp_crypto_session_config_ipsec(odp_crypto_session_t session, + enum odp_ipsec_mode ipsec_mode, + enum odp_ipsec_proto ipsec_proto, + odp_ipsec_params_t ipsec_params); + +/** + * @} + */ + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/platform/linux-generic/include/odp/crypto.h b/platform/linux-generic/include/odp/crypto.h index 7684c1e..718ab7d 100644 --- a/platform/linux-generic/include/odp/crypto.h +++ b/platform/linux-generic/include/odp/crypto.h @@ -20,6 +20,7 @@ extern "C" { #include <odp/std_types.h> #include <odp/plat/packet_types.h> #include <odp/plat/crypto_types.h> +#include <odp/plat/crypto_ipsec_types.h> #include <odp/plat/buffer_types.h> #include <odp/plat/pool_types.h> #include <odp/queue.h> @@ -33,6 +34,7 @@ extern "C" { */ #include <odp/api/crypto.h> +#include <odp/api/crypto_ipsec.h> #ifdef __cplusplus } diff --git a/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h new file mode 100644 index 0000000..74521da --- /dev/null +++ b/platform/linux-generic/include/odp/plat/crypto_ipsec_types.h @@ -0,0 +1,53 @@ +/* Copyright (c) 2015, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + * + * ODP crypto + */ + +#ifndef ODP_CRYPTO_IPSEC_TYPES_H_ +#define ODP_CRYPTO_IPSEC_TYPES_H_ + +#ifdef __cplusplus +extern "C" { +#endif + +/** @addtogroup odp_crypto + * @{ + */ + +enum odp_ipsec_mode { + ODP_IPSEC_MODE_TUNNEL, /**< IPSec tunnel mode */ + ODP_IPSEC_MODE_TRANSPORT, /**< IPSec transport mode */ +}; + +enum odp_ipsec_proto { + ODP_IPSEC_ESP, /**< ESP protocol */ +}; + +enum odp_ipsec_outhdr_type { + ODP_IPSEC_OUTHDR_IPV4, /**< Outer header is IPv4 */ + ODP_IPSEC_OUTHDR_IPV6, /**< Outer header is IPv6 */ +}; + +enum odp_ipsec_ar_ws { + ODP_IPSEC_AR_WS_NONE, /**< Anti-replay is not enabled */ + ODP_IPSEC_AR_WS_32, /**< Anti-replay window size 32 */ + ODP_IPSEC_AR_WS_64, /**< Anti-replay window size 64 */ + ODP_IPSEC_AR_WS_128, /**< Anti-replay window size 128 */ +}; + +/** + * @} + */ + +#ifdef __cplusplus +} +#endif + +#endif