Re: [RFC PATCH 1/3] vu_common: Stick to size of input buffer in vu_send_single()

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 4 Nov 2025 16:01:07 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> On 11/3/25 11:16, Stefano Brivio wrote:
> > ...instead of copying, from the input buffer, the amount of available
> > bytes in the buffer provided via vhost-user.
> > 
> > The existing behaviour should be harmless due to the fact that we
> > don't request overly large buffers from vhost-user, but it doesn't
> > look correct to me.  
> 
> I don't understand where is the problem.
> 
> I think the existing behaviour is correct because the iov array is padded to size in 
> vu_collect() by:
> 
>                  if (iov->iov_len > size - current_size)
>                          iov->iov_len = size - current_size;

Ah, thanks, this is the part I missed. So the buffers could be bigger,
but @frame_size set on return is <= @size. I was misled by:

 * @frame_size:		The total size of the buffers (output)

because that made me think that 'total' would be... well, the total
size of the (output) buffers, not limited to what we requested.

I wonder if we should fix:

- this comment (I would call this "Available buffer length, up to
  @size, set on return") and perhaps the parameter name ("@collected"?)

- the prototype. I see why UDP needs it this way, but wouldn't it be
  more natural to return error if the requested size is not available,
  and set the available buffer count as optional argument on return?

  The way it currently is, we're mostly ignoring the value of
  @frame_size (except for what we need for a comparison) in the only two
  cases where we actually pass a pointer.

  Well, we use that value in tcp_vu_sock_recv(), but just as a sum of
  values we already have in the caller.

- or the interface altogether like I'm doing here, so that vu_collect()
  actually returns the size of the buffers. I think it's more natural
  like that but also looks a bit more complicated so I'm not
  necessarily fond of this

What do you think? I can just fix the comment instead (feel free to do
so, of course).

> total can be lesser than size but in this case we exit.
> 
> So total should be equal to input size + sizeof(struct virtio_net_hdr_mrg_rxbuf).
> 
> And in iov_from_buf() we copy the content of the buffer according the available room in 
> the iovec and the size of the input buffer.

Right, I see now. I wonder if 3/3 is even desired in this case, after
all it's guest memory that's given to us but we don't touch in any way.

If buffers are consistently much larger than what we request (for
example, TCP flows with low MSS), we probably introduce substantial
overhead by zeroing those bytes.

I thought they were filled with random stuff because of the issue I
though we had, but we don't have it, so no need to zero them either.

-- 
Stefano