On Fri, Feb 27, 2026 at 03:03:25PM +0100, Laurent Vivier wrote:
Rework udp_vu_prepare() to use IOV_REMOVE_HEADER() and IOV_PUT_HEADER() to walk through Ethernet, IP and UDP headers instead of the layout-specific helpers (vu_eth(), vu_ip(), vu_payloadv4(), vu_payloadv6()) that assume a contiguous buffer. The payload length is now implicit in the iov_tail, so drop the dlen parameter.
Signed-off-by: Laurent Vivier
LGTM, a few nits below.
--- iov.c | 1 - udp_vu.c | 63 ++++++++++++++++++++++++++++++-------------------------- 2 files changed, 34 insertions(+), 30 deletions(-)
diff --git a/iov.c b/iov.c index 2cf23d284e4a..7c9641968271 100644 --- a/iov.c +++ b/iov.c @@ -304,7 +304,6 @@ void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align) * * Return: number of bytes written */ -/* cppcheck-suppress unusedFunction */ size_t iov_put_header_(struct iov_tail *tail, const void *v, size_t len) { size_t l = len; diff --git a/udp_vu.c b/udp_vu.c index dd8904d65a38..6d87f4872268 100644 --- a/udp_vu.c +++ b/udp_vu.c @@ -101,52 +101,54 @@ static ssize_t udp_vu_sock_recv(struct iov_tail *data, int s, bool v6) * @c: Execution context * @data: IO vector tail for the frame * @toside: Address information for one side of the flow - * @dlen: Packet data length * * Return: Layer-4 length */ static size_t udp_vu_prepare(const struct ctx *c, const struct iov_tail *data, - const struct flowside *toside, ssize_t dlen) + const struct flowside *toside) { - const struct iovec *iov = data->iov; - struct ethhdr *eh; + struct iov_tail current = *data; + struct ethhdr *eh, eh_storage; + struct udphdr *uh, uh_storage; size_t l4len;
/* ethernet header */ - eh = vu_eth(iov[0].iov_base); + eh = IOV_REMOVE_HEADER(¤t, eh_storage);
memcpy(eh->h_dest, c->guest_mac, sizeof(eh->h_dest)); memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source));
/* initialize header */ if (inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr)) { - struct iphdr *iph = vu_ip(iov[0].iov_base); - struct udp_payload_t *bp = vu_payloadv4(iov[0].iov_base); - const struct iovec payload_iov = { - .iov_base = bp->data, - .iov_len = dlen, - }; - struct iov_tail payload = IOV_TAIL(&payload_iov, 1, 0); + struct iphdr *iph, iph_storage;
eh->h_proto = htons(ETH_P_IP);
+ iph = IOV_REMOVE_HEADER(¤t, iph_storage); *iph = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_UDP);
- l4len = udp_update_hdr4(iph, &bp->uh, &payload, toside, true); + uh = IOV_REMOVE_HEADER(¤t, uh_storage); + l4len = udp_update_hdr4(iph, uh, ¤t, toside, true); + + current = *data; + IOV_PUT_HEADER(¤t, eh); + IOV_PUT_HEADER(¤t, iph); + IOV_PUT_HEADER(¤t, uh);
The puts for eh and uh can be factored out of the if/else.
} else { - struct ipv6hdr *ip6h = vu_ip(iov[0].iov_base); - struct udp_payload_t *bp = vu_payloadv6(iov[0].iov_base); - const struct iovec payload_iov = { - .iov_base = bp->data, - .iov_len = dlen, - }; - struct iov_tail payload = IOV_TAIL(&payload_iov, 1, 0); + struct ipv6hdr *ip6h, ip6h_storage;
eh->h_proto = htons(ETH_P_IPV6);
+ ip6h = IOV_REMOVE_HEADER(¤t, ip6h_storage); *ip6h = (struct ipv6hdr)L2_BUF_IP6_INIT(IPPROTO_UDP);
- l4len = udp_update_hdr6(ip6h, &bp->uh, &payload, toside, true); + uh = IOV_REMOVE_HEADER(¤t, uh_storage); + l4len = udp_update_hdr6(ip6h, uh, ¤t, toside, true); + + current = *data; + IOV_PUT_HEADER(¤t, eh); + IOV_PUT_HEADER(¤t, ip6h); + IOV_PUT_HEADER(¤t, uh); }
return l4len; @@ -165,9 +167,10 @@ static void udp_vu_csum(const struct flowside *toside, struct iov_tail payload = *data; struct udphdr *uh, uh_storage; bool ipv4 = src4 && dst4; + int hdrlen = sizeof(struct ethhdr) + + (ipv4 ? sizeof(struct iphdr) : sizeof(struct ipv6hdr));
- iov_drop_header(&payload, - udp_vu_hdrlen(!ipv4) - sizeof(struct udphdr)); + iov_drop_header(&payload, hdrlen);
udp_vu_prepare() and udp_vu_csum() independently locate the UDP header, but they're called in fairly close proximity. Would it make more sense to pass the UDP header and payload tail separately to each of them?
uh = IOV_REMOVE_HEADER(&payload, uh_storage);
if (ipv4) @@ -208,8 +211,8 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, int n, flow_sidx_t tosidx) }
for (i = 0; i < n; i++) { + int elem_cnt, elem_used; ssize_t dlen; - int elem_cnt;
vu_init_elem(elem, iov_vu, ARRAY_SIZE(elem));
@@ -225,18 +228,20 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, int n, flow_sidx_t tosidx) vu_queue_rewind(vq, elem_cnt); continue; } + elem_used = data.cnt;
/* release unused buffers */ - vu_queue_rewind(vq, elem_cnt - data.cnt); + vu_queue_rewind(vq, elem_cnt - elem_used);
if (data.cnt > 0) { - vu_set_vnethdr(vdev, data.iov[0].iov_base, data.cnt); - udp_vu_prepare(c, &data, toside, dlen); + vu_set_vnethdr(vdev, data.iov[0].iov_base, elem_used); + iov_drop_header(&data, VNET_HLEN); + udp_vu_prepare(c, &data, toside); if (*c->pcap) { udp_vu_csum(toside, &data); - pcap_iov(data.iov, data.cnt, VNET_HLEN); + pcap_iov(data.iov, data.cnt, 0); } - vu_flush(vdev, vq, elem, data.cnt); + vu_flush(vdev, vq, elem, elem_used); } } } -- 2.53.0
-- 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