Re: [PATCH v5 29/29] packet: use buf to store iovec array

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 27 May 2025 15:16:04 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 26/05/2025 16:21, Stefano Brivio wrote:
> > On Thu, 17 Apr 2025 18:51:36 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> When we use vhost-user we don't use the memory buffer
> >> of the pool to store the packet, so we can use it to
> >> store iovec array that points to the memory provided
> >> by vhost-user.
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >>   packet.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++++------
> >>   1 file changed, 147 insertions(+), 16 deletions(-)  
> >...  
> >> +/**
> >> + * packet_iov_next_idx() - Give the the next available iovec index
> >> + * @p:		Pointer to packet pool
> >> + * @idx:	Index of packet descriptor in pool
> >> + * @func:	For tracing: name of calling function
> >> + * @line:	For tracing: caller line of function call
> >> + *
> >> + * Return: the next available iovec index
> >> + */
> >> +static size_t packet_iov_next_idx(const struct pool *p, size_t idx,
> >> +				  const char *func, int line)
> >> +{
> >> +	size_t iov_idx, iov_cnt;
> >> +
> >> +	if (idx == 0)
> >> +		return 0;  
> > 
> > I'm a bit lost here. Why is 0 a special value now?  
> 
> See below: we use "idx - 1"
> 
> >   
> >> +
> >> +	iov_idx = packet_iov_idx(p, idx - 1, &iov_cnt, func, line);
> >> +
> >> +	return iov_idx + iov_cnt;
> >> +}
> >> +  
> 
> The next available iovec index for a given packet index is computed using the information 
> from the previous packet index:
> 
> We take first available iovec index after the iovecs used by the previous packet.
> 
> The next available iovec index is the iovec index base of the previous packet plus all the 
> iovecs used by the previous packet:
> 
> next available iovec index = (base iovec index of previous packet) + (number of iovec used 
> by previous packet)
> 
> So, for the first packet (packet index 0), the first available iovec index is 0, we can't 
> compute it from the previous packet because it doesn't exist (and all the iovec are free 
> and available)

Ah-ha, thanks, I followed through the whole path again and it makes
sense now. Maybe we could change the comment to @idx, or say
"subsequent" instead of next... but I can't come up with a good idea at
the moment.

-- 
Stefano