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=202508 header.b=NSl3T7Fi; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id D8C575A0279 for ; Mon, 08 Sep 2025 05:35:57 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1757302554; bh=WmLv1paQ/1BjUPEL+NRCOhClI+Z8yqp17c+xmLZdJP8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=NSl3T7FiEbWZOTCo6ZLTnO9TyAMN2offx1c6u5Nxv/u54D6rTp65nl+I7kkNhvS0F 3KtVrOsl6DQuC1M1J+BAWoGw+MjYugxFaKZkJD54B9nWV8bsblSL3U/pakubvyHSqO 1E16Q0D6t75z1Py26igIU9fjBaDp7EmKUgVTdYfeFCzNZLd2/0xJF9u+k8QrTf8TL+ L8Xi2WJ4wvamaWEdyRlYQj+IeH9Az2fMFjDMr9jR4hcmDFr3L4CQYZsDPotCWjiP/2 O4mxCumnhLWSPkFQnX9H20vbmNIapF4Z5yH6EYgT3hI5LgQdS7hTL6oczUyvcBS0Ls /vEBK6nSG3tvQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4cKszt4hH5z4w2S; Mon, 8 Sep 2025 13:35:54 +1000 (AEST) Date: Mon, 8 Sep 2025 13:35:47 +1000 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v5 10/10] icmp: let icmp use mac address from flowside structure Message-ID: References: <20250906021154.2760611-1-jmaloy@redhat.com> <20250906021154.2760611-11-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="OPdniQj0dXqwN0H/" Content-Disposition: inline In-Reply-To: <20250906021154.2760611-11-jmaloy@redhat.com> Message-ID-Hash: HVIM34CE45YBOE43IXD6LEVV7RDWW7KP X-Message-ID-Hash: HVIM34CE45YBOE43IXD6LEVV7RDWW7KP 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: --OPdniQj0dXqwN0H/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 05, 2025 at 10:11:54PM -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 > v4: - Adapted to name changes in previous commits in this series > v5: - Added conditional lookup in ARP/NDP if the flow's tap_omac is > undefined > --- > icmp.c | 8 ++++++-- > ndp.c | 2 +- > tap.c | 10 ++++++---- > tap.h | 4 ++-- > udp.c | 12 ++++++++---- > 5 files changed, 23 insertions(+), 13 deletions(-) >=20 > diff --git a/icmp.c b/icmp.c > index 6dffafb..2d5bf0c 100644 > --- a/icmp.c > +++ b/icmp.c > @@ -125,17 +125,21 @@ void icmp_sock_handler(const struct ctx *c, union e= poll_ref ref) > flow_dbg(pingf, "echo reply to tap, ID: %"PRIu16", seq: %"PRIu16, > ini->eport, seq); > =20 > + /* Try to find true MAC address in ARP/NDP table if needed */ > + if (mac_undefined(pingf->f.tap_omac)) > + fwd_neigh_mac_get(c, &ini->oaddr, pingf->f.tap_omac); > + > if (pingf->f.type =3D=3D FLOW_PING4) { > const struct in_addr *saddr =3D inany_v4(&ini->oaddr); > 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.tap_omac, n); > } 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.tap_omac, n); > } > return; > =20 > diff --git a/ndp.c b/ndp.c > index 820c556..e0e8938 100644 > --- a/ndp.c > +++ b/ndp.c > @@ -184,7 +184,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 05429aa..6443812 100644 > --- a/tap.c > +++ b/tap.c > @@ -278,13 +278,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, c->our_tap_mac, 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 > @@ -385,14 +386,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, c->our_tap_mac, ETH_P_IPV= 6); > + 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 02f7761..1864173 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 7d98845..26f8c22 100644 > --- a/udp.c > +++ b/udp.c > @@ -385,6 +385,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 > @@ -392,6 +393,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) > @@ -421,7 +423,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.tap_omac, msglen); As I noted on an earlier version, this is only correct if the ICMP is coming from the peer, not an intermediate host. You'd need to look up the MAC based on saddr, which could be different from the peer address. Or... I suspect it's probably good enough to just use our_tap_mac in the case that (saddr !=3D peer address). > } > =20 > =20 > @@ -429,6 +431,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 > @@ -437,6 +440,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) > @@ -466,7 +470,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.tap_omac, msglen); Same thing here. > } > =20 > /** > @@ -626,12 +630,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 > 2.50.1 >=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 --OPdniQj0dXqwN0H/ Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmi+TxIACgkQzQJF27ox 2GfcZxAAqfKj6qZdlNHWc9e9djrt6eoosslzALbrfamC/B+voZr7so7D75FrjCoE NqSsuAXfAz+zJyovhGlcAUiIZ3mG3YJ2CTFriMn8cHzPxwix1VSFKa76q4RUThuj VvjdpB1GSRYvZ6IqEIs636w2atQJnVdSaKgf7N30SnAOlO6zCvVObKtpzC9t/L2F nBhVkgQxzuVxSILqsMD9vRP3ehE/JHbPX3cgOiN/ikc8OrPOdXbfSfcqotXPRDL8 /HbKexDVdnU5cFQ+EqhuQkIprqa9oN+FRXmWLrJ8DXrSTmjQ8/zg/gVqAyeW7QiB 8AAWZ4YB0a1zyiNAizAsOfC86pT7rErmlB+fpqH7159ffbSGKALrvb+5TGkHqeH5 4wB9FdVeB+326yw8yjuGNy4/MVY6XlkpT3vZBIVdD2/qjYeGxqWQ7As/TP6LD/iY HXpjja0hPtnKWjyO7B+sbb/E7XwY8a1Gvi0JtIoOxV92Fg2PhDPvKcV2qa8meEnt cpNrxMRBtQV9OUidhA0A+nuLvZw9p/20SfXN6UYEvvxF0bxf0dE7x/NpDDEuZ4OE +FUKwtxg+HXmHgHKDOPJZ+3QK9TBfb/Yn3Grmg2IgZbdby3WaHAxXLK0d0o0Wxkh irmKdiCShLqhCazQEjn2gG4QEfLdPU6lGpT5BUZZSzqpuDV7pXk= =/YSV -----END PGP SIGNATURE----- --OPdniQj0dXqwN0H/--