Re: [PATCH v8 7/8] vhost-user: add vhost-user

14 Nov 2024


      On 14/11/2024 15:23, Stefano Brivio wrote:
...
On Thu, 14 Nov 2024 11:20:09 +0100
Laurent Vivier  wrote:
...
On 17/10/2024 02:10, Stefano Brivio wrote:
...
...
+		if (frame_size == 0)
+			first = &iov_vu[i + 1];
+
+		if (iov_vu[i + 1].iov_len > (size_t)len)
+			iov_vu[i + 1].iov_len = len;
+
+		len -= iov_vu[i + 1].iov_len;
+		iov_used++;
+
+		frame_size += iov_vu[i + 1].iov_len;
+		num_buffers++;
+
+		if (frame_size >= mss || len == 0 ||
+		    i + 1 == iov_cnt || !vu_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) {
+			if (i + 1 == iov_cnt)
+				check = NULL;
+
+			/* restore first iovec base: point to vnet header */
+			vu_set_vnethdr(vdev, first, num_buffers, l2_hdrlen);
+
+			tcp_vu_prepare(c, conn, first, frame_size, &check);
+			if (*c->pcap)  {
+				tcp_vu_update_check(tapside, first, num_buffers);
+				pcap_iov(first, num_buffers,
+					 sizeof(struct virtio_net_hdr_mrg_rxbuf));
+			}
+
+			conn->seq_to_tap += frame_size;
We always increase this, even if, later...
...
+
+			frame_size = 0;
+			num_buffers = 0;
+		}
+	}
+
+	/* release unused buffers */
+	vu_queue_rewind(vq, iov_cnt - iov_used);
+
+	/* send packets */
+	vu_flush(vdev, vq, elem, iov_used);
we fail to send packets, that is, even if vu_queue_fill_by_index()
returns early because (!vq->vring.avail).
vring.avail is a pointer to a structure. vring.avail is NULL if there is something wrong
during the initialization. It's imported code, I think it's only a sanity check.
So in theory vu_flush() cannot fail.
Oh, I see now. I actually think it's preferable to crash in that
(theoretically impossible) case, without even an ASSERT() (we would
dereference a NULL pointer, eventually, even if not here).
So what you propose is to remove the "if (!vq->vring.avail) return;"?

Thanks
Laurent