From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=a1ex8kjo; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id 88F795A0271 for ; Thu, 19 Dec 2024 10:00:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1734598821; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=PwwrUJ3jo/n5I6JM6hPRPGor6MGzIJo5qtFdKJFR2r4=; b=a1ex8kjop6+EWhM27yFo2y4yNLP1dLabdyHsfgFUI3JckPUyHmHRZsDBgbO2f9FuKlHBG4 w9NJ8Ueh02XafxIoUBQy3la/gMq3dbVTzqkY/PZyJvgxlNKSMoGW99HPZz8S9+ODSitPJc DYHGvLM2CVGVg61WARHxyibV1SNn5AA= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-377-M2Fok0VYP1a8NiQ5XD_vmA-1; Thu, 19 Dec 2024 04:00:20 -0500 X-MC-Unique: M2Fok0VYP1a8NiQ5XD_vmA-1 X-Mimecast-MFC-AGG-ID: M2Fok0VYP1a8NiQ5XD_vmA Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-4361fc2b2d6so3158565e9.3 for ; Thu, 19 Dec 2024 01:00:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734598818; x=1735203618; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=PwwrUJ3jo/n5I6JM6hPRPGor6MGzIJo5qtFdKJFR2r4=; b=ljigxWH2MH1RrYmN0GBRpiAOGOVJlSa3xmzDL+qbaDXJp6YOoEGuDEAyhadk6Lc+SV BBMrcycR3zoCRE6GLXTFzBzEyT9IpqxpjhrkbOZwv/ROQOcEhhe8O6KeEZ+qxFdnuJgM fuDe1OXXYAUYpRk/tBGnBzyIGeb6LT6BxfHLKJXbKQaDH+ojGFBLXAF/0RGuXtgnbE1J xtZ6waelTuywf5N0+vLZ8EPrLqxjCSfbEG0wkJ8uXWppfX7QTy6hdySumXNNEz9wQxTi F6Ic+sf2JVkgFZhpSZ1dNFV/yZQi1H3LdB1U4tc7WCZZNx70x62PA8apO92Lqrs7+mTQ 3KkA== X-Gm-Message-State: AOJu0YwLeVfXd95irHVCcxgTzijz/wrn6fxCaPbJnS1JmyTmiE4HKiOG DF0ZeJpIjmQZs7YumyOfL3+vUHmc3goHPUnni9j4/QondSfHA0buMJy3TDXhsu1nLPwVQMwkTHH 5re4rzk98VKs1bDKa8/85hVnC4wx1xXGqfrnr88eDHtt5OylR8yF8YXgJCw== X-Gm-Gg: ASbGncuhNYwyOb4q0FhwYhh3ixfWdopkD28NdUu/AeaS900pswePEWoPDuuiVdWW0Rp 7rW6X3SvN2YOuEzvbimRgB5j70nt44UAUwW0Pi1iyOsIIiMCOmY5CCg2RjlOGhvjJnT2+kxKwp6 gw7b6glJPl8PqOUhAcpQHoBxkoeDZQsC1/Q3hEWI74EM7EdmMckQjjbDYVzaozanAfY0TUUEXik Yd9GuAFt78lwJCFN1BeSGCTfMugulDQpdZsdPNcD2IvB9OhLyBXJ2yyRfp7Z5OHu8pt X-Received: by 2002:a05:600c:1990:b0:434:a7f1:6545 with SMTP id 5b1f17b1804b1-4365c7dbe5cmr17414095e9.27.1734598818351; Thu, 19 Dec 2024 01:00:18 -0800 (PST) X-Google-Smtp-Source: AGHT+IEpqwxc4kZean2m5iI1XLJ4yfbgIFXmj4idsJaXiOLQ9Q5fM+ooYzMz4528nQIGJYz843GNww== X-Received: by 2002:a05:600c:1990:b0:434:a7f1:6545 with SMTP id 5b1f17b1804b1-4365c7dbe5cmr17413815e9.27.1734598817955; Thu, 19 Dec 2024 01:00:17 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-436611ea42esm12030575e9.9.2024.12.19.01.00.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Dec 2024 01:00:17 -0800 (PST) Date: Thu, 19 Dec 2024 10:00:15 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 3/3] tap: Don't size pool_tap[46] for the maximum number of packets Message-ID: <20241219100015.3e4b7599@elisabeth> In-Reply-To: <20241213120156.4123972-4-david@gibson.dropbear.id.au> References: <20241213120156.4123972-1-david@gibson.dropbear.id.au> <20241213120156.4123972-4-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: IAUHuihytLFfbtNCNhr0SIgMzQ4wPXee19ohLbHcw1Q_1734598819 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: VXDIJY5CRKHLJVSNOI2XG537A5MFHMRN X-Message-ID-Hash: VXDIJY5CRKHLJVSNOI2XG537A5MFHMRN X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri, 13 Dec 2024 23:01:56 +1100 David Gibson wrote: > Currently we attempt to size pool_tap[46] so they have room for the maximum > possible number of packets that could fit in pkt_buf, TAP_MSGS. However, > the calculation isn't quite correct: TAP_MSGS is based on ETH_ZLEN (60) as > the minimum possible L2 frame size. But, we don't enforce that L2 frames > are at least ETH_ZLEN when we receive them from the tap backend, and since > we're dealing with virtual interfaces we don't have the physical Ethernet > limitations requiring that length. Indeed it is possible to generate a > legitimate frame smaller than that (e.g. a zero-payload UDP/IPv4 frame on > the 'pasta' backend is only 42 bytes long). > > It's also unclear if this limit is sufficient for vhost-user which isn't > limited by the size of pkt_buf as the other modes are. > > We could attempt to correct the calculation, but that would leave us with > even larger arrays, which in practice rarely accumulate more than a handful > of packets. So, instead, put an arbitrary cap on the number of packets we > can put in a batch, and if we run out of space, process and flush the > batch. > > Signed-off-by: David Gibson > --- > packet.c | 13 ++++++++++++- > packet.h | 3 +++ > passt.h | 2 -- > tap.c | 18 +++++++++++++++--- > tap.h | 3 ++- > vu_common.c | 3 ++- > 6 files changed, 34 insertions(+), 8 deletions(-) > > diff --git a/packet.c b/packet.c > index 5bfa7304..b68580cc 100644 > --- a/packet.c > +++ b/packet.c > @@ -22,6 +22,17 @@ > #include "util.h" > #include "log.h" > > +/** > + * pool_full() - Is a packet pool full? > + * @p: Pointer to packet pool > + * > + * Return: true if the pool is full, false if more packets can be added > + */ > +bool pool_full(const struct pool *p) > +{ > + return p->count >= p->size; > +} > + > /** > * packet_add_do() - Add data as packet descriptor to given pool > * @p: Existing pool > @@ -35,7 +46,7 @@ void packet_add_do(struct pool *p, size_t len, const char *start, > { > size_t idx = p->count; > > - if (idx >= p->size) { > + if (pool_full(p)) { > trace("add packet index %zu to pool with size %zu, %s:%i", > idx, p->size, func, line); > return; > diff --git a/packet.h b/packet.h > index 98eb8812..3618f213 100644 > --- a/packet.h > +++ b/packet.h > @@ -6,6 +6,8 @@ > #ifndef PACKET_H > #define PACKET_H > > +#include > + > /** > * struct pool - Generic pool of packets stored in nmemory > * @size: Number of usable descriptors for the pool > @@ -23,6 +25,7 @@ void packet_add_do(struct pool *p, size_t len, const char *start, > void *packet_get_do(const struct pool *p, const size_t idx, > size_t offset, size_t len, size_t *left, > const char *func, int line); > +bool pool_full(const struct pool *p); > void pool_flush(struct pool *p); > > #define packet_add(p, len, start) \ > diff --git a/passt.h b/passt.h > index 0dd4efa0..81b2787f 100644 > --- a/passt.h > +++ b/passt.h > @@ -70,8 +70,6 @@ static_assert(sizeof(union epoll_ref) <= sizeof(union epoll_data), > > #define TAP_BUF_BYTES \ > ROUND_DOWN(((ETH_MAX_MTU + sizeof(uint32_t)) * 128), PAGE_SIZE) > -#define TAP_MSGS \ > - DIV_ROUND_UP(TAP_BUF_BYTES, ETH_ZLEN - 2 * ETH_ALEN + sizeof(uint32_t)) > > #define PKT_BUF_BYTES MAX(TAP_BUF_BYTES, 0) > extern char pkt_buf [PKT_BUF_BYTES]; > diff --git a/tap.c b/tap.c > index 68231f09..42370a26 100644 > --- a/tap.c > +++ b/tap.c > @@ -61,6 +61,8 @@ > #include "vhost_user.h" > #include "vu_common.h" > > +#define TAP_MSGS 256 Sorry, I stopped at 2/3, had just a quick look at this one, and I missed this. Assuming 4 KiB pages, this changes from 161319 to 256. You mention that in practice we never have more than a handful of messages, which is probably almost always the case, but I wonder if that's also the case with UDP "real-time" streams, where we could have bursts of a few hundred (thousand?) messages at a time. I wonder: how bad would it be to correct the calculation, instead? We wouldn't actually use more memory, right? -- Stefano