diff mbox series

[API-NEXT,1/2] api: ipsec: change IPSEC result to packet

Message ID 20170609105143.11350-2-petri.savolainen@linaro.org
State New
Headers show
Series IPSEC packet event | expand

Commit Message

Petri Savolainen June 9, 2017, 10:51 a.m. UTC
Input and output of IPSEC operations are packets. Parameter and
result structures are cleaner when packet arrays are direct
parameters to functions. Also API is more flexible for
application or API pipelining when output is packets with
additional metadata. Application or API pipeline stages which
do not care about IPSEC results may work on basic packet metadata.

IPSEC result event type changes from ODP_EVENT_IPSEC_RESULT to
ODP_EVENT_PACKET_IPSEC. Later on, other packet producing APIs
(crypto, comp, etc) may also produce special packet events.

Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

---
 include/odp/api/spec/event.h                       |  27 +-
 include/odp/api/spec/ipsec.h                       | 365 +++++++++++----------
 include/odp/arch/default/api/abi/event.h           |   6 +-
 .../include/odp/api/plat/event_types.h             |   7 +-
 platform/linux-generic/odp_ipsec.c                 |  67 +++-
 5 files changed, 279 insertions(+), 193 deletions(-)

-- 
2.13.0

Comments

Dmitry Eremin-Solenikov June 9, 2017, 11:59 a.m. UTC | #1
On 09.06.2017 13:51, Petri Savolainen wrote:
> Input and output of IPSEC operations are packets. Parameter and

> result structures are cleaner when packet arrays are direct

> parameters to functions. Also API is more flexible for

> application or API pipelining when output is packets with

> additional metadata. Application or API pipeline stages which

> do not care about IPSEC results may work on basic packet metadata.

> 

> IPSEC result event type changes from ODP_EVENT_IPSEC_RESULT to

> ODP_EVENT_PACKET_IPSEC. Later on, other packet producing APIs

> (crypto, comp, etc) may also produce special packet events.

> 

> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

> ---

>  include/odp/api/spec/event.h                       |  27 +-

>  include/odp/api/spec/ipsec.h                       | 365 +++++++++++----------

>  include/odp/arch/default/api/abi/event.h           |   6 +-

>  .../include/odp/api/plat/event_types.h             |   7 +-

>  platform/linux-generic/odp_ipsec.c                 |  67 +++-

>  5 files changed, 279 insertions(+), 193 deletions(-)

> 

> diff --git a/include/odp/api/spec/event.h b/include/odp/api/spec/event.h

> index f22efce5..ad757c7d 100644

> --- a/include/odp/api/spec/event.h

> +++ b/include/odp/api/spec/event.h

> @@ -37,9 +37,30 @@ extern "C" {

>  

>  /**

>   * @typedef odp_event_type_t

> - * ODP event types:

> - * ODP_EVENT_BUFFER, ODP_EVENT_PACKET, ODP_EVENT_TIMEOUT,

> - * ODP_EVENT_CRYPTO_COMPL, ODP_EVENT_IPSEC_RESULT, ODP_EVENT_IPSEC_STATUS

> + * Event type

> + *

> + * Event type specifies the purpose and format of an event. It can be checked

> + * with odp_event_type(). Each event type has functions

> + * (e.g. odp_buffer_from_event()) to convert between the generic event handle

> + * (odp_event_t) and the type specific handle (e.g. odp_buffer_t). Results are

> + * undefined, if conversion function of a wrong event type is used. Application

> + * cannot change event type by chaining conversion functions.

> + *

> + * List of event types:

> + * - ODP_EVENT_BUFFER

> + *     - Buffer event (odp_buffer_t) for simple data storage and message passing

> + * - ODP_EVENT_PACKET

> + *     - Normal packet event (odp_packet_t) containing basic packet metadata.

> + * - ODP_EVENT_PACKET_IPSEC

> + *     - Packet event (odp_packet_t) genereted as a result of an IPsec


generated

> + *       operation. It contains IPSEC specific metadata in addition to the basic

> + *       packet metadata.

> + * - ODP_EVENT_TIMEOUT

> + *     - Timeout event (odp_timeout_t) from a timer

> + * - ODP_EVENT_CRYPTO_COMPL

> + *     - Crypto completion event (odp_crypto_compl_t)

> + * - ODP_EVENT_IPSEC_STATUS

> + *     - IPSEC status update event (odp_ipsec_status_t)

>   */

>  

>  /**

> diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h

> index 65f0b066..0da2b42c 100644

> --- a/include/odp/api/spec/ipsec.h

> +++ b/include/odp/api/spec/ipsec.h

> @@ -552,16 +552,18 @@ typedef enum odp_ipsec_frag_mode_t {

>  

>  /**

>   * Packet lookup mode

> + *

> + * Lookup mode controls how an SA participates in SA lookup offload.

> + * Inbound operations perform SA lookup if application does not provide a SA as

> + * a parameter. In inline mode, a lookup miss directs the packet back to normal

> + * packet input interface processing. SA lookup failure status (error.sa_lookup)

> + * is reported through odp_ipsec_packet_result_t.

>   */

>  typedef enum odp_ipsec_lookup_mode_t {

> -	/** Inbound SA lookup is disabled. */

> +	/** Inbound SA lookup is disabled for the SA. */

>  	ODP_IPSEC_LOOKUP_DISABLED = 0,

>  

> -	/** Inbound SA lookup is enabled. Lookup matches only SPI value.

> -	 *  In inline mode, a lookup miss directs the packet back to normal

> -	 *  packet input interface processing. In other modes, the SA lookup

> -	 *  failure status (error.sa_lookup) is reported through

> -	 *  odp_ipsec_packet_result_t. */

> +	/** Inbound SA lookup is enabled. Lookup matches only SPI value. */

>  	ODP_IPSEC_LOOKUP_SPI,

>  

>  	/** Inbound SA lookup is enabled. Lookup matches both SPI value and


This can go to separate patch.

> @@ -850,17 +852,6 @@ int odp_ipsec_sa_destroy(odp_ipsec_sa_t sa);

>   */

>  uint64_t odp_ipsec_sa_to_u64(odp_ipsec_sa_t sa);

>  

> -/**

> - * IPSEC operation level options

> - *

> - * These may be used to override some SA level options

> - */

> -typedef struct odp_ipsec_op_opt_t {

> -	/** Fragmentation mode */

> -	odp_ipsec_frag_mode_t mode;

> -

> -} odp_ipsec_op_opt_t;

> -

>  /** IPSEC operation status has no errors */

>  #define ODP_IPSEC_OK 0

>  

> @@ -870,7 +861,8 @@ typedef struct odp_ipsec_op_status_t {

>  	union {

>  		/** Error flags */

>  		struct {

> -			/** Protocol error. Not a valid ESP or AH packet. */

> +			/** Protocol error. Not a valid ESP or AH packet,

> +			 *  packet data length error, etc. */

>  			uint32_t proto            : 1;

>  

>  			/** SA lookup failed */

> @@ -934,41 +926,70 @@ typedef struct odp_ipsec_op_status_t {

>  } odp_ipsec_op_status_t;

>  

>  /**

> - * IPSEC operation input parameters

> + * IPSEC outbound operation options

> + *

> + * These may be used to override some SA level options

>   */

> -typedef struct odp_ipsec_op_param_t {

> -	/** Number of packets to be processed */

> -	int num_pkt;

> +typedef struct odp_ipsec_out_opt_t {

> +	/** Fragmentation mode */

> +	odp_ipsec_frag_mode_t mode;

> +

> +} odp_ipsec_out_opt_t;


Maybe we can just inline this into out_param_t ?
With this in/out split, it should be quite straightforward change from
the API point of view.

>  

> +/**

> + * IPSEC outbound operation parameters

> + */

> +typedef struct odp_ipsec_out_param_t {

>  	/** Number of SAs

>  	 *

> +	 *  Outbound IPSEC operation needs SA from application. Use either

> +	 *  single SA for all packets, or a SA per packet.

> +	 *

>  	 *  Valid values are:

> -	 *  * 0:       No SAs (default)

> -	 *  * 1:       Single SA for all packets

> -	 *  * num_pkt: SA per packet

> +	 *  * 1:  Single SA for all packets

> +	 *  * N:  A SA per packet. N must match the number of packets.

>  	 */

>  	int num_sa;

>  

> -	/** Number of operation options

> +	/** Number of outbound operation options

>  	 *

>  	 *  Valid values are:

> -	 *  * 0:       No options (default)

> -	 *  * 1:       Single option for all packets

> -	 *  * num_pkt: An option per packet

> +	 *  * 0:  No options

> +	 *  * 1:  Single option for all packets

> +	 *  * N:  An option per packet. N must match the number of packets.

>  	 */

>  	int num_opt;

>  

> -	/** Pointer to an array of packets

> +	/** Pointer to an array of IPSEC SAs */

> +	odp_ipsec_sa_t *sa;

> +

> +	/** Pointer to an array of outbound operation options

> +	 *

> +	 *  May be NULL when num_opt is zero.

> +	 */

> +	odp_ipsec_out_opt_t *opt;

> +

> +} odp_ipsec_out_param_t;

> +

> +/**

> + * IPSEC inbound operation parameters

> + */

> +typedef struct odp_ipsec_in_param_t {

> +	/** Number of SAs

>  	 *

> -	 *  Each packet must have a valid value for these metadata:

> -	 *  * L3 offset: Offset to the first byte of the (outmost) IP header

> -	 *  * L4 offset: For inbound direction, when udp_encap is enabled -

> -	 *               offset to the first byte of the encapsulating UDP

> -	 *               header

> +	 *  Inbound IPSEC operation processes a packet using the SA provided by

> +	 *  the application. If the application does not provide an SA, the

> +	 *  operation searches for the SA by matching the input packet with all

> +	 *  inbound SAs according to the lookup mode (odp_ipsec_lookup_mode_t)

> +	 *  configured in each SA. When passing SAs, use either single SA for

> +	 *  all packets, or a SA per packet.

>  	 *

> -	 *  @see odp_packet_l3_offset(), odp_packet_l4_offset()

> +	 *  Valid values are:

> +	 *  * 0:  No SAs. SA lookup is done for all packets.

> +	 *  * 1:  Single SA for all packets

> +	 *  * N:  A SA per packet. N must match the number of packets.

>  	 */

> -	odp_packet_t *pkt;

> +	int num_sa;

>  

>  	/** Pointer to an array of IPSEC SAs

>  	 *

> @@ -976,18 +997,12 @@ typedef struct odp_ipsec_op_param_t {

>  	 */

>  	odp_ipsec_sa_t *sa;

>  

> -	/** Pointer to an array of operation options

> -	 *

> -	 *  May be NULL when num_opt is zero.

> -	 */

> -	odp_ipsec_op_opt_t *opt;

> -

> -} odp_ipsec_op_param_t;

> +} odp_ipsec_in_param_t;


I like this idea, it will simplify code for both implementations and
applications. However can we move this op_param -> in_param+out_param to
a separate patch?

>  /**

>   * Outbound inline IPSEC operation parameters

>   */

> -typedef struct odp_ipsec_inline_op_param_t {

> +typedef struct odp_ipsec_out_inline_param_t {

>  	/** Packet output interface for inline output operation

>  	 *

>  	 *  Outbound inline IPSEC operation uses this packet IO interface to

> @@ -1011,7 +1026,7 @@ typedef struct odp_ipsec_inline_op_param_t {

>  		uint32_t len;

>  	} outer_hdr;

>  

> -} odp_ipsec_inline_op_param_t;

> +} odp_ipsec_out_inline_param_t;

>  

>  /**

>   * IPSEC operation result for a packet

> @@ -1020,16 +1035,6 @@ typedef struct odp_ipsec_packet_result_t {

>  	/** IPSEC operation status */

>  	odp_ipsec_op_status_t status;

>  

> -	/** Number of output packets created from the corresponding input packet

> -	 *

> -	 *  Without fragmentation offload this is always one. However, if the

> -	 *  input packet was fragmented during the operation this is larger than

> -	 *  one for the first returned fragment and zero for the rest of the

> -	 *  fragments. All the fragments (of the same source packet) are stored

> -	 *  consecutively in the 'pkt' array.

> -	 */

> -	int num_out;

> -

>  	/** IPSEC SA that was used to create the packet

>  	 *

>  	 *  Operation updates this SA handle value, when SA look up is performed

> @@ -1040,7 +1045,8 @@ typedef struct odp_ipsec_packet_result_t {

>  	odp_ipsec_sa_t sa;

>  

>  	/** Packet outer header status before inbound inline processing.

> -	 *  This is valid only when status.flag.inline_mode is set.

> +	 *  This is valid only when outer headers are retained

> +	 *  (see odp_ipsec_inbound_config_t) and status.flag.inline_mode is set.


Related question: would we like to lift this restriction and enable
headers retaining for all kinds of inline processing?

>  	 */

>  	struct {

>  		/** Points to the first byte of retained outer headers. These

> @@ -1048,7 +1054,7 @@ typedef struct odp_ipsec_packet_result_t {

>  		 *  implementation specific memory space. Since the memory space

>  		 *  may overlap with e.g. packet head/tailroom, the content

>  		 *  becomes invalid if packet data storage is modified in

> -		 *  anyway. The memory space may not be sharable to other

> +		 *  any way. The memory space may not be sharable to other

>  		 *  threads. */

>  		uint8_t *ptr;

>  

> @@ -1059,51 +1065,6 @@ typedef struct odp_ipsec_packet_result_t {

>  } odp_ipsec_packet_result_t;

>  

>  /**

> - * IPSEC operation results

> - */

> -typedef struct odp_ipsec_op_result_t {

> -	/** Number of packets

> -	 *

> -	 *  Application sets this to the maximum number of packets the operation

> -	 *  may output (number of elements in 'pkt' and 'res' arrays).

> -	 *  The operation updates it with the actual number of packets

> -	 *  outputted.

> -	 */

> -	int num_pkt;

> -

> -	/** Pointer to an array of packets

> -	 *

> -	 *  Operation outputs packets into this array. The array must have

> -	 *  at least 'num_pkt' elements.

> -	 *

> -	 *  Each successfully transformed packet has a valid value for these

> -	 *  metadata regardless of the inner packet parse configuration.

> -	 *  (odp_ipsec_inbound_config_t):

> -	 *  * L3 offset: Offset to the first byte of the (outmost) IP header

> -	 *  * pktio:     For inbound inline IPSEC processed packets, original

> -	 *               packet input interface

> -	 *

> -	 *  Other metadata for parse results and error checks depend on

> -	 *  configuration (selected parse and error check levels).

> -	 */

> -	odp_packet_t *pkt;

> -

> -	/** Pointer to an array of per packet operation results

> -	 *

> -	 *  Operation outputs results for each outputted packet into this array.

> -	 *  The array must have at least 'num_pkt' elements. The results include

> -	 *  operation status and packet form information for each outputted

> -	 *  packet.

> -	 *

> -	 *  For example, some packets may not have been transformed due to

> -	 *  an error, but the original packet is returned with appropriate

> -	 *  packet result information instead.

> -	 */

> -	odp_ipsec_packet_result_t *res;

> -

> -} odp_ipsec_op_result_t;

> -

> -/**

>   * IPSEC status ID

>   */

>  typedef enum odp_ipsec_status_id_t {

> @@ -1136,20 +1097,32 @@ typedef struct odp_ipsec_status_t {

>   *

>   * This operation does inbound IPSEC processing in synchronous mode

>   * (ODP_IPSEC_OP_MODE_SYNC). A successful operation returns the number of

> - * packets consumed and outputs a new packet handle as well as an operation

> - * result for each outputted packet. The operation does not modify packets that

> - * it does not consume. It cannot consume all input packets if 'output.num_pkt'

> - * is smaller than 'input.num_pkt'.

> + * packets consumed and outputs a new packet handle for each outputted packet.

> + * Outputted packets contain IPSEC result metadata (odp_ipsec_packet_result_t),

> + * which should be checked for transformation errors, etc. The operation does

> + * not modify packets that it does not consume. It cannot consume all input

> + * packets if 'num_out' is smaller than 'num_in'.

>   *

>   * Packet context pointer and user area content are copied from input to output

>   * packets. Output packets are allocated from the same pool(s) as input packets.

>   *

> - * When 'input.num_sa' is zero, this operation performs SA look up for each

> + * When 'param.num_sa' is zero, this operation performs SA look up for each

>   * packet. Otherwise, application must provide the SA(s) as part of operation

> - * input parameters (odp_ipsec_op_param_t). The operation outputs used SA(s) as

> - * part of per packet operation results (odp_ipsec_packet_result_t), or an error

> + * input parameters (odp_ipsec_in_param_t). The operation outputs used SA(s) as

> + * part of per packet results (odp_ipsec_packet_result_t), or an error

>   * status if a SA was not found.

>   *

> + * Each input packet must have a valid value for these metadata (other metadata

> + * is ignored):

> + *  * L3 offset: Offset to the first byte of the (outmost) IP header

> + *  * L4 offset: When udp_encap is enabled, offset to the first byte of the

> + *               encapsulating UDP header

> + *

> + * Additionally, implementation checks input IP packet length (odp_packet_len()

> + * minus odp_packet_l3_offset()) against protocol headers and reports an error

> + * (status.error.proto) if packet data length is less than protocol headers

> + * indicate.

> + *

>   * Packets are processed in the input order. Packet order is maintained from

>   * input 'pkt' array to output 'pkt' array. Packet order is not guaranteed

>   * between calling threads.

> @@ -1162,35 +1135,50 @@ typedef struct odp_ipsec_status_t {

>   * restored. The amount and content of packet data before the IP header is

>   * undefined.

>   *

> - * @param         input   Operation input parameters

> - * @param[out]    output  Operation results

> + * @param          pkt_in   Packets to be processed

> + * @param          num_in   Number of packets to be processed

> + * @param[out]     pkt_out  Packet handle array for resulting packets

> + * @param[in, out] num_out  Number of resulting packets. Application sets this

> + *                          to 'pkt_out' array size. A successful operation sets

> + *                          this to the number of outputted packets

> + *                          (1 ... num_out).

> + * @param          param    Inbound operation parameters

>   *

> - * @return Number of input packets consumed (0 ... input.num_pkt)

> + * @return Number of input packets consumed (0 ... num_in)

>   * @retval <0     On failure

>   *

> - * @see odp_packet_user_ptr(), odp_packet_user_area()

> + * @see odp_packet_user_ptr(), odp_packet_user_area(), odp_packet_l3_offset(),

> + * odp_packet_l4_offset()

>   */

> -int odp_ipsec_in(const odp_ipsec_op_param_t *input,

> -		 odp_ipsec_op_result_t *output);

> +int odp_ipsec_in(const odp_packet_t pkt_in[], int num_in,

> +		 odp_packet_t pkt_out[], int *num_out,

> +		 const odp_ipsec_in_param_t *param);

>  

>  /**

>   * Outbound synchronous IPSEC operation

>   *

>   * This operation does outbound IPSEC processing in synchronous mode

>   * (ODP_IPSEC_OP_MODE_SYNC). A successful operation returns the number of

> - * packets consumed and outputs a new packet handle as well as an operation

> - * result for each outputted packet. The operation does not modify packets that

> - * it does not consume. It cannot consume all input packets if 'output.num_pkt'

> - * is smaller than 'input.num_pkt'.

> + * packets consumed and outputs a new packet handle for each outputted packet.

> + * Outputted packets contain IPSEC result metadata (odp_ipsec_packet_result_t),

> + * which should be checked for transformation errors, etc. The operation does

> + * not modify packets that it does not consume. It cannot consume all input

> + * packets if 'num_out' is smaller than 'num_in'.

>   *

>   * Packet context pointer and user area content are copied from input to output

>   * packets. Output packets are allocated from the same pool(s) as input packets.

>   *

>   * When outbound IP fragmentation offload is enabled, the number of outputted

> - * packets (and corresponding per packet results) may be greater than

> - * the number of input packets. In that case, application may examine 'num_out'

> - * of each packet result (odp_ipsec_packet_result_t) to find out which

> - * fragments are originated from which input packet.

> + * packets may be greater than the number of input packets.

> + *

> + * Each input packet must have a valid value for these metadata (other metadata

> + * is ignored):

> + *  * L3 offset: Offset to the first byte of the (outmost) IP header

> + *  * L4 offset: Offset to the L4 header if L4 checksum offload is requested

> + *

> + * Additionally, input IP packet length (odp_packet_len() minus

> + * odp_packet_l3_offset()) must match values in protocol headers. Otherwise

> + * results are undefined.

>   *

>   * Packets are processed in the input order. Packet order is maintained from

>   * input 'pkt' array to output 'pkt' array. Packet order is not guaranteed

> @@ -1201,31 +1189,37 @@ int odp_ipsec_in(const odp_ipsec_op_param_t *input,

>   * with IPSEC, etc headers constructed according to the standards. The amount

>   * and content of packet data before the IP header is undefined.

>   *

> - * @param         input   Operation input parameters

> - * @param[out]    output  Operation results

> + * @param          pkt_in   Packets to be processed

> + * @param          num_in   Number of packets to be processed

> + * @param[out]     pkt_out  Packet handle array for resulting packets

> + * @param[in, out] num_out  Number of resulting packets. Application sets this

> + *                          to 'pkt_out' array size. A successful operation sets

> + *                          this to the number of outputted packets

> + *                          (1 ... num_out).

> + * @param          param    Outbound operation parameters

>   *

> - * @return Number of input packets consumed (0 ... input.num_pkt)

> + * @return Number of input packets consumed (0 ... num_in)

>   * @retval <0     On failure

>   *

> - * @see odp_packet_user_ptr(), odp_packet_user_area()

> + * @see odp_packet_user_ptr(), odp_packet_user_area(), odp_packet_l3_offset()

>   */

> -int odp_ipsec_out(const odp_ipsec_op_param_t *input,

> -		  odp_ipsec_op_result_t *output);

> +int odp_ipsec_out(const odp_packet_t pkt_in[], int num_in,

> +		  odp_packet_t pkt_out[], int *num_out,

> +		  const odp_ipsec_out_param_t *param);

>  

>  /**

>   * Inbound asynchronous IPSEC operation

>   *

>   * This operation does inbound IPSEC processing in asynchronous mode. It

> - * processes packets otherwise identically to odp_ipsec_in(), but outputs all

> - * results through one or more ODP_EVENT_IPSEC_RESULT events with the following

> + * processes packets otherwise identically to odp_ipsec_in(), but outputs

> + * resulting packets as ODP_EVENT_PACKET_IPSEC events with the following

>   * ordering considerations.

>   *

>   * Asynchronous mode maintains packet order per SA when application calls the

>   * operation within an ordered or atomic scheduler context of the same queue.

> - * Resulting events for the same SA are enqueued in order and packet handles

> - * (for the same SA) are stored in order within an event. Packet order per SA at

> - * a destination queue is the same as if application would have enqueued packets

> - * there with odp_queue_enq_multi().

> + * Resulting events for the same SA are enqueued in order. Packet order per SA

> + * at a destination queue is the same as if application would have enqueued

> + * packets there with odp_queue_enq_multi().

>   *

>   * Packet order is also maintained when application otherwise guarantees

>   * (e.g. using locks) that the operation is not called simultaneously from

> @@ -1239,29 +1233,31 @@ int odp_ipsec_out(const odp_ipsec_op_param_t *input,

>   * may be processed simultaneously in both modes (initiated by this function

>   * and inline operation).

>   *

> - * @param         input   Operation input parameters

> + * @param          pkt      Packets to be processed

> + * @param          num      Number of packets to be processed

> + * @param          param    Inbound operation parameters

>   *

> - * @return Number of input packets consumed (0 ... input.num_pkt)

> + * @return Number of input packets consumed (0 ... num)

>   * @retval <0     On failure

>   *

>   * @see odp_ipsec_in(), odp_ipsec_result()

>   */

> -int odp_ipsec_in_enq(const odp_ipsec_op_param_t *input);

> +int odp_ipsec_in_enq(const odp_packet_t pkt[], int num,

> +		     const odp_ipsec_in_param_t *param);

>  

>  /**

>   * Outbound asynchronous IPSEC operation

>   *

>   * This operation does outbound IPSEC processing in asynchronous mode. It

>   * processes packets otherwise identically to odp_ipsec_out(), but outputs all

> - * results through one or more ODP_EVENT_IPSEC_RESULT events with the following

> + * resulting packets as ODP_EVENT_PACKET_IPSEC events with the following

>   * ordering considerations.

>   *

>   * Asynchronous mode maintains packet order per SA when application calls the

>   * operation within an ordered or atomic scheduler context of the same queue.

> - * Resulting events for the same SA are enqueued in order and packet handles

> - * (for the same SA) are stored in order within an event. Packet order per SA at

> - * a destination queue is the same as if application would have enqueued packets

> - * there with odp_queue_enq_multi().

> + * Resulting events for the same SA are enqueued in order. Packet order per SA

> + * at a destination queue is the same as if application would have enqueued

> + * packets there with odp_queue_enq_multi().

>   *

>   * Packet order is also maintained when application otherwise guarantees

>   * (e.g. using locks) that the operation is not called simultaneously from

> @@ -1273,14 +1269,17 @@ int odp_ipsec_in_enq(const odp_ipsec_op_param_t *input);

>   * The function may be used also in inline processing mode, e.g. for IPSEC

>   * packets for which inline processing is not possible.

>   *

> - * @param         input   Operation input parameters

> + * @param          pkt      Packets to be processed

> + * @param          num      Number of packets to be processed

> + * @param          param    Outbound operation parameters

>   *

> - * @return Number of input packets consumed (0 ... input.num_pkt)

> + * @return Number of input packets consumed (0 ... num)

>   * @retval <0     On failure

>   *

>   * @see odp_ipsec_out(), odp_ipsec_result()

>   */

> -int odp_ipsec_out_enq(const odp_ipsec_op_param_t *input);

> +int odp_ipsec_out_enq(const odp_packet_t pkt[], int num,

> +		      const odp_ipsec_out_param_t *param);

>  

>  /**

>   * Outbound inline IPSEC operation

> @@ -1291,39 +1290,73 @@ int odp_ipsec_out_enq(const odp_ipsec_op_param_t *input);

>   * result events for those.

>   *

>   * Inline operation parameters are defined per packet. The array of parameters

> - * must have 'op_param.num_pkt' elements and is pointed to by 'inline_param'.

> + * must have 'num' elements and is pointed to by 'inline_param'.

>   *

> - * @param         op_param      Operation parameters

> - * @param         inline_param  Outbound inline operation specific parameters

> + * @param          pkt           Packets to be processed

> + * @param          num           Number of packets to be processed

> + * @param          param         Outbound operation parameters

> + * @param          inline_param  Outbound inline operation specific parameters

>   *

> - * @return Number of packets consumed (0 ... op_param.num_pkt)

> + * @return Number of packets consumed (0 ... num)

>   * @retval <0     On failure

>   *

>   * @see odp_ipsec_out_enq()

>   */

> -int odp_ipsec_out_inline(const odp_ipsec_op_param_t *op_param,

> -			 const odp_ipsec_inline_op_param_t *inline_param);

> +int odp_ipsec_out_inline(const odp_packet_t pkt[], int num,

> +			 const odp_ipsec_out_param_t *param,

> +			 const odp_ipsec_out_inline_param_t *inline_param);

> +

> +/**

> + * Convert ODP_EVENT_IPSEC_PACKET event to packet handle

> + *

> + * Get packet handle for an IPSEC processed packet. Event type must be

> + * ODP_EVENT_IPSEC_PACKET. IPSEC operation results can be examined with

> + * odp_ipsec_result().

> + *

> + * @param ev       Event handle

> + *

> + * @return Packet handle

> + *

> + * @see odp_event_type(), odp_ipsec_result()

> + */

> +odp_packet_t odp_ipsec_packet_from_event(odp_event_t ev);

>  

>  /**

> - * Get IPSEC results from an ODP_EVENT_IPSEC_RESULT event

> + * Convert IPSEC processed packet handle to event

>   *

> - * Copies IPSEC operation results from an event. The event must be of

> - * type ODP_EVENT_IPSEC_RESULT. It must be freed before the application passes

> - * any resulting packet handles to other ODP calls.

> + * The packet handle must be an output of an IPSEC operation.

>   *

> - * @param[out]    result  Pointer to operation result for output. Maybe NULL, if

> - *                        application is interested only on the number of

> - *                        packets.

> - * @param         event   An ODP_EVENT_IPSEC_RESULT event

> + * @param pkt      Packet handle from IPSEC operation

>   *

> - * @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.

> + * @return Event handle

> + */

> +odp_event_t odp_ipsec_packet_to_event(odp_packet_t pkt);


Shouldn't this be an internal API?

> +

> +/**

> + * Get IPSEC operation results from an IPSEC processed packet

> + *

> + * Successful IPSEC operations of all types (SYNC, ASYNC and INLINE) produce

> + * packets which contain IPSEC result metadata. This function copies the

> + * operation results from an IPSEC processed packet. Event type of this kind of

> + * packet is ODP_EVENT_PACKET_IPSEC. Results are undefined if a non-IPSEC

> + * processed packet is passed as input.

> + *

> + * Some packet API operations output a new packet handle

> + * (e.g. odp_packet_concat()). IPSEC metadata remain valid as long as the packet

> + * handle is not changed from the original (output of e.g. odp_ipsec_in() or

> + * odp_ipsec_packet_from_event() call) IPSEC processed packet handle.

> + *

> + * @param[out]    result  Pointer to operation result for output

> + * @param         packet  An IPSEC processed packet (ODP_EVENT_PACKET_IPSEC)

> + *

> + * @retval  0     On success

>   * @retval <0     On failure

>   *

> - * @see odp_ipsec_in_enq(), odp_ipsec_out_enq()

> + * @see odp_ipsec_in(), odp_ipsec_in_enq(), odp_ipsec_out(),

> + *      odp_ipsec_out_enq(), odp_ipsec_out_inline(),

> + *      odp_ipsec_packet_from_event()

>   */

> -int odp_ipsec_result(odp_ipsec_op_result_t *result, odp_event_t event);

> +int odp_ipsec_result(odp_ipsec_packet_result_t *result, odp_packet_t packet);

>  

>  /**

>   * Get IPSEC status information from an ODP_EVENT_IPSEC_STATUS event

> diff --git a/include/odp/arch/default/api/abi/event.h b/include/odp/arch/default/api/abi/event.h

> index 87220d63..d807425b 100644

> --- a/include/odp/arch/default/api/abi/event.h

> +++ b/include/odp/arch/default/api/abi/event.h

> @@ -27,9 +27,9 @@ typedef _odp_abi_event_t *odp_event_t;

>  typedef enum odp_event_type_t {

>  	ODP_EVENT_BUFFER       = 1,

>  	ODP_EVENT_PACKET       = 2,

> -	ODP_EVENT_TIMEOUT      = 3,

> -	ODP_EVENT_CRYPTO_COMPL = 4,

> -	ODP_EVENT_IPSEC_RESULT = 5,

> +	ODP_EVENT_PACKET_IPSEC = 3,

> +	ODP_EVENT_TIMEOUT      = 4,

> +	ODP_EVENT_CRYPTO_COMPL = 5,

>  	ODP_EVENT_IPSEC_STATUS = 6

>  } odp_event_type_t;


Any reason for rearrangement of existing values?

>  

> diff --git a/platform/linux-generic/include/odp/api/plat/event_types.h b/platform/linux-generic/include/odp/api/plat/event_types.h

> index 0f517834..3615b4a4 100644

> --- a/platform/linux-generic/include/odp/api/plat/event_types.h

> +++ b/platform/linux-generic/include/odp/api/plat/event_types.h

> @@ -37,9 +37,10 @@ typedef ODP_HANDLE_T(odp_event_t);

>  typedef enum odp_event_type_t {

>  	ODP_EVENT_BUFFER       = 1,

>  	ODP_EVENT_PACKET       = 2,

> -	ODP_EVENT_TIMEOUT      = 3,

> -	ODP_EVENT_CRYPTO_COMPL = 4,

> -	ODP_EVENT_IPSEC_RESULT = 5

> +	ODP_EVENT_PACKET_IPSEC = 3,

> +	ODP_EVENT_TIMEOUT      = 4,

> +	ODP_EVENT_CRYPTO_COMPL = 5,

> +	ODP_EVENT_IPSEC_STATUS = 6

>  } odp_event_type_t;


And here?

>  

>  /**

> diff --git a/platform/linux-generic/odp_ipsec.c b/platform/linux-generic/odp_ipsec.c

> index 10918dfb..c7eeb4ec 100644

> --- a/platform/linux-generic/odp_ipsec.c

> +++ b/platform/linux-generic/odp_ipsec.c

> @@ -73,51 +73,68 @@ int odp_ipsec_sa_destroy(odp_ipsec_sa_t sa)

>  	return -1;

>  }

>  

> -int odp_ipsec_in(const odp_ipsec_op_param_t *input,

> -		 odp_ipsec_op_result_t *output)

> -{

> -	(void)input;

> -	(void)output;

> +int odp_ipsec_in(const odp_packet_t pkt_in[], int num_in,

> +		 odp_packet_t pkt_out[], int *num_out,

> +		 const odp_ipsec_in_param_t *param)

> +{

> +	(void)pkt_in;

> +	(void)num_in;

> +	(void)pkt_out;

> +	(void)num_out;

> +	(void)param;

>  

>  	return -1;

>  }

>  

> -int odp_ipsec_out(const odp_ipsec_op_param_t *input,

> -		  odp_ipsec_op_result_t *output)

> +int odp_ipsec_out(const odp_packet_t pkt_in[], int num_in,

> +		  odp_packet_t pkt_out[], int *num_out,

> +		  const odp_ipsec_out_param_t *param)

>  {

> -	(void)input;

> -	(void)output;

> +	(void)pkt_in;

> +	(void)num_in;

> +	(void)pkt_out;

> +	(void)num_out;

> +	(void)param;

>  

>  	return -1;

>  }

>  

> -int odp_ipsec_in_enq(const odp_ipsec_op_param_t *input)

> +int odp_ipsec_in_enq(const odp_packet_t pkt[], int num,

> +		     const odp_ipsec_in_param_t *param)

>  {

> -	(void)input;

> +	(void)pkt;

> +	(void)num;

> +	(void)param;

>  

>  	return -1;

>  }

>  

> -int odp_ipsec_out_enq(const odp_ipsec_op_param_t *input)

> +int odp_ipsec_out_enq(const odp_packet_t pkt[], int num,

> +		      const odp_ipsec_out_param_t *param)

>  {

> -	(void)input;

> +	(void)pkt;

> +	(void)num;

> +	(void)param;

>  

>  	return -1;

>  }

>  

> -int odp_ipsec_out_inline(const odp_ipsec_op_param_t *op_param,

> -			 const odp_ipsec_inline_op_param_t *inline_param)

> +int odp_ipsec_out_inline(const odp_packet_t pkt[], int num,

> +			 const odp_ipsec_out_param_t *param,

> +			 const odp_ipsec_out_inline_param_t *inline_param)

>  {

> -	(void)op_param;

> +	(void)pkt;

> +	(void)num;

> +	(void)param;

>  	(void)inline_param;

>  

>  	return -1;

>  }

>  

> -int odp_ipsec_result(odp_ipsec_op_result_t *result, odp_event_t event)

> +int odp_ipsec_result(odp_ipsec_packet_result_t *result, odp_packet_t packet)

>  {

>  	(void)result;

> -	(void)event;

> +	(void)packet;

>  

>  	return -1;

>  }

> @@ -145,6 +162,20 @@ void *odp_ipsec_sa_context(odp_ipsec_sa_t sa)

>  	return NULL;

>  }

>  

> +odp_packet_t odp_ipsec_packet_from_event(odp_event_t ev)

> +{

> +	(void)ev;

> +

> +	return ODP_PACKET_INVALID;

> +}

> +

> +odp_event_t odp_ipsec_packet_to_event(odp_packet_t pkt)

> +{

> +	(void)pkt;

> +

> +	return ODP_EVENT_INVALID;

> +}

> +

>  uint64_t odp_ipsec_sa_to_u64(odp_ipsec_sa_t sa)

>  {

>  	return _odp_pri(sa);

> 



-- 
With best wishes
Dmitry
Savolainen, Petri (Nokia - FI/Espoo) June 9, 2017, 12:32 p.m. UTC | #2
> > + * - ODP_EVENT_PACKET_IPSEC

> > + *     - Packet event (odp_packet_t) genereted as a result of an IPsec

> 

> generated


Ok


> >  /**

> > diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h

> > index 65f0b066..0da2b42c 100644

> > --- a/include/odp/api/spec/ipsec.h

> > +++ b/include/odp/api/spec/ipsec.h

> > @@ -552,16 +552,18 @@ typedef enum odp_ipsec_frag_mode_t {

> >

> >  /**

> >   * Packet lookup mode

> > + *

> > + * Lookup mode controls how an SA participates in SA lookup offload.

> > + * Inbound operations perform SA lookup if application does not provide

> a SA as

> > + * a parameter. In inline mode, a lookup miss directs the packet back

> to normal

> > + * packet input interface processing. SA lookup failure status

> (error.sa_lookup)

> > + * is reported through odp_ipsec_packet_result_t.

> >   */

> >  typedef enum odp_ipsec_lookup_mode_t {

> > -	/** Inbound SA lookup is disabled. */

> > +	/** Inbound SA lookup is disabled for the SA. */

> >  	ODP_IPSEC_LOOKUP_DISABLED = 0,

> >

> > -	/** Inbound SA lookup is enabled. Lookup matches only SPI

> value.

> > -	 *  In inline mode, a lookup miss directs the packet back to

> normal

> > -	 *  packet input interface processing. In other modes, the SA

> lookup

> > -	 *  failure status (error.sa_lookup) is reported through

> > -	 *  odp_ipsec_packet_result_t. */

> > +	/** Inbound SA lookup is enabled. Lookup matches only SPI

> value. */

> >  	ODP_IPSEC_LOOKUP_SPI,

> >

> >  	/** Inbound SA lookup is enabled. Lookup matches both SPI value

> and

> 

> This can go to separate patch.



We updated the lookup offload text in odp_ipsec_in_param_t (0 == no SA, but lookup), which is a new type. This text pair with that and is merely moved from ODP_IPSEC_LOOKUP_SPI mode to up, since it's true for both lookup modes. So, patch is more complete with the move, than without it.



> 

> > @@ -850,17 +852,6 @@ int odp_ipsec_sa_destroy(odp_ipsec_sa_t sa);

> >   */

> >  uint64_t odp_ipsec_sa_to_u64(odp_ipsec_sa_t sa);

> >

> > -/**

> > - * IPSEC operation level options

> > - *

> > - * These may be used to override some SA level options

> > - */

> > -typedef struct odp_ipsec_op_opt_t {

> > -	/** Fragmentation mode */

> > -	odp_ipsec_frag_mode_t mode;

> > -

> > -} odp_ipsec_op_opt_t;

> > -

> >  /** IPSEC operation status has no errors */

> >  #define ODP_IPSEC_OK 0

> >

> > @@ -870,7 +861,8 @@ typedef struct odp_ipsec_op_status_t {

> >  	union {

> >  		/** Error flags */

> >  		struct {

> > -			/** Protocol error. Not a valid ESP or AH

> packet. */

> > +			/** Protocol error. Not a valid ESP or AH

> packet,

> > +			 *  packet data length error, etc. */

> >  			uint32_t proto            : 1;

> >

> >  			/** SA lookup failed */

> > @@ -934,41 +926,70 @@ typedef struct odp_ipsec_op_status_t {

> >  } odp_ipsec_op_status_t;

> >

> >  /**

> > - * IPSEC operation input parameters

> > + * IPSEC outbound operation options

> > + *

> > + * These may be used to override some SA level options

> >   */

> > -typedef struct odp_ipsec_op_param_t {

> > -	/** Number of packets to be processed */

> > -	int num_pkt;

> > +typedef struct odp_ipsec_out_opt_t {

> > +	/** Fragmentation mode */

> > +	odp_ipsec_frag_mode_t mode;

> > +

> > +} odp_ipsec_out_opt_t;

> 

> Maybe we can just inline this into out_param_t ?

> With this in/out split, it should be quite straightforward change from

> the API point of view.



The thing is that it's a pointer to an odp_ipsec_out_opt_t array, so application needs the type above. Otherwise, we'd need to define maximum of opts in the API, which would be the same as maximum number of packets. API doesn't need to limit the burst size.


> 

> >

> > +/**

> > + * IPSEC outbound operation parameters

> > + */

> > +typedef struct odp_ipsec_out_param_t {

> >  	/** Number of SAs

> >  	 *

> > +	 *  Outbound IPSEC operation needs SA from application. Use

> either

> > +	 *  single SA for all packets, or a SA per packet.

> > +	 *

> >  	 *  Valid values are:

> > -	 *  * 0:       No SAs (default)

> > -	 *  * 1:       Single SA for all packets

> > -	 *  * num_pkt: SA per packet

> > +	 *  * 1:  Single SA for all packets

> > +	 *  * N:  A SA per packet. N must match the number of packets.

> >  	 */

> >  	int num_sa;

> >

> > -	/** Number of operation options

> > +	/** Number of outbound operation options

> >  	 *

> >  	 *  Valid values are:

> > -	 *  * 0:       No options (default)

> > -	 *  * 1:       Single option for all packets

> > -	 *  * num_pkt: An option per packet

> > +	 *  * 0:  No options

> > +	 *  * 1:  Single option for all packets

> > +	 *  * N:  An option per packet. N must match the number of

> packets.

> >  	 */

> >  	int num_opt;

> >

> > -	/** Pointer to an array of packets

> > +	/** Pointer to an array of IPSEC SAs */

> > +	odp_ipsec_sa_t *sa;

> > +

> > +	/** Pointer to an array of outbound operation options

> > +	 *

> > +	 *  May be NULL when num_opt is zero.

> > +	 */

> > +	odp_ipsec_out_opt_t *opt;

> > +

> > +} odp_ipsec_out_param_t;

> > +

> > +/**

> > + * IPSEC inbound operation parameters

> > + */

> > +typedef struct odp_ipsec_in_param_t {

> > +	/** Number of SAs

> >  	 *

> > -	 *  Each packet must have a valid value for these metadata:

> > -	 *  * L3 offset: Offset to the first byte of the (outmost) IP

> header

> > -	 *  * L4 offset: For inbound direction, when udp_encap is

> enabled -

> > -	 *               offset to the first byte of the encapsulating

> UDP

> > -	 *               header

> > +	 *  Inbound IPSEC operation processes a packet using the SA

> provided by

> > +	 *  the application. If the application does not provide an SA,

> the

> > +	 *  operation searches for the SA by matching the input packet

> with all

> > +	 *  inbound SAs according to the lookup mode

> (odp_ipsec_lookup_mode_t)

> > +	 *  configured in each SA. When passing SAs, use either single

> SA for

> > +	 *  all packets, or a SA per packet.

> >  	 *

> > -	 *  @see odp_packet_l3_offset(), odp_packet_l4_offset()

> > +	 *  Valid values are:

> > +	 *  * 0:  No SAs. SA lookup is done for all packets.

> > +	 *  * 1:  Single SA for all packets

> > +	 *  * N:  A SA per packet. N must match the number of packets.

> >  	 */

> > -	odp_packet_t *pkt;

> > +	int num_sa;

> >

> >  	/** Pointer to an array of IPSEC SAs

> >  	 *

> > @@ -976,18 +997,12 @@ typedef struct odp_ipsec_op_param_t {

> >  	 */

> >  	odp_ipsec_sa_t *sa;

> >

> > -	/** Pointer to an array of operation options

> > -	 *

> > -	 *  May be NULL when num_opt is zero.

> > -	 */

> > -	odp_ipsec_op_opt_t *opt;

> > -

> > -} odp_ipsec_op_param_t;

> > +} odp_ipsec_in_param_t;

> 

> I like this idea, it will simplify code for both implementations and

> applications. However can we move this op_param -> in_param+out_param to

> a separate patch?



We cannot since it's part of the function prototypes under.

int odp_ipsec_in(const odp_packet_t pkt_in[], int num_in,
		 odp_packet_t pkt_out[], int *num_out,
		 const odp_ipsec_in_param_t *param);





> 

> >  /**

> >   * Outbound inline IPSEC operation parameters

> >   */

> > -typedef struct odp_ipsec_inline_op_param_t {

> > +typedef struct odp_ipsec_out_inline_param_t {

> >  	/** Packet output interface for inline output operation

> >  	 *

> >  	 *  Outbound inline IPSEC operation uses this packet IO

> interface to

> > @@ -1011,7 +1026,7 @@ typedef struct odp_ipsec_inline_op_param_t {

> >  		uint32_t len;

> >  	} outer_hdr;

> >

> > -} odp_ipsec_inline_op_param_t;

> > +} odp_ipsec_out_inline_param_t;

> >

> >  /**

> >   * IPSEC operation result for a packet

> > @@ -1020,16 +1035,6 @@ typedef struct odp_ipsec_packet_result_t {

> >  	/** IPSEC operation status */

> >  	odp_ipsec_op_status_t status;

> >

> > -	/** Number of output packets created from the corresponding

> input packet

> > -	 *

> > -	 *  Without fragmentation offload this is always one. However,

> if the

> > -	 *  input packet was fragmented during the operation this is

> larger than

> > -	 *  one for the first returned fragment and zero for the rest

> of the

> > -	 *  fragments. All the fragments (of the same source packet)

> are stored

> > -	 *  consecutively in the 'pkt' array.

> > -	 */

> > -	int num_out;

> > -

> >  	/** IPSEC SA that was used to create the packet

> >  	 *

> >  	 *  Operation updates this SA handle value, when SA look up is

> performed

> > @@ -1040,7 +1045,8 @@ typedef struct odp_ipsec_packet_result_t {

> >  	odp_ipsec_sa_t sa;

> >

> >  	/** Packet outer header status before inbound inline

> processing.

> > -	 *  This is valid only when status.flag.inline_mode is set.

> > +	 *  This is valid only when outer headers are retained

> > +	 *  (see odp_ipsec_inbound_config_t) and

> status.flag.inline_mode is set.

> 

> Related question: would we like to lift this restriction and enable

> headers retaining for all kinds of inline processing?



What means "all kind of inline processing"?

The limitation here is that *non-inline* IPSEC operations does not need to support retaining, since application sees all those headers before calling IPSEC. Anyway, this can be discussed/expanded later.

> >  /**

> > - * Get IPSEC results from an ODP_EVENT_IPSEC_RESULT event

> > + * Convert IPSEC processed packet handle to event

> >   *

> > - * Copies IPSEC operation results from an event. The event must be of

> > - * type ODP_EVENT_IPSEC_RESULT. It must be freed before the application

> passes

> > - * any resulting packet handles to other ODP calls.

> > + * The packet handle must be an output of an IPSEC operation.

> >   *

> > - * @param[out]    result  Pointer to operation result for output. Maybe

> NULL, if

> > - *                        application is interested only on the number

> of

> > - *                        packets.

> > - * @param         event   An ODP_EVENT_IPSEC_RESULT event

> > + * @param pkt      Packet handle from IPSEC operation

> >   *

> > - * @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.

> > + * @return Event handle

> > + */

> > +odp_event_t odp_ipsec_packet_to_event(odp_packet_t pkt);

> 

> Shouldn't this be an internal API?



No. This is the way an application can forward resulting packets as events (EVENT_PACKET_IPSEC) through queues to e.g. a next stage of application processing.


> >  /**

> >   * Get IPSEC status information from an ODP_EVENT_IPSEC_STATUS event

> > diff --git a/include/odp/arch/default/api/abi/event.h

> b/include/odp/arch/default/api/abi/event.h

> > index 87220d63..d807425b 100644

> > --- a/include/odp/arch/default/api/abi/event.h

> > +++ b/include/odp/arch/default/api/abi/event.h

> > @@ -27,9 +27,9 @@ typedef _odp_abi_event_t *odp_event_t;

> >  typedef enum odp_event_type_t {

> >  	ODP_EVENT_BUFFER       = 1,

> >  	ODP_EVENT_PACKET       = 2,

> > -	ODP_EVENT_TIMEOUT      = 3,

> > -	ODP_EVENT_CRYPTO_COMPL = 4,

> > -	ODP_EVENT_IPSEC_RESULT = 5,

> > +	ODP_EVENT_PACKET_IPSEC = 3,

> > +	ODP_EVENT_TIMEOUT      = 4,

> > +	ODP_EVENT_CRYPTO_COMPL = 5,

> >  	ODP_EVENT_IPSEC_STATUS = 6

> >  } odp_event_type_t;

> 

> Any reason for rearrangement of existing values?


Yes, it's now 

  	ODP_EVENT_BUFFER       = 1,
  	ODP_EVENT_PACKET       = 2,
	ODP_EVENT_PACKET_IPSEC = 3,

which may turn in couple of weeks into 

  	ODP_EVENT_BUFFER       = 1,
  	ODP_EVENT_PACKET       = 2,
	ODP_EVENT_PACKET_IPSEC = 3,
	ODP_EVENT_PACKET_CRYPTO = 4,
	ODP_EVENT_PACKET_COMP = 5,

... normal packet and special ones.

-Petri
Bill Fischofer June 9, 2017, 1:53 p.m. UTC | #3
On Fri, Jun 9, 2017 at 7:32 AM, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
>> > + * - ODP_EVENT_PACKET_IPSEC

>> > + *     - Packet event (odp_packet_t) genereted as a result of an IPsec

>>

>> generated

>

> Ok

>

>

>> >  /**

>> > diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h

>> > index 65f0b066..0da2b42c 100644

>> > --- a/include/odp/api/spec/ipsec.h

>> > +++ b/include/odp/api/spec/ipsec.h

>> > @@ -552,16 +552,18 @@ typedef enum odp_ipsec_frag_mode_t {

>> >

>> >  /**

>> >   * Packet lookup mode

>> > + *

>> > + * Lookup mode controls how an SA participates in SA lookup offload.

>> > + * Inbound operations perform SA lookup if application does not provide

>> a SA as

>> > + * a parameter. In inline mode, a lookup miss directs the packet back

>> to normal

>> > + * packet input interface processing. SA lookup failure status

>> (error.sa_lookup)

>> > + * is reported through odp_ipsec_packet_result_t.

>> >   */

>> >  typedef enum odp_ipsec_lookup_mode_t {

>> > -   /** Inbound SA lookup is disabled. */

>> > +   /** Inbound SA lookup is disabled for the SA. */

>> >     ODP_IPSEC_LOOKUP_DISABLED = 0,

>> >

>> > -   /** Inbound SA lookup is enabled. Lookup matches only SPI

>> value.

>> > -    *  In inline mode, a lookup miss directs the packet back to

>> normal

>> > -    *  packet input interface processing. In other modes, the SA

>> lookup

>> > -    *  failure status (error.sa_lookup) is reported through

>> > -    *  odp_ipsec_packet_result_t. */

>> > +   /** Inbound SA lookup is enabled. Lookup matches only SPI

>> value. */

>> >     ODP_IPSEC_LOOKUP_SPI,

>> >

>> >     /** Inbound SA lookup is enabled. Lookup matches both SPI value

>> and

>>

>> This can go to separate patch.

>

>

> We updated the lookup offload text in odp_ipsec_in_param_t (0 == no SA, but lookup), which is a new type. This text pair with that and is merely moved from ODP_IPSEC_LOOKUP_SPI mode to up, since it's true for both lookup modes. So, patch is more complete with the move, than without it.

>

>

>

>>

>> > @@ -850,17 +852,6 @@ int odp_ipsec_sa_destroy(odp_ipsec_sa_t sa);

>> >   */

>> >  uint64_t odp_ipsec_sa_to_u64(odp_ipsec_sa_t sa);

>> >

>> > -/**

>> > - * IPSEC operation level options

>> > - *

>> > - * These may be used to override some SA level options

>> > - */

>> > -typedef struct odp_ipsec_op_opt_t {

>> > -   /** Fragmentation mode */

>> > -   odp_ipsec_frag_mode_t mode;

>> > -

>> > -} odp_ipsec_op_opt_t;

>> > -

>> >  /** IPSEC operation status has no errors */

>> >  #define ODP_IPSEC_OK 0

>> >

>> > @@ -870,7 +861,8 @@ typedef struct odp_ipsec_op_status_t {

>> >     union {

>> >             /** Error flags */

>> >             struct {

>> > -                   /** Protocol error. Not a valid ESP or AH

>> packet. */

>> > +                   /** Protocol error. Not a valid ESP or AH

>> packet,

>> > +                    *  packet data length error, etc. */

>> >                     uint32_t proto            : 1;

>> >

>> >                     /** SA lookup failed */

>> > @@ -934,41 +926,70 @@ typedef struct odp_ipsec_op_status_t {

>> >  } odp_ipsec_op_status_t;

>> >

>> >  /**

>> > - * IPSEC operation input parameters

>> > + * IPSEC outbound operation options

>> > + *

>> > + * These may be used to override some SA level options

>> >   */

>> > -typedef struct odp_ipsec_op_param_t {

>> > -   /** Number of packets to be processed */

>> > -   int num_pkt;

>> > +typedef struct odp_ipsec_out_opt_t {

>> > +   /** Fragmentation mode */

>> > +   odp_ipsec_frag_mode_t mode;

>> > +

>> > +} odp_ipsec_out_opt_t;

>>

>> Maybe we can just inline this into out_param_t ?

>> With this in/out split, it should be quite straightforward change from

>> the API point of view.

>

>

> The thing is that it's a pointer to an odp_ipsec_out_opt_t array, so application needs the type above. Otherwise, we'd need to define maximum of opts in the API, which would be the same as maximum number of packets. API doesn't need to limit the burst size.

>

>

>>

>> >

>> > +/**

>> > + * IPSEC outbound operation parameters

>> > + */

>> > +typedef struct odp_ipsec_out_param_t {

>> >     /** Number of SAs

>> >      *

>> > +    *  Outbound IPSEC operation needs SA from application. Use

>> either

>> > +    *  single SA for all packets, or a SA per packet.

>> > +    *

>> >      *  Valid values are:

>> > -    *  * 0:       No SAs (default)

>> > -    *  * 1:       Single SA for all packets

>> > -    *  * num_pkt: SA per packet

>> > +    *  * 1:  Single SA for all packets

>> > +    *  * N:  A SA per packet. N must match the number of packets.

>> >      */

>> >     int num_sa;

>> >

>> > -   /** Number of operation options

>> > +   /** Number of outbound operation options

>> >      *

>> >      *  Valid values are:

>> > -    *  * 0:       No options (default)

>> > -    *  * 1:       Single option for all packets

>> > -    *  * num_pkt: An option per packet

>> > +    *  * 0:  No options

>> > +    *  * 1:  Single option for all packets

>> > +    *  * N:  An option per packet. N must match the number of

>> packets.

>> >      */

>> >     int num_opt;

>> >

>> > -   /** Pointer to an array of packets

>> > +   /** Pointer to an array of IPSEC SAs */

>> > +   odp_ipsec_sa_t *sa;

>> > +

>> > +   /** Pointer to an array of outbound operation options

>> > +    *

>> > +    *  May be NULL when num_opt is zero.

>> > +    */

>> > +   odp_ipsec_out_opt_t *opt;

>> > +

>> > +} odp_ipsec_out_param_t;

>> > +

>> > +/**

>> > + * IPSEC inbound operation parameters

>> > + */

>> > +typedef struct odp_ipsec_in_param_t {

>> > +   /** Number of SAs

>> >      *

>> > -    *  Each packet must have a valid value for these metadata:

>> > -    *  * L3 offset: Offset to the first byte of the (outmost) IP

>> header

>> > -    *  * L4 offset: For inbound direction, when udp_encap is

>> enabled -

>> > -    *               offset to the first byte of the encapsulating

>> UDP

>> > -    *               header

>> > +    *  Inbound IPSEC operation processes a packet using the SA

>> provided by

>> > +    *  the application. If the application does not provide an SA,

>> the

>> > +    *  operation searches for the SA by matching the input packet

>> with all

>> > +    *  inbound SAs according to the lookup mode

>> (odp_ipsec_lookup_mode_t)

>> > +    *  configured in each SA. When passing SAs, use either single

>> SA for

>> > +    *  all packets, or a SA per packet.

>> >      *

>> > -    *  @see odp_packet_l3_offset(), odp_packet_l4_offset()

>> > +    *  Valid values are:

>> > +    *  * 0:  No SAs. SA lookup is done for all packets.

>> > +    *  * 1:  Single SA for all packets

>> > +    *  * N:  A SA per packet. N must match the number of packets.

>> >      */

>> > -   odp_packet_t *pkt;

>> > +   int num_sa;

>> >

>> >     /** Pointer to an array of IPSEC SAs

>> >      *

>> > @@ -976,18 +997,12 @@ typedef struct odp_ipsec_op_param_t {

>> >      */

>> >     odp_ipsec_sa_t *sa;

>> >

>> > -   /** Pointer to an array of operation options

>> > -    *

>> > -    *  May be NULL when num_opt is zero.

>> > -    */

>> > -   odp_ipsec_op_opt_t *opt;

>> > -

>> > -} odp_ipsec_op_param_t;

>> > +} odp_ipsec_in_param_t;

>>

>> I like this idea, it will simplify code for both implementations and

>> applications. However can we move this op_param -> in_param+out_param to

>> a separate patch?

>

>

> We cannot since it's part of the function prototypes under.

>

> int odp_ipsec_in(const odp_packet_t pkt_in[], int num_in,

>                  odp_packet_t pkt_out[], int *num_out,

>                  const odp_ipsec_in_param_t *param);

>

>

>

>

>

>>

>> >  /**

>> >   * Outbound inline IPSEC operation parameters

>> >   */

>> > -typedef struct odp_ipsec_inline_op_param_t {

>> > +typedef struct odp_ipsec_out_inline_param_t {

>> >     /** Packet output interface for inline output operation

>> >      *

>> >      *  Outbound inline IPSEC operation uses this packet IO

>> interface to

>> > @@ -1011,7 +1026,7 @@ typedef struct odp_ipsec_inline_op_param_t {

>> >             uint32_t len;

>> >     } outer_hdr;

>> >

>> > -} odp_ipsec_inline_op_param_t;

>> > +} odp_ipsec_out_inline_param_t;

>> >

>> >  /**

>> >   * IPSEC operation result for a packet

>> > @@ -1020,16 +1035,6 @@ typedef struct odp_ipsec_packet_result_t {

>> >     /** IPSEC operation status */

>> >     odp_ipsec_op_status_t status;

>> >

>> > -   /** Number of output packets created from the corresponding

>> input packet

>> > -    *

>> > -    *  Without fragmentation offload this is always one. However,

>> if the

>> > -    *  input packet was fragmented during the operation this is

>> larger than

>> > -    *  one for the first returned fragment and zero for the rest

>> of the

>> > -    *  fragments. All the fragments (of the same source packet)

>> are stored

>> > -    *  consecutively in the 'pkt' array.

>> > -    */

>> > -   int num_out;

>> > -

>> >     /** IPSEC SA that was used to create the packet

>> >      *

>> >      *  Operation updates this SA handle value, when SA look up is

>> performed

>> > @@ -1040,7 +1045,8 @@ typedef struct odp_ipsec_packet_result_t {

>> >     odp_ipsec_sa_t sa;

>> >

>> >     /** Packet outer header status before inbound inline

>> processing.

>> > -    *  This is valid only when status.flag.inline_mode is set.

>> > +    *  This is valid only when outer headers are retained

>> > +    *  (see odp_ipsec_inbound_config_t) and

>> status.flag.inline_mode is set.

>>

>> Related question: would we like to lift this restriction and enable

>> headers retaining for all kinds of inline processing?

>

>

> What means "all kind of inline processing"?

>

> The limitation here is that *non-inline* IPSEC operations does not need to support retaining, since application sees all those headers before calling IPSEC. Anyway, this can be discussed/expanded later.

>

>> >  /**

>> > - * Get IPSEC results from an ODP_EVENT_IPSEC_RESULT event

>> > + * Convert IPSEC processed packet handle to event

>> >   *

>> > - * Copies IPSEC operation results from an event. The event must be of

>> > - * type ODP_EVENT_IPSEC_RESULT. It must be freed before the application

>> passes

>> > - * any resulting packet handles to other ODP calls.

>> > + * The packet handle must be an output of an IPSEC operation.

>> >   *

>> > - * @param[out]    result  Pointer to operation result for output. Maybe

>> NULL, if

>> > - *                        application is interested only on the number

>> of

>> > - *                        packets.

>> > - * @param         event   An ODP_EVENT_IPSEC_RESULT event

>> > + * @param pkt      Packet handle from IPSEC operation

>> >   *

>> > - * @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.

>> > + * @return Event handle

>> > + */

>> > +odp_event_t odp_ipsec_packet_to_event(odp_packet_t pkt);

>>

>> Shouldn't this be an internal API?

>

>

> No. This is the way an application can forward resulting packets as events (EVENT_PACKET_IPSEC) through queues to e.g. a next stage of application processing.

>

>

>> >  /**

>> >   * Get IPSEC status information from an ODP_EVENT_IPSEC_STATUS event

>> > diff --git a/include/odp/arch/default/api/abi/event.h

>> b/include/odp/arch/default/api/abi/event.h

>> > index 87220d63..d807425b 100644

>> > --- a/include/odp/arch/default/api/abi/event.h

>> > +++ b/include/odp/arch/default/api/abi/event.h

>> > @@ -27,9 +27,9 @@ typedef _odp_abi_event_t *odp_event_t;

>> >  typedef enum odp_event_type_t {

>> >     ODP_EVENT_BUFFER       = 1,

>> >     ODP_EVENT_PACKET       = 2,

>> > -   ODP_EVENT_TIMEOUT      = 3,

>> > -   ODP_EVENT_CRYPTO_COMPL = 4,

>> > -   ODP_EVENT_IPSEC_RESULT = 5,

>> > +   ODP_EVENT_PACKET_IPSEC = 3,

>> > +   ODP_EVENT_TIMEOUT      = 4,

>> > +   ODP_EVENT_CRYPTO_COMPL = 5,

>> >     ODP_EVENT_IPSEC_STATUS = 6

>> >  } odp_event_type_t;

>>

>> Any reason for rearrangement of existing values?

>

> Yes, it's now

>

>         ODP_EVENT_BUFFER       = 1,

>         ODP_EVENT_PACKET       = 2,

>         ODP_EVENT_PACKET_IPSEC = 3,

>

> which may turn in couple of weeks into

>

>         ODP_EVENT_BUFFER       = 1,

>         ODP_EVENT_PACKET       = 2,

>         ODP_EVENT_PACKET_IPSEC = 3,

>         ODP_EVENT_PACKET_CRYPTO = 4,

>         ODP_EVENT_PACKET_COMP = 5,

>

> ... normal packet and special ones.


The problem with trying to to this as an ever-growing list of event
types, is that every time we introduce a new one every application
needs to change. ODP_EVENT_PACKET_IPSEC is simply an ODP_EVENT_PACKET
with some additional metadata that may or may not be of interest to a
given application processing stage. So the basic architecture should
reflect this fact.

The typical event loop for a worker thread would look like:

while (1) {
        odp_event_t ev = odp_schedule(&queue, ODP_SCHED_WAIT);
        switch (odp_event_type(ev)) {
                case ODP_EVENT_BUFFER:
                        do_buffer_processing(odp_buffer_from_event(ev), queue);
                        break;
                case ODP_EVENT_TIMEOUT:

do_timeout_processing(odp_timeout_from_event(ev), queue);
                        break;
                case ODP_EVENT_PACKET:
                        do_packet_processing(odp_packet_from_event(ev), queue);
                        break;
                ...
                default:
                        // Unknown event type, log and discard
        }
}

So now that processing loop needs to be updated even if this worker
thread doesn't care about that optional IPsec metadata.

A more flexible approach would be to define a new odp_packet_type_t
and associated odp_packet_type() API that identifies these subtypes.
That way the worker packet processing routine can be written as:

void do_packet_processing(odp_packet_t pkt, odp_queue_t queue)
{
        ....
        switch (odp_packet_type(pkt)) {
                case ODP_PACKET_IPSEC:
                          // do processing of interest with associated
IPsec metadata
                          break;
                case ODP_PACKET_COMP:
                          // do processing of interest with associated
COMP metadata
                          break;
                default:
        }

        ... process the basic packet itself
}

Note that this means that anytime we add a new packet subtype nothing
has to change at the application layer. It only needs to add code to
process that optional metadata in stages that specifically want to do
something with it. Otherwise they are treated as regular packets. So
the fact that this packet was IPsec decrypted before it got to this
stage is something the stage need not be aware of or care about unless
it wants to take special action because the packet arrived via that
offload path.

As we add more pipeline capabilities this sort of structure will
become increasingly important as the whole idea behind pipelining is
that worker threads don't need to be aware of where the packets came
from or their offload history in order to do what that stage needs to
do. Some stages will want to inspect that history and do something
different based on it, but most will not.

This becomes even more important when we consider pipeline chains. For
example, what happens when we support full offloading of packet that
is IPsec decrypted, then decompressed, and then handed to a worker?
Trying to encode these composites in a single event type quickly
becomes completely unmanageable. However, we can easily extend
odp_packet_type() to be:

odp_packet_type_t odp_packet_type(odp_packet_t pkt, int level);

So that odp_packet_type(pkt, 0) gives the current packet subtype,
odp_packet_type(pkt, 1) gives the previous, etc. That way processing
stages that need this sort of "forensic" information can get it if
needed, but other stages can ignore it. We may want to add the API:

int odp_packet_history(odp_packet_t pkt);

to return the number of such levels available, but this sort of
extension may be premature for now.

>

> -Petri

>

>
Savolainen, Petri (Nokia - FI/Espoo) June 12, 2017, 7:34 a.m. UTC | #4
> >> >  /**

> >> >   * Get IPSEC status information from an ODP_EVENT_IPSEC_STATUS event

> >> > diff --git a/include/odp/arch/default/api/abi/event.h

> >> b/include/odp/arch/default/api/abi/event.h

> >> > index 87220d63..d807425b 100644

> >> > --- a/include/odp/arch/default/api/abi/event.h

> >> > +++ b/include/odp/arch/default/api/abi/event.h

> >> > @@ -27,9 +27,9 @@ typedef _odp_abi_event_t *odp_event_t;

> >> >  typedef enum odp_event_type_t {

> >> >     ODP_EVENT_BUFFER       = 1,

> >> >     ODP_EVENT_PACKET       = 2,

> >> > -   ODP_EVENT_TIMEOUT      = 3,

> >> > -   ODP_EVENT_CRYPTO_COMPL = 4,

> >> > -   ODP_EVENT_IPSEC_RESULT = 5,

> >> > +   ODP_EVENT_PACKET_IPSEC = 3,

> >> > +   ODP_EVENT_TIMEOUT      = 4,

> >> > +   ODP_EVENT_CRYPTO_COMPL = 5,

> >> >     ODP_EVENT_IPSEC_STATUS = 6

> >> >  } odp_event_type_t;

> >>

> >> Any reason for rearrangement of existing values?

> >

> > Yes, it's now

> >

> >         ODP_EVENT_BUFFER       = 1,

> >         ODP_EVENT_PACKET       = 2,

> >         ODP_EVENT_PACKET_IPSEC = 3,

> >

> > which may turn in couple of weeks into

> >

> >         ODP_EVENT_BUFFER       = 1,

> >         ODP_EVENT_PACKET       = 2,

> >         ODP_EVENT_PACKET_IPSEC = 3,

> >         ODP_EVENT_PACKET_CRYPTO = 4,

> >         ODP_EVENT_PACKET_COMP = 5,

> >

> > ... normal packet and special ones.

> 

> The problem with trying to to this as an ever-growing list of event

> types, is that every time we introduce a new one every application

> needs to change. ODP_EVENT_PACKET_IPSEC is simply an ODP_EVENT_PACKET

> with some additional metadata that may or may not be of interest to a

> given application processing stage. So the basic architecture should

> reflect this fact.


Actually, the idea is that existing application *does not* have to change. EVENT_PACKET is still what it is today. So, current applications do not need to change anything: e.g. those do not need to check if a packet is coming from IPSEC and if the IPSEC operation has failed, etc.


> 

> The typical event loop for a worker thread would look like:

> 

> while (1) {

>         odp_event_t ev = odp_schedule(&queue, ODP_SCHED_WAIT);

>         switch (odp_event_type(ev)) {

>                 case ODP_EVENT_BUFFER:

>                         do_buffer_processing(odp_buffer_from_event(ev),

> queue);

>                         break;

>                 case ODP_EVENT_TIMEOUT:

> 

> do_timeout_processing(odp_timeout_from_event(ev), queue);

>                         break;

>                 case ODP_EVENT_PACKET:

>                         do_packet_processing(odp_packet_from_event(ev),

> queue);

>                         break;

>                 ...

>                 default:

>                         // Unknown event type, log and discard

>         }

> }

> 

> So now that processing loop needs to be updated even if this worker

> thread doesn't care about that optional IPsec metadata.



No, by default nothing changes. Only if application itself *enabled* IPSEC processing, it would see PACKET_IPSEC events and would need to handle those... but then application was already changed a lot (decided to use IPSEC offload).


> 

> A more flexible approach would be to define a new odp_packet_type_t

> and associated odp_packet_type() API that identifies these subtypes.

> That way the worker packet processing routine can be written as:

> 

> void do_packet_processing(odp_packet_t pkt, odp_queue_t queue)

> {

>         ....

>         switch (odp_packet_type(pkt)) {

>                 case ODP_PACKET_IPSEC:

>                           // do processing of interest with associated

> IPsec metadata

>                           break;

>                 case ODP_PACKET_COMP:

>                           // do processing of interest with associated

> COMP metadata

>                           break;

>                 default:

>         }

> 

>         ... process the basic packet itself

> }



Subtype is not needed - event type is the subtype already.

We have a plan of adding odp_event_class_t instead (ODP_EVENT_CLASS_PACKET, ...) as a super-type, but decided that it can wait for the actual need. The need is not yet there today as all packets are basic packets. After IPSEC/crypto/comp packet types are merged, it could be needed by an application pipeline stage which needs to do simple processing for all type packets. But even with that only successfully decrypted packets would go through that stage, otherwise failed decrypt would leave inner packet as garbage ...


> 

> Note that this means that anytime we add a new packet subtype nothing

> has to change at the application layer. It only needs to add code to

> process that optional metadata in stages that specifically want to do

> something with it. Otherwise they are treated as regular packets. So

> the fact that this packet was IPsec decrypted before it got to this

> stage is something the stage need not be aware of or care about unless

> it wants to take special action because the packet arrived via that

> offload path.


Event class would enable this later on. As explained above, when IPSEC (/crypto/comp) is enabled application needs to have at least one stage which checks the results, otherwise other stage may end up processing garbage.


> 

> As we add more pipeline capabilities this sort of structure will

> become increasingly important as the whole idea behind pipelining is

> that worker threads don't need to be aware of where the packets came

> from or their offload history in order to do what that stage needs to

> do. Some stages will want to inspect that history and do something

> different based on it, but most will not.

> 

> This becomes even more important when we consider pipeline chains. For

> example, what happens when we support full offloading of packet that

> is IPsec decrypted, then decompressed, and then handed to a worker?

> Trying to encode these composites in a single event type quickly

> becomes completely unmanageable. However, we can easily extend

> odp_packet_type() to be:


Pipelining in API would change the type and connection between stages would be (mostly) invisible to the application. So, e.g. pktin -> IPSEC decrypt -> decomp would result PACKET_COMP event for successfully decrypted and decompressed packets. Some small amount of IPSEC metadata could be needed still to be visible (e.g. "packet was ipsec", "ipsec sa was X"). Failed IPSEC step would result PACKET_IPSEC with error flags.


This is roughly the idea of event classes.

typedef enum odp_event_class_t {
     ODP_EVENT_BUFFER,
     ODP_EVENT_PACKET, // basic, ipsec, crypto, comp, ... all share the same basic packet metadata
     ODP_EVENT_TIMER,
     ODP_EVENT_STATUS, // all status events from ODP offloads 
} odp_event_class_t

odp_event_class_t odp_event_class(odp_event_t ev);



> 

> odp_packet_type_t odp_packet_type(odp_packet_t pkt, int level);

> 

> So that odp_packet_type(pkt, 0) gives the current packet subtype,

> odp_packet_type(pkt, 1) gives the previous, etc. That way processing

> stages that need this sort of "forensic" information can get it if

> needed, but other stages can ignore it. We may want to add the API:



Most application code is written against a specific event type (e.g. basic packet vs. ipsec packet). The class spec can be added later when we have gathered more use cases for it.

-Petri



> 

> int odp_packet_history(odp_packet_t pkt);

> 

> to return the number of such levels available, but this sort of

> extension may be premature for now.

> 

> >

> > -Petri

> >

> >
Bill Fischofer June 12, 2017, 12:43 p.m. UTC | #5
On Mon, Jun 12, 2017 at 2:34 AM, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
>> >> >  /**

>> >> >   * Get IPSEC status information from an ODP_EVENT_IPSEC_STATUS event

>> >> > diff --git a/include/odp/arch/default/api/abi/event.h

>> >> b/include/odp/arch/default/api/abi/event.h

>> >> > index 87220d63..d807425b 100644

>> >> > --- a/include/odp/arch/default/api/abi/event.h

>> >> > +++ b/include/odp/arch/default/api/abi/event.h

>> >> > @@ -27,9 +27,9 @@ typedef _odp_abi_event_t *odp_event_t;

>> >> >  typedef enum odp_event_type_t {

>> >> >     ODP_EVENT_BUFFER       = 1,

>> >> >     ODP_EVENT_PACKET       = 2,

>> >> > -   ODP_EVENT_TIMEOUT      = 3,

>> >> > -   ODP_EVENT_CRYPTO_COMPL = 4,

>> >> > -   ODP_EVENT_IPSEC_RESULT = 5,

>> >> > +   ODP_EVENT_PACKET_IPSEC = 3,

>> >> > +   ODP_EVENT_TIMEOUT      = 4,

>> >> > +   ODP_EVENT_CRYPTO_COMPL = 5,

>> >> >     ODP_EVENT_IPSEC_STATUS = 6

>> >> >  } odp_event_type_t;

>> >>

>> >> Any reason for rearrangement of existing values?

>> >

>> > Yes, it's now

>> >

>> >         ODP_EVENT_BUFFER       = 1,

>> >         ODP_EVENT_PACKET       = 2,

>> >         ODP_EVENT_PACKET_IPSEC = 3,

>> >

>> > which may turn in couple of weeks into

>> >

>> >         ODP_EVENT_BUFFER       = 1,

>> >         ODP_EVENT_PACKET       = 2,

>> >         ODP_EVENT_PACKET_IPSEC = 3,

>> >         ODP_EVENT_PACKET_CRYPTO = 4,

>> >         ODP_EVENT_PACKET_COMP = 5,

>> >

>> > ... normal packet and special ones.

>>

>> The problem with trying to to this as an ever-growing list of event

>> types, is that every time we introduce a new one every application

>> needs to change. ODP_EVENT_PACKET_IPSEC is simply an ODP_EVENT_PACKET

>> with some additional metadata that may or may not be of interest to a

>> given application processing stage. So the basic architecture should

>> reflect this fact.

>

> Actually, the idea is that existing application *does not* have to change. EVENT_PACKET is still what it is today. So, current applications do not need to change anything: e.g. those do not need to check if a packet is coming from IPSEC and if the IPSEC operation has failed, etc.


EVENT_PACKET remains the same after the addition of packet subtypes
since every subtype is fully an odp_packet_t. The subtypes simply
qualify that by noting what additional metadata is available for this
packet. Applications (or more properly individual routines within
applications) need only be concerned with subtypes when they want to
process this additional metadata.

>

>

>>

>> The typical event loop for a worker thread would look like:

>>

>> while (1) {

>>         odp_event_t ev = odp_schedule(&queue, ODP_SCHED_WAIT);

>>         switch (odp_event_type(ev)) {

>>                 case ODP_EVENT_BUFFER:

>>                         do_buffer_processing(odp_buffer_from_event(ev),

>> queue);

>>                         break;

>>                 case ODP_EVENT_TIMEOUT:

>>

>> do_timeout_processing(odp_timeout_from_event(ev), queue);

>>                         break;

>>                 case ODP_EVENT_PACKET:

>>                         do_packet_processing(odp_packet_from_event(ev),

>> queue);

>>                         break;

>>                 ...

>>                 default:

>>                         // Unknown event type, log and discard

>>         }

>> }

>>

>> So now that processing loop needs to be updated even if this worker

>> thread doesn't care about that optional IPsec metadata.

>

>

> No, by default nothing changes. Only if application itself *enabled* IPSEC processing, it would see PACKET_IPSEC events and would need to handle those... but then application was already changed a lot (decided to use IPSEC offload).

>


The application enabled IPsec processing in one, likely small, section
of the application code. If I have an existing large body of code it's
best not to have to comb through it updating every reference to packet
events to now have to consider other packet events that don't present
themselves as ODP_EVENT_PACKETs.

>

>>

>> A more flexible approach would be to define a new odp_packet_type_t

>> and associated odp_packet_type() API that identifies these subtypes.

>> That way the worker packet processing routine can be written as:

>>

>> void do_packet_processing(odp_packet_t pkt, odp_queue_t queue)

>> {

>>         ....

>>         switch (odp_packet_type(pkt)) {

>>                 case ODP_PACKET_IPSEC:

>>                           // do processing of interest with associated

>> IPsec metadata

>>                           break;

>>                 case ODP_PACKET_COMP:

>>                           // do processing of interest with associated

>> COMP metadata

>>                           break;

>>                 default:

>>         }

>>

>>         ... process the basic packet itself

>> }

>

>

> Subtype is not needed - event type is the subtype already.

>

> We have a plan of adding odp_event_class_t instead (ODP_EVENT_CLASS_PACKET, ...) as a super-type, but decided that it can wait for the actual need. The need is not yet there today as all packets are basic packets. After IPSEC/crypto/comp packet types are merged, it could be needed by an application pipeline stage which needs to do simple processing for all type packets. But even with that only successfully decrypted packets would go through that stage, otherwise failed decrypt would leave inner packet as garbage ...


That's a disruptive change that again forces applications to be
updated. We can't keep adjusting fundamental concepts like this and
stringing these changes out in a series of API "bumps". The concept of
event classes seems overkill since it's packets that have "flavors".
If timeouts developed the need for subtypes then that could be added
there as well, but so far it's only packets that have this need.

The failed decrypt case is trivially handled because routines that
know nothing about IPsec would simply see that the packet had an error
bit set and treat it as any other uncategorized error (e.g., checksum
failure).

>

>

>>

>> Note that this means that anytime we add a new packet subtype nothing

>> has to change at the application layer. It only needs to add code to

>> process that optional metadata in stages that specifically want to do

>> something with it. Otherwise they are treated as regular packets. So

>> the fact that this packet was IPsec decrypted before it got to this

>> stage is something the stage need not be aware of or care about unless

>> it wants to take special action because the packet arrived via that

>> offload path.

>

> Event class would enable this later on. As explained above, when IPSEC (/crypto/comp) is enabled application needs to have at least one stage which checks the results, otherwise other stage may end up processing garbage.


No, applications need only verify that the packet doesn't have any
error bits set, as noted above. This is something it always needs to
do, so there's no additional work needed here.

>

>

>>

>> As we add more pipeline capabilities this sort of structure will

>> become increasingly important as the whole idea behind pipelining is

>> that worker threads don't need to be aware of where the packets came

>> from or their offload history in order to do what that stage needs to

>> do. Some stages will want to inspect that history and do something

>> different based on it, but most will not.

>>

>> This becomes even more important when we consider pipeline chains. For

>> example, what happens when we support full offloading of packet that

>> is IPsec decrypted, then decompressed, and then handed to a worker?

>> Trying to encode these composites in a single event type quickly

>> becomes completely unmanageable. However, we can easily extend

>> odp_packet_type() to be:

>

> Pipelining in API would change the type and connection between stages would be (mostly) invisible to the application. So, e.g. pktin -> IPSEC decrypt -> decomp would result PACKET_COMP event for successfully decrypted and decompressed packets. Some small amount of IPSEC metadata could be needed still to be visible (e.g. "packet was ipsec", "ipsec sa was X"). Failed IPSEC step would result PACKET_IPSEC with error flags.


Yes, we can special-case specific composites, but this isn't a general
solution to the problem of dealing with multi-stage pipelines. We
don't want to have to keep coming up with one-offs as we add more
types of pipelining, so best to put a the structure for a general
solution to this problem now.

>

>

> This is roughly the idea of event classes.

>

> typedef enum odp_event_class_t {

>      ODP_EVENT_BUFFER,

>      ODP_EVENT_PACKET, // basic, ipsec, crypto, comp, ... all share the same basic packet metadata

>      ODP_EVENT_TIMER,

>      ODP_EVENT_STATUS, // all status events from ODP offloads

> } odp_event_class_t

>

> odp_event_class_t odp_event_class(odp_event_t ev);


It seems this is just renaming events to be event classes and then
using the subtypes I mentioned for packets. An odp_status_type() API
to get the specific status would be in keeping with the
odp_packet_type() API proposed earlier.

>

>

>

>>

>> odp_packet_type_t odp_packet_type(odp_packet_t pkt, int level);

>>

>> So that odp_packet_type(pkt, 0) gives the current packet subtype,

>> odp_packet_type(pkt, 1) gives the previous, etc. That way processing

>> stages that need this sort of "forensic" information can get it if

>> needed, but other stages can ignore it. We may want to add the API:

>

>

> Most application code is written against a specific event type (e.g. basic packet vs. ipsec packet). The class spec can be added later when we have gathered more use cases for it.


When would that be? Before or after the Tiger Moth release? I'd
suggest it needs to be before because again this requires application
change at a fairly fundamental level and we can't keep doing that sort
of thing.

>

> -Petri

>

>

>

>>

>> int odp_packet_history(odp_packet_t pkt);

>>

>> to return the number of such levels available, but this sort of

>> extension may be premature for now.

>>

>> >

>> > -Petri

>> >

>> >
Dmitry Eremin-Solenikov June 14, 2017, 9:53 a.m. UTC | #6
On 09.06.2017 15:32, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>>> @@ -934,41 +926,70 @@ typedef struct odp_ipsec_op_status_t {

>>>  } odp_ipsec_op_status_t;

>>>

>>>  /**

>>> - * IPSEC operation input parameters

>>> + * IPSEC outbound operation options

>>> + *

>>> + * These may be used to override some SA level options

>>>   */

>>> -typedef struct odp_ipsec_op_param_t {

>>> -	/** Number of packets to be processed */

>>> -	int num_pkt;

>>> +typedef struct odp_ipsec_out_opt_t {

>>> +	/** Fragmentation mode */

>>> +	odp_ipsec_frag_mode_t mode;

>>> +

>>> +} odp_ipsec_out_opt_t;

>>

>> Maybe we can just inline this into out_param_t ?

>> With this in/out split, it should be quite straightforward change from

>> the API point of view.

> 

> 

> The thing is that it's a pointer to an odp_ipsec_out_opt_t array, so application needs the type above. Otherwise, we'd need to define maximum of opts in the API, which would be the same as maximum number of packets. API doesn't need to limit the burst size.


I would suggest to move odp_ipsec_out_opt_t contents completely into the
odp_ipsec_out_param_t. One would have to fill it in every param, however
this might result in simpler code. This is just an idea for you to
consider, not a requirement of course.

-- 
With best wishes
Dmitry
Dmitry Eremin-Solenikov June 14, 2017, 2:41 p.m. UTC | #7
On 14.06.2017 12:53, Dmitry Eremin-Solenikov wrote:
> On 09.06.2017 15:32, Savolainen, Petri (Nokia - FI/Espoo) wrote:

>>>> @@ -934,41 +926,70 @@ typedef struct odp_ipsec_op_status_t {

>>>>  } odp_ipsec_op_status_t;

>>>>

>>>>  /**

>>>> - * IPSEC operation input parameters

>>>> + * IPSEC outbound operation options

>>>> + *

>>>> + * These may be used to override some SA level options

>>>>   */

>>>> -typedef struct odp_ipsec_op_param_t {

>>>> -	/** Number of packets to be processed */

>>>> -	int num_pkt;

>>>> +typedef struct odp_ipsec_out_opt_t {

>>>> +	/** Fragmentation mode */

>>>> +	odp_ipsec_frag_mode_t mode;

>>>> +

>>>> +} odp_ipsec_out_opt_t;

>>>

>>> Maybe we can just inline this into out_param_t ?

>>> With this in/out split, it should be quite straightforward change from

>>> the API point of view.

>>

>>

>> The thing is that it's a pointer to an odp_ipsec_out_opt_t array, so application needs the type above. Otherwise, we'd need to define maximum of opts in the API, which would be the same as maximum number of packets. API doesn't need to limit the burst size.

> 

> I would suggest to move odp_ipsec_out_opt_t contents completely into the

> odp_ipsec_out_param_t. One would have to fill it in every param, however

> this might result in simpler code. This is just an idea for you to

> consider, not a requirement of course.


I got what you meant. Please ignore this comment.


-- 
With best wishes
Dmitry
diff mbox series

Patch

diff --git a/include/odp/api/spec/event.h b/include/odp/api/spec/event.h
index f22efce5..ad757c7d 100644
--- a/include/odp/api/spec/event.h
+++ b/include/odp/api/spec/event.h
@@ -37,9 +37,30 @@  extern "C" {
 
 /**
  * @typedef odp_event_type_t
- * ODP event types:
- * ODP_EVENT_BUFFER, ODP_EVENT_PACKET, ODP_EVENT_TIMEOUT,
- * ODP_EVENT_CRYPTO_COMPL, ODP_EVENT_IPSEC_RESULT, ODP_EVENT_IPSEC_STATUS
+ * Event type
+ *
+ * Event type specifies the purpose and format of an event. It can be checked
+ * with odp_event_type(). Each event type has functions
+ * (e.g. odp_buffer_from_event()) to convert between the generic event handle
+ * (odp_event_t) and the type specific handle (e.g. odp_buffer_t). Results are
+ * undefined, if conversion function of a wrong event type is used. Application
+ * cannot change event type by chaining conversion functions.
+ *
+ * List of event types:
+ * - ODP_EVENT_BUFFER
+ *     - Buffer event (odp_buffer_t) for simple data storage and message passing
+ * - ODP_EVENT_PACKET
+ *     - Normal packet event (odp_packet_t) containing basic packet metadata.
+ * - ODP_EVENT_PACKET_IPSEC
+ *     - Packet event (odp_packet_t) genereted as a result of an IPsec
+ *       operation. It contains IPSEC specific metadata in addition to the basic
+ *       packet metadata.
+ * - ODP_EVENT_TIMEOUT
+ *     - Timeout event (odp_timeout_t) from a timer
+ * - ODP_EVENT_CRYPTO_COMPL
+ *     - Crypto completion event (odp_crypto_compl_t)
+ * - ODP_EVENT_IPSEC_STATUS
+ *     - IPSEC status update event (odp_ipsec_status_t)
  */
 
 /**
diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h
index 65f0b066..0da2b42c 100644
--- a/include/odp/api/spec/ipsec.h
+++ b/include/odp/api/spec/ipsec.h
@@ -552,16 +552,18 @@  typedef enum odp_ipsec_frag_mode_t {
 
 /**
  * Packet lookup mode
+ *
+ * Lookup mode controls how an SA participates in SA lookup offload.
+ * Inbound operations perform SA lookup if application does not provide a SA as
+ * a parameter. In inline mode, a lookup miss directs the packet back to normal
+ * packet input interface processing. SA lookup failure status (error.sa_lookup)
+ * is reported through odp_ipsec_packet_result_t.
  */
 typedef enum odp_ipsec_lookup_mode_t {
-	/** Inbound SA lookup is disabled. */
+	/** Inbound SA lookup is disabled for the SA. */
 	ODP_IPSEC_LOOKUP_DISABLED = 0,
 
-	/** Inbound SA lookup is enabled. Lookup matches only SPI value.
-	 *  In inline mode, a lookup miss directs the packet back to normal
-	 *  packet input interface processing. In other modes, the SA lookup
-	 *  failure status (error.sa_lookup) is reported through
-	 *  odp_ipsec_packet_result_t. */
+	/** Inbound SA lookup is enabled. Lookup matches only SPI value. */
 	ODP_IPSEC_LOOKUP_SPI,
 
 	/** Inbound SA lookup is enabled. Lookup matches both SPI value and
@@ -850,17 +852,6 @@  int odp_ipsec_sa_destroy(odp_ipsec_sa_t sa);
  */
 uint64_t odp_ipsec_sa_to_u64(odp_ipsec_sa_t sa);
 
-/**
- * IPSEC operation level options
- *
- * These may be used to override some SA level options
- */
-typedef struct odp_ipsec_op_opt_t {
-	/** Fragmentation mode */
-	odp_ipsec_frag_mode_t mode;
-
-} odp_ipsec_op_opt_t;
-
 /** IPSEC operation status has no errors */
 #define ODP_IPSEC_OK 0
 
@@ -870,7 +861,8 @@  typedef struct odp_ipsec_op_status_t {
 	union {
 		/** Error flags */
 		struct {
-			/** Protocol error. Not a valid ESP or AH packet. */
+			/** Protocol error. Not a valid ESP or AH packet,
+			 *  packet data length error, etc. */
 			uint32_t proto            : 1;
 
 			/** SA lookup failed */
@@ -934,41 +926,70 @@  typedef struct odp_ipsec_op_status_t {
 } odp_ipsec_op_status_t;
 
 /**
- * IPSEC operation input parameters
+ * IPSEC outbound operation options
+ *
+ * These may be used to override some SA level options
  */
-typedef struct odp_ipsec_op_param_t {
-	/** Number of packets to be processed */
-	int num_pkt;
+typedef struct odp_ipsec_out_opt_t {
+	/** Fragmentation mode */
+	odp_ipsec_frag_mode_t mode;
+
+} odp_ipsec_out_opt_t;
 
+/**
+ * IPSEC outbound operation parameters
+ */
+typedef struct odp_ipsec_out_param_t {
 	/** Number of SAs
 	 *
+	 *  Outbound IPSEC operation needs SA from application. Use either
+	 *  single SA for all packets, or a SA per packet.
+	 *
 	 *  Valid values are:
-	 *  * 0:       No SAs (default)
-	 *  * 1:       Single SA for all packets
-	 *  * num_pkt: SA per packet
+	 *  * 1:  Single SA for all packets
+	 *  * N:  A SA per packet. N must match the number of packets.
 	 */
 	int num_sa;
 
-	/** Number of operation options
+	/** Number of outbound operation options
 	 *
 	 *  Valid values are:
-	 *  * 0:       No options (default)
-	 *  * 1:       Single option for all packets
-	 *  * num_pkt: An option per packet
+	 *  * 0:  No options
+	 *  * 1:  Single option for all packets
+	 *  * N:  An option per packet. N must match the number of packets.
 	 */
 	int num_opt;
 
-	/** Pointer to an array of packets
+	/** Pointer to an array of IPSEC SAs */
+	odp_ipsec_sa_t *sa;
+
+	/** Pointer to an array of outbound operation options
+	 *
+	 *  May be NULL when num_opt is zero.
+	 */
+	odp_ipsec_out_opt_t *opt;
+
+} odp_ipsec_out_param_t;
+
+/**
+ * IPSEC inbound operation parameters
+ */
+typedef struct odp_ipsec_in_param_t {
+	/** Number of SAs
 	 *
-	 *  Each packet must have a valid value for these metadata:
-	 *  * L3 offset: Offset to the first byte of the (outmost) IP header
-	 *  * L4 offset: For inbound direction, when udp_encap is enabled -
-	 *               offset to the first byte of the encapsulating UDP
-	 *               header
+	 *  Inbound IPSEC operation processes a packet using the SA provided by
+	 *  the application. If the application does not provide an SA, the
+	 *  operation searches for the SA by matching the input packet with all
+	 *  inbound SAs according to the lookup mode (odp_ipsec_lookup_mode_t)
+	 *  configured in each SA. When passing SAs, use either single SA for
+	 *  all packets, or a SA per packet.
 	 *
-	 *  @see odp_packet_l3_offset(), odp_packet_l4_offset()
+	 *  Valid values are:
+	 *  * 0:  No SAs. SA lookup is done for all packets.
+	 *  * 1:  Single SA for all packets
+	 *  * N:  A SA per packet. N must match the number of packets.
 	 */
-	odp_packet_t *pkt;
+	int num_sa;
 
 	/** Pointer to an array of IPSEC SAs
 	 *
@@ -976,18 +997,12 @@  typedef struct odp_ipsec_op_param_t {
 	 */
 	odp_ipsec_sa_t *sa;
 
-	/** Pointer to an array of operation options
-	 *
-	 *  May be NULL when num_opt is zero.
-	 */
-	odp_ipsec_op_opt_t *opt;
-
-} odp_ipsec_op_param_t;
+} odp_ipsec_in_param_t;
 
 /**
  * Outbound inline IPSEC operation parameters
  */
-typedef struct odp_ipsec_inline_op_param_t {
+typedef struct odp_ipsec_out_inline_param_t {
 	/** Packet output interface for inline output operation
 	 *
 	 *  Outbound inline IPSEC operation uses this packet IO interface to
@@ -1011,7 +1026,7 @@  typedef struct odp_ipsec_inline_op_param_t {
 		uint32_t len;
 	} outer_hdr;
 
-} odp_ipsec_inline_op_param_t;
+} odp_ipsec_out_inline_param_t;
 
 /**
  * IPSEC operation result for a packet
@@ -1020,16 +1035,6 @@  typedef struct odp_ipsec_packet_result_t {
 	/** IPSEC operation status */
 	odp_ipsec_op_status_t status;
 
-	/** Number of output packets created from the corresponding input packet
-	 *
-	 *  Without fragmentation offload this is always one. However, if the
-	 *  input packet was fragmented during the operation this is larger than
-	 *  one for the first returned fragment and zero for the rest of the
-	 *  fragments. All the fragments (of the same source packet) are stored
-	 *  consecutively in the 'pkt' array.
-	 */
-	int num_out;
-
 	/** IPSEC SA that was used to create the packet
 	 *
 	 *  Operation updates this SA handle value, when SA look up is performed
@@ -1040,7 +1045,8 @@  typedef struct odp_ipsec_packet_result_t {
 	odp_ipsec_sa_t sa;
 
 	/** Packet outer header status before inbound inline processing.
-	 *  This is valid only when status.flag.inline_mode is set.
+	 *  This is valid only when outer headers are retained
+	 *  (see odp_ipsec_inbound_config_t) and status.flag.inline_mode is set.
 	 */
 	struct {
 		/** Points to the first byte of retained outer headers. These
@@ -1048,7 +1054,7 @@  typedef struct odp_ipsec_packet_result_t {
 		 *  implementation specific memory space. Since the memory space
 		 *  may overlap with e.g. packet head/tailroom, the content
 		 *  becomes invalid if packet data storage is modified in
-		 *  anyway. The memory space may not be sharable to other
+		 *  any way. The memory space may not be sharable to other
 		 *  threads. */
 		uint8_t *ptr;
 
@@ -1059,51 +1065,6 @@  typedef struct odp_ipsec_packet_result_t {
 } odp_ipsec_packet_result_t;
 
 /**
- * IPSEC operation results
- */
-typedef struct odp_ipsec_op_result_t {
-	/** Number of packets
-	 *
-	 *  Application sets this to the maximum number of packets the operation
-	 *  may output (number of elements in 'pkt' and 'res' arrays).
-	 *  The operation updates it with the actual number of packets
-	 *  outputted.
-	 */
-	int num_pkt;
-
-	/** Pointer to an array of packets
-	 *
-	 *  Operation outputs packets into this array. The array must have
-	 *  at least 'num_pkt' elements.
-	 *
-	 *  Each successfully transformed packet has a valid value for these
-	 *  metadata regardless of the inner packet parse configuration.
-	 *  (odp_ipsec_inbound_config_t):
-	 *  * L3 offset: Offset to the first byte of the (outmost) IP header
-	 *  * pktio:     For inbound inline IPSEC processed packets, original
-	 *               packet input interface
-	 *
-	 *  Other metadata for parse results and error checks depend on
-	 *  configuration (selected parse and error check levels).
-	 */
-	odp_packet_t *pkt;
-
-	/** Pointer to an array of per packet operation results
-	 *
-	 *  Operation outputs results for each outputted packet into this array.
-	 *  The array must have at least 'num_pkt' elements. The results include
-	 *  operation status and packet form information for each outputted
-	 *  packet.
-	 *
-	 *  For example, some packets may not have been transformed due to
-	 *  an error, but the original packet is returned with appropriate
-	 *  packet result information instead.
-	 */
-	odp_ipsec_packet_result_t *res;
-
-} odp_ipsec_op_result_t;
-
-/**
  * IPSEC status ID
  */
 typedef enum odp_ipsec_status_id_t {
@@ -1136,20 +1097,32 @@  typedef struct odp_ipsec_status_t {
  *
  * This operation does inbound IPSEC processing in synchronous mode
  * (ODP_IPSEC_OP_MODE_SYNC). A successful operation returns the number of
- * packets consumed and outputs a new packet handle as well as an operation
- * result for each outputted packet. The operation does not modify packets that
- * it does not consume. It cannot consume all input packets if 'output.num_pkt'
- * is smaller than 'input.num_pkt'.
+ * packets consumed and outputs a new packet handle for each outputted packet.
+ * Outputted packets contain IPSEC result metadata (odp_ipsec_packet_result_t),
+ * which should be checked for transformation errors, etc. The operation does
+ * not modify packets that it does not consume. It cannot consume all input
+ * packets if 'num_out' is smaller than 'num_in'.
  *
  * Packet context pointer and user area content are copied from input to output
  * packets. Output packets are allocated from the same pool(s) as input packets.
  *
- * When 'input.num_sa' is zero, this operation performs SA look up for each
+ * When 'param.num_sa' is zero, this operation performs SA look up for each
  * packet. Otherwise, application must provide the SA(s) as part of operation
- * input parameters (odp_ipsec_op_param_t). The operation outputs used SA(s) as
- * part of per packet operation results (odp_ipsec_packet_result_t), or an error
+ * input parameters (odp_ipsec_in_param_t). The operation outputs used SA(s) as
+ * part of per packet results (odp_ipsec_packet_result_t), or an error
  * status if a SA was not found.
  *
+ * Each input packet must have a valid value for these metadata (other metadata
+ * is ignored):
+ *  * L3 offset: Offset to the first byte of the (outmost) IP header
+ *  * L4 offset: When udp_encap is enabled, offset to the first byte of the
+ *               encapsulating UDP header
+ *
+ * Additionally, implementation checks input IP packet length (odp_packet_len()
+ * minus odp_packet_l3_offset()) against protocol headers and reports an error
+ * (status.error.proto) if packet data length is less than protocol headers
+ * indicate.
+ *
  * Packets are processed in the input order. Packet order is maintained from
  * input 'pkt' array to output 'pkt' array. Packet order is not guaranteed
  * between calling threads.
@@ -1162,35 +1135,50 @@  typedef struct odp_ipsec_status_t {
  * restored. The amount and content of packet data before the IP header is
  * undefined.
  *
- * @param         input   Operation input parameters
- * @param[out]    output  Operation results
+ * @param          pkt_in   Packets to be processed
+ * @param          num_in   Number of packets to be processed
+ * @param[out]     pkt_out  Packet handle array for resulting packets
+ * @param[in, out] num_out  Number of resulting packets. Application sets this
+ *                          to 'pkt_out' array size. A successful operation sets
+ *                          this to the number of outputted packets
+ *                          (1 ... num_out).
+ * @param          param    Inbound operation parameters
  *
- * @return Number of input packets consumed (0 ... input.num_pkt)
+ * @return Number of input packets consumed (0 ... num_in)
  * @retval <0     On failure
  *
- * @see odp_packet_user_ptr(), odp_packet_user_area()
+ * @see odp_packet_user_ptr(), odp_packet_user_area(), odp_packet_l3_offset(),
+ * odp_packet_l4_offset()
  */
-int odp_ipsec_in(const odp_ipsec_op_param_t *input,
-		 odp_ipsec_op_result_t *output);
+int odp_ipsec_in(const odp_packet_t pkt_in[], int num_in,
+		 odp_packet_t pkt_out[], int *num_out,
+		 const odp_ipsec_in_param_t *param);
 
 /**
  * Outbound synchronous IPSEC operation
  *
  * This operation does outbound IPSEC processing in synchronous mode
  * (ODP_IPSEC_OP_MODE_SYNC). A successful operation returns the number of
- * packets consumed and outputs a new packet handle as well as an operation
- * result for each outputted packet. The operation does not modify packets that
- * it does not consume. It cannot consume all input packets if 'output.num_pkt'
- * is smaller than 'input.num_pkt'.
+ * packets consumed and outputs a new packet handle for each outputted packet.
+ * Outputted packets contain IPSEC result metadata (odp_ipsec_packet_result_t),
+ * which should be checked for transformation errors, etc. The operation does
+ * not modify packets that it does not consume. It cannot consume all input
+ * packets if 'num_out' is smaller than 'num_in'.
  *
  * Packet context pointer and user area content are copied from input to output
  * packets. Output packets are allocated from the same pool(s) as input packets.
  *
  * When outbound IP fragmentation offload is enabled, the number of outputted
- * packets (and corresponding per packet results) may be greater than
- * the number of input packets. In that case, application may examine 'num_out'
- * of each packet result (odp_ipsec_packet_result_t) to find out which
- * fragments are originated from which input packet.
+ * packets may be greater than the number of input packets.
+ *
+ * Each input packet must have a valid value for these metadata (other metadata
+ * is ignored):
+ *  * L3 offset: Offset to the first byte of the (outmost) IP header
+ *  * L4 offset: Offset to the L4 header if L4 checksum offload is requested
+ *
+ * Additionally, input IP packet length (odp_packet_len() minus
+ * odp_packet_l3_offset()) must match values in protocol headers. Otherwise
+ * results are undefined.
  *
  * Packets are processed in the input order. Packet order is maintained from
  * input 'pkt' array to output 'pkt' array. Packet order is not guaranteed
@@ -1201,31 +1189,37 @@  int odp_ipsec_in(const odp_ipsec_op_param_t *input,
  * with IPSEC, etc headers constructed according to the standards. The amount
  * and content of packet data before the IP header is undefined.
  *
- * @param         input   Operation input parameters
- * @param[out]    output  Operation results
+ * @param          pkt_in   Packets to be processed
+ * @param          num_in   Number of packets to be processed
+ * @param[out]     pkt_out  Packet handle array for resulting packets
+ * @param[in, out] num_out  Number of resulting packets. Application sets this
+ *                          to 'pkt_out' array size. A successful operation sets
+ *                          this to the number of outputted packets
+ *                          (1 ... num_out).
+ * @param          param    Outbound operation parameters
  *
- * @return Number of input packets consumed (0 ... input.num_pkt)
+ * @return Number of input packets consumed (0 ... num_in)
  * @retval <0     On failure
  *
- * @see odp_packet_user_ptr(), odp_packet_user_area()
+ * @see odp_packet_user_ptr(), odp_packet_user_area(), odp_packet_l3_offset()
  */
-int odp_ipsec_out(const odp_ipsec_op_param_t *input,
-		  odp_ipsec_op_result_t *output);
+int odp_ipsec_out(const odp_packet_t pkt_in[], int num_in,
+		  odp_packet_t pkt_out[], int *num_out,
+		  const odp_ipsec_out_param_t *param);
 
 /**
  * Inbound asynchronous IPSEC operation
  *
  * This operation does inbound IPSEC processing in asynchronous mode. It
- * processes packets otherwise identically to odp_ipsec_in(), but outputs all
- * results through one or more ODP_EVENT_IPSEC_RESULT events with the following
+ * processes packets otherwise identically to odp_ipsec_in(), but outputs
+ * resulting packets as ODP_EVENT_PACKET_IPSEC events with the following
  * ordering considerations.
  *
  * Asynchronous mode maintains packet order per SA when application calls the
  * operation within an ordered or atomic scheduler context of the same queue.
- * Resulting events for the same SA are enqueued in order and packet handles
- * (for the same SA) are stored in order within an event. Packet order per SA at
- * a destination queue is the same as if application would have enqueued packets
- * there with odp_queue_enq_multi().
+ * Resulting events for the same SA are enqueued in order. Packet order per SA
+ * at a destination queue is the same as if application would have enqueued
+ * packets there with odp_queue_enq_multi().
  *
  * Packet order is also maintained when application otherwise guarantees
  * (e.g. using locks) that the operation is not called simultaneously from
@@ -1239,29 +1233,31 @@  int odp_ipsec_out(const odp_ipsec_op_param_t *input,
  * may be processed simultaneously in both modes (initiated by this function
  * and inline operation).
  *
- * @param         input   Operation input parameters
+ * @param          pkt      Packets to be processed
+ * @param          num      Number of packets to be processed
+ * @param          param    Inbound operation parameters
  *
- * @return Number of input packets consumed (0 ... input.num_pkt)
+ * @return Number of input packets consumed (0 ... num)
  * @retval <0     On failure
  *
  * @see odp_ipsec_in(), odp_ipsec_result()
  */
-int odp_ipsec_in_enq(const odp_ipsec_op_param_t *input);
+int odp_ipsec_in_enq(const odp_packet_t pkt[], int num,
+		     const odp_ipsec_in_param_t *param);
 
 /**
  * Outbound asynchronous IPSEC operation
  *
  * This operation does outbound IPSEC processing in asynchronous mode. It
  * processes packets otherwise identically to odp_ipsec_out(), but outputs all
- * results through one or more ODP_EVENT_IPSEC_RESULT events with the following
+ * resulting packets as ODP_EVENT_PACKET_IPSEC events with the following
  * ordering considerations.
  *
  * Asynchronous mode maintains packet order per SA when application calls the
  * operation within an ordered or atomic scheduler context of the same queue.
- * Resulting events for the same SA are enqueued in order and packet handles
- * (for the same SA) are stored in order within an event. Packet order per SA at
- * a destination queue is the same as if application would have enqueued packets
- * there with odp_queue_enq_multi().
+ * Resulting events for the same SA are enqueued in order. Packet order per SA
+ * at a destination queue is the same as if application would have enqueued
+ * packets there with odp_queue_enq_multi().
  *
  * Packet order is also maintained when application otherwise guarantees
  * (e.g. using locks) that the operation is not called simultaneously from
@@ -1273,14 +1269,17 @@  int odp_ipsec_in_enq(const odp_ipsec_op_param_t *input);
  * The function may be used also in inline processing mode, e.g. for IPSEC
  * packets for which inline processing is not possible.
  *
- * @param         input   Operation input parameters
+ * @param          pkt      Packets to be processed
+ * @param          num      Number of packets to be processed
+ * @param          param    Outbound operation parameters
  *
- * @return Number of input packets consumed (0 ... input.num_pkt)
+ * @return Number of input packets consumed (0 ... num)
  * @retval <0     On failure
  *
  * @see odp_ipsec_out(), odp_ipsec_result()
  */
-int odp_ipsec_out_enq(const odp_ipsec_op_param_t *input);
+int odp_ipsec_out_enq(const odp_packet_t pkt[], int num,
+		      const odp_ipsec_out_param_t *param);
 
 /**
  * Outbound inline IPSEC operation
@@ -1291,39 +1290,73 @@  int odp_ipsec_out_enq(const odp_ipsec_op_param_t *input);
  * result events for those.
  *
  * Inline operation parameters are defined per packet. The array of parameters
- * must have 'op_param.num_pkt' elements and is pointed to by 'inline_param'.
+ * must have 'num' elements and is pointed to by 'inline_param'.
  *
- * @param         op_param      Operation parameters
- * @param         inline_param  Outbound inline operation specific parameters
+ * @param          pkt           Packets to be processed
+ * @param          num           Number of packets to be processed
+ * @param          param         Outbound operation parameters
+ * @param          inline_param  Outbound inline operation specific parameters
  *
- * @return Number of packets consumed (0 ... op_param.num_pkt)
+ * @return Number of packets consumed (0 ... num)
  * @retval <0     On failure
  *
  * @see odp_ipsec_out_enq()
  */
-int odp_ipsec_out_inline(const odp_ipsec_op_param_t *op_param,
-			 const odp_ipsec_inline_op_param_t *inline_param);
+int odp_ipsec_out_inline(const odp_packet_t pkt[], int num,
+			 const odp_ipsec_out_param_t *param,
+			 const odp_ipsec_out_inline_param_t *inline_param);
+
+/**
+ * Convert ODP_EVENT_IPSEC_PACKET event to packet handle
+ *
+ * Get packet handle for an IPSEC processed packet. Event type must be
+ * ODP_EVENT_IPSEC_PACKET. IPSEC operation results can be examined with
+ * odp_ipsec_result().
+ *
+ * @param ev       Event handle
+ *
+ * @return Packet handle
+ *
+ * @see odp_event_type(), odp_ipsec_result()
+ */
+odp_packet_t odp_ipsec_packet_from_event(odp_event_t ev);
 
 /**
- * Get IPSEC results from an ODP_EVENT_IPSEC_RESULT event
+ * Convert IPSEC processed packet handle to event
  *
- * Copies IPSEC operation results from an event. The event must be of
- * type ODP_EVENT_IPSEC_RESULT. It must be freed before the application passes
- * any resulting packet handles to other ODP calls.
+ * The packet handle must be an output of an IPSEC operation.
  *
- * @param[out]    result  Pointer to operation result for output. Maybe NULL, if
- *                        application is interested only on the number of
- *                        packets.
- * @param         event   An ODP_EVENT_IPSEC_RESULT event
+ * @param pkt      Packet handle from IPSEC operation
  *
- * @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.
+ * @return Event handle
+ */
+odp_event_t odp_ipsec_packet_to_event(odp_packet_t pkt);
+
+/**
+ * Get IPSEC operation results from an IPSEC processed packet
+ *
+ * Successful IPSEC operations of all types (SYNC, ASYNC and INLINE) produce
+ * packets which contain IPSEC result metadata. This function copies the
+ * operation results from an IPSEC processed packet. Event type of this kind of
+ * packet is ODP_EVENT_PACKET_IPSEC. Results are undefined if a non-IPSEC
+ * processed packet is passed as input.
+ *
+ * Some packet API operations output a new packet handle
+ * (e.g. odp_packet_concat()). IPSEC metadata remain valid as long as the packet
+ * handle is not changed from the original (output of e.g. odp_ipsec_in() or
+ * odp_ipsec_packet_from_event() call) IPSEC processed packet handle.
+ *
+ * @param[out]    result  Pointer to operation result for output
+ * @param         packet  An IPSEC processed packet (ODP_EVENT_PACKET_IPSEC)
+ *
+ * @retval  0     On success
  * @retval <0     On failure
  *
- * @see odp_ipsec_in_enq(), odp_ipsec_out_enq()
+ * @see odp_ipsec_in(), odp_ipsec_in_enq(), odp_ipsec_out(),
+ *      odp_ipsec_out_enq(), odp_ipsec_out_inline(),
+ *      odp_ipsec_packet_from_event()
  */
-int odp_ipsec_result(odp_ipsec_op_result_t *result, odp_event_t event);
+int odp_ipsec_result(odp_ipsec_packet_result_t *result, odp_packet_t packet);
 
 /**
  * Get IPSEC status information from an ODP_EVENT_IPSEC_STATUS event
diff --git a/include/odp/arch/default/api/abi/event.h b/include/odp/arch/default/api/abi/event.h
index 87220d63..d807425b 100644
--- a/include/odp/arch/default/api/abi/event.h
+++ b/include/odp/arch/default/api/abi/event.h
@@ -27,9 +27,9 @@  typedef _odp_abi_event_t *odp_event_t;
 typedef enum odp_event_type_t {
 	ODP_EVENT_BUFFER       = 1,
 	ODP_EVENT_PACKET       = 2,
-	ODP_EVENT_TIMEOUT      = 3,
-	ODP_EVENT_CRYPTO_COMPL = 4,
-	ODP_EVENT_IPSEC_RESULT = 5,
+	ODP_EVENT_PACKET_IPSEC = 3,
+	ODP_EVENT_TIMEOUT      = 4,
+	ODP_EVENT_CRYPTO_COMPL = 5,
 	ODP_EVENT_IPSEC_STATUS = 6
 } odp_event_type_t;
 
diff --git a/platform/linux-generic/include/odp/api/plat/event_types.h b/platform/linux-generic/include/odp/api/plat/event_types.h
index 0f517834..3615b4a4 100644
--- a/platform/linux-generic/include/odp/api/plat/event_types.h
+++ b/platform/linux-generic/include/odp/api/plat/event_types.h
@@ -37,9 +37,10 @@  typedef ODP_HANDLE_T(odp_event_t);
 typedef enum odp_event_type_t {
 	ODP_EVENT_BUFFER       = 1,
 	ODP_EVENT_PACKET       = 2,
-	ODP_EVENT_TIMEOUT      = 3,
-	ODP_EVENT_CRYPTO_COMPL = 4,
-	ODP_EVENT_IPSEC_RESULT = 5
+	ODP_EVENT_PACKET_IPSEC = 3,
+	ODP_EVENT_TIMEOUT      = 4,
+	ODP_EVENT_CRYPTO_COMPL = 5,
+	ODP_EVENT_IPSEC_STATUS = 6
 } odp_event_type_t;
 
 /**
diff --git a/platform/linux-generic/odp_ipsec.c b/platform/linux-generic/odp_ipsec.c
index 10918dfb..c7eeb4ec 100644
--- a/platform/linux-generic/odp_ipsec.c
+++ b/platform/linux-generic/odp_ipsec.c
@@ -73,51 +73,68 @@  int odp_ipsec_sa_destroy(odp_ipsec_sa_t sa)
 	return -1;
 }
 
-int odp_ipsec_in(const odp_ipsec_op_param_t *input,
-		 odp_ipsec_op_result_t *output)
-{
-	(void)input;
-	(void)output;
+int odp_ipsec_in(const odp_packet_t pkt_in[], int num_in,
+		 odp_packet_t pkt_out[], int *num_out,
+		 const odp_ipsec_in_param_t *param)
+{
+	(void)pkt_in;
+	(void)num_in;
+	(void)pkt_out;
+	(void)num_out;
+	(void)param;
 
 	return -1;
 }
 
-int odp_ipsec_out(const odp_ipsec_op_param_t *input,
-		  odp_ipsec_op_result_t *output)
+int odp_ipsec_out(const odp_packet_t pkt_in[], int num_in,
+		  odp_packet_t pkt_out[], int *num_out,
+		  const odp_ipsec_out_param_t *param)
 {
-	(void)input;
-	(void)output;
+	(void)pkt_in;
+	(void)num_in;
+	(void)pkt_out;
+	(void)num_out;
+	(void)param;
 
 	return -1;
 }
 
-int odp_ipsec_in_enq(const odp_ipsec_op_param_t *input)
+int odp_ipsec_in_enq(const odp_packet_t pkt[], int num,
+		     const odp_ipsec_in_param_t *param)
 {
-	(void)input;
+	(void)pkt;
+	(void)num;
+	(void)param;
 
 	return -1;
 }
 
-int odp_ipsec_out_enq(const odp_ipsec_op_param_t *input)
+int odp_ipsec_out_enq(const odp_packet_t pkt[], int num,
+		      const odp_ipsec_out_param_t *param)
 {
-	(void)input;
+	(void)pkt;
+	(void)num;
+	(void)param;
 
 	return -1;
 }
 
-int odp_ipsec_out_inline(const odp_ipsec_op_param_t *op_param,
-			 const odp_ipsec_inline_op_param_t *inline_param)
+int odp_ipsec_out_inline(const odp_packet_t pkt[], int num,
+			 const odp_ipsec_out_param_t *param,
+			 const odp_ipsec_out_inline_param_t *inline_param)
 {
-	(void)op_param;
+	(void)pkt;
+	(void)num;
+	(void)param;
 	(void)inline_param;
 
 	return -1;
 }
 
-int odp_ipsec_result(odp_ipsec_op_result_t *result, odp_event_t event)
+int odp_ipsec_result(odp_ipsec_packet_result_t *result, odp_packet_t packet)
 {
 	(void)result;
-	(void)event;
+	(void)packet;
 
 	return -1;
 }
@@ -145,6 +162,20 @@  void *odp_ipsec_sa_context(odp_ipsec_sa_t sa)
 	return NULL;
 }
 
+odp_packet_t odp_ipsec_packet_from_event(odp_event_t ev)
+{
+	(void)ev;
+
+	return ODP_PACKET_INVALID;
+}
+
+odp_event_t odp_ipsec_packet_to_event(odp_packet_t pkt)
+{
+	(void)pkt;
+
+	return ODP_EVENT_INVALID;
+}
+
 uint64_t odp_ipsec_sa_to_u64(odp_ipsec_sa_t sa)
 {
 	return _odp_pri(sa);