[v3,6/7] cryptodev: update fast path APIs to use new flat array
Checks
Commit Message
Rework fast-path cryptodev functions to use rte_crypto_fp_ops[].
While it is an API/ABI breakage, this change is intended to be
transparent for both users (no changes in user app is required) and
PMD developers (no changes in PMD is required).
Signed-off-by: Akhil Goyal <gakhil@marvell.com>
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
---
lib/cryptodev/rte_cryptodev.h | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
Comments
>
> Rework fast-path cryptodev functions to use rte_crypto_fp_ops[].
> While it is an API/ABI breakage, this change is intended to be
> transparent for both users (no changes in user app is required) and
> PMD developers (no changes in PMD is required).
>
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
> lib/cryptodev/rte_cryptodev.h | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index ce0dca72be..56e3868ada 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -1832,13 +1832,18 @@ static inline uint16_t
> rte_cryptodev_dequeue_burst(uint8_t dev_id, uint16_t qp_id,
> struct rte_crypto_op **ops, uint16_t nb_ops)
> {
> - struct rte_cryptodev *dev = &rte_cryptodevs[dev_id];
> + const struct rte_crypto_fp_ops *fp_ops;
> + void *qp;
>
> rte_cryptodev_trace_dequeue_burst(dev_id, qp_id, (void **)ops, nb_ops);
> - nb_ops = (*dev->dequeue_burst)
> - (dev->data->queue_pairs[qp_id], ops, nb_ops);
> +
> + fp_ops = &rte_crypto_fp_ops[dev_id];
> + qp = fp_ops->qp.data[qp_id];
> +
> + nb_ops = fp_ops->dequeue_burst(qp, ops, nb_ops);
> +
> #ifdef RTE_CRYPTO_CALLBACKS
> - if (unlikely(dev->deq_cbs != NULL)) {
> + if (unlikely(fp_ops->qp.deq_cb != NULL)) {
> struct rte_cryptodev_cb_rcu *list;
> struct rte_cryptodev_cb *cb;
As I ca see you decided to keep call-back related data-structs as public API.
I wonder that's to avoid extra changes with CB related code?
Or performance reasons?
Or probably something else?
>
> @@ -1848,7 +1853,7 @@ rte_cryptodev_dequeue_burst(uint8_t dev_id, uint16_t qp_id,
> * cb and cb->fn/cb->next, __ATOMIC_ACQUIRE memory order is
> * not required.
> */
> - list = &dev->deq_cbs[qp_id];
> + list = &fp_ops->qp.deq_cb[qp_id];
> rte_rcu_qsbr_thread_online(list->qsbr, 0);
> cb = __atomic_load_n(&list->next, __ATOMIC_RELAXED);
>
> @@ -1899,10 +1904,13 @@ static inline uint16_t
> rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
> struct rte_crypto_op **ops, uint16_t nb_ops)
> {
> - struct rte_cryptodev *dev = &rte_cryptodevs[dev_id];
> + const struct rte_crypto_fp_ops *fp_ops;
> + void *qp;
>
> + fp_ops = &rte_crypto_fp_ops[dev_id];
> + qp = fp_ops->qp.data[qp_id];
> #ifdef RTE_CRYPTO_CALLBACKS
> - if (unlikely(dev->enq_cbs != NULL)) {
> + if (unlikely(fp_ops->qp.enq_cb != NULL)) {
> struct rte_cryptodev_cb_rcu *list;
> struct rte_cryptodev_cb *cb;
>
> @@ -1912,7 +1920,7 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
> * cb and cb->fn/cb->next, __ATOMIC_ACQUIRE memory order is
> * not required.
> */
> - list = &dev->enq_cbs[qp_id];
> + list = &fp_ops->qp.enq_cb[qp_id];
> rte_rcu_qsbr_thread_online(list->qsbr, 0);
> cb = __atomic_load_n(&list->next, __ATOMIC_RELAXED);
>
> @@ -1927,8 +1935,7 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
> #endif
>
> rte_cryptodev_trace_enqueue_burst(dev_id, qp_id, (void **)ops, nb_ops);
> - return (*dev->enqueue_burst)(
> - dev->data->queue_pairs[qp_id], ops, nb_ops);
> + return fp_ops->enqueue_burst(qp, ops, nb_ops);
> }
>
>
> --
> 2.25.1
> > @@ -1832,13 +1832,18 @@ static inline uint16_t
> > rte_cryptodev_dequeue_burst(uint8_t dev_id, uint16_t qp_id,
> > struct rte_crypto_op **ops, uint16_t nb_ops)
> > {
> > - struct rte_cryptodev *dev = &rte_cryptodevs[dev_id];
> > + const struct rte_crypto_fp_ops *fp_ops;
> > + void *qp;
> >
> > rte_cryptodev_trace_dequeue_burst(dev_id, qp_id, (void **)ops,
> nb_ops);
> > - nb_ops = (*dev->dequeue_burst)
> > - (dev->data->queue_pairs[qp_id], ops, nb_ops);
> > +
> > + fp_ops = &rte_crypto_fp_ops[dev_id];
> > + qp = fp_ops->qp.data[qp_id];
> > +
> > + nb_ops = fp_ops->dequeue_burst(qp, ops, nb_ops);
> > +
> > #ifdef RTE_CRYPTO_CALLBACKS
> > - if (unlikely(dev->deq_cbs != NULL)) {
> > + if (unlikely(fp_ops->qp.deq_cb != NULL)) {
> > struct rte_cryptodev_cb_rcu *list;
> > struct rte_cryptodev_cb *cb;
>
> As I ca see you decided to keep call-back related data-structs as public API.
> I wonder that's to avoid extra changes with CB related code?
> Or performance reasons?
> Or probably something else?
I just wanted to avoid extra changes and it did not look that important at this point
Compared to other patches.
I would have done the changes if I had some more time.
> > > @@ -1832,13 +1832,18 @@ static inline uint16_t
> > > rte_cryptodev_dequeue_burst(uint8_t dev_id, uint16_t qp_id,
> > > struct rte_crypto_op **ops, uint16_t nb_ops)
> > > {
> > > - struct rte_cryptodev *dev = &rte_cryptodevs[dev_id];
> > > + const struct rte_crypto_fp_ops *fp_ops;
> > > + void *qp;
> > >
> > > rte_cryptodev_trace_dequeue_burst(dev_id, qp_id, (void **)ops,
> > nb_ops);
> > > - nb_ops = (*dev->dequeue_burst)
> > > - (dev->data->queue_pairs[qp_id], ops, nb_ops);
> > > +
> > > + fp_ops = &rte_crypto_fp_ops[dev_id];
> > > + qp = fp_ops->qp.data[qp_id];
> > > +
> > > + nb_ops = fp_ops->dequeue_burst(qp, ops, nb_ops);
> > > +
> > > #ifdef RTE_CRYPTO_CALLBACKS
> > > - if (unlikely(dev->deq_cbs != NULL)) {
> > > + if (unlikely(fp_ops->qp.deq_cb != NULL)) {
> > > struct rte_cryptodev_cb_rcu *list;
> > > struct rte_cryptodev_cb *cb;
> >
> > As I ca see you decided to keep call-back related data-structs as public API.
> > I wonder that's to avoid extra changes with CB related code?
> > Or performance reasons?
> > Or probably something else?
> I just wanted to avoid extra changes and it did not look that important at this point
> Compared to other patches.
> I would have done the changes if I had some more time.
Understood, thanks for explanation.
@@ -1832,13 +1832,18 @@ static inline uint16_t
rte_cryptodev_dequeue_burst(uint8_t dev_id, uint16_t qp_id,
struct rte_crypto_op **ops, uint16_t nb_ops)
{
- struct rte_cryptodev *dev = &rte_cryptodevs[dev_id];
+ const struct rte_crypto_fp_ops *fp_ops;
+ void *qp;
rte_cryptodev_trace_dequeue_burst(dev_id, qp_id, (void **)ops, nb_ops);
- nb_ops = (*dev->dequeue_burst)
- (dev->data->queue_pairs[qp_id], ops, nb_ops);
+
+ fp_ops = &rte_crypto_fp_ops[dev_id];
+ qp = fp_ops->qp.data[qp_id];
+
+ nb_ops = fp_ops->dequeue_burst(qp, ops, nb_ops);
+
#ifdef RTE_CRYPTO_CALLBACKS
- if (unlikely(dev->deq_cbs != NULL)) {
+ if (unlikely(fp_ops->qp.deq_cb != NULL)) {
struct rte_cryptodev_cb_rcu *list;
struct rte_cryptodev_cb *cb;
@@ -1848,7 +1853,7 @@ rte_cryptodev_dequeue_burst(uint8_t dev_id, uint16_t qp_id,
* cb and cb->fn/cb->next, __ATOMIC_ACQUIRE memory order is
* not required.
*/
- list = &dev->deq_cbs[qp_id];
+ list = &fp_ops->qp.deq_cb[qp_id];
rte_rcu_qsbr_thread_online(list->qsbr, 0);
cb = __atomic_load_n(&list->next, __ATOMIC_RELAXED);
@@ -1899,10 +1904,13 @@ static inline uint16_t
rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
struct rte_crypto_op **ops, uint16_t nb_ops)
{
- struct rte_cryptodev *dev = &rte_cryptodevs[dev_id];
+ const struct rte_crypto_fp_ops *fp_ops;
+ void *qp;
+ fp_ops = &rte_crypto_fp_ops[dev_id];
+ qp = fp_ops->qp.data[qp_id];
#ifdef RTE_CRYPTO_CALLBACKS
- if (unlikely(dev->enq_cbs != NULL)) {
+ if (unlikely(fp_ops->qp.enq_cb != NULL)) {
struct rte_cryptodev_cb_rcu *list;
struct rte_cryptodev_cb *cb;
@@ -1912,7 +1920,7 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
* cb and cb->fn/cb->next, __ATOMIC_ACQUIRE memory order is
* not required.
*/
- list = &dev->enq_cbs[qp_id];
+ list = &fp_ops->qp.enq_cb[qp_id];
rte_rcu_qsbr_thread_online(list->qsbr, 0);
cb = __atomic_load_n(&list->next, __ATOMIC_RELAXED);
@@ -1927,8 +1935,7 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
#endif
rte_cryptodev_trace_enqueue_burst(dev_id, qp_id, (void **)ops, nb_ops);
- return (*dev->enqueue_burst)(
- dev->data->queue_pairs[qp_id], ops, nb_ops);
+ return fp_ops->enqueue_burst(qp, ops, nb_ops);
}