Re: [PATCH v2 5/7] tap, tcp, udp: Prepare the to-guest path for vhost-net

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

From: Ammar Yasser

Frames sent through vhost-net carry a virtio-net header where pasta's tap header would otherwise go, this necessitates the following two changes:

- udp_tap_prepare() has been modified to take a context parameter so it can check if vhost is set up. Only if it isn't should tap_hdr_update be called because this means that the active union variant is vnet_len no the virtio net header - tap_l2_offset() a new helper, has been introduced which tells pcap_multiple() how many bytes precede the L2 frame, which now varies with vhost as well as with the mode

tap_send_single() calls tap_send_frames_passt() and tap_send_frames_pasta() directly rather than going through tap_send_frames(). Its callers - ARP, TCP resets, and DHCP, DHCPv6, NDP and ICMP via tap_udp*_send()/tap_icmp*_send() - build their frame in a buffer on the stack, not in the buffers registered with vhost-net. The kernel can only read from registered memory, so these protocols can never take the vhost path and shouldn't be tested for it on every frame.

Signed-off-by: Eugenio Pérez Signed-off-by: Ammar Yasser --- tap.c | 112 ++++++++++++++++++++++++++++++++++-------------------- tcp_buf.c | 12 ++++-- udp.c | 27 +++++++++---- 3 files changed, 99 insertions(+), 52 deletions(-)

diff --git a/tap.c b/tap.c index a5fdee8..fd500f1 100644 --- a/tap.c +++ b/tap.c @@ -123,45 +123,6 @@ unsigned long tap_l2_max_len(const struct ctx *c) return 0; /* Unreachable, for cppcheck's sake */ }

-/** - * tap_send_single() - Send a single frame - * @c: Execution context - * @data: Packet buffer - * @l2len: Total L2 packet length - */ -void tap_send_single(const struct ctx *c, const void *data, size_t l2len) -{ - uint8_t padded[ETH_ZLEN] = { 0 }; - struct iovec iov[2]; - size_t iovcnt = 0; - uint32_t vnet_len; - - if (l2len < ETH_ZLEN) { - memcpy(padded, data, l2len); - data = padded; - l2len = ETH_ZLEN; - } - - vnet_len = htonl(l2len); - - switch (c->mode) { - case MODE_PASST: - iov[iovcnt] = IOV_OF_LVALUE(vnet_len); - iovcnt++; - /* fall through */ - case MODE_PASTA: - iov[iovcnt].iov_base = (void *)data; - iov[iovcnt].iov_len = l2len; - iovcnt++; - - tap_send_frames(c, iov, iovcnt, 1); - break; - case MODE_VU: - vu_send_single(c, data, l2len); - break; - } -} - /** * tap_push_l2h() - Build an L2 header for an inbound packet * @c: Execution context @@ -497,6 +458,76 @@ static size_t tap_send_frames_passt(const struct ctx *c, return i / bufs_per_frame; }

+/** + * tap_l2_offset() - Backend specific header size preceding each L2 frame + * @c: Execution context + * + * Return: offset of the L2 frame within each frame's first buffer + */ +static size_t tap_l2_offset(const struct ctx *c) +{ + if (c->mode == MODE_PASST) + return sizeof(uint32_t); /* vnet_len */ + + if (c->vhost.fd != -1) + return VNET_HLEN; + + return 0; +} + +/** + * tap_send_single() - Send a single frame + * @c: Execution context + * @data: Packet buffer + * @l2len: Total L2 packet length + */ +void tap_send_single(const struct ctx *c, const void *data, size_t l2len) +{ + uint8_t padded[ETH_ZLEN] = { 0 }; + struct iovec iov[2]; + uint32_t vnet_len; + size_t iovcnt = 0; + size_t m = 0; + + if (l2len < ETH_ZLEN) { + memcpy(padded, data, l2len); + data = padded; + l2len = ETH_ZLEN; + } + + vnet_len = htonl(l2len); + + switch (c->mode) { + case MODE_PASST: + /* create an iov for the length */ + iov[iovcnt] = IOV_OF_LVALUE(vnet_len); + iovcnt++; + /* create the data iov */ + iov[iovcnt].iov_base = (void *)data; + iov[iovcnt].iov_len = l2len; + iovcnt++; + + m = tap_send_frames_passt(c, iov, iovcnt, 1); + break; + case MODE_PASTA: + /* don't create a length iov in the case of pasta */ + iov[iovcnt].iov_base = (void *)data; + iov[iovcnt].iov_len = l2len; + iovcnt++; + + m = tap_send_frames_pasta(c, iov, iovcnt, 1); + break; + case MODE_VU: + vu_send_single(c, data, l2len); + return; + } + + if (!m) + debug("tap: failed to send a single frame"); + + pcap_multiple(iov, iovcnt, m, tap_l2_offset(c));

for MODE_PASTA with vhost, tap_l2_offset(c) is VNET_HLEN, but in "case MODE_PASTA" iov_base points directly to data, so it should be 0.

+} + /** * tap_send_frames() - Send out multiple prepared frames * @c: Execution context @@ -537,8 +568,7 @@ size_t tap_send_frames(const struct ctx *c, const struct iovec *iov, debug("tap: failed to send %zu frames of %zu", nframes - m, nframes);

- pcap_multiple(iov, bufs_per_frame, m, - c->mode == MODE_PASST ? sizeof(uint32_t) : 0); + pcap_multiple(iov, bufs_per_frame, m, tap_l2_offset(c));

return m; } diff --git a/tcp_buf.c b/tcp_buf.c index dd80384..493c5c9 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -21,6 +21,7 @@ #include

#include +#include

#include "util.h" #include "ip.h" @@ -44,7 +45,7 @@ /* Ethernet header for IPv4 and IPv6 frames */ static struct ethhdr tcp_eth_hdr[TCP_FRAMES_MEM];

-static struct tap_hdr tcp_payload_tap_hdr[TCP_FRAMES_MEM]; +static struct virtio_net_hdr_mrg_rxbuf tcp_payload_tap_hdr[TCP_FRAMES_MEM];

We can keep "struct tap_hdr" as it is an union of struct virtio_net_hdr_mrg_rxbuf.>

/* IP headers for IPv4 and IPv6 */ static struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; @@ -113,7 +114,7 @@ void tcp_sock_iov_init(const struct ctx *c) for (i = 0; i < TCP_FRAMES_MEM; i++) { struct iovec *iov = tcp_l2_iov[i];

- iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp_payload_tap_hdr[i]); + iov[TCP_IOV_TAP] = tap_hdr_iov(c, (struct tap_hdr *)&tcp_payload_tap_hdr[i]);

and then no need of the cast.

iov[TCP_IOV_ETH].iov_len = sizeof(struct ethhdr); iov[TCP_IOV_PAYLOAD].iov_base = &tcp_payload[i]; iov[TCP_IOV_ETH_PAD].iov_base = eth_pad; @@ -216,7 +217,12 @@ static void tcp_l2_buf_fill_headers(const struct ctx *c,

l2len = tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &tail, iov_tail_size(&tail), csum_flags, seq); - tap_hdr_update(taph, l2len); + + /* With vhost-net this buffer holds a virtio-net header, which a tap + * one must not be written over + */ + if (c->vhost.fd == -1) + tap_hdr_update(taph, l2len); }

/** diff --git a/udp.c b/udp.c index a4355a1..afe1c7f 100644 --- a/udp.c +++ b/udp.c @@ -103,6 +103,7 @@ #include #include #include +#include

#include "checksum.h" #include "util.h" @@ -149,12 +150,16 @@ 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 + * @vnet_hdr: virtio-net header, used when sending through vhost-net + * @taph: Tap backend specific header, used otherwise */ static struct udp_meta_t { struct ipv6hdr ip6h; struct iphdr ip4h; - struct tap_hdr taph; + union { + struct virtio_net_hdr_mrg_rxbuf vnet_hdr; + struct tap_hdr taph; + };

Same here, "struct tap_hdr" is an union of "struct virtio_net_hdr_mrg_rxbuf"

} #ifdef __AVX2__ __attribute__ ((aligned(32))) @@ -356,13 +361,15 @@ static void udp_tap_pad(struct iovec *iov)

/** * udp_tap_prepare() - Convert one datagram into a tap frame + * @c: Execution context * @mmh: Receiving mmsghdr array * @idx: Index of the datagram to prepare * @tap_omac: MAC address of remote endpoint as seen from the guest * @toside: Flowside for destination side * @no_udp_csum: Do not set UDP checksum */ -static void udp_tap_prepare(const struct mmsghdr *mmh, +static void udp_tap_prepare(const struct ctx *c, + const struct mmsghdr *mmh, unsigned int idx, const uint8_t *tap_omac, const struct flowside *toside, @@ -384,8 +391,10 @@ static void udp_tap_prepare(const struct mmsghdr *mmh, udp_update_hdr6(&bm->ip6h, uh, &payload, toside, mmh[idx].msg_len, no_udp_csum);

- l2len = MAX(l4len + sizeof(bm->ip6h) + ETH_HLEN, ETH_ZLEN); - tap_hdr_update(&bm->taph, l2len); + if (c->mode == MODE_PASST) { + l2len = MAX(l4len + sizeof(bm->ip6h) + ETH_HLEN, ETH_ZLEN); + tap_hdr_update(&bm->taph, l2len); + }

eh->h_proto = htons_constant(ETH_P_IPV6); (*tap_iov)[UDP_IOV_IP] = IOV_OF_LVALUE(bm->ip6h); @@ -393,8 +402,10 @@ static void udp_tap_prepare(const struct mmsghdr *mmh, udp_update_hdr4(&bm->ip4h, uh, &payload, toside, mmh[idx].msg_len, no_udp_csum);

- l2len = MAX(l4len + sizeof(bm->ip4h) + ETH_HLEN, ETH_ZLEN); - tap_hdr_update(&bm->taph, l2len); + if (c->mode == MODE_PASST) { + l2len = MAX(l4len + sizeof(bm->ip4h) + ETH_HLEN, ETH_ZLEN); + tap_hdr_update(&bm->taph, l2len); + }

eh->h_proto = htons_constant(ETH_P_IP); (*tap_iov)[UDP_IOV_IP] = IOV_OF_LVALUE(bm->ip4h); @@ -867,7 +878,7 @@ static void udp_buf_sock_to_tap(const struct ctx *c, int s, int n, fwd_neigh_mac_get(c, &toside->oaddr, omac);

for (i = 0; i < n; i++) - udp_tap_prepare(udp_mh_recv, i, omac, toside, false); + udp_tap_prepare(c, udp_mh_recv, i, omac, toside, false);

tap_send_frames(c, &udp_l2_iov[0][0], UDP_NUM_IOVS, n); } -- 2.39.5 (Apple Git-154)