On 9 October 2015 at 12:48, Alexandru Badicioiu <
[email protected]> 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 <[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

Reply via email to