Hi Petri,
1. I am not a big fan of idea that application level data stored per SA needs to be accessed per packet: for example for asynchronous processing you need to know if processing is inbound or outbound, tunnel or transport and also to update stats through this mechanism: sa->context->sa_application_data. It is obvious it will need locks. I would prefer to have some sort of IPsec metadata associated to packet to provide offsets and next layer protocol. I imagine that sooner or later we will need to pass some other data from IPsec engine to application … so mechanism needs to be extensible. I am wonder how will be calculate lifetime in bytes for asynchronous inbound operations and how the application can figure it out from output packets: it is only output packet data or should contain ESP header/trailer + outer IP header? 2. Esthetics: Why use two arrays of equal size inside odp_ipsec_op_result_t instead of one containing both result and output packet? For odp_ipsec_op_param_t (maybe) it make sense as one can set zero SAs and zero operation options. Is the gain big enough? BR, Bogdan On 28 November 2016 at 11:46, Nikhil Agarwal <nikhil.agar...@nxp.com> wrote: > > > -----Original Message----- > From: Savolainen, Petri (Nokia - FI/Espoo) [mailto:petri.savolainen@ > nokia-bell-labs.com] > Sent: Friday, November 25, 2016 6:40 PM > To: Nikhil Agarwal <nikhil.agar...@nxp.com>; lng-odp@lists.linaro.org > Subject: RE: [lng-odp] [API-NEXT PATCH] api: ipsec: added IPSEC API > > > > > -----Original Message----- > > From: Nikhil Agarwal [mailto:nikhil.agar...@nxp.com] > > Sent: Friday, November 25, 2016 1:22 PM > > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- > > labs.com>; lng-odp@lists.linaro.org > > Subject: RE: [lng-odp] [API-NEXT PATCH] api: ipsec: added IPSEC API > > > > > > > > -----Original Message----- > > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of > > Petri Savolainen > > Sent: Wednesday, November 23, 2016 4:10 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [API-NEXT PATCH] api: ipsec: added IPSEC API > > > > Added definitions for a look-a-side IPSEC offload API. In addition to > > IPSEC packet transformations, it also supports: > > * inbound SA look up > > * outbound IP fragmentation > > > > Signed-off-by: Petri Savolainen <petri.savolai...@nokia.com> > > --- > > > > Changes to draft: > > * renamed odp_ipsec_proto_t to renamed odp_ipsec_protocol_t > > * specify that lifetime sec limit is from the SA creation > > * added odp_ipsec_sa_context() > > * pool for output packets is the same as packet input pool > > * added antireplay check and protocol error codes > > * specified which input / output packet offsets and flags are set > > * moved sync/async mode selection to global config > > (odp_ipsec_config()) > > * added IPSEC capability to aid mode selection > > * specify that also packet user area is copied from input to output > > packet > > > > > > include/odp/api/spec/event.h | 2 +- > > include/odp/api/spec/ipsec.h | 856 > > +++++++++++++++++++++ > > include/odp_api.h | 1 + > > platform/Makefile.inc | 1 + > > platform/linux-generic/Makefile.am | 2 + > > platform/linux-generic/include/odp/api/ipsec.h | 36 + > > .../include/odp/api/plat/event_types.h | 1 + > > .../include/odp/api/plat/ipsec_types.h | 39 + > > 8 files changed, 937 insertions(+), 1 deletion(-) create mode 100644 > > include/odp/api/spec/ipsec.h create mode 100644 platform/linux- > > generic/include/odp/api/ipsec.h create mode 100644 platform/linux- > > generic/include/odp/api/plat/ipsec_types.h > > > > + > > +/** > > + * IPSEC capability > > + */ > > +typedef struct odp_ipsec_capability_t { > > Capability should expose fragmentation options and SA lifetime options > > as well. > > Lookup modes capability may also be exposed. > > > Which kind of options? What lacking HW features cannot be filled in with > (reasonable amount of) additional SW ? > > For example, if HW cannot do > * pre-fragmentation: implementation can do that first in SW and then send > fragments to HW > * SPI lookup: implementation can have e.g. a vector instruction optimized > data base of that > * time limit calculation: implementation may use CPU local time source to > check against limit on every (few) calls > As for time limit, usually control plane applications(like strongswan) > already runs a timer for SA life time. It would be overhead to run two > timers(one in application other in SW implementation) in case hardware does > not provide such functionalities. There would be no way for application to > know whether functionality is in HW or SW, so that application may choose > the optimal design. > * byte limit: implementation may use atomic instructions, which may be > costly on high contention SA, but application would do the same > > If too many things are optional (in capability), application have to > implement lot of things in SW - which is often less efficient and degrade > the value of the common API. > > > > +/** > > + * Outbound asynchronous IPSEC operation > > + * > > + * This operation does outbound IPSEC processing in asynchronous mode > > + * (ODP_IPSEC_OP_MODE_ASYNC). It processes packets otherwise > > +identically to > > + * odp_ipsec_out(), but outputs all results through one or multiple > > + * ODP_EVENT_IPSEC_RESULT events. > > + * > > + * Packets are processed in the input order. Packet order is > > +maintained from > > + * input 'pkt' array to 'pkt' array of each resulting event. > > +Resulting events > > + * of the same SA should be enqueued in order and all packets within > > +the same > > + * event must be in order. > > Ordering Cannot be assured for cases where packets from multiple > > session are input because accelerators have multiple engines running > > in Parallel. Packets from same session may be serialized but not all. > > If I enqueue with single call: pkt1/SA1, pkt1/SA2, pkt2/SA1, pkt2/SA2. > Both SAs have the same dest queue. > > For example, these would be acceptable events: > * one event > 1) pkt1/SA1, pkt1/SA2, pkt2/SA1, pkt2/SA2 > * two events > 1) pkt1/SA1, pkt1/SA2 > 2) pkt2/SA1, pkt2/SA2 > * two events (one per SA) > 1) pkt1/SA1, pkt2/SA1 > 2) pkt1/SA2, pkt2/SA2 > * two events (one per SA) > 1) pkt1/SA2, pkt2/SA2 > 2) pkt1/SA1, pkt2/SA1 > * four events > 1) pkt1/SA1 > 2) pkt1/SA2 > 3) pkt2/SA1 > 4) pkt2/SA2 > * four events > 2) pkt1/SA2 > 4) pkt2/SA2 > 1) pkt1/SA1 > 3) pkt2/SA1 > > So, within one event and one SA pkt1 and pkt2 must not be reordered. > Implementation may split an input array into multiple resulting events. > Resulting events for the same SA should not be reordered. > > -- Yes. As per understanding, Ordering should be maintained for same SA > packets. > > > + * > > + * @param input Operation input parameters > > + * > > + * @return Number of input packets consumed (0 ... input.num_pkt) > > + * @retval <0 On failure > > + * > > + * @see odp_ipsec_out(), odp_ipsec_result() */ int > > +odp_ipsec_out_enq(const odp_ipsec_op_param_t *input); > > + > > +/** > > + * Get IPSEC results from an ODP_EVENT_IPSEC_RESULT event > > + * > > + * Copies IPSEC operation results from an event. The event must be of > > + * type ODP_EVENT_IPSEC_RESULT. The event may be freed after the call. > > + * > > + * @param[out] result Pointer to operation result for output > > + * @param event An ODP_EVENT_IPSEC_RESULT event > > + * > > + * @return Number of packets in the event. If this is larger than > > + * 'result.num_pkt', all packets did not fit into result struct > > and > > + * application must call the function again with a larger result > > struct. > > + * @retval <0 On failure > > + * > > + * @see odp_ipsec_in_enq(), odp_ipsec_out_enq() */ int > > +odp_ipsec_result(odp_ipsec_op_result_t *result, odp_event_t event); > > The event enqueued back to completion queue will give one packet at a > > time not the complete burst, Because the input burst may contain > > multiple transaction to accelerator(for different SA). > > In case burst operation is required, application may use dequeue_multi > > operation. > > It's OK design to produce a result event per output packet. This function > would then return always a single packet. > > As per the understanding here, even if application gives a burst operation > request(multiple packets in odp_ipsec_in/out_enq), implementation may > choose to return packets one by one. > > > -Petri > >
