From: Laurent Vivier <lvivier(a)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(a)gibson.dropbear.id.au> [sbrivio: commit message, albeit not really descriptive] Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- 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; -- 2.43.0
On Tue, Feb 11, 2025 at 08:51:33PM +0100, Stefano Brivio wrote:From: Laurent Vivier <lvivier(a)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(a)gibson.dropbear.id.au> [sbrivio: commit message, albeit not really descriptive] Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>I think I've understood the surrounding code enough to say, Reviewed-by: David Gibson <david(a)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
On Wed, 12 Feb 2025 11:57:10 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:On Tue, Feb 11, 2025 at 08:51:33PM +0100, Stefano Brivio wrote:Ah, great, that starts making sense. Applied with this commit message. -- StefanoFrom: Laurent Vivier <lvivier(a)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(a)gibson.dropbear.id.au> [sbrivio: commit message, albeit not really descriptive] Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>I think I've understood the surrounding code enough to say, Reviewed-by: David Gibson <david(a)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. ###