Re: [PATCH 02/10] tcp: Helper to encode IPv4-mapped IPv6 addresses

[Stefano Brivio <sbrivio@redhat.com>]

On Fri,  4 Nov 2022 19:43:25 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> In the tcp_conn structure we have space for an IPv6 address, and use
> IPv4-mapped IPv6 addresses to describe IPv4 connections.  We open code
> the construction of those IPv4-mapped address in two places.
> 
> Avoid the duplication with a helper function.

Much nicer, indeed.

> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  tcp.c  |  9 ++-------
>  util.c | 20 ++++++++++++++++++++
>  util.h |  1 +
>  3 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 3d48d6e..26dd268 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2217,9 +2217,7 @@ static void tcp_conn_from_tap(struct ctx *c, int af, const void *addr,
>  		sa = (struct sockaddr *)&addr4;
>  		sl = sizeof(addr4);
>  
> -		memset(&conn->a.a4.zero, 0,    sizeof(conn->a.a4.zero));
> -		memset(&conn->a.a4.one,  0xff, sizeof(conn->a.a4.one));
> -		memcpy(&conn->a.a4.a,    addr, sizeof(conn->a.a4.a));
> +		encode_ip4mapped_ip6(&conn->a.a6, addr);
>  	} else {
>  		sa = (struct sockaddr *)&addr6;
>  		sl = sizeof(addr6);
> @@ -2902,15 +2900,12 @@ static void tcp_conn_from_sock(struct ctx *c, union epoll_ref ref,
>  
>  		memcpy(&sa4, &sa, sizeof(sa4));
>  
> -		memset(&conn->a.a4.zero,   0, sizeof(conn->a.a4.zero));
> -		memset(&conn->a.a4.one, 0xff, sizeof(conn->a.a4.one));
> -
>  		if (IN4_IS_ADDR_LOOPBACK(&sa4.sin_addr) ||
>  		    IN4_IS_ADDR_UNSPECIFIED(&sa4.sin_addr) ||
>  		    IN4_ARE_ADDR_EQUAL(&sa4.sin_addr, &c->ip4.addr_seen))
>  			sa4.sin_addr = c->ip4.gw;
>  
> -		conn->a.a4.a = sa4.sin_addr;
> +		encode_ip4mapped_ip6(&conn->a.a6, &sa4.sin_addr);
>  
>  		conn->sock_port = ntohs(sa4.sin_port);
>  		conn->tap_port = ref.r.p.tcp.tcp.index;
> diff --git a/util.c b/util.c
> index 514bd44..257d0b6 100644
> --- a/util.c
> +++ b/util.c
> @@ -482,3 +482,23 @@ int write_file(const char *path, const char *buf)
>  	close(fd);
>  	return len == 0 ? 0 : -1;
>  }
> +
> +struct ip4mapped_ip6 {
> +	uint8_t zero[10];
> +	uint8_t one[2];
> +	struct in_addr a4;
> +};

Document fields even if they're obvious. Can we reuse this part in
struct conn instead of using struct in6_addr there as you do later in
7/10? I don't have a strong preference though.

> +
> +/**
> + * encode_ip4mapped_ip6() - Convert an IPv4 address to an IPv4-mapped IPv6 address
> + * @ip6:	Buffer to store the IPv4-mapped IPv6 address
> + * @ip4:	IPv4 address, network order
> + */
> +void encode_ip4mapped_ip6(struct in6_addr *ip6, const void *ip4)
> +{
> +	struct ip4mapped_ip6 *a = (struct ip4mapped_ip6 *)ip6;
> +
> +	memset(&a->zero, 0, sizeof(a->zero));
> +	memset(&a->one, 0xff, sizeof(a->one));
> +	memcpy(&a->a4, ip4, sizeof(a->a4));
> +}
> diff --git a/util.h b/util.h
> index 2d4e1ff..f7d6a6f 100644
> --- a/util.h
> +++ b/util.h
> @@ -209,5 +209,6 @@ void write_pidfile(int fd, pid_t pid);
>  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);
>  
>  #endif /* UTIL_H */

-- 
Stefano