bug in cryptodev enqueue/dequeue callbacks?
Hi everyone,
Looking at implementation of cryptodev callbacks (it uses DPDK RCU), it seems like there is a bug here:
at init time we don't call rte_rcu_qsbr_thread_register(). As I understand without it rte_rcu_qsbr_check() wouldn't work properly for that thread.
Probably need to add:
static intcryptodev_cb_init(struct rte_cryptodev *dev)
{
....
if (rte_rcu_qsbr_init(qsbr, max_threads)) {...}+ rte_rcu_qsbr_thread_register(qsbr, 0);
Unless I am missing something obvious here?
Konstantin
Quoted 12 lines · Konstantin Ananyev
Hi everyone,
Looking at implementation of cryptodev callbacks (it uses DPDK RCU), it seems like there is a bug here:
at init time we don't call
rte_rcu_qsbr_thread_register(). As I understand without itrte_rcu_qsbr_check()wouldn't work properly for that thread.
Yes, this understanding is correct. However, the responsibility of calling the rte_rcu_qsbr_thread_register lies with the application. The roles and responsibilities are documented at [1]
[1] https://doc.dpdk.org/guides/prog_guide/rcu_lib.html#resource-reclamation-framework-for-dpdk
Quoted 12 lines
Probably need to add:
static intcryptodev_cb_init(struct rte_cryptodev *dev)
{
....
if (rte_rcu_qsbr_init(qsbr, max_threads)) {...}
+rte_rcu_qsbr_thread_register(qsbr, 0);Unless I am missing something obvious here?
Konstantin
Quoted 9 lines
Quoted 9 lines · Konstantin Ananyev
Hi everyone,
Looking at implementation of cryptodev callbacks (it uses DPDK RCU), it seems like there is a bug here:
Quoted 7 lines
Quoted 3 lines
at init time we don't call
rte_rcu_qsbr_thread_register(). As I understand without itrte_rcu_qsbr_check()wouldn't work properly for that thread.Yes, this understanding is correct. However, the responsibility of calling the
rte_rcu_qsbr_thread_registerlies with the application. The roles and responsibilities are documented at [1][1] https://doc.dpdk.org/guides/prog_guide/rcu_lib.html#resource-reclamation-framework-for-dpdk
I don't think it is an option here.
cryptodev callbacks adding/removing/invoking mechanism it totally opaque to the user. So these RCU instances are internal for cryptodev devices and shouldn't be updated for the user. In fact, it is sort of 'speacial-case' where we have rcu with just one thread_id(==0), and doing online/offline only.
Here is a link to original discussion:
https://mails.dpdk.org/archives/dev/2020-September/183401.html
So I still think we just forgot to callrte_rcu_qsbr_thread_register(qsbr, 0);
here.
Quoted 12 lines
Quoted 12 lines
Probably need to add:
static intcryptodev_cb_init(struct rte_cryptodev *dev)
{
....
if (rte_rcu_qsbr_init(qsbr, max_threads)) {...}
+rte_rcu_qsbr_thread_register(qsbr, 0);Unless I am missing something obvious here?
Konstantin