On 30 July 2015 at 13:50, Stuart Haslam <[email protected]> wrote:
> On Wed, Jul 22, 2015 at 11:26:03AM +0300, [email protected] > wrote: > > From: Alexandru Badicioiu <[email protected]> > > > > 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 <[email protected]> > > --- > > 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. >
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
