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=CUN2t1w1; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 0B1DA5A0625 for ; Sun, 16 Mar 2025 12:41:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1742125268; bh=EnvWfj6+BsjY66R4QMmwhiq0ySVXnFcRsEy1SAin2mY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=CUN2t1w1mmSJblcBfGerJw2xWMl2JxsNadpyKab853o+lqqvT34YlapjVLkOczC6+ FQEm0lVaD5wDUTxS7zzOt57kmBWLm0pCyP6ghMpQEe8IwHI5bygMauw10z0lT0Gne+ XXCCAuqd+dgP1JPAKcrCVY2mMzRy1TaMGmNN1RxB5G6aq0Kezy0QI3+MTdBIcUJVlL FLE20WYeGZs8n2oG0iuo13gl1JZnXfe+UazSmO5TR5bqWmN0k2aJsiWiDH3BzoSnXr GnSuKGD0W9DjeeQT9uBnjH714ufUT5qnTnpI3vw7ZrYYe563T2mya6IrtHWMnpXUyH /rfVwmZZYolsA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZFx503sSgz4wgp; Sun, 16 Mar 2025 22:41:08 +1100 (AEDT) Date: Sun, 16 Mar 2025 15:47:41 +1100 From: David Gibson To: Jon Maloy Subject: Re: [PATCH 1/2] udp: correct source address for ICMP messages Message-ID: References: <20250315153245.435293-1-jmaloy@redhat.com> <20250315153245.435293-2-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="7Hz88lcAmBbmkOdt" Content-Disposition: inline In-Reply-To: <20250315153245.435293-2-jmaloy@redhat.com> Message-ID-Hash: N6C4QYTALHBMCK3TXRYTA2UMNRYLGVMG X-Message-ID-Hash: N6C4QYTALHBMCK3TXRYTA2UMNRYLGVMG 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: --7Hz88lcAmBbmkOdt Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Mar 15, 2025 at 11:32:44AM -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 The idea looks good, a few minor warts in the implementation. > --- > udp.c | 26 +++++++++++++++----------- > 1 file changed, 15 insertions(+), 11 deletions(-) >=20 > diff --git a/udp.c b/udp.c > index 80520cb..271e570 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,11 +524,11 @@ static int udp_sock_recverr(const struct ctx *c, un= ion epoll_ref ref) > .iov_len =3D sizeof(data) > }; > struct msghdr mh =3D { > - .msg_name =3D &saddr, > - .msg_namelen =3D sizeof(saddr), > + .msg_name =3D 0, > + .msg_namelen =3D 0, I think you can just omit these and they'll be zero initialized. > .msg_iov =3D &iov, > .msg_iovlen =3D 1, > - .msg_control =3D buf, > + .msg_control =3D buf,//(void *)&errhdr, Looks like you have a leftover comment here. > .msg_controllen =3D sizeof(buf), > }; > ssize_t rc; > @@ -553,7 +556,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 +564,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); So, it looks from man ip(7) that you're supposed to use the SO_EE_OFFENDER() macro to get the socket address, rather than just assuming it's immediately after the struct sock_extended_err. Except... without assuming that, I can't really see how you're supposed to properly size the buffer > } 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 --7Hz88lcAmBbmkOdt Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmfWV+AACgkQzQJF27ox 2Gfu9Q/7B4KjMS5YwiMDpWj7mimPLsuMp6l7YQBRiOw5GvC4R5L7frGbKaklCPbJ eB8ArOfN39npRduOZcbJ99e7fB2LRnr4rWLUZAOGdl99DztxV6dXhFTzDdyCFuXK 8E2zNF8AGTgHZqht0DO3haFanLHv0ZKVnp7CSRfYcP+VU+q2PGuICYxW31IrR4Mk UNSW1n4pcaFydOgqVwl5aGUSCb9P8pE5OSOGMfjMMd45rRG2dq85GbqHlqFY6jZY 3v+EYoIDhJcpBpo1W73h9rDIW5aRLX9y+dfZjCb+RDRycQYcxMi3nw4pyQE+7cNp aXXrqmLZqxSJxKwYVt+OO+FGvJ7dCSOLF1E69w0Bs6WWNNRY2mt95U8T/tz3Hkyb +aPg+ufHM0LuUq1EnRckg/Ey6q2/ZapVTIMZZ1KRFaZYBL8JVjGSqp0XNpnUJPlf kf6u3mPwIq97pXk4aoXa/zk/79N+cOjnknwUvwMIQ0bH3PlMu+IuzYYg40g/ZjcP oJ4I1uuwq1oTQn9QZm8pRkeasN8GKHFKiCDED2sMtkx2oflxPsl86kijiyFysbzH 44gdrGH8BhSj46/5a97S9KBNvUlHkaSn3dE0EdgbRX0wp1a+h/gCbuSl48gPV/EB 3JvL3gvieetuR3DR47RyfnEe0vdlc9QNBbIe7dXVZBN/MPjORSI= =jOC9 -----END PGP SIGNATURE----- --7Hz88lcAmBbmkOdt--