Re: [PATCH v2 3/7] virtio: Add the pasta vhost-net interface and implementation

On 10/16/26 22:49, aerosouund wrote:

From: Ammar Yasser

Add vhost.c and vhost.h which define the api and implementation for pasta vhost acceleration

- vq_state/vhost_vq_state[]: our own view of each virtqueue, which the kernel doesn't track for us: descriptors ready to hand back, how far we've read the used ring, and the next descriptor to allocate - vring_desc, vring_avail_all, vring_used_all, vhost_memory: the rings and the region table shared with the kernel - vhost_setup_net: open /dev/vhost-net and negotiate VIRTIO_F_VERSION_1 and VHOST_NET_F_VIRTIO_NET_HDR. Failure is fatal only if --vhost-kernel is set to 'on'. - vhost_setup_eventfds: create, register and watch the call, kick and error eventfds for one queue - vhost_setup_memory_table: register the buffers the kernel may touch. pasta has no guest, so each region maps guest_phys_addr onto the identical userspace_addr, making GPA translation an identity mapping. This function uses the *_register_memory_regions functions created per protocol and for general purpose buffers. - vhost_set_vring: publish a vring's addresses, bind the tap fd as its backend and lay out its descriptors - vhost_rx_descriptor_handoff, vhost_kick: hand consumed descriptors back and notify the kernel unless it says it's already polling

Signed-off-by: Eugenio Pérez Signed-off-by: Ammar Yasser --- Makefile | 2 +- tcp_buf.c | 22 +++- tcp_buf.h | 4 + udp.c | 20 ++++ udp.h | 3 + util.c | 18 ++++ util.h | 2 + vhost.c | 295 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ vhost.h | 91 +++++++++++++++++ 9 files changed, 455 insertions(+), 2 deletions(-) create mode 100644 vhost.c create mode 100644 vhost.h

...

diff --git a/vhost.c b/vhost.c new file mode 100644 index 0000000..a69c4b5 --- /dev/null +++ b/vhost.c ... +/** + * vhost_set_vring() - Register a vring's addresses and bind its backend + * @c: Execution context; c->vhost.fd must already be set + * @queue_idx: Index of the queue (vring) to configure + * @tap_fd: Tap fd to bind as this queue's backend + */ +void vhost_set_vring(struct ctx *c, int queue_idx, int tap_fd) +{ + int vhost_fd = c->vhost.fd; + struct vhost_vring_addr addr = { + .index = queue_idx, + .desc_user_addr = (unsigned long)vring_desc[queue_idx], + .avail_user_addr = (unsigned long)&vring_avail_all[queue_idx], + .used_user_addr = (unsigned long)&vring_used_all[queue_idx], + .log_guest_addr = (unsigned long)&vring_used_all[queue_idx],

I think it would be better to use uintptr_t for the casts.

+ }; + struct vhost_vring_file file = { + .index = queue_idx, + .fd = tap_fd, + }; + unsigned int i; + int rc; + + rc = ioctl(vhost_fd, VHOST_SET_VRING_ADDR, &addr); + if (rc < 0) + die_perror("VHOST_SET_VRING_ADDR ioctl failed, queue %d", + queue_idx); + + if (queue_idx == VHOST_AVAIL_Q_IDX) {

VHOST_AVAIL_Q_IDX (0) is not really meaningful, and if in the future we want to support multiqueue we should use something like VHOST_USER_IS_QUEUE_RX(queue_idx)

+ for (i = 0; i < VHOST_NDESCS; ++i) { + vring_desc[VHOST_AVAIL_Q_IDX][i].addr = + (uintptr_t)pkt_buf + i * VHOST_DESC_BYTES; + vring_desc[VHOST_AVAIL_Q_IDX][i].len = VHOST_DESC_BYTES; + vring_desc[VHOST_AVAIL_Q_IDX][i].flags = + VRING_DESC_F_WRITE; + } + + for (i = 0; i < VHOST_NDESCS; ++i) + vring_avail_all[VHOST_AVAIL_Q_IDX].ring[i] = + htole16(i); + + vhost_rx_descriptor_handoff(c); + } + + if (queue_idx == VHOST_USED_Q_IDX) {

something like VHOST_USER_IS_QUEUE_TX(queue_idx), perhaps we can rename them VHOST_IS_QUEUE_TX()/VHOST_IS_QUEUE_RX(). Please add a comment explaining .addr and .len will be set in tap_send_frames_vhost()

+ for (i = 0; i < (VHOST_NDESCS - 1); ++i) + vring_desc[VHOST_USED_Q_IDX][i].next = i + 1; + } + + rc = ioctl(vhost_fd, VHOST_NET_SET_BACKEND, &file); + if (rc < 0) + die_perror("VHOST_NET_SET_BACKEND ioctl failed, queue %d", + queue_idx); +} + +/** + * vhost_rx_descriptor_handoff() - Announce freed from-guest descriptors + * @c: Execution context + * + * Bumps avail.idx by the number of descriptors accumulated in + * vhost_vq_state[VHOST_AVAIL_Q_IDX].num_free (from prior + * consume_one_rx_descriptor() calls), + * then resets the counter to zero. The kernel will see the new + * avail.idx and consume the freshly-available descriptors. + */ +void vhost_rx_descriptor_handoff(struct ctx *c)

perhaps you can add queue_idx in the parameters.

+{ + smp_wmb(); + + if (!vhost_vq_state[VHOST_AVAIL_Q_IDX].num_free) + return; + + vring_avail_all[VHOST_AVAIL_Q_IDX].idx += + vhost_vq_state[VHOST_AVAIL_Q_IDX].num_free; + vhost_vq_state[VHOST_AVAIL_Q_IDX].num_free = 0; + vhost_kick(&vring_used_all[VHOST_AVAIL_Q_IDX], + c->vhost.vq[VHOST_AVAIL_Q_IDX].kick_fd); +} + +/** + * vhost_kick() - Notify the kernel that new descriptors are available + * @used: Used ring of the queue we're announcing on, checked to see + * whether the kernel's virtio thread is already reading + * descriptors and doesn't want to be notified + * @kick_fd: Kick eventfd of that same queue + */ +void vhost_kick(struct vring_used_pasta *used, int kick_fd) +{ + /* Ensure that the read of used->flags doesn't get reordered to be + * above the avail.idx update + */ + smp_mb(); + + if (!(used->flags & VRING_USED_F_NO_NOTIFY)) + eventfd_write(kick_fd, 1); +}