From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 1C1E85A0275 for ; Wed, 28 Feb 2024 04:55:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1709092542; bh=+x9KV3kNyf80jeOUepNo461VloqmN3/chLASQtMxsek=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RXo4TypsEhQl65CJ8GbkuEgWyaMRMqJGn97YxjxHM5SjF9tm40PFmJC+R4Cutc6IU x/MiIKUyJyh8P2rAHLi19oyNvCyIJl9LvKmFNjnHb1wxiZLWz6whNHwILUjeNQVa6A /dvMmHf8tYqigwrIeAVjGyE9SF12SojYz2qUTujygRKR2MC0ItCEUM6IRk7AcHMaSN JsBmk/KHaAkdWZJAPA3l8QwD8zxl3Jzo/D8pqHEgB8KL9mQxbKUZjRnJxPa74QeUPC kNqWHlT243xJWTeKrb2y3TZORm0NlSGwmc2CWv3SaXEkC4/gnNWxh1eBpamBee0oKf YVUheFneiRkDQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Tl0rG724kz4wc8; Wed, 28 Feb 2024 14:55:42 +1100 (AEDT) Date: Wed, 28 Feb 2024 14:53:27 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 4/7] udp: Separate bound sockets from flags Message-ID: References: <20240221232115.1376333-1-david@gibson.dropbear.id.au> <20240221232115.1376333-5-david@gibson.dropbear.id.au> <20240227125659.6505dae9@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="UOH4ulJsxICszJx1" Content-Disposition: inline In-Reply-To: Message-ID-Hash: 25RHHQOBGHTBVOF5NCB5ZED7OV7KHHE3 X-Message-ID-Hash: 25RHHQOBGHTBVOF5NCB5ZED7OV7KHHE3 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: --UOH4ulJsxICszJx1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 28, 2024 at 01:30:05PM +1100, David Gibson wrote: > On Tue, Feb 27, 2024 at 12:56:59PM +0100, Stefano Brivio wrote: > > On Thu, 22 Feb 2024 10:21:12 +1100 > > David Gibson wrote: [snip] > > I'm having a hard time reviewing the rest because of this, I think. > > Earlier, UDP_ACT_TAP meant that the activity timestamp was observed on > > a non-spliced port. >=20 > Ah! I'd read those as "ACTions we needed to take" in the expiry > timers, rather than tying them to specific activity. It amounts to > the same thing pre-patch, but it makes more sense to me with that new > lens. >=20 > > Now, we have UDP_ACT_FLAGS which seems to mean that activity was seen > > in the direction of (to) the "tap" side, and UDP_ACT_TAP_SOCK meaning > > that activity was seen in the direction of (to) the socket. > >=20 > > But that's somehow the opposite of UDP_ACT_SPLICE_NS and > > UDP_ACT_SPLICE_INIT -- even though they don't actually indicate > > activity, rather the fact that activity timestamps refer to sockets that > > originated in (*from*) the target namespace, or in the initial > > namespace. > >=20 > > Anyway, I'm not quite sure: why do we need two separate flags for "tap" > > (from/to) activity? >=20 > We don't, this is an ugly side effect. The problem is that in > scanning for expiries we only have a single port number. But the > socket is associated with a particular forwarding port, while the > flags are associated with a particular endpoint port. Without flow > tracking, we don't have a way to match one to the other. >=20 > The compromise here is - or is supposed to be - that we have > independent timestamps for the socket and the flags. The two > timestamps associated with a single flow are supposed to be updated at > basically the same time - looks like I missed some things to do that > as well. >=20 > > I mean, as long as we observe activity on a non-spliced flow, we'll > > update the related timestamp, and we make sure the activity flag is > > set. Can't it simply be UDP_ACT_TAP? Urgh. So it seems like every time I re-examine this, I find more pre-existing bugs. So, part of what this patch was attempting to fix originally is that previously, although we updated the timestamp host->guest packets, we updated the wrong one for expiring the socket (based on endpoint rather than forwarding port). In trying to fix it up, I additionally realised, we currently don't update the timestamp for host to guest packets at all, except in the when remapping the src to gateway address, which as noted previously itself doesn't honour the no_map_gw flag. *Then* I realised that if we do update the correct timestamp and ACT flag, we could now set an ACT flag for a socket created by -u, and therefore cause a port that should remain open to be expired. I don't think this any longer counts as a "small" fix, so I will remove it from this series and tackle it elsewhere. --=20 David Gibson | 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 --UOH4ulJsxICszJx1 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXerjYACgkQzQJF27ox 2GdjhQ//Qpg8XMK3bbGhjQFD333D38YAJKxw2agwT2DJ4DTeLl30s2IWmIYUOqhj xgmAcC/HdEIFY+z8DqK6cxEDzk8ZyZqHL+CKOl+dKjw3ENfiZYUPxwo+uv4yRTSA 5FL7sSszWs13XadwzIDpisZqbclDSLuj836S/LjvmNUKQJ96rmcRN2Fz1L94x8dY J7LKfrFBfbaN1vIgQylQ+7zrxrig7ay8wrfaPmyJrbbMbjZ84b+HMX2p5S/4SpTQ KSBcP2+wR2wyxarBvz2DDBo+2ed7bT+IF0a+avgoKkxMSqSvEKbSO4xCKjhb6rMk klalgzzM37MTE2nAOEWFvgxx6GG2456JsO2AqG5kpVC8gMmVTZXlkwKhLft7/WU6 7jknwgp/PbWxuBNUCeB7c5WgTFIgqI2gG/jMtTY0PFXgx4nfgDbhXaUO2sC1lZUC R2iu1XThe9Ah75sYEsaruZAA6/lyrL7jdpO6CGndJK44vfsHAz0sftAwgBlRMM1o aPRmdV+P4hHaO14jxnTOG9LfjwvJVtlJbHBsp4mxc/Kt6OYo3q5Q75923xP9rMBT MYhs1D03Zwib3q6kbF2EqY7grUBV2VvA5O1kMz7Uia0ivhBVHCz0BDcOfWTDePr8 9tiUh3ianPTJjQligljNNKa1jTJmPjuzJBIR5Znt7PhSPNTbYgw= =MiQH -----END PGP SIGNATURE----- --UOH4ulJsxICszJx1--