Hi ,
what this patch wants to accomplish can be done with the actual API this
way : if the implementation really cannot reuse the input packet as the
completion event (are there implementations which cannot?) it can return an
error and the application could allocate a separate completion event:
There is a little overhead with the first assignment and with the failed
crypto operation in case of platforms which do not support reusing the
packet but the API doesn't need to be modified (only adding a new error
code).

odp_buffer_t compl = odp_packet_to_buffer(op_params.pkt);
rc = odp_crypto_operation(&op_params, &posted, compl);
if (odp_unlikely(ODP_CRYPTO_INVALID_COMPL_EVENT == rc)) {
      compl = odp_buffer_alloc();
      rc = odp_crypto_operation(&op_params, &posted, compl);
}

Alex




On 5 December 2014 at 14:42, Taras Kondratiuk <[email protected]>
wrote:

> Some implementations may not need a separate buffer for completion
> event but can reuse packet for this. Make it possible for implementation
> to decide this by passing completion event as pointer to
> odp_crypto_operation(). In synchronous mode implementation will return
> completion event at that pointer. In asynchronous - ODP_BUFFER_INVALID.
>
> This patch doesn't cover CUnit tests.
>
> Signed-off-by: Taras Kondratiuk <[email protected]>
> ---
>  example/ipsec/odp_ipsec.c                       |  106
> ++++++++++++++---------
>  platform/linux-generic/include/api/odp_crypto.h |   34 +++++---
>  platform/linux-generic/odp_crypto.c             |   30 ++++---
>  3 files changed, 101 insertions(+), 69 deletions(-)
>
> diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
> index 76d27c5..c25f631 100644
> --- a/example/ipsec/odp_ipsec.c
> +++ b/example/ipsec/odp_ipsec.c
> @@ -209,6 +209,33 @@ void free_pkt_ctx(pkt_ctx_t *ctx)
>  }
>
>  /**
> + * Convert completion event to completed operation packet
> + *
> + * @note Function frees completion event.
> + *
> + * @param completion  Event containing operation results
> + *
> + * @return Packet structure where data now resides or ODP_PACKET_INVALID
> + *         in case of any operation error.
> + */
> +static
> +odp_packet_t crypto_completion_to_packet(odp_buffer_t completion)
> +{
> +       odp_crypto_compl_status_t cipher_rc;
> +       odp_crypto_compl_status_t auth_rc;
> +       odp_packet_t pkt;
> +
> +       odp_crypto_get_operation_compl_status(completion, &cipher_rc,
> &auth_rc);
> +       if (!is_crypto_compl_status_ok(&cipher_rc))
> +               return ODP_PACKET_INVALID;
> +       if (!is_crypto_compl_status_ok(&auth_rc))
> +               return ODP_PACKET_INVALID;
> +       pkt = odp_crypto_get_operation_compl_packet(completion);
> +       odp_crypto_operation_compl_free(completion);
> +       return pkt;
> +}
> +
> +/**
>   * Use socket I/O workaround to query interface MAC address
>   *
>   * @todo Remove all references to USE_MAC_ADDR_HACK once
> @@ -677,18 +704,18 @@ pkt_disposition_e do_route_fwd_db(odp_packet_t pkt,
> pkt_ctx_t *ctx)
>   * @return PKT_CONTINUE if done else PKT_POSTED
>   */
>  static
> -pkt_disposition_e do_ipsec_in_classify(odp_packet_t pkt,
> +pkt_disposition_e do_ipsec_in_classify(odp_packet_t *pkt,
>                                        pkt_ctx_t *ctx,
>                                        bool *skip)
>  {
> -       uint8_t *buf = odp_packet_addr(pkt);
> -       odph_ipv4hdr_t *ip = (odph_ipv4hdr_t *)odp_packet_l3(pkt);
> +       uint8_t *buf = odp_packet_addr(*pkt);
> +       odp_buffer_t completion = ODP_BUFFER_INVALID;
> +       odph_ipv4hdr_t *ip = (odph_ipv4hdr_t *)odp_packet_l3(*pkt);
>         int hdr_len;
>         odph_ahhdr_t *ah = NULL;
>         odph_esphdr_t *esp = NULL;
>         ipsec_cache_entry_t *entry;
>         odp_crypto_op_params_t params;
> -       bool posted = 0;
>
>         /* Default to skip IPsec */
>         *skip = TRUE;
> @@ -710,8 +737,8 @@ pkt_disposition_e do_ipsec_in_classify(odp_packet_t
> pkt,
>         /* Initialize parameters block */
>         memset(&params, 0, sizeof(params));
>         params.session = entry->state.session;
> -       params.pkt = pkt;
> -       params.out_pkt = entry->in_place ? pkt : ODP_PACKET_INVALID;
> +       params.pkt = *pkt;
> +       params.out_pkt = entry->in_place ? *pkt : ODP_PACKET_INVALID;
>
>         /*Save everything to context */
>         ctx->ipsec.ip_tos = ip->tos;
> @@ -743,12 +770,16 @@ pkt_disposition_e do_ipsec_in_classify(odp_packet_t
> pkt,
>
>         /* Issue crypto request */
>         *skip = FALSE;
> -       if (odp_crypto_operation(&params,
> -                                &posted,
> -                                odp_packet_to_buffer(pkt))) {
> +       if (odp_crypto_operation(&params, &completion))
>                 abort();
> -       }
> -       return (posted) ? PKT_POSTED : PKT_CONTINUE;
> +       if (completion == ODP_BUFFER_INVALID)
> +               return PKT_POSTED;
> +
> +       *pkt = crypto_completion_to_packet(completion);
> +       if (*pkt != ODP_PACKET_INVALID)
> +               return PKT_CONTINUE;
> +
> +       return PKT_DROP;
>  }
>
>  /**
> @@ -763,20 +794,10 @@ static
>  pkt_disposition_e do_ipsec_in_finish(odp_packet_t pkt,
>                                      pkt_ctx_t *ctx)
>  {
> -       odp_buffer_t event;
> -       odp_crypto_compl_status_t cipher_rc;
> -       odp_crypto_compl_status_t auth_rc;
>         odph_ipv4hdr_t *ip;
>         int hdr_len = ctx->ipsec.hdr_len;
>         int trl_len = 0;
>
> -       /* Check crypto result */
> -       event = odp_packet_to_buffer(pkt);
> -       odp_crypto_get_operation_compl_status(event, &cipher_rc, &auth_rc);
> -       if (!is_crypto_compl_status_ok(&cipher_rc))
> -               return PKT_DROP;
> -       if (!is_crypto_compl_status_ok(&auth_rc))
> -               return PKT_DROP;
>         ip = (odph_ipv4hdr_t *)odp_packet_l3(pkt);
>
>         /*
> @@ -956,11 +977,11 @@ pkt_disposition_e do_ipsec_out_classify(odp_packet_t
> pkt,
>   * @return PKT_CONTINUE if done else PKT_POSTED
>   */
>  static
> -pkt_disposition_e do_ipsec_out_seq(odp_packet_t pkt,
> +pkt_disposition_e do_ipsec_out_seq(odp_packet_t *pkt,
>                                    pkt_ctx_t *ctx)
>  {
> -       uint8_t *buf = odp_packet_addr(pkt);
> -       bool posted = 0;
> +       odp_buffer_t completion = ODP_BUFFER_INVALID;
> +       uint8_t *buf = odp_packet_addr(*pkt);
>
>         /* We were dispatched from atomic queue, assign sequence numbers */
>         if (ctx->ipsec.ah_offset) {
> @@ -977,12 +998,16 @@ pkt_disposition_e do_ipsec_out_seq(odp_packet_t pkt,
>         }
>
>         /* Issue crypto request */
> -       if (odp_crypto_operation(&ctx->ipsec.params,
> -                                &posted,
> -                                odp_packet_to_buffer(pkt))) {
> +       if (odp_crypto_operation(&ctx->ipsec.params, &completion))
>                 abort();
> -       }
> -       return (posted) ? PKT_POSTED : PKT_CONTINUE;
> +       if (completion == ODP_BUFFER_INVALID)
> +               return PKT_POSTED;
> +
> +       *pkt = crypto_completion_to_packet(completion);
> +       if (*pkt != ODP_PACKET_INVALID)
> +               return PKT_CONTINUE;
> +
> +       return PKT_DROP;
>  }
>
>  /**
> @@ -997,18 +1022,8 @@ static
>  pkt_disposition_e do_ipsec_out_finish(odp_packet_t pkt,
>                                       pkt_ctx_t *ctx)
>  {
> -       odp_buffer_t event;
> -       odp_crypto_compl_status_t cipher_rc;
> -       odp_crypto_compl_status_t auth_rc;
>         odph_ipv4hdr_t *ip;
>
> -       /* Check crypto result */
> -       event = odp_packet_to_buffer(pkt);
> -       odp_crypto_get_operation_compl_status(event, &cipher_rc, &auth_rc);
> -       if (!is_crypto_compl_status_ok(&cipher_rc))
> -               return PKT_DROP;
> -       if (!is_crypto_compl_status_ok(&auth_rc))
> -               return PKT_DROP;
>         ip = (odph_ipv4hdr_t *)odp_packet_l3(pkt);
>
>         /* Finalize the IPv4 header */
> @@ -1059,7 +1074,13 @@ void *pktio_thread(void *arg EXAMPLE_UNUSED)
>
>                 /* Use schedule to get buf from any input queue */
>                 buf = SCHEDULE(&dispatchq, ODP_SCHED_WAIT);
> -               pkt = odp_packet_from_buffer(buf);
> +               if (completionq == dispatchq)
> +                       pkt = crypto_completion_to_packet(buf);
> +               else
> +                       pkt = odp_packet_from_buffer(buf);
> +
> +               if (pkt == ODP_PACKET_INVALID)
> +                       continue;
>
>                 /* Determine new work versus completion or sequence number
> */
>                 if ((completionq != dispatchq) && (seqnumq != dispatchq)) {
> @@ -1093,7 +1114,7 @@ void *pktio_thread(void *arg EXAMPLE_UNUSED)
>
>                         case PKT_STATE_IPSEC_IN_CLASSIFY:
>
> -                               rc = do_ipsec_in_classify(pkt, ctx, &skip);
> +                               rc = do_ipsec_in_classify(&pkt, ctx,
> &skip);
>                                 ctx->state = (skip) ?
>                                         PKT_STATE_ROUTE_LOOKUP :
>                                         PKT_STATE_IPSEC_IN_FINISH;
> @@ -1124,7 +1145,7 @@ void *pktio_thread(void *arg EXAMPLE_UNUSED)
>
>                         case PKT_STATE_IPSEC_OUT_SEQ:
>
> -                               rc = do_ipsec_out_seq(pkt, ctx);
> +                               rc = do_ipsec_out_seq(&pkt, ctx);
>                                 ctx->state = PKT_STATE_IPSEC_OUT_FINISH;
>                                 break;
>
> @@ -1150,7 +1171,6 @@ void *pktio_thread(void *arg EXAMPLE_UNUSED)
>                 if ((PKT_DROP == rc) || (PKT_DONE == rc))
>                         free_pkt_ctx(ctx);
>
> -
>                 /* Check for drop */
>                 if (PKT_DROP == rc)
>                         odph_packet_free(pkt);
> diff --git a/platform/linux-generic/include/api/odp_crypto.h
> b/platform/linux-generic/include/api/odp_crypto.h
> index 337e7cf..aac44d5 100644
> --- a/platform/linux-generic/include/api/odp_crypto.h
> +++ b/platform/linux-generic/include/api/odp_crypto.h
> @@ -225,29 +225,25 @@
> odp_crypto_session_create(odp_crypto_session_params_t *params,
>                           odp_crypto_session_t *session,
>                           enum odp_crypto_ses_create_err *status);
>
> -
>  /**
>   * Crypto per packet operation
>   *
>   * Performs the cryptographic operations specified during session creation
> - * on the packet.  If the operation is performed synchronously, "posted"
> - * will return FALSE and the result of the operation is immediately
> available
> - * in the completion event.  If "posted" returns TRUE the result will be
> - * delivered via the completion queue specified when the session was
> created.
> - *
> - * @todo Resolve if completion_event is necessary, can/should the output
> - *       packet buffer always be used instead.
> + * on the packet.  If the operation is performed synchronously, result
> will
> + * be returned in completion_event buffer immediately. In case of
> asynchronous
> + * operation completion event will return ODP_INVALID_BUFFER and the
> result
> + * will be delivered via the completion queue specified when the session
> was
> + * created.
>   *
> - * @param params            Operation parameters
> - * @param posted            Pointer to return posted, TRUE for async
> operation
> - * @param completion_event  Event by which the operation results are
> delivered.
> + * @param[in]  params            Operation parameters
> + * @param[out] completion_event  Pointer to completion event by which the
> + *                               synchronous operation results are
> delivered.
>   *
>   * @return 0 if successful else -1
>   */
>  int
>  odp_crypto_operation(odp_crypto_op_params_t *params,
> -                    bool *posted,
> -                    odp_buffer_t completion_event);
> +                    odp_buffer_t *completion_event);
>
>  /**
>   * Crypto per packet operation set user context in completion event
> @@ -297,6 +293,18 @@ void *
>  odp_crypto_get_operation_compl_ctx(odp_buffer_t completion_event);
>
>  /**
> + * Free completion event
> + *
> + * Completion event is an opaque buffer that is managed by implementation.
> + * The event can't be freed directly, because it can point to an output
> packet.
> + * This function should be used instead.
> + *
> + * @param completion_event  Event to be freed
> + */
> +void
> +odp_crypto_operation_compl_free(odp_buffer_t completion_event);
> +
> +/**
>   * Generate random byte string
>   *
>   * @param buf          Pointer to store result
> diff --git a/platform/linux-generic/odp_crypto.c
> b/platform/linux-generic/odp_crypto.c
> index d3cdec7..98cbdb7 100644
> --- a/platform/linux-generic/odp_crypto.c
> +++ b/platform/linux-generic/odp_crypto.c
> @@ -337,15 +337,14 @@
> odp_crypto_session_create(odp_crypto_session_params_t *params,
>
>  int
>  odp_crypto_operation(odp_crypto_op_params_t *params,
> -                    bool *posted,
> -                    odp_buffer_t completion_event)
> +                    odp_buffer_t *completion_event)
>  {
>         enum crypto_alg_err rc_cipher = ODP_CRYPTO_ALG_ERR_NONE;
>         enum crypto_alg_err rc_auth = ODP_CRYPTO_ALG_ERR_NONE;
>         odp_crypto_generic_session_t *session;
>         odp_crypto_generic_op_result_t *result;
> +       odp_buffer_t completion;
>
> -       *posted = 0;
>         session = (odp_crypto_generic_session_t
> *)(intptr_t)params->session;
>
>         /* Resolve output buffer */
> @@ -357,13 +356,12 @@ odp_crypto_operation(odp_crypto_op_params_t *params,
>                 if (odp_unlikely(ODP_PACKET_INVALID == params->out_pkt))
>                         abort();
>                 odp_packet_copy(params->out_pkt, params->pkt);
> -               if (completion_event == odp_packet_to_buffer(params->pkt))
> -                       completion_event =
> -                               odp_packet_to_buffer(params->out_pkt);
>                 odph_packet_free(params->pkt);
>                 params->pkt = ODP_PACKET_INVALID;
>         }
>
> +       completion = odp_packet_to_buffer(params->out_pkt);
> +
>         /* Invoke the functions */
>         if (session->do_cipher_first) {
>                 rc_cipher = session->cipher.func(params, session);
> @@ -374,7 +372,7 @@ odp_crypto_operation(odp_crypto_op_params_t *params,
>         }
>
>         /* Build Result (no HW so no errors) */
> -       result = get_op_result_from_buffer(completion_event);
> +       result = get_op_result_from_buffer(completion);
>         result->magic = OP_RESULT_MAGIC;
>         result->cipher.alg_err = rc_cipher;
>         result->cipher.hw_err = ODP_CRYPTO_HW_ERR_NONE;
> @@ -383,8 +381,10 @@ odp_crypto_operation(odp_crypto_op_params_t *params,
>
>         /* If specified during creation post event to completion queue */
>         if (ODP_QUEUE_INVALID != session->compl_queue) {
> -               odp_queue_enq(session->compl_queue, completion_event);
> -               *posted = 1;
> +               odp_queue_enq(session->compl_queue, completion);
> +               *completion_event = ODP_BUFFER_INVALID;
> +       } else {
> +               *completion_event = completion;
>         }
>         return 0;
>  }
> @@ -421,7 +421,6 @@ odp_hw_random_get(uint8_t *buf, size_t *len, bool
> use_entropy ODP_UNUSED)
>         rc = RAND_bytes(buf, *len);
>         return ((1 == rc) ? 0 : -1);
>  }
> -
>  void
>  odp_crypto_get_operation_compl_status(odp_buffer_t completion_event,
>                                       odp_crypto_compl_status_t *auth,
> @@ -454,8 +453,13 @@ void
>  }
>
>  odp_packet_t
> -odp_crypto_get_operation_compl_packet(odp_buffer_t completion_event
> ODP_UNUSED)
> +odp_crypto_get_operation_compl_packet(odp_buffer_t completion_event)
>  {
> -       ODP_UNIMPLEMENTED();
> -       return ODP_PACKET_INVALID;
> +       return odp_packet_from_buffer(completion_event);
> +}
> +
> +void
> +odp_crypto_operation_compl_free(odp_buffer_t completion_event ODP_UNUSED)
> +{
> +       /* Original packet is used as a completion, so nothing to free */
>  }
> --
> 1.7.9.5
>
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to