| Message ID | 1417783375-9704-1-git-send-email-taras.kondratiuk@linaro.org |
|---|---|
| State | New |
| Headers | show |
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 <taras.kondratiuk@linaro.org>
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 <taras.kondratiuk@linaro.org>
> ---
> 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(¶ms, 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(¶ms,
> - &posted,
> - odp_packet_to_buffer(pkt))) {
> + if (odp_crypto_operation(¶ms, &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
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Hi, In case of synchronous crypto operation the completion event is need only in case of an error hence the application can check the return code for failure and get info from completion event buffer only during that scenario. This will greatly optimize the performance in synchronous crypto operations. Regards, Bala On 5 December 2014 at 19:46, Alexandru Badicioiu < alexandru.badicioiu@linaro.org> wrote: > 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 <taras.kondratiuk@linaro.org > > 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 <taras.kondratiuk@linaro.org> >> --- >> 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(¶ms, 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(¶ms, >> - &posted, >> - odp_packet_to_buffer(pkt))) { >> + if (odp_crypto_operation(¶ms, &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 >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
On 12/05/2014 04:16 PM, Alexandru Badicioiu wrote: > 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); > } This approach really looks like a hack and it has several issues: 1. Application have to remember to free completion in case it was allocated. In asynchronous case completion event may be dequeued in another thread, so 'need to be freed' information should be passed somehow along with the completion event. Implementation is in much better position to handle this case. 2. Implicitly passing packet as a completion event may mislead application developer, because he may assume that he will get it back in exactly the same state (that's exactly what happened in our IPsec application). But accordingly to the specification completion event is an opaque token, so developer should not assume that its state is preserved. Instead we have accessors functions to get information from the completion event.
On 12/05/2014 04:38 PM, Bala Manoharan wrote: > Hi, > > In case of synchronous crypto operation the completion event is need > only in case of an error hence the application can check the return code > for failure and get info from completion event buffer only during that > scenario. This will greatly optimize the performance in synchronous > crypto operations. This RFC aims to eliminate completion event handling by application, so implementation (if capable) can optimize it and use output packet instead. Both sync and async implementation will benefit in the same way. What you are asking is a separate optimization for sync mode which also can be achieved on top of this RFC. We can add a note to odp_crypto_operation() description: "if the function succeeded and it was synchronous an output packet can be queried without status check". ----------------- implementation --------------------------- int odp_crypto_operation(odp_crypto_op_params_t *params, odp_buffer_t *completion_event) { ... process_packet(params->in_pkt, params->out_pkt); *completion_event = odp_packet_to_buffer(params->out_pkt) ... return 0; } odp_packet_t odp_crypto_get_operation_compl_packet(odp_buffer_t completion_event) { return odp_packet_from_buffer(completion_event); } void odp_crypto_operation_compl_free(odp_buffer_t completion_event) { } ----------------- application (odp_ipsec.c)---------------------------- /* Issue crypto request */ if (odp_crypto_operation(¶ms, &completion)) { /* Call odp_crypto_get_operation_status(completion) if details are needed */ return PKT_DROP; } if (completion != ODP_BUFFER_INVALID) { *pkt = odp_crypto_get_operation_compl_packet(completion); return (*pkt != ODP_PACKET_INVALID) ? PKT_CONTINUE : PKT_DROP: } else { return PKT_POSTED; If buffer<->packet conversion is just casting, then overhead is zero. Will it work for you?
Some comments inline. Alex On 5 December 2014 at 17:45, Taras Kondratiuk <taras.kondratiuk@linaro.org> wrote: > On 12/05/2014 04:16 PM, Alexandru Badicioiu wrote: > >> 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); >> } >> > > This approach really looks like a hack and it has several issues: > 1. Application have to remember to free completion in case it was > allocated. [Alex] This is normal, right? If the application allocated something, shouldn't have the responsibility to free it? I think it is the case of your patch when the application has to remember to free something which it did not allocate. You have proposed completion_event_free API but there is no alloc call which is unbalanced. The call is in fact no different than normal buffer free and for implementations which reuse the packet it will be empty, as it is in the patch. > In asynchronous case completion event may be dequeued > in another thread, so 'need to be freed' information should be > passed somehow along with the completion event. Implementation is in > much better position to handle this case. > [Alex] The operation context is meant to "pass something along with the completion event", in case the application really needs this. However, the code example was only to show that the application has a mean to handle this packet vs new completion event issue. I think the API should be changed only if there's no other acceptable way of doing something. The cleanest approach is probably to introduce a capability call and the application can check if it can pass the input packet as the completion event. > 2. Implicitly passing packet as a completion event may mislead > application developer, because he may assume that he will get it back > in exactly the same state (that's exactly what happened in our IPsec > application). [Alex] The underlying buffer is passed, not the packet as such. Implementation knows where the packet and its metadata are in the packet and if no INVALID_COMPLETION_EVENT error is returned it should be guaranteed that the packet "state" is preserved. As I remember from the design, the completion event is just a buffer where the crypto operation returns the completion information. The only "opaque" thing is how the crypto uses the buffer and how it lays out the output information. > But accordingly to the specification completion event > is an opaque token, so developer should not assume that its state is > preserved. Instead we have accessors functions to get information > from the completion event. >
On 12/08/2014 10:58 AM, Alexandru Badicioiu wrote: > Some comments inline. > > Alex > > On 5 December 2014 at 17:45, Taras Kondratiuk > <taras.kondratiuk@linaro.org <mailto:taras.kondratiuk@linaro.org>> wrote: > > On 12/05/2014 04:16 PM, Alexandru Badicioiu wrote: > > 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); > } > > > This approach really looks like a hack and it has several issues: > > > 1. Application have to remember to free completion in case it was > allocated. > > [Alex] This is normal, right? If the application allocated something, > shouldn't have the responsibility to free it? Sure freeing previously allocated buffer is normal, but *allocating* it in a first place is *not necessary*. So no need to free anything. > I think it is the case of your patch when the application has to > remember to free something which it did not allocate. > You have proposed completion_event_free API but there is no alloc call > which is unbalanced. The call is in fact no different > than normal buffer free and for implementations which reuse the packet > it will be empty, as it is in the patch. No. Difference is that application have to 'free' it *always*. No need to determine action per event. Regarding API balancing. It is the same use-case as ingress packets processing: application can free them, but they are not allocated by application. > > In asynchronous case completion event may be dequeued > in another thread, so 'need to be freed' information should be > passed somehow along with the completion event. Implementation is in > much better position to handle this case. > > [Alex] The operation context is meant to "pass something along with the > completion event", > in case the application really needs this. However, the code example was > only to show > that the application has a mean to handle this packet vs new completion > event issue. > I think the API should be changed only if there's no other acceptable > way of doing something. > The cleanest approach is probably to introduce a capability call and the > application can check if > it can pass the input packet as the completion event. Right, but IMO the approach you have proposed is not acceptable. It doesn't make sense to freeze the API if small API changes can optimize application and implementation sides. Especially at these early ODP versions. Unfortunately capability call won't solve the issue. Current crypto API design was build on assumption that implementation can chose to process a packet differently depending on its characteristics (size, cypher algorithm, etc.). Depending on a type of processing chosen a completion event may or may not be needed. > > > 2. Implicitly passing packet as a completion event may mislead > application developer, because he may assume that he will get it > back > in exactly the same state (that's exactly what happened in our IPsec > application). > > [Alex] The underlying buffer is passed, not the packet as such. > Implementation > knows where the packet and its metadata are in the packet and if no > INVALID_COMPLETION_EVENT error is returned it should be guaranteed > that the packet "state" is preserved. As I remember from the design, > the completion event is just a buffer where the crypto operation > returns the completion information. The only "opaque" > thing is how the crypto uses the buffer and how it lays out the output > information. Preserving an original completion buffer content is a wrong assumption. Current specification doesn't say anything about it. Robby initially used this as a workaround to avoid completion event allocation on application side. That's exactly what this RFC is addressing. > > But accordingly to the specification completion event > is an opaque token, so developer should not assume that its state is > preserved. Instead we have accessors functions to get information > from the completion event. > >
On Friday 05 December 2014 10:16 PM, Taras Kondratiuk wrote: > On 12/05/2014 04:38 PM, Bala Manoharan wrote: >> Hi, >> >> In case of synchronous crypto operation the completion event is need >> only in case of an error hence the application can check the return code >> for failure and get info from completion event buffer only during that >> scenario. This will greatly optimize the performance in synchronous >> crypto operations. > This RFC aims to eliminate completion event handling by application, so > implementation (if capable) can optimize it and use output packet > instead. Both sync and async implementation will benefit in the same > way. > > What you are asking is a separate optimization for sync mode which also > can be achieved on top of this RFC. We can add a note to > odp_crypto_operation() description: "if the function succeeded and it > was synchronous an output packet can be queried without status check". > > > ----------------- implementation --------------------------- > int > odp_crypto_operation(odp_crypto_op_params_t *params, > odp_buffer_t *completion_event) > { > ... > process_packet(params->in_pkt, params->out_pkt); > *completion_event = odp_packet_to_buffer(params->out_pkt) > ... > return 0; > } > > odp_packet_t > odp_crypto_get_operation_compl_packet(odp_buffer_t completion_event) > { > return odp_packet_from_buffer(completion_event); > } > > void > odp_crypto_operation_compl_free(odp_buffer_t completion_event) > { > } > > ----------------- application (odp_ipsec.c)---------------------------- > > /* Issue crypto request */ > if (odp_crypto_operation(¶ms, &completion)) { > /* Call odp_crypto_get_operation_status(completion) if details are needed */ > return PKT_DROP; > } > > if (completion != ODP_BUFFER_INVALID) { > *pkt = odp_crypto_get_operation_compl_packet(completion); > return (*pkt != ODP_PACKET_INVALID) ? PKT_CONTINUE : PKT_DROP: > } else { > return PKT_POSTED; > > If buffer<->packet conversion is just casting, then overhead is zero. > Will it work for you? In Sync crypto operation the output pkt is specified as part of the odp_crypto_op_params_t by the original design. Hence it will not required to get the packet from completion event. The completion event needs to be checked only during failure and during success the same can be ignored by the application. Regards, Bala
On 12/08/2014 01:30 PM, Bala wrote: > > On Friday 05 December 2014 10:16 PM, Taras Kondratiuk wrote: >> On 12/05/2014 04:38 PM, Bala Manoharan wrote: >>> Hi, >>> >>> In case of synchronous crypto operation the completion event is need >>> only in case of an error hence the application can check the return code >>> for failure and get info from completion event buffer only during that >>> scenario. This will greatly optimize the performance in synchronous >>> crypto operations. >> This RFC aims to eliminate completion event handling by application, so >> implementation (if capable) can optimize it and use output packet >> instead. Both sync and async implementation will benefit in the same >> way. >> >> What you are asking is a separate optimization for sync mode which also >> can be achieved on top of this RFC. We can add a note to >> odp_crypto_operation() description: "if the function succeeded and it >> was synchronous an output packet can be queried without status check". >> >> >> ----------------- implementation --------------------------- >> int >> odp_crypto_operation(odp_crypto_op_params_t *params, >> odp_buffer_t *completion_event) >> { >> ... >> process_packet(params->in_pkt, params->out_pkt); >> *completion_event = odp_packet_to_buffer(params->out_pkt) >> ... >> return 0; >> } >> >> odp_packet_t >> odp_crypto_get_operation_compl_packet(odp_buffer_t completion_event) >> { >> return odp_packet_from_buffer(completion_event); >> } >> >> void >> odp_crypto_operation_compl_free(odp_buffer_t completion_event) >> { >> } >> >> ----------------- application (odp_ipsec.c)---------------------------- >> >> /* Issue crypto request */ >> if (odp_crypto_operation(¶ms, &completion)) { >> /* Call odp_crypto_get_operation_status(completion) if details >> are needed */ >> return PKT_DROP; >> } >> >> if (completion != ODP_BUFFER_INVALID) { >> *pkt = odp_crypto_get_operation_compl_packet(completion); >> return (*pkt != ODP_PACKET_INVALID) ? PKT_CONTINUE : >> PKT_DROP: >> } else { >> return PKT_POSTED; >> >> If buffer<->packet conversion is just casting, then overhead is zero. >> Will it work for you? > In Sync crypto operation the output pkt is specified as part of the > odp_crypto_op_params_t by the original design. > Hence it will not required to get the packet from completion event. The > completion event needs to be checked > only during failure and during success the same can be ignored by the > application. Actually sync/async mode doesn't change a way how output packet is handled from application point of view. There are two options: 1. Packet is allocated by application and specified in out_pkt. If operation is finished synchronously and successfully, then output packet is present in out_pkt. 2. out_pkt is set to ODP_PACKET_INVALID, so implementation allocates it. In this case output packet have to be obtained as in the code above. So in general, will it help you to have next to zero overhead?
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(¶ms, 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(¶ms, - &posted, - odp_packet_to_buffer(pkt))) { + if (odp_crypto_operation(¶ms, &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 */ }
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 <taras.kondratiuk@linaro.org> --- 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(-)