public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: Laurent Vivier <lvivier@redhat.com>
Cc: passt-dev@passt.top
Subject: Re: [PATCH v7 00/31] Introduce discontiguous frames management
Date: Fri, 25 Jul 2025 16:18:23 +0200	[thread overview]
Message-ID: <20250725161823.4329b411@elisabeth> (raw)
In-Reply-To: <a7496d32-c6a1-4f48-979f-16c9e8a7b611@redhat.com>

On Fri, 25 Jul 2025 13:45:36 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 24/07/2025 15:01, Laurent Vivier wrote:
> > On 18/07/2025 20:45, Stefano Brivio wrote:  
> >> On Mon, 23 Jun 2025 13:06:04 +0200
> >> Laurent Vivier <lvivier@redhat.com> wrote:
> >>  
> >>> This series introduces iov_tail to convey frame information
> >>> between functions.
> >>>
> >>> This is only an API change, for the moment the memory pool
> >>> is only able to store contiguous buffer, so, except for
> >>> vhost-user in a special case, we only play with iovec array
> >>> with only one entry.
> >>>
> >>> v7:
> >>>      - Add a patch to fix comment style of 'Return:'
> >>>      - Fix ignore_arp()/accept_arp()
> >>>      - Fix coverity error
> >>>      - Fix several comments  
> >>
> >> I was about to apply this without 1/31 (I applied the v2 of it you sent
> >> outside of this series instead, which is actually up to date) and with
> >> the minor comment fix to 31/31... but the test perf/passt_vu_tcp fails
> >> rather consistently now (and I triple checked without this series):
> >>
> >> - "TCP throughput over IPv6: guest to host" with MTU 1500 and 9000
> >>    bytes now reports between 0 and 0.6 Gbps. The guest kernel prints a
> >>    series of two messages with ~1-10 µs interval:
> >>
> >> [   21.159827] TCP: out of memory -- consider tuning tcp_mem
> >> [   21.159831] TCP: out of memory -- consider tuning tcp_mem
> >>
> >> - "TCP throughput over IPv4: guest to host" never reports 0 Gbps, but
> >>    the throughput figure for large MTU (65520 bytes) is very low (5.4
> >>    Gbps in the last run). Here I'm getting four messages:
> >>
> >> [   40.807818] TCP: out of memory -- consider tuning tcp_mem
> >> [   40.807829] TCP: out of memory -- consider tuning tcp_mem
> >> [   40.807829] TCP: out of memory -- consider tuning tcp_mem
> >> [   40.807830] TCP: out of memory -- consider tuning tcp_mem
> >>
> >> - on the reverse direction, "TCP throughput over IPv4: host to guest"
> >>    (but not with IPv6), the iperf3 client gets SIGSEGV, but not
> >>    consistently, it happened once out of five times.
> >>
> >> To me it smells a bit like we're leaking virtqueue slots but I looked
> >> again at the whole series and I couldn't find anything obvious... at
> >> least not yet.
> >>
> >> UDP tests never fail and the throughput is the same as before.
> >>  
> > 
> > I think the problem is the way we use the iovec array.
> > 
> > In tap4_handler() we have a packet_get() that provides a pointer to the iovec array from 
> > pool. Idx is 0, iovec idx is 0.
> > 
> > Then we have a pool_flush(), so first available idx is now 0 again.
> > 
> > And then we have packet_add() with the iovec idx (in "data") of the previous packet_get() 
> > that we try to add at the same index (as pool is empty again, and first available idx is 0).
> > 
> > When I wrote this patch I guessed that when we release packet (pool_flush()) we don't use 
> > anymore the iovec array of the packet, it appears to be not true.  
> 
> Could you try the following patch (my iperf3 continue to crash <defunct> on my host 
> system, not related to this problem), it's a little bit ugly (use of alloca()) but it's an 
> easy fix.

Weird, I don't get the "TCP: out of memory" messages anymore, but
throughput still has some obvious issues:

=== perf/passt_vu_tcp
> passt: throughput and latency
Throughput in Gbps, latency in µs, 6 threads at 3.6 GHz
                            MTU: |  256B  |  576B  |  1280B |  1500B |  9000B | 65520B |
                                 |--------|--------|--------|--------|--------|--------|
  TCP throughput over IPv6: guest to host   |      - |      - |    0.6 |    2.0 |      0 |   13.3 |
  TCP RR latency over IPv6: guest to host   |      - |      - |      - |      - |      - |     38 |
   TCP CRR latency over IPv6: guest to host   |      - |      - |      - |      - |      - |     95 |
                                 |--------|--------|--------|--------|--------|--------|
  TCP throughput over IPv4: guest to host   |      0 |    2.2 |    1.0 |    1.7 |    9.9 |   16.7 |
  TCP RR latency over IPv4: guest to host   |      - |      - |      - |      - |      - |     35 |
   TCP CRR latency over IPv4: guest to host   |      - |      - |      - |      - |      - |     91 |
                                 |--------|--------|--------|--------|--------|--------|

...we should never have < 0.1 Gbps, and with 64k MTU, we otherwise have
30 - 40 Gbps.

Even more weird, iperf3 now reliably crashes for me too, on the other path
(host to guest). It never did without this series.

I wonder if the issue is actually "fixed" with this patch but the alloca()
is so large as to actually mess with throughput. I can play with that if
it helps, or bisect... let me know.

I have to admit I didn't fully grasp the problem at hand, simply because
I haven't read carefully what you wrote yet.

-- 
Stefano


      reply	other threads:[~2025-07-25 14:18 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-23 11:06 [PATCH v7 00/31] Introduce discontiguous frames management Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 01/31] style: Fix 'Return' comment style Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 02/31] arp: Don't mix incoming and outgoing buffers Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 03/31] iov: Introduce iov_tail_clone() and iov_tail_drop() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 04/31] iov: Update IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 05/31] tap: Use iov_tail with tap_add_packet() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 06/31] packet: Use iov_tail with packet_add() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 07/31] packet: Add packet_data() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 08/31] arp: Convert to iov_tail Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 09/31] ndp: " Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 10/31] icmp: " Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 11/31] udp: " Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 12/31] tcp: Convert tcp_tap_handler() to use iov_tail Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 13/31] tcp: Convert tcp_data_from_tap() " Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 14/31] dhcpv6: move offset initialization out of dhcpv6_opt() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 15/31] dhcpv6: Extract sending of NotOnLink status Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 16/31] dhcpv6: Convert to iov_tail Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 17/31] dhcpv6: Use iov_tail in dhcpv6_opt() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 18/31] dhcp: Convert to iov_tail Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 19/31] ip: Use iov_tail in ipv6_l4hdr() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 20/31] tap: Convert tap4_handler() to iov_tail Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 21/31] tap: Convert tap6_handler() " Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 22/31] packet: rename packet_data() to packet_get() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 23/31] arp: use iov_tail rather than pool Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 24/31] dhcp: " Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 25/31] dhcpv6: " Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 26/31] icmp: " Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 27/31] ndp: " Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 28/31] packet: remove PACKET_POOL() and PACKET_POOL_P() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 29/31] packet: remove unused parameter from PACKET_POOL_DECL() Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 30/31] packet: add memory regions information into pool Laurent Vivier
2025-06-23 11:06 ` [PATCH v7 31/31] packet: use buf to store iovec array Laurent Vivier
2025-07-18 18:45   ` Stefano Brivio
2025-07-18 18:45 ` [PATCH v7 00/31] Introduce discontiguous frames management Stefano Brivio
2025-07-24 13:01   ` Laurent Vivier
2025-07-25 11:45     ` Laurent Vivier
2025-07-25 14:18       ` Stefano Brivio [this message]

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=20250725161823.4329b411@elisabeth \
    --to=sbrivio@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=passt-dev@passt.top \
    /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).