On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote: > 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.
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. -- Stuart. _______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
