This work is based on the original v2 RFC by Eugenio Perez, and continues its version numbering. Link: https://archives.passt.top/passt-dev/20250709174748.3514693-1-eperezma@redha... vhost-net is a kernel device that allows reading and writing packets to a tap device using virtqueues instead of read(2) and write(2) and their vectored counterparts. Frames are handed over through a shared descriptor ring, so several of them can be passed to the kernel at once rather than issuing a syscall per frame. This RFC introduces an option to pasta, --vhost, which if specified will leverage this mode of acceleration. I haven't benchmarked this against the existing path yet, so the performance argument above is so far unquantified. TODO: test what happens when the namespace wants to send data but we don't have available rx descriptors, and vice versa for tx. TODO: tap_send_frames() currently carries an implicit contract that it is synchronous: once it returns, the caller is free to reuse its frame buffers. That holds for writev() and for vhost-user, but not for vhost-kernel, where returning only means the descriptors have been queued. pasta can overwrite a buffer while the kernel vhost worker thread is still reading the descriptor pointing at it. I haven't settled on how to restore that contract, and would appreciate direction before building further on top of it. Ammar Yasser (8): tap: Move the tap_hdr file to a separate file udp,tcp: Make protocol specific buffers public conf: Add context fields, epoll types and the --vhost flag to pasta virtio: Define the pasta vhost interface virtio: Implement the pasta vhost functions virtio: Implement pasta vhost acceleration guest->pasta path pasta: Implement pasta vhost TX (pasta->guest) prerequisites tap: Implement pasta vhost TX conf.c | 6 + epoll_type.h | 4 + passt.c | 9 ++ passt.h | 17 ++- tap.c | 317 +++++++++++++++++++++++++++++++++++++++++-------- tap.h | 10 +- tap_hdr.h | 19 +++ tcp_buf.c | 51 +++++--- tcp_buf.h | 14 +++ udp.c | 68 ++++++----- udp.h | 53 +++++++++ udp_internal.h | 13 -- virtio.c | 283 ++++++++++++++++++++++++++++++++++++++++++- virtio.h | 76 ++++++++++++ 14 files changed, 827 insertions(+), 113 deletions(-) create mode 100644 tap_hdr.h -- 2.34.1
It was defined in tap.h, put it on its own so that future callers won't
need to depend on all definitions in tap.h
Signed-off-by: Ammar Yasser
Move the buffers used to split tcp and udp packets coming from the host
from being static definitions in tcp_buf.c and udp.c to structs exported
through extern on the their respective header files.
Also move the udp_meta_t definition to the public udp.h file instead of
it living in udp_internal.h
Signed-off-by: Ammar Yasser
The --vhost controls whether or no to use vhost acceleration for pasta.
The EPOLL_TYPE_VHOST_CALL means that 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 notify us that something went
wrong.
Add the fd_vhost field on the context which will carry the file
descriptor of the device and will indicate that the setup was
successful. The vq field contains the kick, call and err file
descriptors for both queues.
Also add vhost (mark if vhost acceleration was requested) and
virtio_features field.
Signed-off-by: Ammar Yasser
- Include the ioctls needed in opening, initializing and registering
queues and fds with vhost-net
- Create the vq_state type which represents from pasta's perspective our
state of virtqueue processing
- Define the structures that will represent the actual descriptor
queues: vring_desc (the actual descriptor chain), vring_used_all (used
ring for each queue), vring_avail_all (available ring for each queue),
vhost_memory (the structure that carries the shape of the memory
region as it should be shared with the kernel)
- Increase PKT_BUF_BYTES by 1536 because the byte array that carries
the frames needs to also account for virtio_net_hdr_mrg_rxbuf header
sizes per each frame.
- Define the signatures of functions that will be used in setting up the
virtqueues with the kernel (set_vring_for_queue, setup_memory_table,
setup_vhost_net, setup_eventfds).
- Define the signature of rx_descriptor_handoff, which will be used to
mark descriptors as available again to the driver, and vhost_kick so
we can notify the driver of our updates
Signed-off-by: Ammar Yasser
- setup_vhost_net: try to open /dev/vhost-net and negotiate the needed
features on the device file descriptor. Set the descriptor on
c->fd_host on success.
- setup_eventfds: setup the call, kick and err files for a given
queue_index.
- setup_memory_table: create the memory table object that gets shared
with the kernel so it writes directly to the global pkt_buf on rx, and
when we receive data from the host and eventually write it the
protocol specific buffers the kernel is able to read from them. we
share only buffers for tcp and udp as they are the only two protocols
that will have vhost acceleration. besides, other protocols don't
have a global buffer from which they allocate packets to send to the
guest and would be trickier to support
Signed-off-by: Ammar Yasser
Add a new function called tap_vhost_input that will be used to indicate
that the guest is trying to send data to us through the rx queue and the
kernel is informing us to handle this data.
We will however still recieve epoll events on the fd normally. We should
explicitly use either the vhost path of the tap_handler_pasta path and
so if vhost was requested the tap_handler_pasta path gets skipped.
Add vhost bootstrapping to tap_sock_tun_init in case vhost was
requested.
consume_one_rx_descriptor: fetch one descriptor from what the kernel has
advertised to us was written to, advance the tracking data structure to
reflect the last index we used, and the count of descriptors we are
ready to hand back to the kernel. return a void pointer to the start of
this descriptor's data
tap_vhost_input: fetch descriptors using consume_one_rx_descriptor,
and skip over the vnet header, then build an iov tail from data and
queue it for processing, process the frames, and lastly, advertise to
the kernel that the descriptors are free to reuse
Signed-off-by: Ammar Yasser
- tap_send_single was changed to directly call
tap_send_frames_passt/pasta instead of relying on tap_send_frames
because protocols that use tap_send_single won't use vhost
acceleration, only tcp and udp will. The function was also moved lower
in the file to by the time its called tap_send_frames_* functions are
defined and a forward declaration isn't needed
- extend udp_meta_t to instead of always carrying a tap_hdr, to carry a
union of a tap_hdr and a virtio_net header because if vhost is used
the will the latter type of header and then build an iov from this
virtio net header in udp.c
- create iov_from_virtio_net_hdr to build an iov from the vnet header
- in the case that vhost is requested, set the first frame fragment to
be a virtio_net header through calling iov_from_virtio_net_hdr
- convert the tcp_payload_tap_hdr array to be an array of virtio_net
header. in the case where we aren't using vhost, we will just be using
the first 4 bytes of it
- add a boolean argumen to tap_send_frames_pasta to determine if vhost
will be used
Signed-off-by: Ammar Yasser
Callers will specify whether vhost should be used or no by passing the
vhost argument to tap_send_frames_pasta. If specified, sending will go
through a function called tap_send_frames_vhost, which pops a descriptor
from the queue shared with the kernel and sets the address of that
descriptor to be the base address of a single iov from the group of
buffers_per_frame * nframes iovs we pass to the function
Signed-off-by: Ammar Yasser
On Sun, Aug 02, 2026 at 01:21:48PM +0000, Ammar Yasser wrote:
It was defined in tap.h, put it on its own so that future callers won't need to depend on all definitions in tap.h
Signed-off-by: Ammar Yasser
Reviewed-by: David Gibson
--- tap.h | 9 +-------- tap_hdr.h | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 8 deletions(-) create mode 100644 tap_hdr.h
diff --git a/tap.h b/tap.h index b335933..1625975 100644 --- a/tap.h +++ b/tap.h @@ -10,6 +10,7 @@ #include
#include "passt.h" +#include "tap_hdr.h"
/** L2_MAX_LEN_PASTA - Maximum frame length for pasta mode (with L2 header) * @@ -38,14 +39,6 @@
struct udphdr;
-/** - * struct tap_hdr - tap backend specific headers - * @vnet_len: Frame length (for qemu socket transport) - */ -struct tap_hdr { - uint32_t vnet_len; -} __attribute__((packed)); - /** * tap_hdr_iov() - struct iovec for a tap header * @c: Execution context diff --git a/tap_hdr.h b/tap_hdr.h new file mode 100644 index 0000000..15bdb5b --- /dev/null +++ b/tap_hdr.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later + * Copyright (c) 2021 Red Hat GmbH + * Author: Stefano Brivio
+ */ + +#ifndef TAP_HDR_H +#define TAP_HDR_H + +#include + +/** + * struct tap_hdr - tap backend specific headers + * @vnet_len: Frame length (for qemu socket transport) + */ +struct tap_hdr { + uint32_t vnet_len; +} __attribute__((packed)); + +#endif /* TAP_HDR_H */ -- 2.34.1
-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
On Sun, Aug 02, 2026 at 01:21:49PM +0000, Ammar Yasser wrote:
Move the buffers used to split tcp and udp packets coming from the host from being static definitions in tcp_buf.c and udp.c to structs exported through extern on the their respective header files.
Also move the udp_meta_t definition to the public udp.h file instead of it living in udp_internal.h
This commit message needs more of a rationale as to why this is a desirable change. I presume it's because something later in the series needs to access these variables and types, but this commit message should summarise that for easier review. The content looks fine assuming the premise makes sense.
Signed-off-by: Ammar Yasser
--- tcp_buf.c | 14 +++++--------- tcp_buf.h | 16 +++++++++++++++- udp.c | 40 ++++++++++++---------------------------- udp.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ udp_internal.h | 13 ------------- 5 files changed, 81 insertions(+), 51 deletions(-) diff --git a/tcp_buf.c b/tcp_buf.c index 72c4541..4452337 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -33,23 +33,19 @@ #include "tcp_internal.h" #include "tcp_buf.h"
-#define TCP_FRAMES_MEM 128 -#define TCP_FRAMES \ - (c->mode == MODE_PASTA ? 1 : TCP_FRAMES_MEM) - /* Static buffers */
/* Ethernet header for IPv4 and IPv6 frames */ -static struct ethhdr tcp_eth_hdr[TCP_FRAMES_MEM]; +struct ethhdr tcp_eth_hdr[TCP_FRAMES_MEM];
-static struct tap_hdr tcp_payload_tap_hdr[TCP_FRAMES_MEM]; +struct tap_hdr tcp_payload_tap_hdr[TCP_FRAMES_MEM];
/* IP headers for IPv4 and IPv6 */ -static struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; -static struct ipv6hdr tcp6_payload_ip[TCP_FRAMES_MEM]; +struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; +struct ipv6hdr tcp6_payload_ip[TCP_FRAMES_MEM];
/* TCP segments with payload for IPv4 and IPv6 frames */ -static struct tcp_payload_t tcp_payload[TCP_FRAMES_MEM]; +struct tcp_payload_t tcp_payload[TCP_FRAMES_MEM];
static_assert(MSS4 <= sizeof(tcp_payload[0].data), "MSS4 is greater than 65516"); static_assert(MSS6 <= sizeof(tcp_payload[0].data), "MSS6 is greater than 65516"); diff --git a/tcp_buf.h b/tcp_buf.h index 5d31cea..c749038 100644 --- a/tcp_buf.h +++ b/tcp_buf.h @@ -6,11 +6,25 @@ #ifndef TCP_BUF_H #define TCP_BUF_H
-void tcp_sock_iov_init(const struct ctx *c); +#include "tcp_conn.h" +#include "tcp_internal.h" + +void tcp_sock_iov_init(); void tcp_payload_flush(const struct ctx *c, const struct timespec *now); int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn, uint32_t already_sent, const struct timespec *now); int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags, const struct timespec *now);
+#define TCP_FRAMES_MEM 128 +#define TCP_FRAMES \ +(c->mode == MODE_PASTA ? 1 : TCP_FRAMES_MEM) + +extern struct tap_hdr tcp_payload_tap_hdr[TCP_FRAMES_MEM]; +extern struct ethhdr tcp_eth_hdr[TCP_FRAMES_MEM]; +extern struct tcp_payload_t tcp_payload[TCP_FRAMES_MEM]; + +extern struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; +extern struct ipv6hdr tcp6_payload_ip[TCP_FRAMES_MEM]; + #endif /*TCP_BUF_H */ diff --git a/udp.c b/udp.c index 505e554..d05ee66 100644 --- a/udp.c +++ b/udp.c @@ -119,7 +119,6 @@ #include "udp_vu.h" #include "epoll_ctl.h"
-#define UDP_MAX_FRAMES 32 /* max # of frames to receive at once */
#define UDP_TIMEOUT "/proc/sys/net/netfilter/nf_conntrack_udp_timeout" #define UDP_TIMEOUT_STREAM \ @@ -134,30 +133,6 @@ - sizeof(struct udphdr) \ - sizeof(struct ipv6hdr))
-/* Static buffers */ - -/* UDP header and data for inbound messages */ -static struct udp_payload_t udp_payload[UDP_MAX_FRAMES]; - -/* Ethernet headers for IPv4 and IPv6 frames */ -static struct ethhdr udp_eth_hdr[UDP_MAX_FRAMES]; - -/** - * struct udp_meta_t - Pre-cooked headers for UDP packets - * @ip6h: Pre-filled IPv6 header (except for payload_len and addresses) - * @ip4h: Pre-filled IPv4 header (except for tot_len and saddr) - * @taph: Tap backend specific header - */ -static struct udp_meta_t { - struct ipv6hdr ip6h; - struct iphdr ip4h; - struct tap_hdr taph; -} -#ifdef __AVX2__ -__attribute__ ((aligned(32))) -#endif -udp_meta[UDP_MAX_FRAMES]; - #define PKTINFO_SPACE \ MAX(CMSG_SPACE(sizeof(struct in_pktinfo)), \ CMSG_SPACE(sizeof(struct in6_pktinfo))) @@ -186,9 +161,6 @@ enum udp_iov_idx { UDP_NUM_IOVS, };
-/* IOVs and msghdr arrays for receiving datagrams from sockets */ -static struct iovec udp_iov_recv [UDP_MAX_FRAMES]; -static struct mmsghdr udp_mh_recv [UDP_MAX_FRAMES];
/* IOVs and msghdr arrays for sending "spliced" datagrams to sockets */ static union sockaddr_inany udp_splice_to; @@ -199,6 +171,18 @@ static struct mmsghdr udp_mh_splice [UDP_MAX_FRAMES]; /* IOVs for L2 frames */ static struct iovec udp_l2_iov [UDP_MAX_FRAMES][UDP_NUM_IOVS];
+struct udp_payload_t udp_payload[UDP_MAX_FRAMES]; + +/* Ethernet headers for IPv4 and IPv6 frames */ +struct ethhdr udp_eth_hdr[UDP_MAX_FRAMES]; + +/* IOVs and msghdr arrays for receiving datagrams from sockets */ +struct iovec udp_iov_recv [UDP_MAX_FRAMES]; +struct mmsghdr udp_mh_recv [UDP_MAX_FRAMES]; + +/* Pre-cooked headers for UDP packets */ +struct udp_meta_t udp_meta[UDP_MAX_FRAMES]; + /** * udp_update_l2_buf() - Update L2 buffers with Ethernet and IPv4 addresses * @eth_d: Ethernet destination address, NULL if unchanged diff --git a/udp.h b/udp.h index b50283e..b7a367c 100644 --- a/udp.h +++ b/udp.h @@ -8,9 +8,58 @@
#include
#include +#include +#include "tap_hdr.h" #include "fwd.h"
+/** + * struct udp_payload_t - UDP header and data for inbound messages + * @uh: UDP header + * @data: UDP data + */ +struct udp_payload_t { + struct udphdr uh; + char data[USHRT_MAX - sizeof(struct udphdr)]; +#ifdef __AVX2__ +} __attribute__ ((packed, aligned(32))); +#else +} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); +#endif + +#define UDP_MAX_FRAMES 32 /* max # of frames to receive at once */ + +/* UDP header and data for inbound messages */ +extern struct udp_payload_t udp_payload[UDP_MAX_FRAMES]; + +/* Ethernet headers for IPv4 and IPv6 frames */ +extern struct ethhdr udp_eth_hdr[UDP_MAX_FRAMES]; + +/* IOVs and msghdr arrays for receiving datagrams from sockets */ +extern struct iovec udp_iov_recv [UDP_MAX_FRAMES]; +extern struct mmsghdr udp_mh_recv [UDP_MAX_FRAMES]; + + +/** + * struct udp_meta_t - Pre-cooked headers for UDP packets + * @ip6h: Pre-filled IPv6 header (except for payload_len and addresses) + * @ip4h: Pre-filled IPv4 header (except for tot_len and saddr) + * @taph: Tap backend specific header + */ +struct udp_meta_t { + struct ipv6hdr ip6h; + struct iphdr ip4h; + struct tap_hdr taph; +#ifdef __AVX2__ +} __attribute__((aligned(32))); +#else +}; +#endif + +/* Pre-cooked headers for UDP packets */ +extern struct udp_meta_t udp_meta[UDP_MAX_FRAMES]; + + void udp_listen_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events, const struct timespec *now); void udp_sock_handler(const struct ctx *c, union epoll_ref ref, diff --git a/udp_internal.h b/udp_internal.h index 361cc74..6804843 100644 --- a/udp_internal.h +++ b/udp_internal.h @@ -11,19 +11,6 @@
#include "tap.h" /* needed by udp_meta_t */
-/** - * struct udp_payload_t - UDP header and data for inbound messages - * @uh: UDP header - * @data: UDP data - */ -struct udp_payload_t { - struct udphdr uh; - char data[USHRT_MAX - sizeof(struct udphdr)]; -#ifdef __AVX2__ -} __attribute__ ((packed, aligned(32))); -#else -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); -#endif
size_t udp_update_hdr4(struct iphdr *ip4h, struct udphdr *uh, struct iov_tail *payload, -- 2.34.1
-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
On Sun, Aug 02, 2026 at 01:21:50PM +0000, Ammar Yasser wrote:
The --vhost controls whether or no to use vhost acceleration for pasta. The EPOLL_TYPE_VHOST_CALL means that 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 notify us that something went wrong.
Add the fd_vhost field on the context which will carry the file descriptor of the device and will indicate that the setup was successful. The vq field contains the kick, call and err file descriptors for both queues.
Also add vhost (mark if vhost acceleration was requested) and virtio_features field.
A couple of concerns about the command line interface. IIUC, once this is all ready the difference between vhost-kernel and regular tuntap should be neither guest-visible nor user-visible. So, assuming it does perform better, we'll probably want to make it the default. So, making this explicitly an opt-in probably isn't what we want. Maybe --vhost on|off|auto would be a better idea, defaulting to off now, but auto (use if available) in future. "on" would be if the user definitely wants vhost, and would rather we exit than fall back if it's not available. A more minor concern is that there's the potential confusion between this and --vhost-user. Not immediately sure what we can do about this, since that's just reflecting the confusingly similar names of the underlying features.
Signed-off-by: Ammar Yasser
--- conf.c | 6 ++++++ epoll_type.h | 4 ++++ passt.c | 3 +++ passt.h | 15 +++++++++++++++ 4 files changed, 28 insertions(+) diff --git a/conf.c b/conf.c index faf2681..c31546e 100644 --- a/conf.c +++ b/conf.c @@ -736,6 +736,7 @@ pasta_opts: " default: auto\n" " --host-lo-to-ns-lo Translate host-loopback forwards to\n" " namespace loopback\n" + " --vhost\t\tUse vhost-kernel acceleration\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" @@ -766,6 +767,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", no_argument, NULL, 0 }, { 0 }, }; char argv0[PATH_MAX], *basearg0; @@ -1309,6 +1311,7 @@ void conf(struct ctx *c, int argc, char **argv) {"ipv4-only", no_argument, NULL, '4' }, {"ipv6-only", no_argument, NULL, '6' }, {"one-off", no_argument, NULL, '1' }, + {"vhost", no_argument, &c->vhost, 1 }, {"tcp-ports", required_argument, NULL, 't' }, {"udp-ports", required_argument, NULL, 'u' }, {"tcp-ns", required_argument, NULL, 'T' }, @@ -1838,6 +1841,9 @@ void conf(struct ctx *c, int argc, char **argv) die("--no-copy-addrs needs --config-net"); }
+ if (c->vhost && c->mode != MODE_PASTA) + die("--vhost is only available in pasta mode"); + if (c->mode == MODE_PASTA && c->splice_only) { if (c->no_splice) die("--splice-only is incompatible with --no-splice"); 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..865b331 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, + .fd_vhost = -1, .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 51ccd4f..c729316 100644 --- a/passt.h +++ b/passt.h @@ -183,8 +183,11 @@ struct ip6_ctx { * @fd_repair_listen: File descriptor for listening TCP_REPAIR socket, if any * @fd_repair: Connected AF_UNIX socket for TCP_REPAIR helper * @our_tap_mac: Pasta/passt's MAC on the tap link + * @fd_vhost: File descriptor for /dev/vhost-net, set on vhost setup * @guest_mac: MAC address of guest or namespace, seen or configured * @hash_secret: 128-bit secret for siphash functions + * @virtio_features: Negotiated virtio feature bits + * @vhost: Enable vhost-kernel acceleration for pasta * @ifi4: Template interface for IPv4, -1: none, 0: IPv4 disabled * @ip4: IPv4 configuration * @dns_search: DNS search list @@ -202,6 +205,8 @@ struct ip6_ctx { * @no_udp: Disable UDP operation * @udp: Context for UDP protocol handler * @no_icmp: Disable ICMP operation + * @vq: Per-virtqueue eventfd descriptors for vhost-kernel + * ([0] is RX, [1] is TX; each holds kick_fd, call_fd, err_fd) * @mtu: MTU passed via DHCP/NDP * @no_dns: Do not source/use DNS servers for any purpose * @no_dns_search: Do not source/use domain search lists for any purpose @@ -258,11 +263,14 @@ struct ctx { int fd_control; int fd_repair_listen; int fd_repair; + + int fd_vhost; unsigned char our_tap_mac[ETH_ALEN]; unsigned char guest_mac[ETH_ALEN]; uint16_t mtu;
uint64_t hash_secret[2]; + uint64_t virtio_features;
int ifi4; struct ip4_ctx ip4; @@ -288,6 +296,12 @@ struct ctx { struct udp_ctx udp; int no_icmp;
+ struct { + int kick_fd; + int call_fd; + int err_fd; + } vq[2]; + int no_dns; int no_dns_search; int no_dhcp_dns; @@ -300,6 +314,7 @@ struct ctx { int splice_only; int host_lo_to_ns_lo; int freebind; + int vhost; bool chroot_fallback;
int low_wmem;
It might be a bit neater to put the new vhost-kernel related fields into a substructure, rather than spreading them across struct ctx. At some point it would be nice to make the various tap backends a bit more pluggable / independent from each other. Doing that's obviously not in scope for this series, but keeping it's internal data all together will at least not make that job harder in the future.
-- 2.34.1
-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
On Sun, Aug 02, 2026 at 01:21:51PM +0000, Ammar Yasser wrote:
- Include the ioctls needed in opening, initializing and registering queues and fds with vhost-net - Create the vq_state type which represents from pasta's perspective our state of virtqueue processing - Define the structures that will represent the actual descriptor queues: vring_desc (the actual descriptor chain), vring_used_all (used ring for each queue), vring_avail_all (available ring for each queue), vhost_memory (the structure that carries the shape of the memory region as it should be shared with the kernel) - Increase PKT_BUF_BYTES by 1536 because the byte array that carries the frames needs to also account for virtio_net_hdr_mrg_rxbuf header sizes per each frame. - Define the signatures of functions that will be used in setting up the virtqueues with the kernel (set_vring_for_queue, setup_memory_table, setup_vhost_net, setup_eventfds). - Define the signature of rx_descriptor_handoff, which will be used to mark descriptors as available again to the driver, and vhost_kick so we can notify the driver of our updates
Signed-off-by: Ammar Yasser
--- passt.h | 2 +- virtio.h | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/passt.h b/passt.h index c729316..88a1b03 100644 --- a/passt.h +++ b/passt.h @@ -36,7 +36,7 @@ union epoll_ref; ((uint8_t [ETH_ALEN]){0x9a, 0x55, 0x9a, 0x55, 0x9a, 0x55})
/* Large enough for ~128 maximum size frames */ -#define PKT_BUF_BYTES (8UL << 20) +#define PKT_BUF_BYTES ((8UL << 20) + 1536) /* 128 * sizeof(virtio_net_hdr_mrg_rxbuf) */
I think the rationale for this change needs to be clearer (granted, the comment here beforehand is also kind of confusing). IIRC - and based on the "~" in the comment, I don't think there's a strict requirement that this can hold 128 full frames - that's just setting a reasonable sense of scale, and then a round number was picked near it: ~64kiB * ~64 ~= 8MiB So, I'm not sure if this change is necessary - if it really is, we need a clearer analysis of why.
extern char pkt_buf [PKT_BUF_BYTES];
diff --git a/virtio.h b/virtio.h index 8f2ae06..2d1eef1 100644 --- a/virtio.h +++ b/virtio.h @@ -9,14 +9,82 @@ #ifndef VIRTIO_H #define VIRTIO_H
+#include
#include +#include #include +struct ctx; +
AIUI, virtio.h is supposed to expose the virtio interfaces to passt, not contain any passt specific logic, so including things that use struct ctx (even just by reference) is probably not a good idea. I believe this header was originally formed as a cut down version of one from qemu or a virtio library, so I'd probably also restrict it to things that (conceptually) were in there. Laurent will know more about the history, since he introduced these for vhost-user. We also want to be clear what definitions are general to virtio versus which are specific to vhost-kernel versus vhost-user.
/* Maximum size of a virtqueue */ #define VIRTQUEUE_MAX_SIZE 1024
#define VNET_HLEN (sizeof(struct virtio_net_hdr_mrg_rxbuf))
+/* Keep in sync with PKT_BUF_BYTES in passt.h */ +#define PKT_BUF_BYTES ((8UL << 20) + 1536) /* 128 * sizeof(virtio_net_hdr_mrg_rxbuf) */
Ouch. We really want to avoid that sort of duplication, even if it means splitting out a new small header included from both places.
+#define VHOST_NDESCS (PKT_BUF_BYTES / 65520)
I'm not sure 65520 is the right number here. That's the max MTU at the IP level, but the packet buffer will also hold the 14 byte L2 header. I think you probably want one of the L2_MAX_LEN_* constants (or to define a new one for vhost-kernel).
+static_assert(!(VHOST_NDESCS & (VHOST_NDESCS - 1)), + "Number of vhost descs must be a power of two by standard"); + + +#define VIRTQUEUE_MAX_SIZE 1024
Duplicate #define.
+ +#define VHOST_VIRTIO 0xAF +#define VHOST_GET_FEATURES _IOR(VHOST_VIRTIO, 0x00, __u64) +#define VHOST_SET_FEATURES _IOW(VHOST_VIRTIO, 0x00, __u64) +#define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01) +#define VHOST_SET_MEM_TABLE _IOW(VHOST_VIRTIO, 0x03, struct vhost_memory) +#define VHOST_SET_VRING_NUM _IOW(VHOST_VIRTIO, 0x10, struct vhost_vring_state) +#define VHOST_SET_VRING_ADDR _IOW(VHOST_VIRTIO, 0x11, struct vhost_vring_addr) +#define VHOST_SET_VRING_KICK _IOW(VHOST_VIRTIO, 0x20, struct vhost_vring_file) +#define VHOST_SET_VRING_CALL _IOW(VHOST_VIRTIO, 0x21, struct vhost_vring_file) +#define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file) +#define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64) +#define VHOST_NET_SET_BACKEND _IOW(VHOST_VIRTIO, 0x30, struct vhost_vring_file)
It's probably preferable to #include
+/** + * struct vq_state - Per-virtqueue local descriptor tracking + * @num_free: Number of descriptors ready to be announced + * to the kernel as available via rx_descriptor_handoff() + * @last_used_idx: Number of used-ring entries consumed so far; + * lagging read cursor vs. vring_used->idx (the + * kernel's write cursor) + */ +extern struct vq_state { + uint16_t num_free; + uint16_t last_used_idx; + uint16_t next_free; +} vqs[2];
This is a vhost-kernel relevant view of a vq - we also have vhost-user relevant views, which is a bit confusing. Renaming and/or moving to a vhost-kernel specific header is probably a good idea.
+extern struct vring_desc vring_desc[2][VHOST_NDESCS];
Do we need these externs, or could we make these structures local to the .c file actually doing the vhost-kernel handling?
+union vring_avail_u { + struct vring_avail avail; + char buf[offsetof(struct vring_avail, ring[VHOST_NDESCS])]; +}; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +extern union vring_avail_u vring_avail_all[2]; +#pragma GCC diagnostic pop + +union vring_used_u { + struct vring_used used; + char buf[offsetof(struct vring_used, ring[VHOST_NDESCS])]; +}; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +extern union vring_used_u vring_used_all[2]; +#pragma GCC diagnostic pop + +#define N_VHOST_REGIONS 12 +union vhost_memory_u { + struct vhost_memory mem; + char buf[offsetof(struct vhost_memory, regions[N_VHOST_REGIONS])]; +}; +extern union vhost_memory_u vhost_memory; + /** * struct vu_ring - Virtqueue rings * @num: Size of the queue @@ -147,6 +215,7 @@ struct vu_virtq_element { struct iovec *out_sg; };
+void vu_queue_notify(const struct vu_dev *dev, struct vu_virtq *vq); /** * has_feature() - Check a feature bit in a features set * @features: Features set @@ -199,4 +268,11 @@ void vu_queue_fill(const struct vu_dev *vdev, struct vu_virtq *vq, unsigned int idx); void vu_queue_flush(const struct vu_dev *vdev, struct vu_virtq *vq, unsigned int count); +void set_vring_for_queue(struct ctx *c, int queue_idx, int tap_fd); +int setup_memory_table(struct ctx *c); +void setup_vhost_net(struct ctx *c); +void setup_eventfds(struct ctx *c, int queue_idx); +void rx_descriptor_handoff(struct ctx *c); +void vhost_kick(struct vring_used *used, int kick_fd); +
We generally don't like to introduce function signatures separate from function implementations. It's sometimes a fuzzy line, but the idea is to split patches on logical concepts / subfeatures, not on different parts of the code for the same thing. That generally makes review easier.
#endif /* VIRTIO_H */ -- 2.34.1
-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
On Sun, Aug 02, 2026 at 01:21:52PM +0000, Ammar Yasser wrote:
- setup_vhost_net: try to open /dev/vhost-net and negotiate the needed features on the device file descriptor. Set the descriptor on c->fd_host on success. - setup_eventfds: setup the call, kick and err files for a given queue_index. - setup_memory_table: create the memory table object that gets shared with the kernel so it writes directly to the global pkt_buf on rx, and when we receive data from the host and eventually write it the protocol specific buffers the kernel is able to read from them. we share only buffers for tcp and udp as they are the only two protocols that will have vhost acceleration. besides, other protocols don't have a global buffer from which they allocate packets to send to the guest and would be trickier to support
Signed-off-by: Ammar Yasser
Similar to notes on the earlier patches, I think this will be clearer with the new vhost-kernel related functions in new .c and .h files, rather than mixed in with the general virtio helpers.
--- virtio.c | 283 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 282 insertions(+), 1 deletion(-)
diff --git a/virtio.c b/virtio.c index d7016cc..d89d485 100644 --- a/virtio.c +++ b/virtio.c @@ -72,19 +72,47 @@ * SUCH DAMAGE. */
+#include "passt.h" #include
+#include #include #include #include #include #include +#include #include +#include +#include +#include +#include +#include +#include +#include #include "util.h" #include "virtio.h" #include "vhost_user.h" +#include "tcp_buf.h" +#include "epoll_ctl.h" +#include "udp.h" + + +struct vq_state vqs[2];
'vqs' isn't really an adequate name for a global variable, especially since this is fully global, not 'static'.
+ +struct vring_desc vring_desc[2][VHOST_NDESCS] __attribute__((aligned(PAGE_SIZE))); + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +union vring_avail_u vring_avail_all[2] __attribute__((aligned(PAGE_SIZE))); +union vring_used_u vring_used_all[2] __attribute__((aligned(PAGE_SIZE))); +#pragma GCC diagnostic pop
-#define VIRTQUEUE_MAX_SIZE 1024 +union vhost_memory_u vhost_memory = { + .mem = { + .nregions = N_VHOST_REGIONS, + }, +};
/** * vu_gpa_to_va() - Translate guest physical address to our virtual address. @@ -766,3 +794,256 @@ void vu_queue_flush(const struct vu_dev *vdev, struct vu_virtq *vq, if ((uint16_t)(new - vq->signalled_used) < (uint16_t)(new - old)) vq->signalled_used_valid = false; } + +/** + * setup_vhost_net() - Open and negotiate features on /dev/vhost-net + * @c: Execution context; c->fd_vhost is set on success + * + */ +void setup_vhost_net(struct ctx *c) +{ + static const uint64_t req_features = + (1ULL << VIRTIO_F_VERSION_1) | (1ULL << VHOST_NET_F_VIRTIO_NET_HDR);
Since it's also 'const' anyway, I'm not sure the 'static' does anything useful.
+ int vhost_fd, rc; + + vhost_fd = open("/dev/vhost-net", O_RDWR | O_NONBLOCK | O_CLOEXEC); + if (vhost_fd < 0) + die_perror("Failed to open /dev/vhost-net");
We probably want to be able to fall back to the /dev/net/tun character device if vhost doesn't work. So making this setup function fallible would be preferable to using die() on errors.
+ rc = ioctl(vhost_fd, VHOST_SET_OWNER, NULL); + if (rc < 0) + die_perror("VHOST_SET_OWNER ioctl on /dev/vhost-net failed"); + + rc = ioctl(vhost_fd, VHOST_GET_FEATURES, &c->virtio_features); + if (rc < 0) + die_perror("VHOST_GET_FEATURES ioctl on /dev/vhost-net failed"); + + debug("vhost features: %lx", c->virtio_features); + debug("req features: %lx", req_features); + + c->virtio_features &= req_features; + if (c->virtio_features != req_features) + die("vhost does not support required features"); + + rc = ioctl(vhost_fd, VHOST_SET_FEATURES, &c->virtio_features); + if (rc < 0) + die_perror("VHOST_SET_FEATURES ioctl on /dev/vhost-net failed"); + + c->fd_vhost = vhost_fd; +} + +/** + * setup_eventfds() - Set up call/kick eventfds and vring size for one queue + * @c: Execution context; c->fd_vhost must already be set + * @queue_idx: Index of the queue (vring) to configure + * + */ +void setup_eventfds(struct ctx *c, int queue_idx) +{ + int vhost_fd = c->fd_vhost;
We use (when possible) the "reverse christmas tree" convention for ordering locals, which would but this further down (see CONTRIBUTING.md for more details).
+ struct vhost_vring_file call_file = { .index = queue_idx }; + struct vhost_vring_file kick_file = { .index = queue_idx }; + struct vhost_vring_file err_file = { .index = queue_idx }; + + struct vhost_vring_state state = { + .index = queue_idx, + .num = VHOST_NDESCS, + }; + union epoll_ref ref = { + .type = EPOLL_TYPE_VHOST_CALL, + .queue = queue_idx, + }; + struct epoll_event ev; + int rc; + + call_file.fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); + if (call_file.fd < 0) + die_perror("Failed to create call eventfd");
This error isn't particularly meaningful to an end user, since it doesn't mention vhost-kernel at all.
+ ref.fd = call_file.fd; + + rc = ioctl(vhost_fd, VHOST_SET_VRING_CALL, &call_file); + if (rc < 0) + die_perror("VHOST_SET_VRING_CALL ioctl on /dev/vhost-net failed"); + + ev = (struct epoll_event){ .data.u64 = ref.u64, .events = EPOLLIN }; + rc = epoll_ctl(c->epollfd, EPOLL_CTL_ADD, ref.fd, &ev); + if (rc < 0) + die_perror("Failed to add call eventfd to epoll"); + c->vq[queue_idx].call_fd = call_file.fd; + + err_file.fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); + if (err_file.fd < 0) + die_perror("Failed to create error eventfd"); + + rc = ioctl(vhost_fd, VHOST_SET_VRING_ERR, &err_file); + if (rc < 0) + die_perror("VHOST_SET_VRING_ERR ioctl on /dev/vhost-net failed"); + + ref.type = EPOLL_TYPE_VHOST_ERROR; + ref.fd = err_file.fd; + ev.data.u64 = ref.u64; + rc = epoll_ctl(c->epollfd, EPOLL_CTL_ADD, ref.fd, &ev); + if (rc < 0) + die_perror("Failed to add error eventfd to epoll"); + c->vq[queue_idx].err_fd = err_file.fd;
We'd generally prefer to use the existing epoll_add() helper, rather than open coding an EPOLL_CTL_ADD call.
+ + rc = ioctl(vhost_fd, VHOST_SET_VRING_NUM, &state); + if (rc < 0) { + die_perror("VHOST_SET_VRING_NUM ioctl on /dev/vhost-net failed (queue %d)", + queue_idx); + } + + kick_file.fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); + if (kick_file.fd < 0) + die_perror("Failed to create kick eventfd"); + + rc = ioctl(vhost_fd, VHOST_SET_VRING_KICK, &kick_file); + if (rc < 0) { + die_perror("VHOST_SET_VRING_KICK ioctl on /dev/vhost-net failed (queue %d)", + queue_idx); + } + + c->vq[queue_idx].kick_fd = kick_file.fd; + + vqs[queue_idx].num_free = VHOST_NDESCS; +} + +/** + * setup_memory_table() - Register the GPA/HVA translation table + * @c: Execution context; c->fd_vhost must already be set + * + * pasta has no real guest, so container->host addresses are 1:1 and can + * be interpreted directly rather than translated.
This is a little unclear, I'd say explicitly that we use a mapping where GPA == HVA.
+ */ +int setup_memory_table(struct ctx *c) { +#define VHOST_MEMORY_REGION_PTR(addr, size) \ + (struct vhost_memory_region) { \ + .guest_phys_addr = (uintptr_t)addr, \ + .memory_size = size, \ + .userspace_addr = (uintptr_t)addr, \ + } +#define VHOST_MEMORY_REGION(elem) VHOST_MEMORY_REGION_PTR(&elem, sizeof(elem))
"elem" is probably not a good name here. Here it's any contiguous variable, but in the vhost-user code, "elem" means a specific data structure within the descriptor rings.
+ + /* general purpose buffers */ + vhost_memory.mem.regions[0] = VHOST_MEMORY_REGION(pkt_buf); + vhost_memory.mem.regions[1] = VHOST_MEMORY_REGION(eth_pad); + + /* tcp specific buffers */ + vhost_memory.mem.regions[2] = VHOST_MEMORY_REGION(tcp_payload_tap_hdr); + vhost_memory.mem.regions[3] = VHOST_MEMORY_REGION(tcp4_payload_ip); + vhost_memory.mem.regions[4] = VHOST_MEMORY_REGION(tcp6_payload_ip); + vhost_memory.mem.regions[5] = VHOST_MEMORY_REGION(tcp_payload); + vhost_memory.mem.regions[6] = VHOST_MEMORY_REGION(tcp_eth_hdr); + + /* udp specific buffers */ + vhost_memory.mem.regions[7] = VHOST_MEMORY_REGION(udp_payload); + vhost_memory.mem.regions[8] = VHOST_MEMORY_REGION(udp_eth_hdr); + vhost_memory.mem.regions[9] = VHOST_MEMORY_REGION(udp_iov_recv); + vhost_memory.mem.regions[10] = VHOST_MEMORY_REGION(udp_mh_recv); + vhost_memory.mem.regions[11] = VHOST_MEMORY_REGION(udp_meta);
Not sure if there would be value in delegating these to helpers in tcp.c and udp.c
+ vhost_memory.mem.nregions = 12; + + return ioctl(c->fd_vhost, VHOST_SET_MEM_TABLE, &vhost_memory.mem); +} + + + +/** + * set_vring_for_queue() - Register a vring's addresses and bind its backend + * @c: Execution context; c->fd_vhost 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 set_vring_for_queue(struct ctx *c, int queue_idx, int tap_fd) +{ + int vhost_fd = c->fd_vhost; + 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], + }; + struct vhost_vring_file file = { + .index = queue_idx, + .fd = tap_fd, + }; + unsigned int i; + int rc; + + debug("qid: %d", queue_idx); + debug("vhost desc addr: 0x%llx", addr.desc_user_addr); + debug("vhost avail addr: 0x%llx", addr.avail_user_addr); + debug("vhost used addr: 0x%llx", addr.used_user_addr); + + rc = ioctl(vhost_fd, VHOST_SET_VRING_ADDR, &addr); + if (rc < 0) + die_perror("VHOST_SET_VRING_ADDR ioctl on /dev/vhost-net failed"); + + if (queue_idx == 0) { + for (i = 0; i < VHOST_NDESCS; ++i) { + vring_desc[0][i].addr = (uintptr_t)pkt_buf + + i * (PKT_BUF_BYTES / VHOST_NDESCS); + vring_desc[0][i].len = PKT_BUF_BYTES / VHOST_NDESCS; + vring_desc[0][i].flags = VRING_DESC_F_WRITE; + } + + for (i = 0; i < VHOST_NDESCS; ++i) + vring_avail_all[0].avail.ring[i] = htole16(i); + + rx_descriptor_handoff(c); + } + + if (queue_idx == 1) { + for (i = 0; i < (VHOST_NDESCS - 1); ++i) { + vring_desc[1][i].next = i+1; + } + } + + debug("qid: %d", file.index); + debug("tap fd: %d", file.fd); + rc = ioctl(vhost_fd, VHOST_NET_SET_BACKEND, &file); + if (rc < 0) + die_perror("VHOST_NET_SET_BACKEND ioctl on /dev/vhost-net failed"); +} + +/** + * rx_descriptor_handoff() - Batch-announce freed RX descriptors to the kernel + * @c: Execution context + * + * Bumps avail.idx by the number of descriptors accumulated in + * vqs[0].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 rx_descriptor_handoff(struct ctx *c)
Can this be static? That's the sort of thing that's harder to review when signatures are split from implementations. If not, it should have a properly prefixed name.
+{ + smp_wmb(); + + if (!vqs[0].num_free) + return; + + vring_avail_all[0].avail.idx += vqs[0].num_free; + vqs[0].num_free = 0; + vhost_kick(&vring_used_all[0].used, c->vq[0].kick_fd); +} + + +/** + * vhost_kick() - Notify the kernel that new descriptors are available + * @used: The vring_used queue. Taken as a parameter to check if the kernel + * virtio thread is actively reading descriptors or no + * @kick_fd: Which fd to notify about (will differ by which queue we are about + * to announce availability in) + */ +void vhost_kick(struct vring_used *used, int kick_fd) {
Again, does this need to be global?
+ /* Ensure that the read to 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); +} \ No newline at end of file ^ What diff said.
-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
participants (2)
-
Ammar Yasser
-
David Gibson