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

On 11/4/25 17:09, Stefano Brivio wrote:

On Tue, 4 Nov 2025 16:01:07 +0100 Laurent Vivier 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). There is a problem in the code as it's not obvious how it works (some part are a little bit touchy...). So I think it needs to be changed, not especially in the logic but in the clarity.

I would say that it would be more natural to return an error if the requested size is not available. Choose the solution you prefer to be clear for you but avoid to change the behaviour as it can introduce unexpected results... Thanks, Laurent