Re: [PATCH v2 08/13] udp_vu: Use iov_tail in udp_vu_prepare()

On 3/12/26 05:30, David Gibson wrote:

On Mon, Mar 09, 2026 at 10:47:39AM +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 --- iov.c | 1 - udp_vu.c | 64 ++++++++++++++++++++++++++++++-------------------------- 2 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/iov.c b/iov.c index 296f24b61067..1f554f5ac297 100644 --- a/iov.c +++ b/iov.c @@ -313,7 +313,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 2a5d3f822bf6..a21a03dbf23e 100644 --- a/udp_vu.c +++ b/udp_vu.c @@ -101,52 +101,54 @@ static ssize_t udp_vu_sock_recv(struct iovec *iov, size_t *cnt, 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;

Oof, having to reset the tail to the original position in order to replay the PUTs in the right order seems kind of fragile. I'm beginning to wonder if for writing (not reading) headers, trying to use an in-place pointer is more trouble than it's worth. It would certainly be easier to reason about this, if you always construct the header in an external buffer, then there's just a sequence of IOV_PUT_HEADER()s to stack them into the iov tail.

I can use one PEEK and PUT with something like: struct { struct ethhdr eh; struct struct iphdr struct udphdr *uh } __attribute__((__packed__)) *hdr4, hdr4_storage; hdr4 = IOV_PEEK_HEADER(¤t, hdr4_storage); ... IOV_PUT_HEADER(¤t, hdr4); or as you proposed: with_header(hdr4, ¤t) { .... } Thanks, Laurent