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

[Laurent Vivier <lvivier@redhat.com>]

On 11/4/25 17:09, Stefano Brivio wrote:
> 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).
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