> > + * - 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