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 (;;) { + /* keep going until we find a descriptor without the + * next flag + */ + vhost_vq_state[VHOST_USED_Q_IDX].num_free++; + 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); + } + + 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) + 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. + */ + desc->addr = (uint64_t)iov_i->iov_base; + desc->len = iov_i->iov_len; + 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); + + 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)

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

From: Ammar Yasser

--vhost-kernel controls whether to use vhost-net acceleration for pasta. It takes one of three modes: "on" requires acceleration and fails if it isn't available, "off" never uses it, and "auto" uses it if it's available and falls back to plain tap operation otherwise. "auto" is the default, so an unmodified invocation picks up acceleration wherever the kernel offers it, and behaves as before where it doesn't.

EPOLL_TYPE_VHOST_CALL means the kernel wants to notify us about data it has written. EPOLL_TYPE_VHOST_ERROR means the kernel encountered an internal error on vhost and wants to tell us something went wrong.

Collect the vhost state in a single struct vhost_ctx with the following fields:

- mode: which of the three modes above was requested - fd: file descriptor for /dev/vhost-net, which doubles as our indication that setup succeeded - features: virtio feature bits negotiated with vhost-net - vq[]: kick, call and err eventfds for both queues

Modify tap_hdr_iov to create an iov with the size of a virtio_net_mrg_rxbuf if vhost initialization has been successful (we are in mode 'on' or 'auto' and we didn't need the tap fallback)

Move the L2_MAX_LEN_XX constants from tap.h to passt.h to avoid the circular dependency between both files. VHOST_DESC_BYTES is defined as the maximum l2 frame size + the size of the virtio net header, if the length constant remains in tap.h passt.h would need to include it, but tap.h needs to include passt.h to get the definition of `struct ctx`. Moving the length constants to passt.h makes it free of dependency on tap.h

Signed-off-by: Eugenio Pérez Signed-off-by: Ammar Yasser --- conf.c | 19 +++++++++++++ epoll_type.h | 4 +++ passt.c | 3 ++ passt.h | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++-- tap.h | 36 ++++++------------------ 5 files changed, 112 insertions(+), 29 deletions(-)

diff --git a/conf.c b/conf.c index 149a4c7..fe578fc 100644 --- a/conf.c +++ b/conf.c @@ -736,6 +736,9 @@ pasta_opts: " default: auto\n" " --host-lo-to-ns-lo Translate host-loopback forwards to\n" " namespace loopback\n" + " --vhost-kernel[=MODE] Use vhost-kernel acceleration\n" + " MODE is \"on\", \"off\" or \"auto\"\n" + " default: auto\n" " --userns NSPATH Target user namespace to join\n" " --netns PATH|NAME Target network namespace to join\n" " --netns-only Don't join existing user namespace\n" @@ -767,6 +770,7 @@ enum passt_modes conf_mode(int argc, char *argv[]) int vhost_user = 0; const struct option optvu[] = { {"vhost-user", no_argument, &vhost_user, 1 }, + {"vhost-kernel", optional_argument, NULL, 0 },

As I said in my previous review, it's not needed in conf_mode.

{ 0 }, }; char argv0[PATH_MAX], *basearg0; @@ -1317,6 +1321,7 @@ void conf(struct ctx *c, int argc, char **argv) {"conf-path", required_argument, NULL, 'c' }, {"chroot-fallback", no_argument, NULL, 32 }, {"no-pidns", no_argument, NULL, 33 }, + {"vhost-kernel", optional_argument, NULL, 34 }, { 0 }, }; const char *optstring = "+dqfel:hs:c:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:"; @@ -1566,6 +1571,20 @@ void conf(struct ctx *c, int argc, char **argv) die("--no-pidns is for pasta mode only");

c->no_pidns = true; + break; + case 34: + if (c->mode != MODE_PASTA) + die("--vhost-kernel is for pasta mode only"); + + if (!optarg || !strcmp(optarg, "auto")) + c->vhost.mode = VHOST_MODE_AUTO; + else if (!strcmp(optarg, "on")) + c->vhost.mode = VHOST_MODE_ON; + else if (!strcmp(optarg, "off")) + c->vhost.mode = VHOST_MODE_OFF; + else + die("Invalid vhost-kernel mode: %s", optarg);

This means: - no "--vhost-kernel" parameter -> AUTO - --vhost-kernel without parameter -> AUTO I think it would be more intuitive if --vhost-kernel without parameter means "ON". No "--vhost-kernel" parameter would mean "AUTO": - no "--vhost-kernel" parameter -> AUTO - "--vhost-kernel" without parameter -> ON

+ break; case 'd': c->debug = 1; diff --git a/epoll_type.h b/epoll_type.h index 061325a..e3206d1 100644 --- a/epoll_type.h +++ b/epoll_type.h @@ -50,6 +50,10 @@ enum epoll_type { EPOLL_TYPE_CONF_LISTEN, /* Configuration socket */ EPOLL_TYPE_CONF, + /* vhost-kernel call socket */ + EPOLL_TYPE_VHOST_CALL, + /* vhost-kernel error socket */ + EPOLL_TYPE_VHOST_ERROR,

EPOLL_NUM_TYPES, }; diff --git a/passt.c b/passt.c index 5054551..6002231 100644 --- a/passt.c +++ b/passt.c @@ -65,6 +65,7 @@ char pkt_buf[PKT_BUF_BYTES] __attribute__ ((aligned(PAGE_SIZE))); struct ctx passt_ctx = { .pidfile_fd = -1, .fd_tap = -1, + .vhost = { .fd = -1 },

You should also initialize .kick_fd, .err_fd, .call_fd to -1 to avoid close stdin on vhost cleanup.

.fd_tap_listen = -1, .fd_control_listen = -1, .fd_repair_listen = -1, @@ -92,6 +93,8 @@ char *epoll_type_str[] = { [EPOLL_TYPE_NL_NEIGH] = "netlink neighbour notifier socket", [EPOLL_TYPE_CONF_LISTEN] = "configuration listening socket", [EPOLL_TYPE_CONF] = "configuration socket", + [EPOLL_TYPE_VHOST_CALL] = "vhost-kernel call socket", + [EPOLL_TYPE_VHOST_ERROR] = "vhost-kernel error socket", }; static_assert(ARRAY_SIZE(epoll_type_str) == EPOLL_NUM_TYPES, "epoll_type_str[] doesn't match enum epoll_type"); diff --git a/passt.h b/passt.h index afd8e9f..be68002 100644 --- a/passt.h +++ b/passt.h @@ -13,8 +13,11 @@ union epoll_ref;

#include #include +#include #include

+#include + #include "pif.h" #include "packet.h" #include "siphash.h" @@ -35,8 +38,43 @@ union epoll_ref; #define MAC_OUR_LAA \ ((uint8_t [ETH_ALEN]){0x9a, 0x55, 0x9a, 0x55, 0x9a, 0x55})

-/* Large enough for ~128 maximum size frames */ -#define PKT_BUF_BYTES (8UL << 20) + +/** L2_MAX_LEN_PASTA - Maximum frame length for pasta mode (with L2 header) + * + * The kernel tuntap device imposes a maximum frame size of 65535 including + * 'hard_header_len' (14 bytes for L2 Ethernet in the case of "tap" mode). + */ +#define L2_MAX_LEN_PASTA USHRT_MAX + +/** L2_MAX_LEN_PASST - Maximum frame length for passt mode (with L2 header) + * + * The only structural limit the QEMU socket protocol imposes on frames is + * (2^32-1) bytes, but that would be ludicrously long in practice. For now, + * limit it somewhat arbitrarily to 65535 bytes. FIXME: Work out an appropriate + * limit with more precision. + */ +#define L2_MAX_LEN_PASST USHRT_MAX + +/** L2_MAX_LEN_VU - Maximum frame length for vhost-user mode (with L2 header) + * + * vhost-user allows multiple buffers per frame, each of which can be quite + * large, so the inherent frame size limit is rather large. Much larger than is + * actually useful for IP. For now limit arbitrarily to 65535 bytes. FIXME: + * Work out an appropriate limit with more precision. + */ +#define L2_MAX_LEN_VU USHRT_MAX + +/* Number of descriptors in each vhost-net virtqueue */ +#define VHOST_NDESCS 128 + +/* Bytes of pkt_buf backing one from-guest descriptor: a maximum size frame + * plus the virtio-net header the kernel writes in front of it + */ +#define VHOST_DESC_BYTES (L2_MAX_LEN_PASTA + \ + sizeof(struct virtio_net_hdr_mrg_rxbuf)) + +/* One maximum size frame per vhost-net descriptor */ +#define PKT_BUF_BYTES (VHOST_NDESCS * VHOST_DESC_BYTES)

You agreed to keep (8UL << 20)

extern char pkt_buf [PKT_BUF_BYTES];

@@ -156,6 +194,40 @@ struct ip6_ctx {

#include

+/** + * enum vhost_mode - Whether to use vhost-kernel acceleration + * @VHOST_MODE_AUTO: Use it if it's available, fall back to plain tap if not + * @VHOST_MODE_ON: Require it, fail if it's not available + * @VHOST_MODE_OFF: Never use it + */ +enum vhost_mode { + VHOST_MODE_AUTO = 0, + VHOST_MODE_ON, + VHOST_MODE_OFF, +}; + +/** + * struct vhost_ctx - Execution context for vhost-kernel acceleration + * @mode: Whether to use acceleration at all, see enum vhost_mode + * @fd: File descriptor for /dev/vhost-net, -1 if not set up + * @features: virtio feature bits negotiated with vhost-net + * @vq: Per-virtqueue eventfds ([0] is from-guest, [1] is to-guest) + * @vq.kick_fd: Written by us, to tell the kernel we queued something + * @vq.call_fd: Written by the kernel, to tell us it queued something + * @vq.err_fd: Written by the kernel on an internal vhost error + */ +struct vhost_ctx { + enum vhost_mode mode; + int fd; + uint64_t features; + + struct { + int kick_fd; + int call_fd; + int err_fd; + } vq[2]; +}; + /** * struct ctx - Execution context * @mode: Operation mode, qemu/UNIX domain socket or namespace/tap @@ -185,6 +257,7 @@ struct ip6_ctx { * @our_tap_mac: Pasta/passt's MAC on the tap link * @guest_mac: MAC address of guest or namespace, seen or configured * @hash_secret: 128-bit secret for siphash functions + * @vhost: vhost-kernel acceleration context, pasta mode only * @ifi4: Template interface for IPv4, -1: none, 0: IPv4 disabled * @ip4: IPv4 configuration * @dns_search: DNS search list @@ -265,6 +338,8 @@ struct ctx {

uint64_t hash_secret[2];

+ struct vhost_ctx vhost; + int ifi4; struct ip4_ctx ip4;

diff --git a/tap.h b/tap.h index 1625975..f8c30e2 100644 --- a/tap.h +++ b/tap.h @@ -12,31 +12,6 @@ #include "passt.h" #include "tap_hdr.h"

-/** L2_MAX_LEN_PASTA - Maximum frame length for pasta mode (with L2 header) - * - * The kernel tuntap device imposes a maximum frame size of 65535 including - * 'hard_header_len' (14 bytes for L2 Ethernet in the case of "tap" mode). - */ -#define L2_MAX_LEN_PASTA USHRT_MAX - -/** L2_MAX_LEN_PASST - Maximum frame length for passt mode (with L2 header) - * - * The only structural limit the QEMU socket protocol imposes on frames is - * (2^32-1) bytes, but that would be ludicrously long in practice. For now, - * limit it somewhat arbitrarily to 65535 bytes. FIXME: Work out an appropriate - * limit with more precision. - */ -#define L2_MAX_LEN_PASST USHRT_MAX - -/** L2_MAX_LEN_VU - Maximum frame length for vhost-user mode (with L2 header) - * - * vhost-user allows multiple buffers per frame, each of which can be quite - * large, so the inherent frame size limit is rather large. Much larger than is - * actually useful for IP. For now limit arbitrarily to 65535 bytes. FIXME: - * Work out an appropriate limit with more precision. - */ -#define L2_MAX_LEN_VU USHRT_MAX - struct udphdr;

/** @@ -50,10 +25,17 @@ struct udphdr; static inline struct iovec tap_hdr_iov(const struct ctx *c, struct tap_hdr *thdr) { - return (struct iovec){ + struct iovec ret = { .iov_base = thdr, - .iov_len = c->mode == MODE_PASST ? sizeof(*thdr) : 0, }; + + if (c->vhost.fd != -1) { + ret.iov_len = sizeof(struct virtio_net_hdr_mrg_rxbuf); + } else { + ret.iov_len = c->mode == MODE_PASST ? sizeof(thdr->vnet_len) : 0; + } + + return ret; }

/** -- 2.39.5 (Apple Git-154)