From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id A97E75A0272 for ; Fri, 19 Jan 2024 01:26:39 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1705623993; bh=EfaHVRddY4IfaXrTuI3TuLP709jR2xrHSbAGNO7P7GY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=J6v7k7WSDwzMIJwU8P/79h8wqfdMGOWpYEeNU9g7HiAUBIOel0ChQAowbSza3YYPw M7QkYxEc1HIExxcqNx5dW+XdhqGtpFxQbf+98giUCHCuaDwUduIzZiFIqY6EzTF0eU v/lXYi1G83Wr0AtzgJWG1VEuvR632/H/hBcHB/1aHdlUeM3JD3o7iEqrx0cNlT1M0O +H5idlTzl6U6zuJSwCEqfA69o+0paUHBSN7M3MGYs2O07fWAGrvXTnNS9am1GGit5y IblX0Hjo0PDxOJIKoZqTu+V0DLGMDeNjouc8/i51gAgEZN8dw7JQrX+KvdL8ieU/wj p6FYUEVidGi3g== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TGL5P6CHFz4xPR; Fri, 19 Jan 2024 11:26:33 +1100 (AEDT) Date: Fri, 19 Jan 2024 10:58:44 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 12/15] icmp: Populate and use host side flow information Message-ID: References: <20231221070237.1422557-1-david@gibson.dropbear.id.au> <20231221070237.1422557-13-david@gibson.dropbear.id.au> <20240117205941.2dffe678@elisabeth> <20240118164305.3ab441c4@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="iyeDc+cKzHSfOaTd" Content-Disposition: inline In-Reply-To: <20240118164305.3ab441c4@elisabeth> Message-ID-Hash: QG3JXSVNYJB6AYODH4UVMNETBV6A4WOG X-Message-ID-Hash: QG3JXSVNYJB6AYODH4UVMNETBV6A4WOG 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: --iyeDc+cKzHSfOaTd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jan 18, 2024 at 04:43:05PM +0100, Stefano Brivio wrote: > On Thu, 18 Jan 2024 12:22:29 +1100 > David Gibson wrote: >=20 > > On Wed, Jan 17, 2024 at 08:59:41PM +0100, Stefano Brivio wrote: > > > On Thu, 21 Dec 2023 18:02:34 +1100 > > > David Gibson wrote: > > > =20 > > > > Complete filling out the common flow information for "ping" flows by > > > > storing the host side information for the ping socket. We can only > > > > retrieve this from the socket after we send the echo-request, becau= se > > > > that's when the kernel will assign an ID. > > > >=20 > > > > Signed-off-by: David Gibson > > > > --- > > > > icmp.c | 36 +++++++++++++++++++++++++++++++++--- > > > > 1 file changed, 33 insertions(+), 3 deletions(-) > > > >=20 > > > > diff --git a/icmp.c b/icmp.c > > > > index 53ad087..917c810 100644 > > > > --- a/icmp.c > > > > +++ b/icmp.c > > > > @@ -310,11 +310,41 @@ int icmp_tap_handler(const struct ctx *c, uin= t8_t pif, int af, > > > > if (sendto(pingf->sock, pkt, plen, MSG_NOSIGNAL, &sa.sa, sl) < 0)= { > > > > debug("%s: failed to relay request to socket: %s", > > > > pname, strerror(errno)); > > > > - } else { > > > > - debug("%s: echo request to socket, ID: %"PRIu16", seq: %"PRIu16, > > > > - pname, id, seq); > > > > + if (flow) > > > > + goto cancel; > > > > + } > > > > + > > > > + debug("%s: echo request to socket, ID: %"PRIu16", seq: %"PRIu16, > > > > + pname, id, seq); > > > > + > > > > + if (!flow) > > > > + /* Nothing more to do for an existing flow */ > > > > + return 1; > > > > + > > > > + /* We need to wait until after the sendto() to fill in the SOCKSI= DE > > > > + * information, so that we can find out the host side id the kern= el > > > > + * assigned. If there's no bind address specified, this will sti= ll have > > > > + * 0.0.0.0 or :: as the host side forwarding address. There's not > > > > + * really anything we can do to fill that in, which means we can = never > > > > + * insert the SOCKSIDE of a ping flow into the hash table. > > > > + */ =20 > > >=20 > > > Well, if it was just because of that we could argue at some point tha= t, > > > with wildcards or suchlike, we could insert that in the hash table as > > > well. =20 > >=20 > > If we allow wildcards, it's probably not a hash table any more, or at > > least not a normal one. Hashing is pretty inherently tied to a single > > value lookup. >=20 > Well, unless you consider 0.0.0.0 / :: as the actual value (which makes > it a wildcard, but not in the hash table sense). I'm not suggesting we > should, though (at least for the moment being). Right, ok. So, I've been looking at what we need to do flow based NAT, and I am now leaning towards revising the existing series so that it's less insistent on "complete" flowside information, except in the case where we really are hashing. That should also let us remove many of the getsockname() calls. > > > But that wouldn't make sense anyway, right? That is, unless I'm missi= ng > > > something, we would have no use for a hash table lookup of the SOCKSI= DE > > > of a ping flow anyway. =20 > >=20 > > That's correct, which is why we can get away with this. Likewise > > SOCKSIDE of tcp flows is also never hashed. However in some cases we > > may want to hash the host sides of flows: I think we'll want to for > > UDP, for example, because we can receive multiple flows via the same > > bound socket. That's why I want to be very clear about when and why > > we're constructing a flowside that can't be hashed. >=20 > The comment makes this sound like a compromise on functionality, or a > problem we have (and that's what mostly caught my attention: can we > "solve" this?). >=20 > Maybe you could add something like "(not that there's any use for it)" > at the end, or similar. Right, the revision I'm thinking of should make that clearer, I hope. --=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 --iyeDc+cKzHSfOaTd Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmWpuzMACgkQzQJF27ox 2GdOTw//bOYmAGPIZwcu/u/DU9o9M/9B1oxpDWKcZX4QJgMohfGmthYCNO4qOpYR X7vVV4LNkwgEayqOjG1obqKCgnXNyeM9DdtlIy8BZvl+MNNYhfGuhyZuOGfyh70v Rgpo86BOCTf4PPiDZMAJxiROy0BWCgWxR1kD0Xum+6VOKThhOUutaxwF3Kg0K31n Xy/VXHQ1engcjKuP9EZr6sjq8sEC3Te6PO6pVzh5theT5I8ZveTeJ3yjPv5VeUMr J4R1K8f2VrWgvWMcdxGE4KuYY3iNaVmFGycIySsKhIMWdoNF+jf9oMIgh8wKh4jg Exh9YnQ4EDkSvwqE9TrCHus9/i7fjkAy9eLQ5tddksvxBPqeYbZsuUXsmCXhKAzJ 6F0KqM8iXiQPtm2j9knyXOIKG+In0NxSJXrhqDVyTBdzw30E/1ymN6DY3gTuzWgs GakxniQfMfFwscVCdNWqOekMNeph1/IUV7hG2O5HvhJg77uAmyJu42S9N88ILGOY oN01MLt3YnrIHfJMy+/6NFozUNOORLIvp+IXenoJ52wNAloSL0qD68yHzysUb0wM lNy1/oADtfem7/NJdV5NytyjTKjby1/6bonGvqH5b5OyNLdfuT9gYPRikwmTGYDP nx1zpgAVDXzO3fnx2u8OITNJ7p2XSfxljoyJHVLe2IS9JUNlLSE= =B+4N -----END PGP SIGNATURE----- --iyeDc+cKzHSfOaTd--