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 <[email protected]> wrote: > On 9 October 2015 at 10:37, Alexandru Badicioiu < > [email protected]> 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 <[email protected]> >> wrote: >> >>> On 9 October 2015 at 09:34, Alexandru Badicioiu < >>> [email protected]> 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 <[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
