From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202510 header.b=PMXtfMJP; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id B71845A0271 for ; Wed, 19 Nov 2025 10:39:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1763545193; bh=CVSufIe85EcMVyD6PumXx/jUVJ1SzfLoGqTaRewpqIg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=PMXtfMJP/UqvE1O1gPKdFDyWiZ4bmkwhcuxDqIpR/NsgKvVxoKIEjdffgSQOc2OQz Hiuou68oDKilLMU6KXaqeg9LAcgxlTvywf2TkMWswvBH3FDoeVNt7TvBmP9Sstsx5C hSmgtGe6f9/do22O6dCqxkDqR/C2qLaOfPt29yRT9LnMAXtQns9ZKjqK8uF2py3wuO E365oM+WwgwxYSNtefjXTVO2G8FpxEf1XL0hvlfDs7wnVhCba/EZktdlDlP1mvfZtt dixrHriW4Pq5OpktV7Nhvtja5NBZkHY216gSZsrIOwTKIqUavYi/nE2UGl+tYV0B5W qYRuwHEnGHvFw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4dBGfd15Q5z4wGX; Wed, 19 Nov 2025 20:39:53 +1100 (AEDT) Date: Wed, 19 Nov 2025 20:38:24 +1100 From: David Gibson To: Yumei Huang Subject: Re: [PATCH v8 2/6] util: Introduce read_file() and read_file_integer() function Message-ID: References: <20251110093137.87705-1-yuhuang@redhat.com> <20251110093137.87705-3-yuhuang@redhat.com> <20251118011938.47d5c1e2@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="4V00aWagKwv1iSOr" Content-Disposition: inline In-Reply-To: Message-ID-Hash: 23ITGG6NXY7WDNCVD4KHZYTNYOTEEVXG X-Message-ID-Hash: 23ITGG6NXY7WDNCVD4KHZYTNYOTEEVXG X-MailFrom: dgibson@gandalf.ozlabs.org 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: Stefano Brivio , 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: --4V00aWagKwv1iSOr Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 19, 2025 at 05:04:31PM +0800, Yumei Huang wrote: > On Tue, Nov 18, 2025 at 11:36=E2=80=AFAM David Gibson > wrote: > > > > On Tue, Nov 18, 2025 at 01:19:38AM +0100, Stefano Brivio wrote: > > > On Mon, 10 Nov 2025 17:31:33 +0800 > > > Yumei Huang wrote: > > > > > > > Signed-off-by: Yumei Huang > > > > Reviewed-by: David Gibson > > > > --- > > > > util.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++ > > > > util.h | 2 ++ > > > > 2 files changed, 88 insertions(+) > > > > > > > > diff --git a/util.c b/util.c > > > > index 44c21a3..c4c849c 100644 > > > > --- a/util.c > > > > +++ b/util.c > > > > @@ -590,6 +590,92 @@ int write_file(const char *path, const char *b= uf) > > > > return len =3D=3D 0 ? 0 : -1; > > > > } > > > > > > > > +/** > > > > + * read_file() - Read contents of file into a NULL-terminated buff= er > > > > + * @path: Path to file to read > > > > + * @buf: Buffer to store file contents > > > > + * @buf_size: Size of buffer > > > > + * > > > > + * Return: number of bytes read on success, -1 on error, -ENOBUFS = on truncation > > > > + */ > > > > +ssize_t read_file(const char *path, char *buf, size_t buf_size) > > > > +{ > > > > + int fd =3D open(path, O_RDONLY | O_CLOEXEC); > > > > + size_t total_read =3D 0; > > > > + ssize_t rc; > > > > + > > > > + if (fd < 0) { > > > > + warn_perror("Could not open %s", path); > > > > > > While testing something unrelated I realised that this looks like a > > > serious failure, but it's perfectly normal on (even slightly) older > > > kernels: > > > > > > --- > > > $ git describe --contains ccce324dabfe # tcp: make the first N SYN RT= O backoffs linear > > > v6.5-rc1~163^2~299 > > > $ git describe --contains 1280c26228bd # tcp: add tcp_rto_max_ms sysc= tl > > > v6.15-rc1~160^2~352^2 > > > --- > > > > > > On a 6.1 kernel, for example: > > > > > > --- > > > $ ./pasta -d --config-net > > > [...] > > > 0.0258: Could not open /proc/sys/net/ipv4/tcp_syn_linear_timeouts: No= such file or directory > > > 0.0258: Could not open /proc/sys/net/ipv4/tcp_rto_max_ms: No such fil= e or directory > > > 0.0258: Read sysctl values syn_retries: 6, syn_linear_timeouts: 4, rt= o_max: 120 > > > --- > > > > > > and actually the third message isn't accurate, because we didn't read > > > those values, we are just using them. > > > > Right. In fact I'd say here the fact they came from sysctl is less > > important here than what we're using them for. So maybe > > Using TCP RTO parameters, syn_retries: 6, ... >=20 > Got it. > > > > > > > > Worse, yet: > > > > > > --- > > > $ ./pasta > > > Could not open /proc/sys/net/ipv4/tcp_syn_linear_timeouts: No such fi= le or directory > > > Could not open /proc/sys/net/ipv4/tcp_rto_max_ms: No such file or dir= ectory > > > --- > > > > > > so this would be logged *with default options* by Podman, rootlesskit= / > > > Docker, via libvirt, and by other users too, meaning we would get > > > submerged by reports about this "error" if we release it like this (6= =2E15 > > > is fairly recent). > > > > > > So maybe we could turn this (and the messages below) to debug() / > > > debug_perror(), and then: > > > > I think we should remove the error message from read_file() entirely, > > just returning an error code. Essentially it is too low level to know > > the severity and meaning of a failure, so reporting the problem to the > > user is better left to the valler. >=20 > Then we will have a few more values to return, like errno for open or > read file fails, -ENOBUFS, and another error code for when buf size is > 0. I'd say it's more complicated than write_fiel(). Maybe we could > update with debug()/debug_perror() first as Stefano suggested, and > then fix them together later? Right, returning full error details is somewhere between awkward and impossible in C. I think returning -errno for the first error to occur is probably good enough, though. --=20 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 --4V00aWagKwv1iSOr Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmkdj/YACgkQzQJF27ox 2GfrQA/+OgkJA9aebthWI1ydxfCgTc8XNr2UagBB+AMJijNjdDJfwWUn7n/Ghymn lZNboyJ7mNiPiOxnY3iYBYWAol1ltSQohWDsTbbWH951D1vTv27H46+erbMKgAmo jOzHDRGcP4AbmkMivMgQOviw4l4sdl/ubJteSMwnbIw7teIpjDt0+qngWHCKfALz ziCwbBqbFzKDyG8lJgJHjXYK2oGkC87WRFQ3ZgrySYCNIFKhJQVE9WCYs3o27AWT vHhNhB3YVacXxeovnWmuXyhCbsthTcPJdoTHmjR7RC5c9M1pZRX2WOJHfvV8l/Ym heg46Il2LfO7onutJYZOv/BONhnljtX/J2kSnhnq0ddGU1cHoVtTi3puJUUOW5uh h+HeK2qEcrYy0csYc4UH+uJpnzHU/7XnBb+QDERFqE5qtI5CARdoVhneMojB0wkT VLnehh4FaJ9S8tK+m0aGh6nEnboUNE8bMURGb+BWdKOEVXKZe2OQ5J0vvYktloqI q8xR2qTauv12OJbvsUET4dO7U7RP9jA/+L066RPEsYHfIKhZvIUZK5EGf8huCKS/ Ygyt99XYbaCPUF+b70RBboI0kfRajrJd9ncTdXxtbUHqwu8nVz5s2z9+ITKmfb4I B8FPLDSpweWIw5qUtIphcUQrz0PkhWswSBZ8EJ/COCjvr0iEIhQ= =37CD -----END PGP SIGNATURE----- --4V00aWagKwv1iSOr--