Hi Fan,
> Hi Akhil,
>
> > +__rte_internal
> > +static inline void *
> > +_rte_cryptodev_dequeue_prolog(uint8_t dev_id, uint8_t qp_id)
> > +{
> > + struct rte_cryptodev *dev = &rte_cryptodevs[dev_id];
> > +
> > + return dev->data->queue_pairs[qp_id];
> > +}
>
> [Fan: the function name looks unclear to me - maybe
> rte_cryptodev_dequeue_prepare?
The naming convention is same as Konstantin did for ethdev and
Subsequently by Pavan in eventdev. I think it is better to align all
With similar naming.
> Also the function didn't do any check as the description suggested - the
> qp is later checked in _RTE_CRYPTO_DEQ_DEF, maybe remove the
> description?]
>
Ok will update the description in next version
> > +_rte_cryptodev_dequeue_epilog(uint16_t dev_id, uint16_t qp_id,
> > + struct rte_crypto_op **ops, uint16_t nb_ops)
> > +{
> > +#ifdef RTE_CRYPTO_CALLBACKS
> > + struct rte_cryptodev *dev = &rte_cryptodevs[dev_id];
> > +
> > + if (unlikely(dev->deq_cbs != NULL)) {
> > + struct rte_cryptodev_cb_rcu *list;
> > + struct rte_cryptodev_cb *cb;
> > +
> > + /* __ATOMIC_RELEASE memory order was used when the
> > + * call back was inserted into the list.
> > + * Since there is a clear dependency between loading
> > + * cb and cb->fn/cb->next, __ATOMIC_ACQUIRE memory
> > order is
> > + * not required.
> > + */
> > + list = &dev->deq_cbs[qp_id];
> > + rte_rcu_qsbr_thread_online(list->qsbr, 0);
> > + cb = __atomic_load_n(&list->next, __ATOMIC_RELAXED);
> > +
> > + while (cb != NULL) {
> > + nb_ops = cb->fn(dev_id, qp_id, ops, nb_ops,
> > + cb->arg);
> > + cb = cb->next;
> > + };
> > +
> > + rte_rcu_qsbr_thread_offline(list->qsbr, 0);
> > + }
> > +#endif
> > +
> > + return nb_ops;
> > +}
>
> [Fan: same naming issue - maybe rte_cryptodev_dequeue_post, so
> as the enqueue part]
Same comment as above. Aligned with ethdev and eventdev changes.
>
> > +#define _RTE_CRYPTO_DEQ_FUNC(fn) _rte_crypto_deq_##fn
> > +
> > +/**
> > + * @internal
> > + * Helper macro to create new API wrappers for existing PMD dequeue
> > functions.
> > + */
> > +#define _RTE_CRYPTO_DEQ_PROTO(fn) \
> > + uint16_t _RTE_CRYPTO_DEQ_FUNC(fn)(uint8_t dev_id, uint8_t
> > qp_id, \
> > + struct rte_crypto_op **ops, uint16_t nb_ops)
> > +
> > +/**
> > + * @internal
> > + * Helper macro to create new API wrappers for existing PMD dequeue
> > functions.
> > + */
> > +#define _RTE_CRYPTO_DEQ_DEF(fn) \
> > +_RTE_CRYPTO_DEQ_PROTO(fn) \
> > +{ \
> > + void *qp = _rte_cryptodev_dequeue_prolog(dev_id, qp_id); \
> > + if (qp == NULL) \
> [Fan: suggest to add "unlikely" above]
Ok
