public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: passt-dev@passt.top, Laurent Vivier <lvivier@redhat.com>
Subject: Re: [PATCH] tcp_vu: Fix off-by one in header count array adjustment
Date: Wed, 12 Feb 2025 11:57:10 +1100	[thread overview]
Message-ID: <Z6vx5tj4gr004rFr@zatzit> (raw)
In-Reply-To: <20250211195133.198082-1-sbrivio@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1972 bytes --]

On Tue, Feb 11, 2025 at 08:51:33PM +0100, Stefano Brivio wrote:
> From: Laurent Vivier <lvivier@redhat.com>
> 
> Not entirely clear to me why, but Laurent proposed this patch to fix
> an issue were we would end up with a zero buf_cnt in
> tcp_vu_data_from_sock() and all sorts of weirdnesses.
> 
> Reported-by: David Gibson <david@gibson.dropbear.id.au>
> [sbrivio: commit message, albeit not really descriptive]
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

I think I've understood the surrounding code enough to say,

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

and offer this possible description if we don't hear from Laurent in
time.

###

head_cnt represents the number of frames we're going to forward to the
guest in tcp_vu_sock_recv(), each of which could require multiple
buffers ("elements").  We initialise it with as many frames as we can
find space for in vu buffers, and we then need to adjust it down to
the number of frames we actually (partially) filled.

We adjust it down based on number of individual buffers used by the
data from recvmsg().  At this point 'i' is *one greater than* that
number of buffers, so we need to discard all (unused) frames with a
buffer index >= i, instead of > i.

###

> ---
>  tcp_vu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tcp_vu.c b/tcp_vu.c
> index fad7065..0622f17 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -261,7 +261,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c,
>  		len -= iov->iov_len;
>  	}
>  	/* adjust head count */
> -	while (head_cnt > 0 && head[head_cnt - 1] > i)
> +	while (head_cnt > 0 && head[head_cnt - 1] >= i)
>  		head_cnt--;
>  	/* mark end of array */
>  	head[head_cnt] = i;

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2025-02-12  0:57 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-11 19:51 [PATCH] tcp_vu: Fix off-by one in header count array adjustment Stefano Brivio
2025-02-12  0:57 ` David Gibson [this message]
2025-02-12 19:44   ` Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z6vx5tj4gr004rFr@zatzit \
    --to=david@gibson.dropbear.id.au \
    --cc=lvivier@redhat.com \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).