> -----Original Message-----
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
> Github ODP bot
> Sent: Wednesday, October 25, 2017 12:00 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH API-NEXT v4 1/3] api: ipsec: rework
> ODP_IPSEC_SA_DISABLE into packet error
> 
> From: Dmitry Eremin-Solenikov <dmitry.ereminsoleni...@linaro.org>
> 
> According to the discussion on mailing list, most of implementations
> will not be able to support odp_ipsec_sa_disable() status event
> directly.  Instead they will submit a dummy packet to that SA. Then
> after receiving this packet after odp_ipsec_result() will detect this
> packet and will report it as a packet with
> odp_ipsec_error_t->sa_disabled bit set.
> 
> Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsoleni...@linaro.org>
> Cc: Nikhil Agarwal <nikhil.agar...@linaro.org>
> Cc: Balasubramanian Manoharan <bala.manoha...@linaro.org>
> ---
> /** Email created from pull request 256 (lumag:ipsec_sa_disable_v2)
>  ** https://github.com/Linaro/odp/pull/256
>  ** Patch: https://github.com/Linaro/odp/pull/256.patch
>  ** Base sha: 825f75ed8644ef57c5648961e7982daf13cd9375
>  ** Merge commit sha: ba520d0a3f4c46777c7aedca029e9979a89c69e7
>  **/
>  include/odp/api/spec/ipsec.h | 44 ++++++++++++++++++++-------------------
> -----
>  1 file changed, 20 insertions(+), 24 deletions(-)
> 
> diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h
> index 26e852fca..b9ad282ce 100644
> --- a/include/odp/api/spec/ipsec.h
> +++ b/include/odp/api/spec/ipsec.h
> @@ -843,10 +843,12 @@ odp_ipsec_sa_t odp_ipsec_sa_create(const
> odp_ipsec_sa_param_t *param);
>   *
>   * When in synchronous operation mode, the call will return when it's
> possible
>   * to destroy the SA. In asynchronous mode, the same is indicated by an
> - * ODP_EVENT_IPSEC_STATUS event sent to the queue specified for the SA.
> The
> - * status event is guaranteed to be the last event for the SA, i.e. all
> - * in-progress operations have completed and resulting events (including
> status
> - * events) have been enqueued before it.
> + * artificial packet event sent to the queue specified for the SA having
> + * sa_disabled error bit set in the odp_ipsec_packet_result_t returned by
> + * odp_ipsec_result(). The packet is guaranteed to be the last event for
> + * the SA, i.e. all in-progress operations have completed and resulting
> events
> + * (including status events) have been enqueued before it. No packets
> will come
> + * from SA after this one.

This still lacks the definition of what is the difference between an artificial 
packet vs a normal packet. We could define it e.g. by saying that length is 
always zero and application must not do anything else with it than free it. But 
then, why it's a packet anyway? Why it would not then be e.g. a ipsec status 
event (which is not a packet, but could be implemented as an artificial packet).

The idea of event type vs sub-event type is that:
 1) all events of the same base type (e.g. packet) contain the same metadata 
and can be processed the same way. For example, someone may write a generic 
packet statistics (application) module, and plug that before and after an ipsec 
termination (application) module, without need to know that packets before 
ipsec module carry extra ipsec metadata (from inline inbound ipsec).
 2) sub-type adds metadata and other properties to the base type

An artificial packet would not fit in either category, it would be a non-packet.

Usually, application receives multiple event types anyway: e.g. packets and 
timeouts. So, for application it would not be an issue to have one additional 
switch-case for IPSEC status events. API integrity is more important than 
(potential) save of couple if-else branches.

So, I think this solution is not complete yet and I don't see how it would in 
the end make a real difference to the current status event API.

-Petri


Reply via email to