[v1] crypto: fix build issues on crypto callbacks macro undefined
Checks
Commit Message
Crypto callbacks macro is defined with value 1 and being used with ifdef,
on config value is changed to 0 to disable, crypto callback changes
still being compiled.
Defined crypto callbacks macro without value, undef to disable
Wrapped crypto callback changes with RTE_CRYPTO_CALLBACKS macro
to fix build issues when macro is undefined.
As callback head nodes have valid pointer, this patch checks the next
node from the head if callbacks registered.
Fixes: 1c3ffb9 ("cryptodev: add enqueue and dequeue callbacks")
Fixes: 5523a75 ("test/crypto: add case for enqueue/dequeue callbacks")
Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
Comments
> -----Original Message-----
> From: Kundapura, Ganapati <ganapati.kundapura@intel.com>
> Sent: Tuesday, April 16, 2024 1:42 PM
> To: dev@dpdk.org
> Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Power, Ciara
> <ciara.power@intel.com>; gakhil@marvell.com; fanzhang.oss@gmail.com
> Subject: [PATCH v1] crypto: fix build issues on crypto callbacks macro undefined
>
> Crypto callbacks macro is defined with value 1 and being used with ifdef, on
> config value is changed to 0 to disable, crypto callback changes still being
> compiled.
>
> Defined crypto callbacks macro without value, undef to disable
>
> Wrapped crypto callback changes with RTE_CRYPTO_CALLBACKS macro to fix
> build issues when macro is undefined.
>
> As callback head nodes have valid pointer, this patch checks the next node from
> the head if callbacks registered.
>
> Fixes: 1c3ffb9 ("cryptodev: add enqueue and dequeue callbacks")
> Fixes: 5523a75 ("test/crypto: add case for enqueue/dequeue callbacks")
>
> Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
>
Acked-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c index
> 1703ebccf1..1a592f2302 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -14547,6 +14547,7 @@ test_null_burst_operation(void)
> return TEST_SUCCESS;
> }
>
> +#ifdef RTE_CRYPTO_CALLBACKS
> static uint16_t
> test_enq_callback(uint16_t dev_id, uint16_t qp_id, struct rte_crypto_op **ops,
> uint16_t nb_ops, void *user_param)
> @@ -14784,6 +14785,7 @@ test_deq_callback_setup(void)
>
> return TEST_SUCCESS;
> }
> +#endif /* RTE_CRYPTO_CALLBACKS */
>
> static void
> generate_gmac_large_plaintext(uint8_t *data) @@ -18069,8 +18071,10 @@
> static struct unit_test_suite cryptodev_gen_testsuite = {
> TEST_CASE_ST(ut_setup, ut_teardown,
>
> test_device_configure_invalid_queue_pair_ids),
> TEST_CASE_ST(ut_setup, ut_teardown, test_stats),
> +#ifdef RTE_CRYPTO_CALLBACKS
> TEST_CASE_ST(ut_setup, ut_teardown,
> test_enq_callback_setup),
> TEST_CASE_ST(ut_setup, ut_teardown,
> test_deq_callback_setup),
> +#endif
> TEST_CASES_END() /**< NULL terminate unit test array */
> }
> };
> diff --git a/config/rte_config.h b/config/rte_config.h index
> dd7bb0d35b..b647a69ba8 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -72,7 +72,7 @@
> /* cryptodev defines */
> #define RTE_CRYPTO_MAX_DEVS 64
> #define RTE_CRYPTODEV_NAME_LEN 64
> -#define RTE_CRYPTO_CALLBACKS 1
> +#define RTE_CRYPTO_CALLBACKS /* No Value, undef/comment out to
> disable */
>
> /* compressdev defines */
> #define RTE_COMPRESS_MAX_DEVS 64
> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c index
> 886eb7adc4..5f3b17a517 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -628,6 +628,7 @@ rte_cryptodev_asym_xform_capability_check_hash(
> return ret;
> }
>
> +#ifdef RTE_CRYPTO_CALLBACKS
> /* spinlock for crypto device enq callbacks */ static rte_spinlock_t
> rte_cryptodev_callback_lock = RTE_SPINLOCK_INITIALIZER;
>
> @@ -744,6 +745,7 @@ cryptodev_cb_init(struct rte_cryptodev *dev)
> cryptodev_cb_cleanup(dev);
> return -ENOMEM;
> }
> +#endif /* RTE_CRYPTO_CALLBACKS */
>
> const char *
> rte_cryptodev_get_feature_name(uint64_t flag) @@ -1244,9 +1246,11 @@
> rte_cryptodev_configure(uint8_t dev_id, struct rte_cryptodev_config *config)
> if (*dev->dev_ops->dev_configure == NULL)
> return -ENOTSUP;
>
> +#ifdef RTE_CRYPTO_CALLBACKS
> rte_spinlock_lock(&rte_cryptodev_callback_lock);
> cryptodev_cb_cleanup(dev);
> rte_spinlock_unlock(&rte_cryptodev_callback_lock);
> +#endif
>
> /* Setup new number of queue pairs and reconfigure device. */
> diag = rte_cryptodev_queue_pairs_config(dev, config->nb_queue_pairs,
> @@ -1257,6 +1261,7 @@ rte_cryptodev_configure(uint8_t dev_id, struct
> rte_cryptodev_config *config)
> return diag;
> }
>
> +#ifdef RTE_CRYPTO_CALLBACKS
> rte_spinlock_lock(&rte_cryptodev_callback_lock);
> diag = cryptodev_cb_init(dev);
> rte_spinlock_unlock(&rte_cryptodev_callback_lock);
> @@ -1264,6 +1269,7 @@ rte_cryptodev_configure(uint8_t dev_id, struct
> rte_cryptodev_config *config)
> CDEV_LOG_ERR("Callback init failed for dev_id=%d", dev_id);
> return diag;
> }
> +#endif
>
> rte_cryptodev_trace_configure(dev_id, config);
> return (*dev->dev_ops->dev_configure)(dev, config); @@ -1485,6
> +1491,7 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t
> queue_pair_id,
> socket_id);
> }
>
> +#ifdef RTE_CRYPTO_CALLBACKS
> struct rte_cryptodev_cb *
> rte_cryptodev_add_enq_callback(uint8_t dev_id,
> uint16_t qp_id,
> @@ -1763,6 +1770,7 @@ rte_cryptodev_remove_deq_callback(uint8_t
> dev_id,
> rte_spinlock_unlock(&rte_cryptodev_callback_lock);
> return ret;
> }
> +#endif /* RTE_CRYPTO_CALLBACKS */
>
> int
> rte_cryptodev_stats_get(uint8_t dev_id, struct rte_cryptodev_stats *stats) diff
> --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h index
> 00ba6a234a..b811b458d5 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -1910,7 +1910,7 @@ rte_cryptodev_dequeue_burst(uint8_t dev_id,
> uint16_t qp_id,
> nb_ops = fp_ops->dequeue_burst(qp, ops, nb_ops);
>
> #ifdef RTE_CRYPTO_CALLBACKS
> - if (unlikely(fp_ops->qp.deq_cb != NULL)) {
> + if (unlikely(fp_ops->qp.deq_cb[qp_id].next != NULL)) {
> struct rte_cryptodev_cb_rcu *list;
> struct rte_cryptodev_cb *cb;
>
> @@ -1977,7 +1977,7 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id,
> uint16_t qp_id,
> fp_ops = &rte_crypto_fp_ops[dev_id];
> qp = fp_ops->qp.data[qp_id];
> #ifdef RTE_CRYPTO_CALLBACKS
> - if (unlikely(fp_ops->qp.enq_cb != NULL)) {
> + if (unlikely(fp_ops->qp.enq_cb[qp_id].next != NULL)) {
> struct rte_cryptodev_cb_rcu *list;
> struct rte_cryptodev_cb *cb;
>
> --
> 2.23.0
> -----Original Message-----
> From: Kundapura, Ganapati <ganapati.kundapura@intel.com>
> Sent: Tuesday, April 16, 2024 9:12 AM
> To: dev@dpdk.org
> Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Power, Ciara
> <ciara.power@intel.com>; gakhil@marvell.com; fanzhang.oss@gmail.com
> Subject: [PATCH v1] crypto: fix build issues on crypto callbacks macro undefined
>
> Crypto callbacks macro is defined with value 1 and being used with ifdef, on
> config value is changed to 0 to disable, crypto callback changes still being
> compiled.
>
> Defined crypto callbacks macro without value, undef to disable
>
> Wrapped crypto callback changes with RTE_CRYPTO_CALLBACKS macro to fix
> build issues when macro is undefined.
>
> As callback head nodes have valid pointer, this patch checks the next node from
> the head if callbacks registered.
>
> Fixes: 1c3ffb9 ("cryptodev: add enqueue and dequeue callbacks")
> Fixes: 5523a75 ("test/crypto: add case for enqueue/dequeue callbacks")
>
> Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
>
Acked-by: Ciara Power <ciara.power@intel.com>
Hi Akhil,
2 acked for this patch but it's still not yet accepted.
Anything more required for this patch?
Thanks,
Ganapati
> -----Original Message-----
> From: Power, Ciara <ciara.power@intel.com>
> Sent: Wednesday, April 17, 2024 5:11 PM
> To: Kundapura, Ganapati <ganapati.kundapura@intel.com>; dev@dpdk.org
> Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>;
> gakhil@marvell.com; fanzhang.oss@gmail.com
> Subject: RE: [PATCH v1] crypto: fix build issues on crypto callbacks macro
> undefined
>
>
>
> > -----Original Message-----
> > From: Kundapura, Ganapati <ganapati.kundapura@intel.com>
> > Sent: Tuesday, April 16, 2024 9:12 AM
> > To: dev@dpdk.org
> > Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Power, Ciara
> > <ciara.power@intel.com>; gakhil@marvell.com; fanzhang.oss@gmail.com
> > Subject: [PATCH v1] crypto: fix build issues on crypto callbacks macro
> > undefined
> >
> > Crypto callbacks macro is defined with value 1 and being used with
> > ifdef, on config value is changed to 0 to disable, crypto callback
> > changes still being compiled.
> >
> > Defined crypto callbacks macro without value, undef to disable
> >
> > Wrapped crypto callback changes with RTE_CRYPTO_CALLBACKS macro to fix
> > build issues when macro is undefined.
> >
> > As callback head nodes have valid pointer, this patch checks the next
> > node from the head if callbacks registered.
> >
> > Fixes: 1c3ffb9 ("cryptodev: add enqueue and dequeue callbacks")
> > Fixes: 5523a75 ("test/crypto: add case for enqueue/dequeue callbacks")
> >
> > Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> >
>
> Acked-by: Ciara Power <ciara.power@intel.com>
> Subject: [EXTERNAL] RE: [PATCH v1] crypto: fix build issues on crypto callbacks
> macro undefined
>
> Hi Akhil,
> 2 acked for this patch but it's still not yet accepted.
> Anything more required for this patch?
>
Hi Ganapati,
I haven't started picking patches on crypto tree yet. Will start from next week.
However, for this patch, I see some other issues as well in crypto callbacks test in dpdk-test.
Will comment by tomorrow.
Regards,
Akhil
> Subject: RE: [PATCH v1] crypto: fix build issues on crypto callbacks macro
> undefined
>
> > Subject: [EXTERNAL] RE: [PATCH v1] crypto: fix build issues on crypto callbacks
> > macro undefined
> >
> > Hi Akhil,
> > 2 acked for this patch but it's still not yet accepted.
> > Anything more required for this patch?
> >
> Hi Ganapati,
>
> I haven't started picking patches on crypto tree yet. Will start from next week.
> However, for this patch, I see some other issues as well in crypto callbacks test in
> dpdk-test.
> Will comment by tomorrow.
>
Please check this patch also. This patch will fix the runtime issues for the case.
https://patches.dpdk.org/project/dpdk/patch/20240522134835.3453044-1-gakhil@marvell.com/
Hi Akhil,
My patch addresses build issue on setting RTE_CRYPTO_CALLBACKS to 0 and
Checks for callback registered or not from the next node instead of head node as callback head node is always valid pointer.
Thanks,
Ganapati
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Wednesday, May 22, 2024 7:21 PM
> To: Kundapura, Ganapati <ganapati.kundapura@intel.com>; dev@dpdk.org
> Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>;
> fanzhang.oss@gmail.com; 'Power, Ciara' <ciara.power@intel.com>
> Subject: RE: [PATCH v1] crypto: fix build issues on crypto callbacks macro
> undefined
>
> > Subject: RE: [PATCH v1] crypto: fix build issues on crypto callbacks
> > macro undefined
> >
> > > Subject: [EXTERNAL] RE: [PATCH v1] crypto: fix build issues on
> > > crypto callbacks macro undefined
> > >
> > > Hi Akhil,
> > > 2 acked for this patch but it's still not yet accepted.
> > > Anything more required for this patch?
> > >
> > Hi Ganapati,
> >
> > I haven't started picking patches on crypto tree yet. Will start from next
> week.
> > However, for this patch, I see some other issues as well in crypto
> > callbacks test in dpdk-test.
> > Will comment by tomorrow.
> >
> Please check this patch also. This patch will fix the runtime issues for the case.
> https://patches.dpdk.org/project/dpdk/patch/20240522134835.3453044-
> 1-gakhil@marvell.com/
> Hi Akhil,
> My patch addresses build issue on setting RTE_CRYPTO_CALLBACKS to 0 and
> Checks for callback registered or not from the next node instead of head node as
> callback head node is always valid pointer.
>
Agreed but your patch cannot be verified without the fix as the callbacks are not getting called if using PMD other than NULL.
Hi Akhil
Agreed that callbacks are not getting called if not NULL PMD as it is designed to be run for NULL PMD.
Tested your patch with qat pmd and callbacks are getting called. Both the patches can go in.
+ ------------------------------------------------------- +
+ Test Suite : Crypto General Unit Test Suite
+ ------------------------------------------------------- +
CRYPTODEV: rte_cryptodev_configure() line 1150: Invalid dev_id=4
CRYPTODEV: rte_cryptodev_configure() line 1150: Invalid dev_id=255
CRYPTODEV: rte_cryptodev_stop() line 1247: Device with dev_id=0 already stopped
+ TestCase [ 0] : test_device_configure_invalid_dev_id succeeded
CRYPTODEV: rte_cryptodev_queue_pair_setup() line 1370: Invalid queue_pair_id=1
CRYPTODEV: rte_cryptodev_queue_pair_setup() line 1370: Invalid queue_pair_id=65535
CRYPTODEV: rte_cryptodev_stop() line 1247: Device with dev_id=0 already stopped
+ TestCase [ 1] : test_queue_pair_descriptor_setup succeeded
CRYPTODEV: rte_cryptodev_queue_pairs_config() line 1088: invalid param: dev 0x7fb24d547940, nb_queues 0
CRYPTODEV: rte_cryptodev_configure() line 1175: dev0 rte_crypto_dev_queue_pairs_config = -22
CRYPTODEV: rte_cryptodev_queue_pairs_config() line 1103: Invalid num queue_pairs (65535) for dev 0
CRYPTODEV: rte_cryptodev_configure() line 1175: dev0 rte_crypto_dev_queue_pairs_config = -22
CRYPTODEV: rte_cryptodev_queue_pairs_config() line 1103: Invalid num queue_pairs (2) for dev 0
CRYPTODEV: rte_cryptodev_configure() line 1175: dev0 rte_crypto_dev_queue_pairs_config = -22
CRYPTODEV: rte_cryptodev_stop() line 1247: Device with dev_id=0 already stopped
+ TestCase [ 2] : test_device_configure_invalid_queue_pair_ids succeeded
CRYPTODEV: rte_cryptodev_stats_get() line 1695: Invalid dev_id=88
CRYPTODEV: rte_cryptodev_stats_get() line 1700: Invalid stats ptr
CRYPTODEV: rte_cryptodev_stats_reset() line 1723: Invalid dev_id=44
+ TestCase [ 3] : test_stats succeeded
CRYPTODEV: rte_cryptodev_add_enq_callback() line 1428: Invalid dev_id=64
CRYPTODEV: rte_cryptodev_add_enq_callback() line 1435: Invalid queue_pair_id=2
CRYPTODEV: rte_cryptodev_add_enq_callback() line 1422: Callback is NULL on dev_id=0
crypto enqueue callback called
CRYPTODEV: rte_cryptodev_remove_enq_callback() line 1495: Invalid dev_id=64
CRYPTODEV: rte_cryptodev_remove_enq_callback() line 1503: Invalid queue_pair_id=2
CRYPTODEV: rte_cryptodev_remove_enq_callback() line 1490: Callback is NULL
+ TestCase [ 4] : test_enq_callback_setup succeeded
CRYPTODEV: rte_cryptodev_add_deq_callback() line 1566: Invalid dev_id=64
CRYPTODEV: rte_cryptodev_add_deq_callback() line 1573: Invalid queue_pair_id=2
CRYPTODEV: rte_cryptodev_add_deq_callback() line 1560: Callback is NULL on dev_id=0
crypto dequeue callback called
crypto dequeue callback called
crypto dequeue callback called
CRYPTODEV: rte_cryptodev_remove_deq_callback() line 1634: Invalid dev_id=64
CRYPTODEV: rte_cryptodev_remove_deq_callback() line 1642: Invalid queue_pair_id=2
CRYPTODEV: rte_cryptodev_remove_deq_callback() line 1629: Callback is NULL
+ TestCase [ 5] : test_deq_callback_setup succeeded
+ ------------------------------------------------------- +
+ Test Suite Summary : Crypto General Unit Test Suite
+ ------------------------------------------------------- +
+ Tests Total : 6
+ Tests Skipped : 0
+ Tests Executed : 6
+ Tests Unsupported: 0
+ Tests Passed : 6
+ Tests Failed : 0
+ ------------------------------------------------------- +
+ -------------------------------------------------------
Thanks,
Ganapati
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Thursday, May 23, 2024 12:06 AM
> To: Kundapura, Ganapati <ganapati.kundapura@intel.com>; dev@dpdk.org
> Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>;
> fanzhang.oss@gmail.com; 'Power, Ciara' <ciara.power@intel.com>
> Subject: RE: [PATCH v1] crypto: fix build issues on crypto callbacks macro
> undefined
>
> > Hi Akhil,
> > My patch addresses build issue on setting RTE_CRYPTO_CALLBACKS to 0
> > and Checks for callback registered or not from the next node instead
> > of head node as callback head node is always valid pointer.
> >
> Agreed but your patch cannot be verified without the fix as the callbacks are
> not getting called if using PMD other than NULL.
> Crypto callbacks macro is defined with value 1 and being used with ifdef,
> on config value is changed to 0 to disable, crypto callback changes
> still being compiled.
I believe we can use #if instead of ifdefs.
It seems convenient to enable/disable in my opinion.
We can use both, but whatever we use should be same as that for ethdev callbacks.
The same issue would be for ethdev callbacks too.
Ferruh, can you check?
>
> Defined crypto callbacks macro without value, undef to disable
>
> Wrapped crypto callback changes with RTE_CRYPTO_CALLBACKS macro
> to fix build issues when macro is undefined.
>
> As callback head nodes have valid pointer, this patch checks the next
> node from the head if callbacks registered.
>
> Fixes: 1c3ffb9 ("cryptodev: add enqueue and dequeue callbacks")
> Fixes: 5523a75 ("test/crypto: add case for enqueue/dequeue callbacks")
>
> Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> diff --git a/config/rte_config.h b/config/rte_config.h
> index dd7bb0d35b..b647a69ba8 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -72,7 +72,7 @@
> /* cryptodev defines */
> #define RTE_CRYPTO_MAX_DEVS 64
> #define RTE_CRYPTODEV_NAME_LEN 64
> -#define RTE_CRYPTO_CALLBACKS 1
> +#define RTE_CRYPTO_CALLBACKS /* No Value, undef/comment out to
> disable */
>
> /* compressdev defines */
> #define RTE_COMPRESS_MAX_DEVS 64
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index 00ba6a234a..b811b458d5 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -1910,7 +1910,7 @@ rte_cryptodev_dequeue_burst(uint8_t dev_id,
> uint16_t qp_id,
> nb_ops = fp_ops->dequeue_burst(qp, ops, nb_ops);
>
> #ifdef RTE_CRYPTO_CALLBACKS
> - if (unlikely(fp_ops->qp.deq_cb != NULL)) {
> + if (unlikely(fp_ops->qp.deq_cb[qp_id].next != NULL)) {
> struct rte_cryptodev_cb_rcu *list;
> struct rte_cryptodev_cb *cb;
>
> @@ -1977,7 +1977,7 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id,
> uint16_t qp_id,
> fp_ops = &rte_crypto_fp_ops[dev_id];
> qp = fp_ops->qp.data[qp_id];
> #ifdef RTE_CRYPTO_CALLBACKS
> - if (unlikely(fp_ops->qp.enq_cb != NULL)) {
> + if (unlikely(fp_ops->qp.enq_cb[qp_id].next != NULL)) {
> struct rte_cryptodev_cb_rcu *list;
> struct rte_cryptodev_cb *cb;
>
This is a separate issue. Please create a separate patch.
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Wednesday, May 29, 2024 2:05 PM
> To: Kundapura, Ganapati <ganapati.kundapura@intel.com>; dev@dpdk.org;
> Ferruh Yigit <ferruh.yigit@amd.com>; thomas@monjalon.net; Richardson,
> Bruce <bruce.richardson@intel.com>
> Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>;
> ciara.power@intel.com; fanzhang.oss@gmail.com
> Subject: RE: [EXTERNAL] [PATCH v1] crypto: fix build issues on crypto callbacks
> macro undefined
>
> > Crypto callbacks macro is defined with value 1 and being used with
> > ifdef, on config value is changed to 0 to disable, crypto callback
> > changes still being compiled.
>
> I believe we can use #if instead of ifdefs.
> It seems convenient to enable/disable in my opinion.
> We can use both, but whatever we use should be same as that for ethdev
> callbacks.
>
Using #if requires check for equality like
#if RTE_CRYPTO_CALLBACKS == 1 for a macro defined with value 1
> The same issue would be for ethdev callbacks too.
> Ferruh, can you check?
>
> >
> > Defined crypto callbacks macro without value, undef to disable
> >
> > Wrapped crypto callback changes with RTE_CRYPTO_CALLBACKS macro to fix
> > build issues when macro is undefined.
> >
> > As callback head nodes have valid pointer, this patch checks the next
> > node from the head if callbacks registered.
> >
> > Fixes: 1c3ffb9 ("cryptodev: add enqueue and dequeue callbacks")
> > Fixes: 5523a75 ("test/crypto: add case for enqueue/dequeue callbacks")
> >
> > Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
>
>
> > diff --git a/config/rte_config.h b/config/rte_config.h index
> > dd7bb0d35b..b647a69ba8 100644
> > --- a/config/rte_config.h
> > +++ b/config/rte_config.h
> > @@ -72,7 +72,7 @@
> > /* cryptodev defines */
> > #define RTE_CRYPTO_MAX_DEVS 64
> > #define RTE_CRYPTODEV_NAME_LEN 64
> > -#define RTE_CRYPTO_CALLBACKS 1
> > +#define RTE_CRYPTO_CALLBACKS /* No Value, undef/comment out to
> > disable */
> >
> > /* compressdev defines */
> > #define RTE_COMPRESS_MAX_DEVS 64
>
>
> > diff --git a/lib/cryptodev/rte_cryptodev.h
> > b/lib/cryptodev/rte_cryptodev.h index 00ba6a234a..b811b458d5 100644
> > --- a/lib/cryptodev/rte_cryptodev.h
> > +++ b/lib/cryptodev/rte_cryptodev.h
> > @@ -1910,7 +1910,7 @@ rte_cryptodev_dequeue_burst(uint8_t dev_id,
> > uint16_t qp_id,
> > nb_ops = fp_ops->dequeue_burst(qp, ops, nb_ops);
> >
> > #ifdef RTE_CRYPTO_CALLBACKS
> > - if (unlikely(fp_ops->qp.deq_cb != NULL)) {
> > + if (unlikely(fp_ops->qp.deq_cb[qp_id].next != NULL)) {
> > struct rte_cryptodev_cb_rcu *list;
> > struct rte_cryptodev_cb *cb;
> >
> > @@ -1977,7 +1977,7 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id,
> > uint16_t qp_id,
> > fp_ops = &rte_crypto_fp_ops[dev_id];
> > qp = fp_ops->qp.data[qp_id];
> > #ifdef RTE_CRYPTO_CALLBACKS
> > - if (unlikely(fp_ops->qp.enq_cb != NULL)) {
> > + if (unlikely(fp_ops->qp.enq_cb[qp_id].next != NULL)) {
> > struct rte_cryptodev_cb_rcu *list;
> > struct rte_cryptodev_cb *cb;
> >
> This is a separate issue. Please create a separate patch.
ok
> > Subject: RE: [EXTERNAL] [PATCH v1] crypto: fix build issues on crypto callbacks
> > macro undefined
> >
> > > Crypto callbacks macro is defined with value 1 and being used with
> > > ifdef, on config value is changed to 0 to disable, crypto callback
> > > changes still being compiled.
> >
> > I believe we can use #if instead of ifdefs.
> > It seems convenient to enable/disable in my opinion.
> > We can use both, but whatever we use should be same as that for ethdev
> > callbacks.
> >
> Using #if requires check for equality like
> #if RTE_CRYPTO_CALLBACKS == 1 for a macro defined with value 1
No
If it is set as 0, it will be disabled, else for any other value it will be enabled.
No need for check. Right?
>
> > The same issue would be for ethdev callbacks too.
> > Ferruh, can you check?
@@ -14547,6 +14547,7 @@ test_null_burst_operation(void)
return TEST_SUCCESS;
}
+#ifdef RTE_CRYPTO_CALLBACKS
static uint16_t
test_enq_callback(uint16_t dev_id, uint16_t qp_id, struct rte_crypto_op **ops,
uint16_t nb_ops, void *user_param)
@@ -14784,6 +14785,7 @@ test_deq_callback_setup(void)
return TEST_SUCCESS;
}
+#endif /* RTE_CRYPTO_CALLBACKS */
static void
generate_gmac_large_plaintext(uint8_t *data)
@@ -18069,8 +18071,10 @@ static struct unit_test_suite cryptodev_gen_testsuite = {
TEST_CASE_ST(ut_setup, ut_teardown,
test_device_configure_invalid_queue_pair_ids),
TEST_CASE_ST(ut_setup, ut_teardown, test_stats),
+#ifdef RTE_CRYPTO_CALLBACKS
TEST_CASE_ST(ut_setup, ut_teardown, test_enq_callback_setup),
TEST_CASE_ST(ut_setup, ut_teardown, test_deq_callback_setup),
+#endif
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
@@ -72,7 +72,7 @@
/* cryptodev defines */
#define RTE_CRYPTO_MAX_DEVS 64
#define RTE_CRYPTODEV_NAME_LEN 64
-#define RTE_CRYPTO_CALLBACKS 1
+#define RTE_CRYPTO_CALLBACKS /* No Value, undef/comment out to disable */
/* compressdev defines */
#define RTE_COMPRESS_MAX_DEVS 64
@@ -628,6 +628,7 @@ rte_cryptodev_asym_xform_capability_check_hash(
return ret;
}
+#ifdef RTE_CRYPTO_CALLBACKS
/* spinlock for crypto device enq callbacks */
static rte_spinlock_t rte_cryptodev_callback_lock = RTE_SPINLOCK_INITIALIZER;
@@ -744,6 +745,7 @@ cryptodev_cb_init(struct rte_cryptodev *dev)
cryptodev_cb_cleanup(dev);
return -ENOMEM;
}
+#endif /* RTE_CRYPTO_CALLBACKS */
const char *
rte_cryptodev_get_feature_name(uint64_t flag)
@@ -1244,9 +1246,11 @@ rte_cryptodev_configure(uint8_t dev_id, struct rte_cryptodev_config *config)
if (*dev->dev_ops->dev_configure == NULL)
return -ENOTSUP;
+#ifdef RTE_CRYPTO_CALLBACKS
rte_spinlock_lock(&rte_cryptodev_callback_lock);
cryptodev_cb_cleanup(dev);
rte_spinlock_unlock(&rte_cryptodev_callback_lock);
+#endif
/* Setup new number of queue pairs and reconfigure device. */
diag = rte_cryptodev_queue_pairs_config(dev, config->nb_queue_pairs,
@@ -1257,6 +1261,7 @@ rte_cryptodev_configure(uint8_t dev_id, struct rte_cryptodev_config *config)
return diag;
}
+#ifdef RTE_CRYPTO_CALLBACKS
rte_spinlock_lock(&rte_cryptodev_callback_lock);
diag = cryptodev_cb_init(dev);
rte_spinlock_unlock(&rte_cryptodev_callback_lock);
@@ -1264,6 +1269,7 @@ rte_cryptodev_configure(uint8_t dev_id, struct rte_cryptodev_config *config)
CDEV_LOG_ERR("Callback init failed for dev_id=%d", dev_id);
return diag;
}
+#endif
rte_cryptodev_trace_configure(dev_id, config);
return (*dev->dev_ops->dev_configure)(dev, config);
@@ -1485,6 +1491,7 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
socket_id);
}
+#ifdef RTE_CRYPTO_CALLBACKS
struct rte_cryptodev_cb *
rte_cryptodev_add_enq_callback(uint8_t dev_id,
uint16_t qp_id,
@@ -1763,6 +1770,7 @@ rte_cryptodev_remove_deq_callback(uint8_t dev_id,
rte_spinlock_unlock(&rte_cryptodev_callback_lock);
return ret;
}
+#endif /* RTE_CRYPTO_CALLBACKS */
int
rte_cryptodev_stats_get(uint8_t dev_id, struct rte_cryptodev_stats *stats)
@@ -1910,7 +1910,7 @@ rte_cryptodev_dequeue_burst(uint8_t dev_id, uint16_t qp_id,
nb_ops = fp_ops->dequeue_burst(qp, ops, nb_ops);
#ifdef RTE_CRYPTO_CALLBACKS
- if (unlikely(fp_ops->qp.deq_cb != NULL)) {
+ if (unlikely(fp_ops->qp.deq_cb[qp_id].next != NULL)) {
struct rte_cryptodev_cb_rcu *list;
struct rte_cryptodev_cb *cb;
@@ -1977,7 +1977,7 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
fp_ops = &rte_crypto_fp_ops[dev_id];
qp = fp_ops->qp.data[qp_id];
#ifdef RTE_CRYPTO_CALLBACKS
- if (unlikely(fp_ops->qp.enq_cb != NULL)) {
+ if (unlikely(fp_ops->qp.enq_cb[qp_id].next != NULL)) {
struct rte_cryptodev_cb_rcu *list;
struct rte_cryptodev_cb *cb;