Re: [PATCH v4 3/4] vhost-user: introduce vhost-user API

12 Sep 2024


      On Thu, 12 Sep 2024 14:41:53 +0200
Laurent Vivier  wrote:
...
On 10/09/2024 17:47, Stefano Brivio wrote:
...
Nits and a couple of questions only:
On Fri,  6 Sep 2024 18:04:48 +0200
Laurent Vivier  wrote:
...
Add vhost_user.c and vhost_user.h that define the functions needed
to implement vhost-user backend.
Signed-off-by: Laurent Vivier 
---
  Makefile     |    4 +-
  iov.c        |    1 -
  vhost_user.c | 1265 ++++++++++++++++++++++++++++++++++++++++++++++++++
  vhost_user.h |  203 ++++++++
  virtio.c     |    5 -
  virtio.h     |    2 +-
  6 files changed, 1471 insertions(+), 9 deletions(-)
  create mode 100644 vhost_user.c
  create mode 100644 vhost_user.h  
...
diff --git a/vhost_user.c b/vhost_user.c
new file mode 100644
index 000000000000..6008a8adc967
--- /dev/null
+++ b/vhost_user.c  
...
+/**
+ * vu_wait_queue() - wait for new free entries in the virtqueue
+ * @vq:		virtqueue to wait on
+ */
+static int vu_wait_queue(const struct vu_virtq *vq)
+{
+	eventfd_t kick_data;
+	ssize_t rc;
+	int status;
+
+	/* wait for the kernel to put new entries in the queue */
+	status = fcntl(vq->kick_fd, F_GETFL);
+	if (status == -1)
+		return -1;
Same as on v3 (I see you changed this below, but not here): if you
don't use status later, you can omit storing it.
We need status with F_SETFL below:
Oops, sorry, of course!
...
...
...
+
+	if (fcntl(vq->kick_fd, F_SETFL, status & ~O_NONBLOCK))
+		return -1;
+
+	rc = eventfd_read(vq->kick_fd, &kick_data);
+
+	if (fcntl(vq->kick_fd, F_SETFL, status))
+		return -1;
+
+	if (rc == -1)
+		return -1;
+
+	return 0;
+}  
...
+/**
+ * vu_handle_tx() - Receive data from the TX virtqueue
+ * @vdev:	vhost-user device
+ * @index:	index of the virtqueue
+ * @now:	Current timestamp
+ */
+static void vu_handle_tx(struct vu_dev *vdev, int index,
+			 const struct timespec *now)
+{
+	struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE];
+	struct iovec out_sg[VIRTQUEUE_MAX_SIZE];
+	struct vu_virtq *vq = &vdev->vq[index];
+	int hdrlen = vdev->hdrlen;
+	int out_sg_count;
+	int count;
+
Excess newline (same as v3).
Done.
...
...
+
+	if (!VHOST_USER_IS_QUEUE_TX(index)) {
+		debug("vhost-user: index %d is not a TX queue", index);
+		return;
+	}
+
+	tap_flush_pools();
+
+	count = 0;
+	out_sg_count = 0;
+	while (count < VIRTQUEUE_MAX_SIZE) {
So, I see that this is limited to 1024 iterations now (it was limited
also earlier, but I didn't realise that).
If we loop at most VIRTQUEUE_MAX_SIZE times, that means, I guess, that
while we're popping elements, the queue can't be written to, correct?
No, I think the queue can be read and write at the same time.
...
Or it can be written to, but we'll get an additional kick after
vu_queue_notify() if that happens?
I could check the protocol and the code, but I think it should work like that.
Well, okay, it should be obvious enough.

-- 
Stefano