On Fri, 12 Jul 2024 17:32:43 +0200
Laurent Vivier <lvivier(a)redhat.com> wrote:
> Add vhost_user.c and vhost_user.h that define the functions needed
> to implement vhost-user backend.
>
> Signed-off-by: Laurent Vivier <lvivier(a)redhat.com>
> ---
> Makefile | 4 +-
> iov.c | 1 -
> vhost_user.c | 1267 ++++++++++++++++++++++++++++++++++++++++++++++++++
> vhost_user.h | 197 ++++++++
> virtio.c | 5 -
> virtio.h | 2 +-
> 6 files changed, 1467 insertions(+), 9 deletions(-)
> create mode 100644 vhost_user.c
> create mode 100644 vhost_user.h
>
...
+/**
+ * vu_send() - Send a buffer to the front-end using the RX virtqueue
+ * @vdev: vhost-user device
+ * @buf: address of the buffer
+ * @size: size of the buffer
+ *
+ * Return: number of bytes sent, -1 if there is an error
+ */
+/* cppcheck-suppress unusedFunction */
+int vu_send(struct vu_dev *vdev, const void *buf, size_t size)
+{
+ size_t hdrlen = vdev->hdrlen;
+ struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
+ struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE];
+ struct iovec in_sg[VIRTQUEUE_MAX_SIZE];
+ size_t lens[VIRTQUEUE_MAX_SIZE];
+ size_t offset;
+ int i, j;
+ __virtio16 *num_buffers_ptr;
+ int in_sg_count;
Can those be aligned in the usual way (from longest to shortest)?
+
+ debug("vu_send size %zu hdrlen %zu", size, hdrlen);
+
+ if (!vu_queue_enabled(vq) || !vu_queue_started(vq)) {
+ err("Got packet, but no available descriptors on RX virtq.");
+ return 0;
+ }
+
+ offset = 0;
+ i = 0;
+ num_buffers_ptr = NULL;
+ in_sg_count = 0;
Could those be initialised when you declare them?
+ while (offset < size) {
+ size_t len;
+ int total;
+ int ret;
+
+ total = 0;
+
+ if (i == ARRAY_SIZE(elem) ||
+ in_sg_count == ARRAY_SIZE(in_sg)) {
+ err("virtio-net unexpected long buffer chain");
+ goto err;
+ }
+
+ elem[i].out_num = 0;
+ elem[i].out_sg = NULL;
+ elem[i].in_num = ARRAY_SIZE(in_sg) - in_sg_count;
+ elem[i].in_sg = &in_sg[in_sg_count];
+
+ ret = vu_queue_pop(vdev, vq, &elem[i]);
+ if (ret < 0) {
+ if (vu_wait_queue(vq) != -1)
+ continue;
+ if (i) {
+ err("virtio-net unexpected empty queue: "
+ "i %d mergeable %d offset %zd, size %zd, "
+ "features 0x%" PRIx64,
+ i, vu_has_feature(vdev,
+ VIRTIO_NET_F_MRG_RXBUF),
+ offset, size, vdev->features);
+ }
+ offset = -1;
+ goto err;
+ }
+ in_sg_count += elem[i].in_num;
+
+ if (elem[i].in_num < 1) {
+ err("virtio-net receive queue contains no in buffers");
+ vu_queue_detach_element(vdev, vq, elem[i].index, 0);
+ offset = -1;
+ goto err;
+ }
+
+ if (i == 0) {
+ struct virtio_net_hdr hdr = {
+ .flags = VIRTIO_NET_HDR_F_DATA_VALID,
+ .gso_type = VIRTIO_NET_HDR_GSO_NONE,
+ };
+
+ ASSERT(offset == 0);
+ ASSERT(elem[i].in_sg[0].iov_len >= hdrlen);
+
+ len = iov_from_buf(elem[i].in_sg, elem[i].in_num, 0,
+ &hdr, sizeof(hdr));
+
+ num_buffers_ptr = (__virtio16 *)((char *)elem[i].in_sg[0].iov_base +
+ len);
+
+ total += hdrlen;
Shouldn't this be 'total += len' or, alternatively, shouldn't there
be
a check that len == hdrlen?
len is sizeof(virtio_net_hdr) but hdrlen can be either sizeof(struct virtio_net_hdr) or
sizeof(struct virtio_net_hdr_mrg_rxbuf). It depends on VIRTIO_NET_F_MRG_RXBUF.
We actually want to add hdrlen to total.
struct virtio_net_hdr_mrg_rxbuf {
struct virtio_net_hdr hdr;
__virtio16 num_buffers; /* Number of merged rx buffers */
};
At this point we initialize hdr, num_buffers will be set later only if hdrlen is
sizeof(struct virtio_net_hdr_mrg_rxbuf).
Thanks,
Laurent