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 AC5B85A0082 for ; Tue, 15 Nov 2022 05:38:47 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4NBD2q726bz4xYV; Tue, 15 Nov 2022 15:38:43 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1668487123; bh=FaBL9akCxsB4c0RLRwqXLBpqCexzl7Ov1G7GZCm5fcw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=P+HDOtTl1YloW2+1UaYXvNDGA9LZPTgRNQixkW+rvkTb4senes1kdhV3z6fx6lQ/a FjMB9ZnlmZMbsjWdcZ8Dv2ZPqKsBbDOK7aG7AdsmujR7DQNSNRTVsB7bpDdQ1LXmEk FTllY9qdsEiUZBwpA29jp+1EXCIQygvk+dQsh0WE= Date: Tue, 15 Nov 2022 13:36:23 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] conf, udp: Drop mostly duplicated dns_send arrays, rename related fields Message-ID: References: <20221110193508.968090-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="XHIKQ8ZdrRT5YaXQ" Content-Disposition: inline In-Reply-To: <20221110193508.968090-1-sbrivio@redhat.com> Message-ID-Hash: I5U4YWTQBX7DLXKIXHVRK5JYUAPNB3W3 X-Message-ID-Hash: I5U4YWTQBX7DLXKIXHVRK5JYUAPNB3W3 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: --XHIKQ8ZdrRT5YaXQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Nov 10, 2022 at 08:35:08PM +0100, Stefano Brivio wrote: > Given that we use just the first valid DNS resolver address > configured, or read from resolv.conf(5) on the host, to forward DNS > queries to, in case --dns-forward is used, we don't need to duplicate > dns[] to dns_send[]: >=20 > - rename dns_send[] back to dns[]: those are the resolvers we > advertise to the guest/container >=20 > - for forwarding purposes, instead of dns[], use a single field (for > each protocol version): dns_host >=20 > - and rename dns_fwd to dns_match, so that it's clear this is the > address we are matching DNS queries against, to decide if they need > to be forwarded >=20 > Suggested-by: David Gibson > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > conf.c | 76 ++++++++++++++++++++++++-------------------------------- > dhcp.c | 5 ++-- > dhcpv6.c | 5 ++-- > ndp.c | 6 ++--- > passt.h | 20 +++++++-------- > udp.c | 21 +++++++++------- > 6 files changed, 62 insertions(+), 71 deletions(-) >=20 > diff --git a/conf.c b/conf.c > index 1adcf83..a995eb7 100644 > --- a/conf.c > +++ b/conf.c > @@ -355,11 +355,9 @@ overlap: > */ > static void get_dns(struct ctx *c) > { > - struct in6_addr *dns6_send =3D &c->ip6.dns_send[0]; > - struct in_addr *dns4_send =3D &c->ip4.dns_send[0]; > + struct in6_addr *dns6 =3D &c->ip6.dns[0], dns6_tmp; > + struct in_addr *dns4 =3D &c->ip4.dns[0], dns4_tmp; > int dns4_set, dns6_set, dnss_set, dns_set, fd; > - struct in6_addr *dns6 =3D &c->ip6.dns[0]; > - struct in_addr *dns4 =3D &c->ip4.dns[0]; > struct fqdn *s =3D c->dns_search; > struct lineread resolvconf; > int line_len; > @@ -388,49 +386,44 @@ static void get_dns(struct ctx *c) > *end =3D 0; > =20 > if (!dns4_set && > - dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1 && > - inet_pton(AF_INET, p + 1, dns4)) { > + dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1 > + && inet_pton(AF_INET, p + 1, &dns4_tmp)) { > /* Guest or container can only access local > * addresses via local redirect > */ > - if (IN4_IS_ADDR_LOOPBACK(dns4)) { > + if (IN4_IS_ADDR_LOOPBACK(&dns4_tmp)) { > if (!c->no_map_gw) { > - *dns4_send =3D c->ip4.gw; > - dns4_send++; > + *dns4 =3D c->ip4.gw; > + dns4++; > } > } else { > - *dns4_send =3D *dns4; > - dns4_send++; > + *dns4 =3D dns4_tmp; > + dns4++; > } > =20 > - dns4++; > - > - dns4->s_addr =3D htonl(INADDR_ANY); > - dns4_send->s_addr =3D htonl(INADDR_ANY); > + if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host)) > + c->ip4.dns_host =3D dns4_tmp; > } > =20 > if (!dns6_set && > - dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) - 1 && > - inet_pton(AF_INET6, p + 1, dns6)) { > + dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) - 1 > + && inet_pton(AF_INET6, p + 1, &dns6_tmp)) { > /* Guest or container can only access local > * addresses via local redirect > */ > - if (IN6_IS_ADDR_LOOPBACK(dns6)) { > + if (IN6_IS_ADDR_LOOPBACK(&dns6_tmp)) { > if (!c->no_map_gw) { > - memcpy(dns6_send, &c->ip6.gw, > - sizeof(*dns6_send)); > - dns6_send++; > + memcpy(dns6, &c->ip6.gw, > + sizeof(*dns6)); > + dns6++; > } > } else { > - memcpy(dns6_send, dns6, > - sizeof(*dns6_send)); > - dns6_send++; > + memcpy(dns6, &dns6_tmp, sizeof(*dns6)); > + dns6++; > } > =20 > - dns6++; > - > - memset(dns6, 0, sizeof(*dns6)); > - memset(dns6_send, 0, sizeof(*dns6_send)); > + if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host)) > + c->ip6.dns_host =3D dns6_tmp; > } > } else if (!dnss_set && strstr(line, "search ") =3D=3D line && > s =3D=3D c->dns_search) { > @@ -897,12 +890,10 @@ static void conf_print(const struct ctx *c) > inet_ntop(AF_INET, &c->ip4.gw, buf4, sizeof(buf4))); > } > =20 > - for (i =3D 0; !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_send[i]); > - i++) { > + for (i =3D 0; !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]); i++) { > if (!i) > info("DNS:"); > - inet_ntop(AF_INET, &c->ip4.dns_send[i], buf4, > - sizeof(buf4)); > + inet_ntop(AF_INET, &c->ip4.dns[i], buf4, sizeof(buf4)); > info(" %s", buf4); > } > =20 > @@ -933,8 +924,7 @@ static void conf_print(const struct ctx *c) > inet_ntop(AF_INET6, &c->ip6.addr_ll, buf6, sizeof(buf6))); > =20 > dns6: > - for (i =3D 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_send[i]); > - i++) { > + for (i =3D 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) { > if (!i) > info("DNS:"); > inet_ntop(AF_INET6, &c->ip6.dns[i], buf6, sizeof(buf6)); > @@ -1230,17 +1220,17 @@ void conf(struct ctx *c, int argc, char **argv) > c->no_dhcp_dns_search =3D 1; > break; > case 9: > - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_fwd) && > - inet_pton(AF_INET6, optarg, &c->ip6.dns_fwd) && > - !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_fwd) && > - !IN6_IS_ADDR_LOOPBACK(&c->ip6.dns_fwd)) > + if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) && > + inet_pton(AF_INET6, optarg, &c->ip6.dns_match) && > + !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) && > + !IN6_IS_ADDR_LOOPBACK(&c->ip6.dns_match)) > break; > =20 > - if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_fwd) && > - inet_pton(AF_INET, optarg, &c->ip4.dns_fwd) && > - !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_fwd) && > - !IN4_IS_ADDR_BROADCAST(&c->ip4.dns_fwd) && > - !IN4_IS_ADDR_LOOPBACK(&c->ip4.dns_fwd)) > + if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) && > + inet_pton(AF_INET, optarg, &c->ip4.dns_match) && > + !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) && > + !IN4_IS_ADDR_BROADCAST(&c->ip4.dns_match) && > + !IN4_IS_ADDR_LOOPBACK(&c->ip4.dns_match)) > break; > =20 > err("Invalid DNS forwarding address: %s", optarg); > diff --git a/dhcp.c b/dhcp.c > index 12da47a..6088886 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -359,9 +359,8 @@ int dhcp(const struct ctx *c, const struct pool *p) > } > =20 > for (i =3D 0, opts[6].slen =3D 0; > - !c->no_dhcp_dns && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_send[i]); > - i++) { > - ((struct in_addr *)opts[6].s)[i] =3D c->ip4.dns_send[i]; > + !c->no_dhcp_dns && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]); i++) { > + ((struct in_addr *)opts[6].s)[i] =3D c->ip4.dns[i]; > opts[6].slen +=3D sizeof(uint32_t); > } > =20 > diff --git a/dhcpv6.c b/dhcpv6.c > index 67262e6..e763aed 100644 > --- a/dhcpv6.c > +++ b/dhcpv6.c > @@ -379,7 +379,7 @@ static size_t dhcpv6_dns_fill(const struct ctx *c, ch= ar *buf, int offset) > if (c->no_dhcp_dns) > goto search; > =20 > - for (i =3D 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_send[i]); i++) { > + for (i =3D 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) { > if (!i) { > srv =3D (struct opt_dns_servers *)(buf + offset); > offset +=3D sizeof(struct opt_hdr); > @@ -387,8 +387,7 @@ static size_t dhcpv6_dns_fill(const struct ctx *c, ch= ar *buf, int offset) > srv->hdr.l =3D 0; > } > =20 > - memcpy(&srv->addr[i], &c->ip6.dns_send[i], > - sizeof(srv->addr[i])); > + memcpy(&srv->addr[i], &c->ip6.dns[i], sizeof(srv->addr[i])); > srv->hdr.l +=3D sizeof(srv->addr[i]); > offset +=3D sizeof(srv->addr[i]); > } > diff --git a/ndp.c b/ndp.c > index 6d79477..80e1f19 100644 > --- a/ndp.c > +++ b/ndp.c > @@ -121,7 +121,7 @@ int ndp(struct ctx *c, const struct icmp6hdr *ih, con= st struct in6_addr *saddr) > if (c->no_dhcp_dns) > goto dns_done; > =20 > - for (n =3D 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_send[n]); n++); > + for (n =3D 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]); n++); > if (n) { > *p++ =3D 25; /* RDNSS */ > *p++ =3D 1 + 2 * n; /* length */ > @@ -130,8 +130,8 @@ int ndp(struct ctx *c, const struct icmp6hdr *ih, con= st struct in6_addr *saddr) > p +=3D 4; > =20 > for (i =3D 0; i < n; i++) { > - memcpy(p, &c->ip6.dns_send[i], 16); > - p +=3D 16; /* address */ > + memcpy(p, &c->ip6.dns[i], 16); /* address */ > + p +=3D 16; > } > =20 > for (n =3D 0; *c->dns_search[n].n; n++) > diff --git a/passt.h b/passt.h > index e93eea8..6649c0a 100644 > --- a/passt.h > +++ b/passt.h > @@ -101,9 +101,9 @@ enum passt_modes { > * @addr_seen: Latest IPv4 address seen as source from tap > * @prefixlen: IPv4 prefix length (netmask) > * @gw: Default IPv4 gateway, network order > - * @dns: Host IPv4 DNS addresses, zero-terminated, network order > - * @dns_send: Offered IPv4 DNS, zero-terminated, network order > - * @dns_fwd: Address forwarded (UDP) to first IPv4 DNS, network order > + * @dns: DNS addresses for DHCP, zero-terminated, network order > + * @dns_match: Forward DNS query if sent to this address, network order > + * @dns_host: Use this DNS on the host for forwarding, network order > */ > struct ip4_ctx { > struct in_addr addr; > @@ -111,8 +111,8 @@ struct ip4_ctx { > int prefix_len; > struct in_addr gw; > struct in_addr dns[MAXNS + 1]; > - struct in_addr dns_send[MAXNS + 1]; > - struct in_addr dns_fwd; > + struct in_addr dns_match; > + struct in_addr dns_host; > }; > =20 > /** > @@ -122,9 +122,9 @@ struct ip4_ctx { > * @addr_seen: Latest IPv6 global/site address seen as source from tap > * @addr_ll_seen: Latest IPv6 link-local address seen as source from tap > * @gw: Default IPv6 gateway > - * @dns: Host IPv6 DNS addresses, zero-terminated > - * @dns_send: Offered IPv6 DNS addresses, zero-terminated > - * @dns_fwd: Address forwarded (UDP) to first IPv6 DNS, network order > + * @dns: DNS addresses for DHCPv6 and NDP, zero-terminated > + * @dns_match: Forward DNS query if sent to this address > + * @dns_host: Use this DNS on the host for forwarding > */ > struct ip6_ctx { > struct in6_addr addr; > @@ -133,8 +133,8 @@ struct ip6_ctx { > struct in6_addr addr_ll_seen; > struct in6_addr gw; > struct in6_addr dns[MAXNS + 1]; > - struct in6_addr dns_send[MAXNS + 1]; > - struct in6_addr dns_fwd; > + struct in6_addr dns_match; > + struct in6_addr dns_host; > }; > =20 > #include > diff --git a/udp.c b/udp.c > index ff7f993..0aa6308 100644 > --- a/udp.c > +++ b/udp.c > @@ -678,10 +678,10 @@ static void udp_sock_fill_data_v4(const struct ctx = *c, int n, > =20 > src_port =3D ntohs(b->s_in.sin_port); > =20 > - if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_fwd) && > - IN4_ARE_ADDR_EQUAL(&b->s_in.sin_addr, &c->ip4.dns[0]) && > + if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) && > + IN4_ARE_ADDR_EQUAL(&b->s_in.sin_addr, &c->ip4.dns_host) && > src_port =3D=3D 53) { > - b->iph.saddr =3D c->ip4.dns_fwd.s_addr; > + b->iph.saddr =3D c->ip4.dns_match.s_addr; > } else if (IN4_IS_ADDR_LOOPBACK(&b->s_in.sin_addr) || > IN4_IS_ADDR_UNSPECIFIED(&b->s_in.sin_addr)|| > IN4_ARE_ADDR_EQUAL(&b->s_in.sin_addr, &c->ip4.addr_seen)) { > @@ -770,10 +770,11 @@ static void udp_sock_fill_data_v6(const struct ctx = *c, int n, > if (IN6_IS_ADDR_LINKLOCAL(src)) { > b->ip6h.daddr =3D c->ip6.addr_ll_seen; > b->ip6h.saddr =3D b->s_in6.sin6_addr; > - } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_fwd) && > - IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns[0]) && src_port =3D=3D 53) { > + } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) && > + IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_host) && > + src_port =3D=3D 53) { > b->ip6h.daddr =3D c->ip6.addr_seen; > - b->ip6h.saddr =3D c->ip6.dns_fwd; > + b->ip6h.saddr =3D c->ip6.dns_match; > } else if (IN6_IS_ADDR_LOOPBACK(src) || > IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen) || > IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr)) { > @@ -1016,13 +1017,15 @@ int udp_tap_handler(struct ctx *c, int af, const = void *addr, > =20 > udp_tap_map[V4][src].ts =3D now->tv_sec; > =20 > - if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) && !c->no_map_gw) { > + if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) && > + !c->no_map_gw) { > if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) || > (udp_tap_map[V4][dst].flags & PORT_LOOPBACK)) > s_in.sin_addr.s_addr =3D htonl(INADDR_LOOPBACK); > else > s_in.sin_addr =3D c->ip4.addr_seen; > - } else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.dns_fwd) && > + } else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, > + &c->ip4.dns_match) && > ntohs(s_in.sin_port) =3D=3D 53) { > s_in.sin_addr =3D c->ip4.dns[0]; > } > @@ -1045,7 +1048,7 @@ int udp_tap_handler(struct ctx *c, int af, const vo= id *addr, > s_in6.sin6_addr =3D c->ip6.addr; > else > s_in6.sin6_addr =3D c->ip6.addr_seen; > - } else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_fwd) && > + } else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_match) && > ntohs(s_in6.sin6_port) =3D=3D 53) { > s_in6.sin6_addr =3D c->ip6.dns[0]; > } else if (IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) { --=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 --XHIKQ8ZdrRT5YaXQ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEoULxWu4/Ws0dB+XtgypY4gEwYSIFAmNy+wgACgkQgypY4gEw YSJcURAAiKzZMZHJ9AwxB8HdErL2g9WusgGl06qQTo0PixDFXjv2nMRLRvIYiMAK Nxzd8GOApUznZVCzrLiRqf9gq0AfiDJgCqz6vp5eh/UxsqNFfrpdxCzLYdMFMpUk vP9q84kXco2L3gbb/+FAcmA0wW7azATQMeqWQJ5+rjVzUdMzNeHwZSNuK2Fk6L1D 0TxriDrwSV2PleLI9s6+P/ipm8rBbpD8+DeZfScfr4YEX5MzF2lWvoNrsXvtb20R EcNNk1dkfnYVDykNNGRNTR0glEUzZwdvx5OAd9M7vRaWaJoOUXMFOcLkIUsVrLXX 7WydrSroekCKeCEKhMkW6m6EbbTn8zBem7K/vHevwPoOzS8irFDBuhLt8UE7oNTz HqanZlX651pi/s5JuRQJ3rAeHxBO0WrM1Quxp4Va/rKNi36X7MgUgtigd7N5Fm3o 9AtdzsSiuGgnwGfiN0NNE4wtNUz1vHLfvPtsGvJgr4n3CpFcQ31R/XZWUFbAuffA mzmWhqstEa6YUDmjS14NbhNZrUm1d9iIk96UjrH7Y7xNYVG2Dt6D6y5haNTTHmCt uwdPSRUHrbqSdw/hj1HNwgNEQ1SdTp7tFK5v7lyY2DF1yr5ZI0hX+rWy+2C2erNB dkz+WR5mV6SP1YhvwRc9h8biukRTc9WS3EZIjHGX36Z4qZmRvqA= =R908 -----END PGP SIGNATURE----- --XHIKQ8ZdrRT5YaXQ--