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=202504 header.b=T4BfrbFX; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id B2AD65A0275 for ; Wed, 16 Apr 2025 02:38:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202504; t=1744763923; bh=i9nJJYoQLcwX+gOsLcJHoVzjexZ/fMLUhnuEdn8SD6g=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=T4BfrbFXqk6VnRN878ZD+fiTTRAPzXQqfbpS7j3m/TJdp7HC2IIMFLQfc4N6co175 eid5dlFOhTSuQWNbJimv7wOQ5uC8LtHOYQ89L1y7omWX6/d/gkOfS2Dy42WewfC1ur FvsIkixQzgFyNcJw837dFc6dHz3EOnN4KJqOnltZzaPLljgP/5oQWX9S0tHNb0MLi2 BUZdJykTe6N18MnaKVjTsZOlMliIMYdFAOSUJjO3C7lKI5X45FobRRPFWMtN5ARUHB Lc4VE7U7yNxRS47leWaxUxR2Dlv5VOL53RG5rKURBo3om+GSRgEojNdAlPnVVFZfEA l/Edt/Jk2KNYg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZchwM1cg0z4xQ2; Wed, 16 Apr 2025 10:38:43 +1000 (AEST) Date: Wed, 16 Apr 2025 10:37:54 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 5/7] udp: Add udp_pktinfo() helper Message-ID: References: <20250415071624.2618589-1-david@gibson.dropbear.id.au> <20250415071624.2618589-6-david@gibson.dropbear.id.au> <20250415205400.68c7c1d2@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ocwgTF3LY2OwE6M4" Content-Disposition: inline In-Reply-To: <20250415205400.68c7c1d2@elisabeth> Message-ID-Hash: Q65ICQHX4BPQ6Q5RLQRGU2CWI5BJ42JS X-Message-ID-Hash: Q65ICQHX4BPQ6Q5RLQRGU2CWI5BJ42JS 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, Jon Maloy 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: --ocwgTF3LY2OwE6M4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Apr 15, 2025 at 08:54:00PM +0200, Stefano Brivio wrote: > I took the liberty of applying this with two minor changes: >=20 > On Tue, 15 Apr 2025 17:16:22 +1000 > David Gibson wrote: >=20 > > Currently we open code parsing the control message for IP_PKTINFO in > > udp_peek_addr(). We have an upcoming case where we want to parse PKTIN= FO > > in another place, so split this out into a helper function. > >=20 > > While we're there, make the parsing a bit more robust: scan all cmsgs to > > look for the one we want, rather than assuming there's only one. > >=20 > > Signed-off-by: David Gibson > > --- > > udp.c | 52 ++++++++++++++++++++++++++++++++++++---------------- > > 1 file changed, 36 insertions(+), 16 deletions(-) > >=20 > > diff --git a/udp.c b/udp.c > > index 0bec499d..352ab83b 100644 > > --- a/udp.c > > +++ b/udp.c > > @@ -464,6 +464,41 @@ static void udp_send_tap_icmp6(const struct ctx *c, > > tap_icmp6_send(c, saddr, eaddr, &msg, msglen); > > } > > =20 > > +/** > > + * udp_pktinfo() - Retreive packet destination address from cmsg >=20 > Nit: "Retrieve", and: Oops, thanks. > > + * @msg: msghdr into which message has been received > > + * @dst: (Local) destination address of message in @mh (output) > > + * > > + * Return: 0 on success, -1 if the information was missing (@dst is se= t to > > + * inany_any6). > > + */ > > +static int udp_pktinfo(struct msghdr *msg, union inany_addr *dst) > > +{ > > + struct cmsghdr *hdr; > > + > > + for (hdr =3D CMSG_FIRSTHDR(msg); hdr; hdr =3D CMSG_NXTHDR(msg, hdr)) { > > + if (hdr->cmsg_level =3D=3D IPPROTO_IP && > > + hdr->cmsg_type =3D=3D IP_PKTINFO) { > > + const struct in_pktinfo *i4 =3D (void *)CMSG_DATA(hdr); > > + > > + *dst =3D inany_from_v4(i4->ipi_addr); > > + return 0; > > + } > > + > > + if (hdr->cmsg_level =3D=3D IPPROTO_IPV6 && > > + hdr->cmsg_type =3D=3D IPV6_PKTINFO) { > > + const struct in6_pktinfo *i6 =3D (void *)CMSG_DATA(hdr); > > + > > + dst->a6 =3D i6->ipi6_addr; > > + return 0; > > + } > > + } > > + > > + err("Missing PKTINFO cmsg on datagram"); >=20 > ...from a quick glance at the matching kernel path, this looks a bit > too likely for err(), so I got a bit scared, and turned it to debug(). Ehh.. fair enough. > I think warn() is actually more correct than debug() (I think it > shouldn't really be err() though because we can keep using this flow), So, technically we don't have a flow at this point: the point here is that if we don't get the pktinfo we don't have the information we need to determine the right flow. But, regardless, you're right, warn() would be the right level in theory. > but I would feel a lot more confident about it once we have > rate-limited versions of those. Ok. > Of course, I'll change this back to err() or warn() if you have a > compelling reason. >=20 > Similar reasoning in 7/7 where this is called. >=20 > By the way, two messages (here and in caller) look redundant to me, > regardless of their severity, but I'm not sure if you have further > changes in mind where these separate prints would make sense. In this patch there aren't two messages, it's only in this function. There are two when I add the other caller in 7/7 which was an oversight. --=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 --ocwgTF3LY2OwE6M4 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmf++9MACgkQzQJF27ox 2GfHUg/8CqVh9W/Qvf8W5YsJNMhsFxBYPu30A2gyD71oJE97rQzg/LLSGf5hXBn5 mwS0Scut3SXUyHDlhv9vOpq0vbfzxoaWhDS1vFNq3JOQu7hhrHAq1npuT4LKAzmS T+hX4b0Lc6gHgnNVqsGnz/AqftiLXsZUfNQMDT82+X/KJTyGe/LToe4q09s+Wcr4 hTWHycQT1dhdG5SBi/8N/ThnhiAuqNdUCBnwtF+cP04ftVUzv3wun5bqCUAtfTgc KSoxpsFzGkOFhg4wZrtbdlAMLbX97aNAG5iWjVtI33d9nvwidLk9fMzIGoO8eazS XzjPCji69c4CVQ45qAqUf0U//l/Stc7VnMLdp1Kg7ZPwx9HFcgN2ZXMf9zeKsLK9 iiYwMUg5Wodr2brpHdISImBFWvAgOuhlWjlEV07NJTtkh6zqYVuUmifKfBlvYLEq +BXlJdAOc0sxWiymEA13zQXTon3At7HePwd/uUrpr0peF72Hs0oyMXPozSrnv4Xp aUD9KHf0+gOQzUOoGM6qVVUN464SmZ8gqNMCQrWEkCJQTO+OOpRk+34h9CJYjPPL JnwT5FYiULIDQJeQWE5IMc9/pUgkSOBGUceepsspr536n5vPvQpAqrfzZIKfIQt2 i++S3CzgOJsDzhkQtpbAlKUBTYk/p41/ZwJFBAJUBwaoqHhfLWg= =H0Ic -----END PGP SIGNATURE----- --ocwgTF3LY2OwE6M4--