From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from imap.gmail.com [173.194.76.109] by localhost with POP3 (fetchmail-6.3.26) for (single-drop); Mon, 20 May 2024 07:57:27 +0200 (CEST) Received: by 2002:a05:6a10:9148:b0:55f:c3c0:ed08 with SMTP id n8csp190795pxb; Sun, 19 May 2024 22:56:50 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCVzI+J1Ern1Cu4CzKLJw/yaDEbDqrBluuxin686O95pxkroD+ARI23xFP8rlM39a+souU42Q6+uCXDNeOau407GHlmCChCkl8w= X-Google-Smtp-Source: AGHT+IEQUu0B7L/DyYra3v8DSWArw7dKyS/vD8vPo1ourAHVDfPLRiXg9JHWAwF8/R7Lo7muCmN7 X-Received: by 2002:a05:620a:d85:b0:790:edd8:ac18 with SMTP id af79cd13be357-79470f300f7mr965982785a.35.1716184610329; Sun, 19 May 2024 22:56:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1716184610; cv=none; d=google.com; s=arc-20160816; b=SnWZBgqpFhdr24Or3/uLop7yWx5IEpQFE8NZc2Ek2AvOMM0VP8pWiyZw57MqVLiTKZ rGSmI5CVJeuLBHheuaecHF4QT+gMPm3bE0qUl+uP0qzTvHSeXaNDsxvr3UcshL7QH3YX gxBEM+VZU67q9I4BUBeV7tDX//RrYKbLRJrcfwxBkUKAwOzTBwMlXQOp15t8IEsfIwue +laJQl+3XR+h3CYlN6xSSlscTtmBFX9PYj2hL04wumBrHf8qCrc2fuuE0JjlSNLEdIZm mJWDz1dos11Gth6XR6BmhalFCJJ9BH7XLQ6UPeGuiRFlOIt1hF2285rreh+ast54nQyf eWNg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:dkim-signature:delivered-to; bh=He2ov5DJq00JIbQeimBIpUFY9P1c4b/fHMCLulGXVu4=; fh=htis3z6nDPD3FFPmKX7SGQyhLv1B9tDlnJYWrC8d10o=; b=oTaNpcf9uWLKlaepyrIvyMpgvOv1C1fZCMiiy6+WKD1YnCR/H81UrXr+ZtVeLfH80D r1iRCCW67qURzjh07alLSkrw0d3FeamL/Qg8PrDNuFY727rgATjhkzgHs+rpjpJd5n/+ YwKsQRnEMhnTVmpPh8pgt0qCfcrVEClwA6VdyVrkeyuuA27mUBNTSnJXoz3DIDmLxK1L x42J+AFkEgEYO9SSaFGwrNvt4it03zpWHio4IzGg76Q8w7iB/udI09ZUCRLR2FlM2tvY wEgRih0uimkDJ7EPjHdlx/SVnUK6QxBZiWkBWNFlyemtUgqvmhhiSN9djpOx1OtDG6iO e12A==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=temperror (no key for signature) header.i=@gibson.dropbear.id.au header.s=202312 header.b=lkFNEf+T; spf=pass (google.com: domain of dgibson@gandalf.ozlabs.org designates 150.107.74.76 as permitted sender) smtp.mailfrom=dgibson@gandalf.ozlabs.org Return-Path: Received: from us-smtp-inbound-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com. [170.10.128.131]) by mx.google.com with ESMTPS id af79cd13be357-792bf309ca8si2394606585a.273.2024.05.19.22.56.49 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 19 May 2024 22:56:49 -0700 (PDT) Received-SPF: pass (google.com: domain of dgibson@gandalf.ozlabs.org designates 150.107.74.76 as permitted sender) client-ip=150.107.74.76; Authentication-Results: mx.google.com; dkim=temperror (no key for signature) header.i=@gibson.dropbear.id.au header.s=202312 header.b=lkFNEf+T; spf=pass (google.com: domain of dgibson@gandalf.ozlabs.org designates 150.107.74.76 as permitted sender) smtp.mailfrom=dgibson@gandalf.ozlabs.org Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-49-HHekUh8OPeiNL5l0QrSMbQ-1; Mon, 20 May 2024 01:56:46 -0400 X-MC-Unique: HHekUh8OPeiNL5l0QrSMbQ-1 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.17]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 20B5A1955E78 for ; Mon, 20 May 2024 05:56:45 +0000 (UTC) Received: by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) id 10D611943288; Mon, 20 May 2024 05:56:45 +0000 (UTC) Received: from mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.33]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 0E48A1943280 for ; Mon, 20 May 2024 05:56:45 +0000 (UTC) Received: from us-smtp-inbound-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 9E7CB1954196 for ; Mon, 20 May 2024 05:56:44 +0000 (UTC) Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-622-Px0iiHSlNqmGtc97ACqI3g-1; Mon, 20 May 2024 01:56:40 -0400 X-MC-Unique: Px0iiHSlNqmGtc97ACqI3g-1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1716184595; bh=He2ov5DJq00JIbQeimBIpUFY9P1c4b/fHMCLulGXVu4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=lkFNEf+T4mn+ZFkHUFik0ZLgJTsH9QiiP493B+M9b/FI3peSTal/J2d10U6vKbVMh rse6a/EQL96d0GYv379skwVL2dBj9GP9u3ULSIAVor+BjKcifDa6tCUKmN2sq80qqn bybe0SIg6VkeuouLdEJzQe4X+dsyD4VjwPQ136kYuF54ZGHN1RyAVbbEeie3spto7j fZ7zinduSPtAA4MLbTsWdXOjf2ij907BPXcFPlrYkP+BTZCUyFvBkrvBa7Gh6SsSZS vRefEXeldDsFgx9THwUa2PYzftJpYGxVa5iQq/3LUH41kg2FQ8HKOqczr9/g4qwaVV 4Wa0iUYSVh5xQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4VjRdv3Fkgz4wqK; Mon, 20 May 2024 15:56:35 +1000 (AEST) Date: Mon, 20 May 2024 15:56:28 +1000 From: David Gibson To: Stefano Brivio Cc: passt-dev@passt.top Subject: Re: [PATCH v5 19/19] flow, icmp: Use general flow forwarding rules for ICMP Message-ID: References: <20240514010337.1104606-1-david@gibson.dropbear.id.au> <20240514010337.1104606-20-david@gibson.dropbear.id.au> <20240518001408.004011b2@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="niaDrpUPXOnMUedY" Content-Disposition: inline In-Reply-To: <20240518001408.004011b2@elisabeth> X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 List-Id: --niaDrpUPXOnMUedY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, May 18, 2024 at 12:14:08AM +0200, Stefano Brivio wrote: > On Tue, 14 May 2024 11:03:37 +1000 > David Gibson wrote: >=20 > > Current ICMP hard codes its forwarding rules, and never applies any > > translations. Change it to use the flow_forward() function, so that > > it's translated the same as TCP (excluding TCP specific port > > redirection). > >=20 > > This means that gw mapping now applies to ICMP so "ping " w= ill > > now ping the host's loopback instead of the actual gw machine. This > > removes the surprising behaviour that the target you ping might not be = the > > same as you connect to with TCP. > >=20 > > Signed-off-by: David Gibson > > --- > > flow.c | 1 + > > icmp.c | 14 ++++++++++++-- > > 2 files changed, 13 insertions(+), 2 deletions(-) > >=20 > > diff --git a/flow.c b/flow.c > > index a6afe39..b43a079 100644 > > --- a/flow.c > > +++ b/flow.c > > @@ -285,6 +285,7 @@ const struct flowside *flow_initiate_sa(union flow = *flow, uint8_t pif, > > * > > * Return: pointer to the forwarded flowside information > > */ > > +/* cppcheck-suppress unusedFunction */ > > const struct flowside *flow_forward_af(union flow *flow, uint8_t pif, > > sa_family_t af, > > const void *saddr, in_port_t sport, > > diff --git a/icmp.c b/icmp.c > > index 0112fd9..6310178 100644 > > --- a/icmp.c > > +++ b/icmp.c > > @@ -153,6 +153,7 @@ static struct icmp_ping_flow *icmp_ping_new(const s= truct ctx *c, > > sa_family_t af, uint16_t id, > > const void *saddr, const void *daddr) > > { > > + uint8_t proto =3D af =3D=3D AF_INET ? IPPROTO_ICMP : IPPROTO_ICMPV6; > > uint8_t flowtype =3D af =3D=3D AF_INET ? FLOW_PING4 : FLOW_PING6; > > union epoll_ref ref =3D { .type =3D EPOLL_TYPE_PING }; > > union flow *flow =3D flow_alloc(); > > @@ -163,9 +164,18 @@ static struct icmp_ping_flow *icmp_ping_new(const = struct ctx *c, > > if (!flow) > > return NULL; > > =20 > > - > > flow_initiate_af(flow, PIF_TAP, af, saddr, id, daddr, id); > > - flow_forward_af(flow, PIF_HOST, af, NULL, 0, daddr, 0); > > + if (!flow_forward(c, flow, proto)) > > + goto cancel; > > + > > + if (flow->f.pif[FWDSIDE] !=3D PIF_HOST) { > > + flow_err(flow, "No support for forwarding %s from %s to %s", > > + proto =3D=3D IPPROTO_ICMP ? "ICMP" : "ICMPv6", >=20 > Which brings me to two remarks: >=20 > - having the protocol name also in the flow_err() message printed in > flow_forward() could be helpful It would, and I've thought about it, but haven't seen a great way to go about it. * The flow type is not set at this point, so we can't use that. We can't trivially move setting the type earlier, because for TCP at least we need the information from flow_froward() to determine if we're spliced and set the type based on that. * Including both flow type and protocol in flow_common is annoyingly redundant, as well as adding a full 32-bits to the structure because of padding. * We could possibly eliminate flow type and make it implicit based on the protocol+pifs: regular TCP flow is (TCP, HOST, TAP) or (TCP, TAP, HOST), whereas TCP spliced is (TCP, HOST, SPLICE) or (TCP, SPLICE, HOST). Type is the selector field for which of the union variants is valid, and I don't love that being something with a kind of complicated calculation behind it. * We could make the type field hold the protocol until FLOW_SET_TYPE(), but I don't love semantics of a field changing like that. * We could just pass either a protocol number or a string to flow_forward() etc., but that seems a bit awkward. Hrm... actually thinking on that last one. It might make sense to add a descriptive string to flow_initiate(), not just the protocol but something like "TCP SYN" versus "TCP accept()" or the like. That wouldn't directly help flow_forward(), but the info line from flow_initiate() is likely to be in close proximity, so it would help. > - then, perhaps, we should re-introduce ip_proto_str[] which was dropped > with 340164445341 ("epoll: Generalize epoll_ref to cover things other > than sockets") Maybe. I guess the standard way to do that is with getprotobyname(3), but that probably won't work in our isolated namespace, I guess. --=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 --niaDrpUPXOnMUedY Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmZK5gsACgkQzQJF27ox 2GdUnRAAoiWSIRPKLLf5gE5ti8PBBa22bdyoYwOcaGMKIDhptxtgxQXdDyAOKtVx 9Oj6eLEU/69koLHQSHiUX9ZF2Sin0Kbs6YyIy5y7Ko4tPKB/gcbeuFXct/mKax6v VmHncoGDbKEX1/Io7m/nSlwp8oK0U5z18bYVGU3V0/nIiBhLYaB/tB5ENbo/NDDW zw75LHLK+PFZU1ns0Lix2v9IdcNf2Uxj7F6z9qTowlv+C1zZVRTl8q896dNLbgS7 yrGVbuEEpzcZuThFVal6BmgeJ7bMaNq7kGvzF0nxb7VKy9lD4dcJ38hO7ZLtmAoA ShvOZrkjhcewlPytyRdOU8hkKJvEuQyDgFUDgHPDyXOU8C5COR8MjYQtHjH9+7ik Dry79y5d6+zhHT/7wYdAWpiwovi3fr2KuP70+9vjjpGch9Y2t6QpWPHwx0cmG4dB TVCacrIOcNznIZ9+CU6H0EG8GfsOhuDJwDyqw2XLYQBhhTxVkurw+f+fjXYwUTIg 42Cf7iATMwkrvbUxEXXaE2TbcGjlTVYdwe3hn8i0Gb0d1k6jZUjpoCyjeHyG/6Ir R1Nf8wPmBvJUTsmkVUp7eYIWXhz/vBsfUBsbQEI3EcRI64P3Xn6zOrBjF405DZ5A H6RuRFuNolLzHjqrF7AcUhJqIjMsJSIPgJQvQQ8cNRl8+k1GAZU= =av7K -----END PGP SIGNATURE----- --niaDrpUPXOnMUedY--