Hi Gowri,
Thanks for the change, but I think there is an issue with the locking,
more below:
On 2/26/25 7:43 PM, Gowrishankar Muthukrishnan wrote:
Until virtio avail ring is initialized (by VHOST_USER_SET_VRING_ADDR),
worker thread should not try to fetch crypto op, which would lead to
memory fault.
Fixes: 939066d96563 ("vhost/crypto: add public function implementation")
Cc: sta...@dpdk.org
Signed-off-by: Gowrishankar Muthukrishnan <gmuthukri...@marvell.com>
Acked-by: Akhil Goyal <gak...@marvell.com>
---
v6:
- added lock checks.
---
lib/vhost/vhost_crypto.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
index 3dc41a3bd5..d3d13eff07 100644
--- a/lib/vhost/vhost_crypto.c
+++ b/lib/vhost/vhost_crypto.c
@@ -8,6 +8,7 @@
#include <rte_mbuf.h>
#include <rte_cryptodev.h>
+#include "iotlb.h"
#include "rte_vhost_crypto.h"
#include "vhost.h"
#include "vhost_user.h"
@@ -1580,7 +1581,26 @@ rte_vhost_crypto_fetch_requests(int vid, uint32_t qid,
vq = dev->virtqueue[qid];
+ if (unlikely(vq == NULL)) {
+ VC_LOG_ERR("Invalid virtqueue %u", qid);
+ return 0;
+ }
+
+ if (unlikely(rte_rwlock_read_trylock(&vq->access_lock) != 0))
+ return 0;
+
+ vhost_user_iotlb_rd_lock(vq);
+ if (unlikely(!vq->access_ok)) {
+ VC_LOG_DBG("Virtqueue %u vrings not yet initialized", qid);
+ vhost_user_iotlb_rd_unlock(vq);
+ rte_rwlock_read_unlock(&vq->access_lock);
+ return 0;
+ }
+
avail_idx = *((volatile uint16_t *)&vq->avail->idx);
+ vhost_user_iotlb_rd_unlock(vq);
+ rte_rwlock_read_unlock(&vq->access_lock);
+
You should only unlock at the end of the function, otherwise there is
not much protection.
start_idx = vq->last_used_idx;
count = avail_idx - start_idx;
count = RTE_MIN(count, VHOST_CRYPTO_MAX_BURST_SIZE);
