On 9 October 2015 at 09:34, Alexandru Badicioiu <
alexandru.badici...@linaro.org> 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 <ola.liljed...@linaro.org>
> 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 <
>> alexandru.badici...@linaro.org> wrote:
>>
>>>
>>>
>>> On 30 July 2015 at 17:44, Stuart Haslam <stuart.has...@linaro.org>
>>> wrote:
>>>
>>>> On Thu, Jul 30, 2015 at 02:42:08PM +0300, Alexandru Badicioiu wrote:
>>>> > On 30 July 2015 at 13:50, Stuart Haslam <stuart.has...@linaro.org>
>>>> wrote:
>>>> >
>>>> > > On Wed, Jul 22, 2015 at 11:26:03AM +0300,
>>>> alexandru.badici...@linaro.org
>>>> > > wrote:
>>>> > > > From: Alexandru Badicioiu <alexandru.badici...@linaro.org>
>>>> > > >
>>>> > > > 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 <
>>>> alexandru.badici...@linaro.org>
>>>> > > > ---
>>>> > > > 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
>>> lng-odp@lists.linaro.org
>>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>>
>>
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
