From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id C369E5A0265 for ; Tue, 8 Nov 2022 02:02:13 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4N5qZB2649z4xx8; Tue, 8 Nov 2022 12:02:10 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1667869330; bh=WjXzGboku3vmsjNjW9Nf8y/ugwcqGlPVDvk+591rYx8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=YfbFRcQ+UoU8+Ay/2BJ5vt/4P8bEGGf3n3a4oj/e4jl/IRQVnrC1XcS3/neX9IbzK QhkPK2ConslVZn73v1B4FNJiQSpI13Qs0Zaca8o+0BHZBz5EoEPHgX+nILcJfpRrSR 3vF3T5ueIZFAtVADY7OtK0tKv5ruOwoadeU7XIzw= Date: Tue, 8 Nov 2022 11:54:26 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 07/10] tcp: Remove ugly address union from struct tcp_conn Message-ID: References: <20221104084333.3761760-1-david@gibson.dropbear.id.au> <20221104084333.3761760-8-david@gibson.dropbear.id.au> <20221107190838.6765aa8f@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="RF0w68st+ghNmV8g" Content-Disposition: inline In-Reply-To: <20221107190838.6765aa8f@elisabeth> Message-ID-Hash: OLHGRN3HK76URGUEMKEWI5TZFM26MWYM X-Message-ID-Hash: OLHGRN3HK76URGUEMKEWI5TZFM26MWYM 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.3 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: --RF0w68st+ghNmV8g Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Nov 07, 2022 at 07:08:38PM +0100, Stefano Brivio wrote: > On Fri, 4 Nov 2022 19:43:30 +1100 > David Gibson wrote: >=20 > > We now only extract the v4 part of the tcp_conn::a union in one place. > > Make this neater by using a helper to extract the IPv4 address from the > > mapped address in that place. > >=20 > > Signed-off-by: David Gibson > > --- > > tcp.c | 40 +++++++++++++++------------------------- > > util.c | 13 +++++++++++++ > > util.h | 1 + > > 3 files changed, 29 insertions(+), 25 deletions(-) > >=20 > > diff --git a/tcp.c b/tcp.c > > index 3816a1c..6634abb 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -416,10 +416,7 @@ struct tcp6_l2_head { /* For MSS6 macro: keep in s= ync with tcp6_l2_buf_t */ > > * @ws_to_tap: Window scaling factor advertised to tap/guest > > * @sndbuf: Sending buffer in kernel, rounded to 2 ^ SNDBUF_BITS > > * @seq_dup_ack_approx: Last duplicate ACK number sent to tap > > - * @a.a6: IPv6 remote address, can be IPv4-mapped > > - * @a.a4.zero: Zero prefix for IPv4-mapped, see RFC 6890, Table 20 > > - * @a.a4.one: Ones prefix for IPv4-mapped > > - * @a.a4.a: IPv4 address > > + * @addr: IPv6 remote address, can be IPv4-mapped > > * @tap_port: Guest-facing tap port > > * @sock_port: Remote, socket-facing port > > * @wnd_from_tap: Last window size from tap, unscaled (as received) > > @@ -489,15 +486,8 @@ struct tcp_conn { > > uint8_t seq_dup_ack_approx; > > =20 > > =20 > > - union { > > - struct in6_addr a6; > > - struct { > > - uint8_t zero[10]; > > - uint8_t one[2]; > > - struct in_addr a; > > - } a4; > > - } a; > > -#define CONN_V4(conn) IN6_IS_ADDR_V4MAPPED(&conn->a.a6) > > + struct in6_addr addr; > > +#define CONN_V4(conn) IN6_IS_ADDR_V4MAPPED(&conn->addr) > > #define CONN_V6(conn) (!CONN_V4(conn)) > > =20 > > in_port_t tap_port; > > @@ -960,7 +950,7 @@ static int tcp_rtt_dst_low(const struct tcp_conn *c= onn) > > int i; > > =20 > > for (i =3D 0; i < LOW_RTT_TABLE_SIZE; i++) > > - if (IN6_ARE_ADDR_EQUAL(&conn->a.a6, low_rtt_dst + i)) > > + if (IN6_ARE_ADDR_EQUAL(&conn->addr, low_rtt_dst + i)) > > return 1; > > =20 > > return 0; > > @@ -982,7 +972,7 @@ static void tcp_rtt_dst_check(const struct tcp_conn= *conn, > > return; > > =20 > > for (i =3D 0; i < LOW_RTT_TABLE_SIZE; i++) { > > - if (IN6_ARE_ADDR_EQUAL(&conn->a.a6, low_rtt_dst + i)) > > + if (IN6_ARE_ADDR_EQUAL(&conn->addr, low_rtt_dst + i)) > > return; > > if (hole =3D=3D -1 && IN6_IS_ADDR_UNSPECIFIED(low_rtt_dst + i)) > > hole =3D i; > > @@ -994,10 +984,10 @@ static void tcp_rtt_dst_check(const struct tcp_co= nn *conn, > > if (hole =3D=3D -1) > > return; > > =20 > > - memcpy(low_rtt_dst + hole++, &conn->a.a6, sizeof(conn->a.a6)); > > + memcpy(low_rtt_dst + hole++, &conn->addr, sizeof(conn->addr)); > > if (hole =3D=3D LOW_RTT_TABLE_SIZE) > > hole =3D 0; > > - memcpy(low_rtt_dst + hole, &in6addr_any, sizeof(conn->a.a6)); > > + memcpy(low_rtt_dst + hole, &in6addr_any, sizeof(conn->addr)); > > #else > > (void)conn; > > (void)tinfo; > > @@ -1285,7 +1275,7 @@ static int tcp_hash_match(const struct tcp_conn *= conn, > > const struct in6_addr *addr, > > in_port_t tap_port, in_port_t sock_port) > > { > > - if (IN6_ARE_ADDR_EQUAL(&conn->a.a6, addr) && > > + if (IN6_ARE_ADDR_EQUAL(&conn->addr, addr) && > > conn->tap_port =3D=3D tap_port && conn->sock_port =3D=3D sock_por= t) > > return 1; > > =20 > > @@ -1330,7 +1320,7 @@ static void tcp_hash_insert(const struct ctx *c, = struct tcp_conn *conn) > > { > > int b; > > =20 > > - b =3D tcp_hash(c, &conn->a.a6, conn->tap_port, conn->sock_port); > > + b =3D tcp_hash(c, &conn->addr, conn->tap_port, conn->sock_port); > > conn->next_index =3D tc_hash[b] ? tc_hash[b] - tc : -1; > > tc_hash[b] =3D conn; > > conn->hash_bucket =3D b; > > @@ -1660,7 +1650,7 @@ do { \ > > ip_len =3D plen + sizeof(struct ipv6hdr) + sizeof(struct tcphdr); > > =20 > > b->ip6h.payload_len =3D htons(plen + sizeof(struct tcphdr)); > > - b->ip6h.saddr =3D conn->a.a6; > > + b->ip6h.saddr =3D conn->addr; > > if (IN6_IS_ADDR_LINKLOCAL(&b->ip6h.saddr)) > > b->ip6h.daddr =3D c->ip6.addr_ll_seen; > > else > > @@ -1684,7 +1674,7 @@ do { \ > > =20 > > ip_len =3D plen + sizeof(struct iphdr) + sizeof(struct tcphdr); > > b->iph.tot_len =3D htons(ip_len); > > - b->iph.saddr =3D conn->a.a4.a.s_addr; > > + b->iph.saddr =3D decode_ip4mapped_ip6(&conn->addr).s_addr; > > b->iph.daddr =3D c->ip4.addr_seen.s_addr; > > =20 > > if (check) > > @@ -2202,12 +2192,12 @@ static void tcp_conn_from_tap(struct ctx *c, in= t af, const void *addr, > > sa =3D (struct sockaddr *)&addr4; > > sl =3D sizeof(addr4); > > =20 > > - encode_ip4mapped_ip6(&conn->a.a6, addr); > > + encode_ip4mapped_ip6(&conn->addr, addr); > > } else { > > sa =3D (struct sockaddr *)&addr6; > > sl =3D sizeof(addr6); > > =20 > > - memcpy(&conn->a.a6, addr, sizeof(conn->a.a6)); > > + memcpy(&conn->addr, addr, sizeof(conn->addr)); > > } > > =20 > > conn->sock_port =3D ntohs(th->dest); > > @@ -2869,7 +2859,7 @@ static void tcp_conn_from_sock(struct ctx *c, uni= on epoll_ref ref, > > memcpy(&sa6.sin6_addr, src, sizeof(*src)); > > } > > =20 > > - memcpy(&conn->a.a6, &sa6.sin6_addr, sizeof(conn->a.a6)); > > + memcpy(&conn->addr, &sa6.sin6_addr, sizeof(conn->addr)); > > =20 > > conn->sock_port =3D ntohs(sa6.sin6_port); > > conn->tap_port =3D ref.r.p.tcp.tcp.index; > > @@ -2888,7 +2878,7 @@ static void tcp_conn_from_sock(struct ctx *c, uni= on epoll_ref ref, > > IN4_ARE_ADDR_EQUAL(&sa4.sin_addr, &c->ip4.addr_seen)) > > sa4.sin_addr =3D c->ip4.gw; > > =20 > > - encode_ip4mapped_ip6(&conn->a.a6, &sa4.sin_addr); > > + encode_ip4mapped_ip6(&conn->addr, &sa4.sin_addr); > > =20 > > conn->sock_port =3D ntohs(sa4.sin_port); > > conn->tap_port =3D ref.r.p.tcp.tcp.index; > > diff --git a/util.c b/util.c > > index 257d0b6..ec7f57f 100644 > > --- a/util.c > > +++ b/util.c > > @@ -502,3 +502,16 @@ void encode_ip4mapped_ip6(struct in6_addr *ip6, co= nst void *ip4) > > memset(&a->one, 0xff, sizeof(a->one)); > > memcpy(&a->a4, ip4, sizeof(a->a4)); > > } > > + > > +/** > > + * decode_ip4mapped_ip6() - Decode an IPv4-mapped IPv6 address > > + * @ip6: IPv6 address > > + * > > + * Returns: IPv4 address >=20 > Return: IPv4 address Fixed. I note there are a few other instances of this mistake in the code, most by fault by the looks of it. I'll try to fix those others at some point. > > + */ > > +struct in_addr decode_ip4mapped_ip6(const struct in6_addr *ip6) > > +{ > > + const struct ip4mapped_ip6 *a =3D (const struct ip4mapped_ip6 *)ip6; >=20 > ...if you use struct ip4_mapped_ip6 directly in struct conn, you could > drop this cast, and perhaps this helper too, but I'm not sure if it has > other implications. True. As I said, I'm considering a slightly different approach now. >=20 > > + > > + return a->a4; > > +} > > diff --git a/util.h b/util.h > > index f7d6a6f..103211d 100644 > > --- a/util.h > > +++ b/util.h > > @@ -210,5 +210,6 @@ int __daemon(int pidfile_fd, int devnull_fd); > > int fls(unsigned long x); > > int write_file(const char *path, const char *buf); > > void encode_ip4mapped_ip6(struct in6_addr *ip6, const void *ip4); > > +struct in_addr decode_ip4mapped_ip6(const struct in6_addr *ip6); > > =20 > > #endif /* UTIL_H */ >=20 --=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 --RF0w68st+ghNmV8g Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEoULxWu4/Ws0dB+XtgypY4gEwYSIFAmNpqLwACgkQgypY4gEw YSKJhw//Q+MKw4X44ivrwMHsk4QBp2ZBSD3F7nAy6iR/ag4zynO8RS4UIJ/CiR6w QVNGmjuS85C4SrJTR1CMVdojYvQoEFsNLsdbYLqvGSDWS84Fwi0Dz0cvcFqA3ZFX nUN7robvMKuM5cBk9t34VN8aozYJVfDIGE8dml2tSvHOJbPP4/lKxaX4CI7R5IR7 R78R82uG6IJEa3tOViFjYUFS4j33N3RjLtjQk2hhMl5AvkAzq/sLNdUHwiENW8T6 vLdc3b8WFQqVgYsYJncZ5dHcNa385SBw99odIvUgzqLZgUqotS1uoLSNmT80c3HN SmuNGuHRGhy7VXcE+zDKRz50TSy2WQS3TbhjjqyYWH9RRliZQ9Gm+9IzWtvhhKo8 T/0ZhrgMv/aMw9fVGB8mrKsLVX6TuYDgbqAHOm5+1GORgY8YWCmrJj0hM60JTpZJ 2H9coBrBN6SzM8vOkIc/ZNrZe34IFskkWV3LOZqLYHabTDTQPoTZ6B7X6srPb28A Ls8nUC26SkbSsifMSGuf3yYsJ4D8FMZ9c/AEJGH435zCOQziUIhKaazZRdfD8G3J CYNtk0+nGqn8Qv0gUyvDqqbqtH7XpjV4eKYyX1tf670ZofMPN0kGYYGp+YsnNMl0 s2wT0DKyg/0AZeLe9tcB7bF741mWzldlbWE0GC7vKaiQUb06iKg= =on8U -----END PGP SIGNATURE----- --RF0w68st+ghNmV8g--