Re: [PATCH v2 6/7] tap: Implement the pasta vhost-net to-guest path

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

From: Ammar Yasser

Add a vhost argument to tap_send_frames_pasta(). Callers pass it to say whether this send should go through vhost-net, and when it does sending goes through tap_send_frames_vhost(), which takes a descriptor from the queue shared with the kernel for each iov and points it at that iov's base address, chaining the bufs_per_frame descriptors of a frame together so the guest receives it in one go.

tx_reap() reclaims the descriptors the kernel has finished with, walking each used chain to its end so that every descriptor in it is counted as free again.

Signed-off-by: Eugenio Pérez Signed-off-by: Ammar Yasser --- tap.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 147 insertions(+), 3 deletions(-)

diff --git a/tap.c b/tap.c index fd500f1..bc2700f 100644 --- a/tap.c +++ b/tap.c @@ -359,12 +359,151 @@ void tap_icmp6_send(const struct ctx *c, tap_send_single(c, buf, l4len + ((char *)icmp6h - buf)); }

+/** + * tx_reap() - Reclaim the descriptors the kernel has already processed + */ +static void tx_reap(void) +{ + struct vring_used_pasta *used = &vring_used_all[VHOST_USED_Q_IDX]; + uint16_t used_idx = le16toh(used->idx); + + smp_rmb(); + + /* increment last_used_idx until it reaches the kernel's used index */ + while (vhost_vq_state[VHOST_USED_Q_IDX].last_used_idx != used_idx) { + uint16_t last_used; + uint32_t desc_id; + + last_used = vhost_vq_state[VHOST_USED_Q_IDX].last_used_idx % VHOST_NDESCS; + desc_id = le32toh(used->ring[last_used].id); + + for (;;) {

We should check we don't have a circular chain rather than allowing an infinite loop

+ /* keep going until we find a descriptor without the + * next flag + */ + vhost_vq_state[VHOST_USED_Q_IDX].num_free++;

Check desc_id < VHOST_NDESCS

+ if (!(le16toh(vring_desc[VHOST_USED_Q_IDX][desc_id].flags) & + VRING_DESC_F_NEXT)) + break; + + /* this descriptor wasn't the last, set desc_id to the + * next one and keep going + */ + desc_id = le32toh(vring_desc[VHOST_USED_Q_IDX][desc_id].next);

.next is __virtio16(), le16toh().

+ } + + vhost_vq_state[VHOST_USED_Q_IDX].last_used_idx++; + } +} + +/** + * tap_send_frames_vhost() - Send multiple frames to the pasta tap + * @c: Execution context + * @iov: Array of buffers + * @bufs_per_frame: Number of buffers (iovec entries) per frame + * @nframes: Number of frames to send + * + * @iov must have total length @bufs_per_frame * @nframes, with each set of + * @bufs_per_frame contiguous buffers representing a single frame. + * + * Return: number of frames successfully sent + */ +static size_t tap_send_frames_vhost(const struct ctx *c, + const struct iovec *iov, + size_t bufs_per_frame, size_t nframes) +{ + struct vring_avail_pasta *avail = + &vring_avail_all[VHOST_USED_Q_IDX]; + size_t processed_frames = 0; + size_t i; + + /* reclaim descriptors if we don't have enough available buffers to + * perform this send + */ + if (vhost_vq_state[VHOST_USED_Q_IDX].num_free < bufs_per_frame * nframes) + tx_reap(); + + for (i = 0; i < nframes; i++) { + uint16_t head; + size_t j; + + /* it's likely that tx_reap returned to us less than + * bufs_per_frame descs + */ + if (vhost_vq_state[VHOST_USED_Q_IDX].num_free < bufs_per_frame)

Perhaps you can add a log message here.

+ break; + + /* set the index of the avail ring in the tx queue to be our + * last_used_idx + */ + head = vhost_vq_state[VHOST_USED_Q_IDX].next_free % VHOST_NDESCS; + avail->ring[(avail->idx + i) % VHOST_NDESCS] = htole16(head); + + /* we will be consuming bufs_per_frame descriptors for every + * frame, decrement the local num_free + */ + vhost_vq_state[VHOST_USED_Q_IDX].num_free -= bufs_per_frame; + + for (j = 0; j < bufs_per_frame; ++j) { + uint16_t next = + vhost_vq_state[VHOST_USED_Q_IDX].next_free % + VHOST_NDESCS; + /* get the last_used_idx descriptor */ + struct vring_desc_pasta *desc = + &vring_desc[VHOST_USED_Q_IDX][next]; + const struct iovec *iov_i; + + /* the iov variable contains the iovecs for all frames + * we will send. access a single fragment of a frame + * (each fragment is one of tcp_iov_parts) denoted by + * iov at index i (index of the frame being processed) + * * bufs_per_frame plus j (the index of the fragment + * being processed) + */ + iov_i = &iov[i * bufs_per_frame + j]; + + /* set that descriptor's address to the base of the iov + * and set the VRING_DESC_F_NEXT flag on the descriptor + * if its not the last frame fragment, so that the + * guest would recieve the entire frame in one go. + */

I think you should skip the iov when iov_len is 0. I can happen with UDP_IOV_ETH_PAD.

+ desc->addr = (uint64_t)iov_i->iov_base;

We need htole64()

+ desc->len = iov_i->iov_len;

htole32()

+ desc->flags = (j == bufs_per_frame - 1) ? + 0 : htole16(VRING_DESC_F_NEXT); + + vhost_vq_state[VHOST_USED_Q_IDX].next_free++; + } + + processed_frames++; + } + + /* we didn't process any frames, no need to notify the kernel */ + if (!processed_frames) + return 0; + + smp_wmb(); + + /* we will have used nframes descriptor chains */ + avail->idx = htole16(le16toh(avail->idx) + processed_frames);

The value to increment is not processed_frames but the number of desc that have been consumed.

+ + vhost_kick(&vring_used_all[VHOST_USED_Q_IDX], + c->vhost.vq[VHOST_USED_Q_IDX].kick_fd); + + /* wait until the kernel finishes processing this send */ + while (avail->idx != vring_used_all[1].used.idx) + ; + + return processed_frames; +} + /** * tap_send_frames_pasta() - Send multiple frames to the pasta tap * @c: Execution context * @iov: Array of buffers * @bufs_per_frame: Number of buffers (iovec entries) per frame * @nframes: Number of frames to send + * @vhost: Send through vhost-net rather than writing to the tap * * @iov must have total length @bufs_per_frame * @nframes, with each set of * @bufs_per_frame contiguous buffers representing a single frame. @@ -375,11 +514,15 @@ void tap_icmp6_send(const struct ctx *c, */ static size_t tap_send_frames_pasta(const struct ctx *c, const struct iovec *iov, - size_t bufs_per_frame, size_t nframes) + size_t bufs_per_frame, size_t nframes, + bool vhost) { size_t nbufs = bufs_per_frame * nframes; size_t i;

+ if (vhost) + return tap_send_frames_vhost(c, iov, bufs_per_frame, nframes); + for (i = 0; i < nbufs; i += bufs_per_frame) { ssize_t rc = writev(c->fd_tap, iov + i, bufs_per_frame); size_t framelen = iov_size(iov + i, bufs_per_frame); @@ -515,7 +658,7 @@ void tap_send_single(const struct ctx *c, const void *data, size_t l2len) iov[iovcnt].iov_len = l2len; iovcnt++;

- m = tap_send_frames_pasta(c, iov, iovcnt, 1); + m = tap_send_frames_pasta(c, iov, iovcnt, 1, false); break; case MODE_VU: vu_send_single(c, data, l2len); @@ -553,7 +696,8 @@ size_t tap_send_frames(const struct ctx *c, const struct iovec *iov,

switch (c->mode) { case MODE_PASTA: - m = tap_send_frames_pasta(c, iov, bufs_per_frame, nframes); + m = tap_send_frames_pasta(c, iov, bufs_per_frame, nframes, + c->vhost.fd != -1); break; case MODE_PASST: m = tap_send_frames_passt(c, iov, bufs_per_frame, nframes); -- 2.39.5 (Apple Git-154)