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=202502 header.b=NZUkX56w; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id C4D425A0008 for ; Fri, 28 Mar 2025 10:40:35 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1743154833; bh=xa+1fxXUJ6ug2TQ8chO12zYynpYWJLdXfoSWwhBcjd8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=NZUkX56wwigwfWXlNEY4oTtVOsaA8zD2lzZMdBJGHN412eSAMqtrQjdjxGKGBZYve aNECNVAHBLs6fSDlQvBZPy3JoutFKt0K0TAiBfeDDnjmYrZ6FvGLbyuxu+pjp4NZ5G N97loX1m84pGTfPlmQ+FeaPvwLg1NgAlfj2rKpCUFtmAtkfU0Xy64VkpyP+nUrsQBL tx9HFHMthpIJ3h0LsUFnmfObeq5RrQVavYZzOKEyzNAENYXfgZMWULwrc6wVFoi4yE r7fFW0Dl9N4D+yT5yfOMZ0m8ITL7zAhRh9er0LcZONvvIKE77+JqUAwjSEQhiO8LPc 0Tjkp76hx80Jg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZPFrK3NGNz4xNs; Fri, 28 Mar 2025 20:40:33 +1100 (AEDT) Date: Fri, 28 Mar 2025 20:32:50 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [RFC PATCH] pasta, passt-repair: Support multiple events per read() in inotify handlers Message-ID: References: <20250327212822.3315053-1-sbrivio@redhat.com> <20250328102340.0ede3ca8@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="8dbkA8hugnuGB/ep" Content-Disposition: inline In-Reply-To: <20250328102340.0ede3ca8@elisabeth> Message-ID-Hash: L6EMVLFVI27MF7BMGLM7BPKPPIYGQO6D X-Message-ID-Hash: L6EMVLFVI27MF7BMGLM7BPKPPIYGQO6D 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: 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: --8dbkA8hugnuGB/ep Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Mar 28, 2025 at 10:23:40AM +0100, Stefano Brivio wrote: > On Fri, 28 Mar 2025 10:36:39 +1100 > David Gibson wrote: >=20 > > On Thu, Mar 27, 2025 at 10:28:22PM +0100, Stefano Brivio wrote: > > > The current code assumes that we'll get one event per read() on > > > inotify descriptors, but that's not the case, not from documentation, > > > and not from reports. > > >=20 > > > Add loops in the two inotify handlers we have, in pasta-specific code > > > and passt-repair, to go through all the events we receive. > > >=20 > > > While in pasta the the inotify watch is in an epoll list, and we'll = =20 > >=20 > > "the the" > >=20 > > > get epoll wakeups as long as there's data to read, in passt-repair > > > that's not the case. So, in pasta we can simply size the buffer for > > > a single event and try to read one, but in passt-repair, we'll need > > > to size the buffer to a safer, reasonable amount of events. =20 > >=20 > > I'm not following the reasoning here. In passt-repair we're just > > looping on the read() until we find what we want, so we'll still > > eventually get what we want. AFAICT it's a stream-like interface, so > > we won't lose events just because we didn't get them in the first > > read(). >=20 > Under the assumption that message (event) boundaries are preserved > across read() calls, yes. Right. My observation was that was the case, and inotify(7) also seems to state so outright: | Each successful read(2) returns a buffer containing one or more of the fo= llowing structures: | | struct inotify_event { | ... | }; > And... this assumption actually holds (I had forgotten about that), so > expanding the buffer as I did makes no sense, because by reading > sizeof(struct inotify_event) + NAME_MAX + 1) bytes we always read one > or more events, but never a partial buffer. >=20 > So, yes, never mind. >=20 > > > Link: https://bugs.passt.top/show_bug.cgi?id=3D119 > > > Signed-off-by: Stefano Brivio =20 > >=20 > > Otherwise, LGTM. > >=20 > > > --- > > > I'm posting this as RFC because, while it seems to do the job and I > > > tested all the code paths, Coverity isn't amused by the fact that > > > we assume that inotify 'name' attributes (ev->name) are > > > NULL-terminated. They actually are, but the code is not very robust. > > >=20 > > > Addressing that is kind of trivial but I keep getting it wrong, so > > > I'll start posting this and fix that up later (direct fixes/edits of > > > this patch are also welcome, of course). =20 > >=20 > > Ok, I might try to polish this up today >=20 > Thanks! >=20 --=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 --8dbkA8hugnuGB/ep Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmfmbLUACgkQzQJF27ox 2Gd9wA/+Oyr2jOUrGSmtoQ9H20w+ZWoYiKV9wCSqOolwgUOi73HdAv/zFibY/VO6 XIdhJ/1jzLkphjJ25WwCuzIn3cl1dL9W2hr4fmLd9FR4vwB61dH3sftz7vYVp0rw a1mWA1VgZyFLz2CA+6gprd0V9+RwrE1pui/8GA6RJTgJw/pYsGNV8twtSOTeS6CY PlkLrEtYNiolQ2Un2JzWzvoDnRUAkmnWDZlSEOrtpMulFT4piroSafVxKNr51UuG NqEMyauaxmMKWqJLfeN8V8b0aApH8yFH8BSr+WOxJ8jDSPJ02usfCiae6AG6kwCF 2RQPCoNXLsq6tx3TKW4UBAYT+3koGmwi0J3aVOV5wS66WpaG7momSfmmI+eekaPs 4A3fGaEVrStDBjrXqLwIo0tfMtHuoboIEk+x20Z0G0OkvFRVQt3xj/prReA9Zdjo ZLwS7mzn1n9jjOKoLOAI8LK+UtAvjzOOWn/2bVtTZkx4dCClbSrOguPhO40q4kZH gle1BEBoUiCRxyj1zi0dLSJWdYLkOgBLD1InIaJaOPBjO/jRuzqw7rSE9Cp5cMlK 9ZltgYiTqEKQ8JRF1OtyvJsUfBYvRi+dAsJ+5wFGWZc4H5UKImqbEY/i6q5dOKKS 2Z3WlaGYGcJT1IkUimLOftgHHnWSHcJGzj/dwW51WB62SRUWo2o= =r8AL -----END PGP SIGNATURE----- --8dbkA8hugnuGB/ep--