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=202502 header.b=egyKiqWU; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 98E115A0008 for ; Thu, 27 Mar 2025 00:28:51 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1743031727; bh=pegocoz/zq4Np3P5AweIXsJvdF0PJN7sl1B5sb5LpjE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=egyKiqWUFIb1LtsoYvfCNX2BzIAYD5MlOA5ss8ncmuElWg4+agzjmVb9MccWsJgUn YZc+0YLwk/BjWhsc/k+C+Gt89o/QShzi0F9rMapou98XLD7rnJIo9ytvJTz8sbd62y 39Dou/Xq0Kh1E0mXcvX4Oq7MpG/hc+glR87w2au5+n7xXbnpRdqUctlg5W60eNU2m4 ktlS+ZEO2ZrvPcqMM3n9czuRyy+7BlWd2pEVO4bDH3+GwQ4DK9U5gF9cfdlKkn0aPW tXq2Dj4fZ6cwswAZZ1TKuDV02tYCRLzeJAiU6mgzmAu4j/PeMLZmzvTZrG7Y/PAI8V hpRqQ71lAejnA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZNNJv0hSwz4x8Z; Thu, 27 Mar 2025 10:28:47 +1100 (AEDT) Date: Thu, 27 Mar 2025 10:28:43 +1100 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v2] udp: correct source address for ICMP messages Message-ID: References: <20250326155902.2903258-1-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="zdZBPJfK+aETvJXg" Content-Disposition: inline In-Reply-To: <20250326155902.2903258-1-jmaloy@redhat.com> Message-ID-Hash: SRY3CPDGLFFZONJIEMKGCMEDGLG2K227 X-Message-ID-Hash: SRY3CPDGLFFZONJIEMKGCMEDGLG2K227 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, sbrivio@redhat.com, lvivier@redhat.com, dgibson@redhat.com 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: --zdZBPJfK+aETvJXg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 26, 2025 at 11:59:02AM -0400, Jon Maloy wrote: > While developing traceroute forwarding tap-to-sock we found that > struct msghdr.msg_name for the ICMPs in the opposite direction always > contains the destination address of the original UDP message, and not, > as one might expect, the one of the host which created the error message. >=20 > Study of the kernel code reveals that this address instead is appended > as extra data after the received struct sock_extended_err area. >=20 > We now change the ICMP receive code accordingly. >=20 > Fixes: 55431f0077b6 ("udp: create and send ICMPv4 to local peer when appl= icable") > Fixes: 68b04182e07d ("udp: create and send ICMPv6 to local peer when appl= icable") >=20 > Signed-off-by: Jon Maloy Reviewed-by: David Gibson This will conflict boringly with my rename of the "send_conn_fail" functions of course. I think this is more important, so I suggest this go first, and I'll rebase my patch. > --- > v2: Removed stray comment and unecessary initializations, as per > feedback from David Gibson > --- > udp.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) >=20 > diff --git a/udp.c b/udp.c > index 80520cb..9d16529 100644 > --- a/udp.c > +++ b/udp.c > @@ -510,10 +510,13 @@ static void udp_send_conn_fail_icmp6(const struct c= tx *c, > */ > static int udp_sock_recverr(const struct ctx *c, union epoll_ref ref) > { > - const struct sock_extended_err *ee; > + struct errhdr { > + struct sock_extended_err ee; > + union sockaddr_inany saddr; > + }; > + const struct errhdr *eh; > const struct cmsghdr *hdr; > - union sockaddr_inany saddr; > - char buf[CMSG_SPACE(sizeof(*ee))]; > + char buf[CMSG_SPACE(sizeof(struct errhdr))]; > char data[ICMP6_MAX_DLEN]; > int s =3D ref.fd; > struct iovec iov =3D { > @@ -521,8 +524,6 @@ static int udp_sock_recverr(const struct ctx *c, unio= n epoll_ref ref) > .iov_len =3D sizeof(data) > }; > struct msghdr mh =3D { > - .msg_name =3D &saddr, > - .msg_namelen =3D sizeof(saddr), > .msg_iov =3D &iov, > .msg_iovlen =3D 1, > .msg_control =3D buf, > @@ -553,7 +554,7 @@ static int udp_sock_recverr(const struct ctx *c, unio= n epoll_ref ref) > return -1; > } > =20 > - ee =3D (const struct sock_extended_err *)CMSG_DATA(hdr); > + eh =3D (const struct errhdr *)CMSG_DATA(hdr); > if (ref.type =3D=3D EPOLL_TYPE_UDP_REPLY) { > flow_sidx_t sidx =3D flow_sidx_opposite(ref.flowside); > const struct flowside *toside =3D flowside_at_sidx(sidx); > @@ -561,18 +562,19 @@ static int udp_sock_recverr(const struct ctx *c, un= ion epoll_ref ref) > =20 > if (hdr->cmsg_level =3D=3D IPPROTO_IP) { > dlen =3D MIN(dlen, ICMP4_MAX_DLEN); > - udp_send_conn_fail_icmp4(c, ee, toside, saddr.sa4.sin_addr, > + udp_send_conn_fail_icmp4(c, &eh->ee, toside, > + eh->saddr.sa4.sin_addr, > data, dlen); > } else if (hdr->cmsg_level =3D=3D IPPROTO_IPV6) { > - udp_send_conn_fail_icmp6(c, ee, toside, > - &saddr.sa6.sin6_addr, > + udp_send_conn_fail_icmp6(c, &eh->ee, toside, > + &eh->saddr.sa6.sin6_addr, > data, dlen, sidx.flowi); > } > } else { > trace("Ignoring received IP_RECVERR cmsg on listener socket"); > } > debug("%s error on UDP socket %i: %s", > - str_ee_origin(ee), s, strerror_(ee->ee_errno)); > + str_ee_origin(&eh->ee), s, strerror_(eh->ee.ee_errno)); > =20 > 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 --zdZBPJfK+aETvJXg Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmfkjasACgkQzQJF27ox 2Gd8Lw/+Nv+xAefMMJcL1XsgaH5RUcTaGicq2/K2Fnxc1SIKFUT11Qa/crtEpKBh YxZh7hIfoKfMmKWwEATzl80k31bVq/W3OPTLMYIKljD/H7qZG4+fnXz/qN6jfy/5 DdqISRDr4BlJ8Nb7+VEoWp8/6Wd78n3En26p+xAPV1ljSnnUl4qUMjGRKa0C2z5I e0Q7gD4pNPHsXUmKYKI9pXkA3enYuCh+SYABkMwVTWDgxfKA+m3V2oNGzzdq3c3o rtYYQH4rnjx8WXdbr6jrvPWg7bzAEJZ9eK3o7nDWHtdfgmhjoLAYIXeJ0L7+UTmA 2FPzpXzeRRhn38Xa5HOG5Adf7uQgSQB2c61ec1SAyEGqx+5S6svTXWqvNpolBEcg cgYW/8P4yFJ36AS3bzZUvZ85M0IpsDbhHqqz4RaVppiHI9Iy3tq5lOtIb2yEdEkz 1I53otLW8VnCgtGf1LENJTsRyo1tZI49CFgtyr5EzFTxVXCJVy7mdusDbbcX+Zfv Hu+nwZFnMfyyebDF62DdSR47jtKnVvVMXmI/pVqFJrIFzPB936OzHW+TSsscleb9 0l8nujypZP2vXaHIqL7GGpJVEDKbtv577fSV97fWGvKbrh82pySdQITvlbCcWRTV E2+f9ALcaChiDTX1rlcxPIiCvDA7+lFe1VQe0qh7KlAfy/B3V0w= =qvz+ -----END PGP SIGNATURE----- --zdZBPJfK+aETvJXg--