Re: [dpdk-dev] [PATCH v2 1/2] lib/crypto: add callback handlers for crypto

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi Vipin,

> 
> Add callback handlers for enqueue-dequeue operation on crypto
> device. The pre-enqueue and post-dequeue are selected on invoke
> user registered callback functions.
> 
> Use cases:
>  - allow user to investigate the contents pre-enqueue.
>  - allow user to investigate the contents post-dequeue.
>  - modify pre-enqueue and post-dequeue stage content.
>  - investigate PMD meta data.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> ---

[..]

> @@ -907,6 +926,19 @@ rte_cryptodev_dequeue_burst(uint8_t dev_id, uint16_t
> qp_id,
>  	nb_ops = (*dev->dequeue_burst)
>  			(dev->data->queue_pairs[qp_id], ops, nb_ops);
> 
> +#ifdef RTE_CRYPTODEV_ENQDEQ_CALLBACKS
> +	struct rte_cryptodev_enqdeq_callback *cb = NULL;
> +
> +	TAILQ_FOREACH(cb, &(dev->deq_cbs), next) {
> +		if (cb->cb_fn == NULL)
> +			continue;
> +
> +		cb->active = 1;
> +		nb_ops = cb->cb_fn(dev_id, qp_id, ops, nb_ops, cb->cb_arg);
> +		cb->active = 0;
> +	}

Shouldn't this cb->active be set atomically.

Will it be thread safe? There may be multiple threads enqueuing on the same device.
One may be executing while the other finished and set the active to 0, and may unregister
While the some other thread is still executing.

One more thing, it would be better to have a debug prints about how many nb_ops have been
Successfully passed through each of the callback.
And in the callback, it should be assumed that it will return back just after the first failed ops, so that
The application can free the remaining one. This should be documented in the callback API.

> +#endif
> +
>  	return nb_ops;
>  }
>