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=202506 header.b=IZQuu91m; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id C475A5A0280 for ; Tue, 22 Jul 2025 04:44:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202506; t=1753152124; bh=6Ro70+YtOy4DOVenXq5/23O4zZlDP+KlilYBzmiU5gg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=IZQuu91mlRNdEIBVWMMKDbL14l2MhC0wji/hEYF0P0ZqYE6KfFzf1uH255fUn+qqS DNJmw9RccI58a9RNK+wBme94VQvAI6XLuxgVXUTvdojVFQpg9K0dukuXWfvoC3OGDt N4/C4nw0ZK9VNX+X7+8dWw3sIFhhKUpKcWK31WW+BVpT8AZ38lv8iSyIqvWu6akt4w 55xmQP6KiY3YlxzvuAoe8ZTaIHLWJf+dmdTQY8zLwYE0i8gjcZHdBIPN7wi9wuTGAZ 8Mb6y6VamiI2VsWdek3pdy70F3lrO0LRR3VRQsOqgMKMnvxJSCqILp80bWYQx+4LBM AxDWoitv+zgaw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4bmM3w3v4Wz4x11; Tue, 22 Jul 2025 12:42:04 +1000 (AEST) Date: Tue, 22 Jul 2025 12:44:36 +1000 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v3 8/8] icmp: let icmp use mac address from flowside structure Message-ID: References: <20250629171348.86323-1-jmaloy@redhat.com> <20250629171348.86323-9-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="hhBlq9i2M3wB3hep" Content-Disposition: inline In-Reply-To: <20250629171348.86323-9-jmaloy@redhat.com> Message-ID-Hash: 2LZDQXWZUV6D5ACP3OAQKJGGDWIX2UJ6 X-Message-ID-Hash: 2LZDQXWZUV6D5ACP3OAQKJGGDWIX2UJ6 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: sbrivio@redhat.com, dgibson@redhat.com, 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: --hhBlq9i2M3wB3hep Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Jun 29, 2025 at 01:13:47PM -0400, Jon Maloy wrote: > Even ICMP needs to be updated to use the external MAC address instead > of just the own tap address when applicable. We do that here. >=20 > Signed-off-by: Jon Maloy >=20 > --- > v3: - Adapted to the move of external MAC address from struct flowside > to struct flow_common > --- > icmp.c | 4 ++-- > ndp.c | 2 +- > tap.c | 10 ++++++---- > tap.h | 4 ++-- > udp.c | 12 ++++++++---- > 5 files changed, 19 insertions(+), 13 deletions(-) >=20 > diff --git a/icmp.c b/icmp.c > index 7e2b342..6a3b636 100644 > --- a/icmp.c > +++ b/icmp.c > @@ -129,12 +129,12 @@ void icmp_sock_handler(const struct ctx *c, union e= poll_ref ref) > const struct in_addr *daddr =3D inany_v4(&ini->eaddr); > =20 > ASSERT(saddr && daddr); /* Must have IPv4 addresses */ > - tap_icmp4_send(c, *saddr, *daddr, buf, n); > + tap_icmp4_send(c, *saddr, *daddr, buf, pingf->f.omac, n); The usual concern about first contact. But more so here, because ping flows are *always* outbound initiated. > } else if (pingf->f.type =3D=3D FLOW_PING6) { > const struct in6_addr *saddr =3D &ini->oaddr.a6; > const struct in6_addr *daddr =3D &ini->eaddr.a6; > =20 > - tap_icmp6_send(c, saddr, daddr, buf, n); > + tap_icmp6_send(c, saddr, daddr, buf, pingf->f.omac, n); > } > return; > =20 > diff --git a/ndp.c b/ndp.c > index 2e7f0bc..09a9e0a 100644 > --- a/ndp.c > +++ b/ndp.c > @@ -185,7 +185,7 @@ static void ndp_send(const struct ctx *c, const struc= t in6_addr *dst, > { > const struct in6_addr *src =3D &c->ip6.our_tap_ll; > =20 > - tap_icmp6_send(c, src, dst, buf, l4len); > + tap_icmp6_send(c, src, dst, buf, c->our_tap_mac, l4len); > } > =20 > /** > diff --git a/tap.c b/tap.c > index ceb9d9f..e62c9a6 100644 > --- a/tap.c > +++ b/tap.c > @@ -279,13 +279,14 @@ void tap_udp4_send(const struct ctx *c, struct in_a= ddr src, in_port_t sport, > * @src: IPv4 source address > * @dst: IPv4 destination address > * @in: ICMP packet, including ICMP header > + * @src_mac: MAC address to be used as source for message > * @l4len: ICMP packet length, including ICMP header > */ > void tap_icmp4_send(const struct ctx *c, struct in_addr src, struct in_a= ddr dst, > - const void *in, size_t l4len) > + const void *in, const void *src_mac, size_t l4len) > { > char buf[USHRT_MAX]; > - struct iphdr *ip4h =3D tap_push_l2h(c, buf, NULL, ETH_P_IP); > + struct iphdr *ip4h =3D tap_push_l2h(c, buf, src_mac, ETH_P_IP); > struct icmphdr *icmp4h =3D tap_push_ip4h(ip4h, src, dst, > l4len, IPPROTO_ICMP); > =20 > @@ -386,14 +387,15 @@ void tap_udp6_send(const struct ctx *c, > * @src: IPv6 source address > * @dst: IPv6 destination address > * @in: ICMP packet, including ICMP header > + * @src_mac: MAC address to be used as source for message > * @l4len: ICMP packet length, including ICMP header > */ > void tap_icmp6_send(const struct ctx *c, > const struct in6_addr *src, const struct in6_addr *dst, > - const void *in, size_t l4len) > + const void *in, const void *src_mac, size_t l4len) > { > char buf[USHRT_MAX]; > - struct ipv6hdr *ip6h =3D tap_push_l2h(c, buf, NULL, ETH_P_IPV6); > + struct ipv6hdr *ip6h =3D tap_push_l2h(c, buf, src_mac, ETH_P_IPV6); > struct icmp6hdr *icmp6h =3D tap_push_ip6h(ip6h, src, dst, l4len, > IPPROTO_ICMPV6, 0); > =20 > diff --git a/tap.h b/tap.h > index e640d95..7025eaa 100644 > --- a/tap.h > +++ b/tap.h > @@ -91,7 +91,7 @@ void tap_udp4_send(const struct ctx *c, struct in_addr = src, in_port_t sport, > struct in_addr dst, in_port_t dport, > const void *in, size_t dlen); > void tap_icmp4_send(const struct ctx *c, struct in_addr src, struct in_a= ddr dst, > - const void *in, size_t l4len); > + const void *in, const void *src_mac, size_t l4len); > const struct in6_addr *tap_ip6_daddr(const struct ctx *c, > const struct in6_addr *src); > void *tap_push_ip6h(struct ipv6hdr *ip6h, > @@ -103,7 +103,7 @@ void tap_udp6_send(const struct ctx *c, > uint32_t flow, void *in, size_t dlen); > void tap_icmp6_send(const struct ctx *c, > const struct in6_addr *src, const struct in6_addr *dst, > - const void *in, size_t l4len); > + const void *in, const void *src_mac, size_t l4len); > void tap_send_single(const struct ctx *c, const void *data, size_t l2len= ); > size_t tap_send_frames(const struct ctx *c, const struct iovec *iov, > size_t bufs_per_frame, size_t nframes); > diff --git a/udp.c b/udp.c > index 5942088..453bb51 100644 > --- a/udp.c > +++ b/udp.c > @@ -386,6 +386,7 @@ static void udp_tap_prepare(const struct mmsghdr *mmh, > * udp_send_tap_icmp4() - Construct and send ICMPv4 to local peer > * @c: Execution context > * @ee: Extended error descriptor > + * @uflow: UDP flow > * @toside: Destination side of flow > * @saddr: Address of ICMP generating node > * @in: First bytes (max 8) of original UDP message body > @@ -393,6 +394,7 @@ static void udp_tap_prepare(const struct mmsghdr *mmh, > */ > static void udp_send_tap_icmp4(const struct ctx *c, > const struct sock_extended_err *ee, > + const struct udp_flow *uflow, > const struct flowside *toside, > struct in_addr saddr, > const void *in, size_t dlen) > @@ -422,7 +424,7 @@ static void udp_send_tap_icmp4(const struct ctx *c, > tap_push_uh4(&msg.uh, eaddr, eport, oaddr, oport, in, dlen); > memcpy(&msg.data, in, dlen); > =20 > - tap_icmp4_send(c, saddr, eaddr, &msg, msglen); > + tap_icmp4_send(c, saddr, eaddr, &msg, uflow->f.omac, msglen); I don't think this is correct. We'd want the MAC corresponding to the source address of the ICMP, which is not necessarily the same as the peer of the flow (it could be an intermediate host). Actually trying to get that right for intermediate hosts is probably pointless and complex, so I think it would make sense to just use our_tap_mac if saddr !=3D eaddr. > } > =20 > =20 > @@ -430,6 +432,7 @@ static void udp_send_tap_icmp4(const struct ctx *c, > * udp_send_tap_icmp6() - Construct and send ICMPv6 to local peer > * @c: Execution context > * @ee: Extended error descriptor > + * @uflow: UDP flow > * @toside: Destination side of flow > * @saddr: Address of ICMP generating node > * @in: First bytes (max 1232) of original UDP message body > @@ -438,6 +441,7 @@ static void udp_send_tap_icmp4(const struct ctx *c, > */ > static void udp_send_tap_icmp6(const struct ctx *c, > const struct sock_extended_err *ee, > + const struct udp_flow *uflow, > const struct flowside *toside, > const struct in6_addr *saddr, > void *in, size_t dlen, uint32_t flow) > @@ -467,7 +471,7 @@ static void udp_send_tap_icmp6(const struct ctx *c, > tap_push_uh6(&msg.uh, eaddr, eport, oaddr, oport, in, dlen); > memcpy(&msg.data, in, dlen); > =20 > - tap_icmp6_send(c, saddr, eaddr, &msg, msglen); > + tap_icmp6_send(c, saddr, eaddr, &msg, uflow->f.omac, msglen); Same here. > } > =20 > /** > @@ -627,12 +631,12 @@ static int udp_sock_recverr(const struct ctx *c, in= t s, flow_sidx_t sidx, > if (hdr->cmsg_level =3D=3D IPPROTO_IP && > (o4 =3D inany_v4(&otap)) && inany_v4(&toside->eaddr)) { > dlen =3D MIN(dlen, ICMP4_MAX_DLEN); > - udp_send_tap_icmp4(c, ee, toside, *o4, data, dlen); > + udp_send_tap_icmp4(c, ee, uflow, toside, *o4, data, dlen); > return 1; > } > =20 > if (hdr->cmsg_level =3D=3D IPPROTO_IPV6 && !inany_v4(&toside->eaddr)) { > - udp_send_tap_icmp6(c, ee, toside, &otap.a6, data, dlen, > + udp_send_tap_icmp6(c, ee, uflow, toside, &otap.a6, data, dlen, > FLOW_IDX(uflow)); > return 1; > } --=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 --hhBlq9i2M3wB3hep Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmh++xMACgkQzQJF27ox 2GfuHQ//YumVcc2OMns/6pyn1YaWL4YUOKem78FhmRNO/BxMxmqL8Iq634bb3ZyB r8crblzbLq6EU0tzG7v39HX3v5whJPq4+HHF3A2QYs8M1DoV+uKdpfy+WLlYB8Wb xjXPZvahLCFR2GSaXfxZZge6GaRVPILNhqJFxelT2X0gdWAS3XRCRYUHsAIvRzHA 3GjQewvjQwuld+KrYjzQ4vLWVoAGBVcj4IdLo0T4gPmxIneW5Fk2NPGqcWtBJakD aql1Gd3hgI4XcpIKlA4eJiafvl9njpcWikhPbNzgD/OKPINDpwHa7S/RmhPPto0k gEhpYOEVYKZyiU1L7myPUzferqOW+WKWxYt4zXCLxLuf6E4KcFgoeI/TCqNWW9kQ S3dlfoiAxOUnkH+/hc99nZunRxW1G/pXGX/9+2xo/oh4I7oIlMgOZRTcCXBAjhK/ 2jw2fAR5lB2EVr3JkomJqauzmpOZSQeTPrpSrfFntYxdnRIqMCHzhprEcI+FKxBl 1HEwkjp5vrob3JaNjGuqMV7RYeEPQFuP9MqVWh+AgNhRZzz0zD4JIvNid8/+d1dd XbrbUrSedsTowpPZ61pTmYPd0sEZ+/VbsHn5bWrdn7G66Qm/Ngpg11Ovi3oh7kYr swki6xA12mY0OkQ2Wn/3i6GMWC0JUu/1+pVSFYKwK2LqkHap30Y= =kEaM -----END PGP SIGNATURE----- --hhBlq9i2M3wB3hep--