Re: [PATCH v6 1/3] udp_vu: Allow virtqueue elements with multiple iovec entries

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 3 Apr 2026 17:18:23 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 4/3/26 13:53, Stefano Brivio wrote:
> > On Wed,  1 Apr 2026 21:23:24 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> The previous code assumed a 1:1 mapping between virtqueue elements and
> >> iovec entries (enforced by an assert).  Drop that assumption to allow
> >> elements that span multiple iovecs: track elem_used separately by
> >> walking the element list against the iov count returned after padding.
> >> This also fixes vu_queue_rewind() and vu_flush() to use the element
> >> count rather than the iov count.
> >>
> >> Use iov_tail_clone() in udp_vu_sock_recv() to handle header offset,
> >> replacing the manual base/len adjustment and restore pattern.
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >>   udp_vu.c | 29 ++++++++++++++---------------
> >>   1 file changed, 14 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/udp_vu.c b/udp_vu.c
> >> index 30af64034516..5608a3a96ff5 100644
> >> --- a/udp_vu.c
> >> +++ b/udp_vu.c
> >> @@ -64,30 +64,25 @@ static size_t udp_vu_hdrlen(bool v6)
> >>    */
> >>   static ssize_t udp_vu_sock_recv(struct iovec *iov, size_t *cnt, int s, bool v6)
> >>   {
> >> +	struct iovec msg_iov[*cnt];  
> > 
> > Variable-length Arrays (VLAs) are allowed starting from C99 but we
> > should really really avoid them. If 'cnt' is big enough, we risk
> > writing all over the place. That's the main reason why they were more
> > or less banned from the Linux kernel some years ago and eventually
> > eradicated:  
> 
> I can use alloca() if you prefer ;)

Claude, is this you? ;)

I guess if you come up with a sufficient convoluted maze of "elem" /
"iov" / "head" macros using concatenation to strategically place calls
to strndupa(), with an abstraction based on IOV "tails" called "strain"
indicated by "strn" for brevity... one day I might miss that, yes.

But I'll try to remember that, the next time we discuss whether it's
really needed to duplicate strain A or if a single copy of it is enough.

> >    https://lore.kernel.org/lkml/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com/
> > 
> >    https://lore.kernel.org/lkml/20181028172401.GA41102@beast/
> > 
> > Can we use VIRTQUEUE_MAX_SIZE as upper bound like udp_vu_sock_to_tap()
> > does?  
> 
> Yes, but the idea here is we have always *cnt < VIRTQUEUE_MAX_SIZE
> (because the value comes from vu_collect() and in vu_collect():
> *in_sg (&iov_cnt or *cnt) < max_in_sg (ARRAY_SIZE(iov_vu) or VIRTQUEUE_MAX_SIZE or 1024)

...until somebody, running this somewhere where we don't have gcc's
stack protector stuff (or equivalent), without having quite obtained
full arbitrary code execution yet, finds a way to manipulate *cnt...

> And vu_collect(), in this case, sets generally *in_sg to a value lower than 44:
> we want to create a frame of ETH_MAX_MTU by coalescing kernel buffers of size ETH_FRAME_LEN)

If it _can_ be 16 KiB, then I would suggest it's better to _just_ have
16 KiB. It's more auditable, and it's not like we "allocate" it anyway.

On top of it, udp_vu_sock_to_tap() already does that, and other
functions (with potentially deeper call trees) do worse. I'm not
claiming it's a good idea to do it "because it's bad anyway", in
general, but in this case the maximum is what matters.

> For me 16 kB on the stack is a lot of memory (but I started programming on a 48 kB RAM 
> computer...).

I guess I started with around ten times that but I tend to agree. What
I'm suggesting is that if it can be a lot, better just make it that lot.

An alternative I'm pondering about is whether we can make things
recoverably / gracefully fail if that's > 64 or something like that. At
this point we still have that data on the socket and we could dequeue
it in a later pass I suppose. But maybe it gets very complicated...

-- 
Stefano