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 <[email protected]> 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 < > [email protected]> wrote: > >> >> >> On 30 July 2015 at 17:44, Stuart Haslam <[email protected]> wrote: >> >>> 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. >>> [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 >> [email protected] >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
