From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 539C25A026D for ; Sun, 7 Jan 2024 05:47:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1704602855; bh=ZMwdReteHjzOkBMnW8cp9DWkh6Ng7FnO1PBin/vqbAc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=r5BEM1+cvJA4bvRiHHHsY3zPH24nsOFvls0o765xPb1c+cTzzM//UjDstHUuvrn/q Jbi3/JMDqzXLE08F/llIvSYUo5auCx9fNzNk2lBHA3MTQKzj47y9qhRya3kxMN9tMp W+TjRO5FZXuW5w6PHAOzzVftduec76FK6K7vaeXpvv5owbBllmNTulOwzRu/Pke7bo TznLCQP9tQqCNDHDpxJirxdM6cQEHjvyfvtJHtcPO7/tjK0McdAglqcZT8xjo+ajAq AxDx7QjTaTmfCGr5m7FztWSL6H+2hhAA9riHpap9/dL72rc2p5yTB/EJEoQUxkhaDb SlmQ2MwKnHxDA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4T74S75lPPz4wdC; Sun, 7 Jan 2024 15:47:35 +1100 (AEDT) Date: Sun, 7 Jan 2024 15:41:02 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 08/12] icmp: Share more between IPv4 and IPv6 paths in icmp_tap_handler() Message-ID: References: <20231221065327.1307827-1-david@gibson.dropbear.id.au> <20231221065327.1307827-9-david@gibson.dropbear.id.au> <20240106170021.1bd09965@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="MxHv2cnTJaMHLfvc" Content-Disposition: inline In-Reply-To: <20240106170021.1bd09965@elisabeth> Message-ID-Hash: CJ2LF7SDSU452TEMYTZGFPNQIADO6LOY X-Message-ID-Hash: CJ2LF7SDSU452TEMYTZGFPNQIADO6LOY 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 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: --MxHv2cnTJaMHLfvc Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Jan 06, 2024 at 05:00:21PM +0100, Stefano Brivio wrote: > On Thu, 21 Dec 2023 17:53:23 +1100 > David Gibson wrote: >=20 > > Currently icmp_tap_handler() consists of two almost disjoint paths for = the > > IPv4 and IPv6 cases. The only thing they share is an error message. > > We can use some intermediate variables to refactor this to share some m= ore > > code between those paths. > >=20 > > Signed-off-by: David Gibson > > --- > > icmp.c | 140 ++++++++++++++++++++++++++++----------------------------- > > 1 file changed, 69 insertions(+), 71 deletions(-) > >=20 > > diff --git a/icmp.c b/icmp.c > > index 02739b9..f6c744a 100644 > > --- a/icmp.c > > +++ b/icmp.c > > @@ -154,102 +154,100 @@ int icmp_tap_handler(const struct ctx *c, uint8= _t pif, int af, > > const void *saddr, const void *daddr, > > const struct pool *p, const struct timespec *now) > > { > > + uint8_t proto =3D af =3D=3D AF_INET ? IPPROTO_ICMP : IPPROTO_ICMPV6; > > + const char *const pname =3D af =3D=3D AF_INET ? "ICMP" : "ICMPv6"; > > + union { > > + struct sockaddr sa; > > + struct sockaddr_in sa4; > > + struct sockaddr_in6 sa6; > > + } sa =3D { .sa.sa_family =3D af }; > > + const socklen_t sl =3D af =3D=3D AF_INET ? sizeof(sa.sa4) : sizeof(sa= =2Esa6); > > + struct icmp_id_sock *id_map; > > + uint16_t id, seq; > > size_t plen; > > + void *pkt; > > + int s; > > =20 > > (void)saddr; > > (void)pif; > > =20 > > + pkt =3D packet_get(p, 0, 0, 0, &plen); > > + if (!pkt) > > + return 1; > > + > > if (af =3D=3D AF_INET) { > > - struct sockaddr_in sa =3D { > > - .sin_family =3D AF_INET, > > - }; > > - union icmp_epoll_ref iref; > > - struct icmphdr *ih; > > - int id, s; > > - > > - ih =3D packet_get(p, 0, 0, sizeof(*ih), &plen); > > - if (!ih) > > + struct icmphdr *ih =3D (struct icmphdr *)pkt; > > + > > + if (plen < sizeof(*ih)) > > return 1; >=20 > This is obviously the same as the existing check. On the other hand, > I've been trying to use packet_get() to validate any packet read length > we need. >=20 > Here, the first call sets plen, but the only purpose is to get the > length of the message we need to relay. Given that we need at least > sizeof(*ih) here, I would prefer keep the explicit packet_get() > validation. >=20 > Same for the IPv6 path below. Ok, fair enough. Updated. --=20 David Gibson | 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 --MxHv2cnTJaMHLfvc Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmWaK08ACgkQzQJF27ox 2GeDlA//XYaHyq8IMvBpfqgOmYKkd1QAk5QwOTeijW9+naLVtwj66c3cl0dccP3+ JejaDoSdstmM4K8KgTkn4JDPkS2ZhhVOcgqFGbIPomW4auQA6MdsJWik3fbTPXrs nDADuH3sD0+CG2kAW7XmBV+41qN49eNcbsarNYcLhbqIZfRc6INhPsA/1Ath24UX ACcE77iHhleUaiZHozl73V0KbI+frWH1C4gqV9C1mFR74Fxpco0cfqE1dJ1mLTaA 0Kn5KPFxQzypEBeBht3MMHLw9u2mtJp6P8UndIUljQkQwC8JyLXmqAMW+4tCxkn4 AbLQ+bwy9Ldsq294mvJOhK454Jh5RnKZbjG00CaiZujn976/T1sy7LRBADo9f5VG ITYlEKVImR4QuZVRHjc2lCuVIEGLpdE7IlP1qoroco+5/2Ku9kxOOqw1jHvaShlN JQoqd6vQGcwkUi9FF8Ug1tK8WRwQnQkloiEZk6uQOWJTEjnZc8b9xxSbld3rFPP1 k10pze3ltVqoHdUgEDxg5IPtBRSEBeJlnM3dHthqLoXqozkfAbxz1+PHR04uKkr+ TEuzuZAxTWHtT6ErtadExnq2ZtUMXXb4zCYE1WsHoQkdaiTlBplfq8jbPSQbL/Gj Dj0tq738j2C69nbvOBrNoqQGO1H9dck+27gm93hKK2JabHdulNw= =0I77 -----END PGP SIGNATURE----- --MxHv2cnTJaMHLfvc--