* [PATCH 01/16] conf: Don't ignore -t and -u options after -D
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 12:18 ` Stefano Brivio
2024-08-14 4:30 ` [PATCH 02/16] treewide: Use "our address" instead of "forwarding address" David Gibson
` (15 subsequent siblings)
16 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
f6d5a5239264 moved handling of -D into a later loop. However as a side
effect it moved this from a switch block to an if block. I left a couple
of 'break' statements that don't make sense in the new context. They
should be 'continue' so that we go onto the next option, rather than
leaving the loop entirely.
Fixes: f6d5a5239264 ("conf: Delay handling -D option until after...")
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/conf.c b/conf.c
index 76d37da0..ed097bdc 100644
--- a/conf.c
+++ b/conf.c
@@ -1682,13 +1682,13 @@ void conf(struct ctx *c, int argc, char **argv)
if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) &&
inet_pton(AF_INET, optarg, &dns4_tmp)) {
add_dns4(c, &dns4_tmp, &dns4);
- break;
+ continue;
}
if (dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) &&
inet_pton(AF_INET6, optarg, &dns6_tmp)) {
add_dns6(c, &dns6_tmp, &dns6);
- break;
+ continue;
}
die("Cannot use DNS address %s", optarg);
--
@@ -1682,13 +1682,13 @@ void conf(struct ctx *c, int argc, char **argv)
if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) &&
inet_pton(AF_INET, optarg, &dns4_tmp)) {
add_dns4(c, &dns4_tmp, &dns4);
- break;
+ continue;
}
if (dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) &&
inet_pton(AF_INET6, optarg, &dns6_tmp)) {
add_dns6(c, &dns6_tmp, &dns6);
- break;
+ continue;
}
die("Cannot use DNS address %s", optarg);
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 01/16] conf: Don't ignore -t and -u options after -D
2024-08-14 4:30 ` [PATCH 01/16] conf: Don't ignore -t and -u options after -D David Gibson
@ 2024-08-14 12:18 ` Stefano Brivio
0 siblings, 0 replies; 20+ messages in thread
From: Stefano Brivio @ 2024-08-14 12:18 UTC (permalink / raw)
To: David Gibson; +Cc: passt-dev
On Wed, 14 Aug 2024 14:30:35 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> f6d5a5239264 moved handling of -D into a later loop. However as a side
> effect it moved this from a switch block to an if block. I left a couple
> of 'break' statements that don't make sense in the new context. They
> should be 'continue' so that we go onto the next option, rather than
> leaving the loop entirely.
>
> Fixes: f6d5a5239264 ("conf: Delay handling -D option until after...")
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Applied (just this one).
--
Stefano
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 02/16] treewide: Use "our address" instead of "forwarding address"
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
2024-08-14 4:30 ` [PATCH 01/16] conf: Don't ignore -t and -u options after -D David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 03/16] util: Helper for formatting MAC addresses David Gibson
` (14 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
The term "forwarding address" to indicate the local-to-passt address was
well-intentioned, but ends up being kinda confusing. As discussed on a
recent call, let's try "our" instead.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
flow.c | 74 +++++++++++++++++++++++++-------------------------
flow.h | 18 ++++++------
fwd.c | 70 +++++++++++++++++++++++------------------------
icmp.c | 4 +--
tcp.c | 33 +++++++++++-----------
tcp_internal.h | 2 +-
udp.c | 12 ++++----
7 files changed, 107 insertions(+), 106 deletions(-)
diff --git a/flow.c b/flow.c
index 687e9fd0..669accf3 100644
--- a/flow.c
+++ b/flow.c
@@ -127,18 +127,18 @@ static struct timespec flow_timer_run;
* @af: Address family (AF_INET or AF_INET6)
* @eaddr: Endpoint address (pointer to in_addr or in6_addr)
* @eport: Endpoint port
- * @faddr: Forwarding address (pointer to in_addr or in6_addr)
- * @fport: Forwarding port
+ * @oaddr: Our address (pointer to in_addr or in6_addr)
+ * @oport: Our port
*/
static void flowside_from_af(struct flowside *side, sa_family_t af,
const void *eaddr, in_port_t eport,
- const void *faddr, in_port_t fport)
+ const void *oaddr, in_port_t oport)
{
- if (faddr)
- inany_from_af(&side->faddr, af, faddr);
+ if (oaddr)
+ inany_from_af(&side->oaddr, af, oaddr);
else
- side->faddr = inany_any6;
- side->fport = fport;
+ side->oaddr = inany_any6;
+ side->oport = oport;
if (eaddr)
inany_from_af(&side->eaddr, af, eaddr);
@@ -193,8 +193,8 @@ static int flowside_sock_splice(void *arg)
* @tgt: Target flowside
* @data: epoll reference portion for protocol handlers
*
- * Return: socket fd of protocol @proto bound to the forwarding address and port
- * from @tgt (if specified).
+ * Return: socket fd of protocol @proto bound to our address and port from @tgt
+ * (if specified).
*/
int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
const struct flowside *tgt, uint32_t data)
@@ -205,11 +205,11 @@ int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
ASSERT(pif_is_socket(pif));
- pif_sockaddr(c, &sa, &sl, pif, &tgt->faddr, tgt->fport);
+ pif_sockaddr(c, &sa, &sl, pif, &tgt->oaddr, tgt->oport);
switch (pif) {
case PIF_HOST:
- if (inany_is_loopback(&tgt->faddr))
+ if (inany_is_loopback(&tgt->oaddr))
ifname = NULL;
else if (sa.sa_family == AF_INET)
ifname = c->ip4.ifname_out;
@@ -309,11 +309,11 @@ static void flow_set_state(struct flow_common *f, enum flow_state state)
pif_name(f->pif[INISIDE]),
inany_ntop(&ini->eaddr, estr0, sizeof(estr0)),
ini->eport,
- inany_ntop(&ini->faddr, fstr0, sizeof(fstr0)),
- ini->fport,
+ inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)),
+ ini->oport,
pif_name(f->pif[TGTSIDE]),
- inany_ntop(&tgt->faddr, fstr1, sizeof(fstr1)),
- tgt->fport,
+ inany_ntop(&tgt->oaddr, fstr1, sizeof(fstr1)),
+ tgt->oport,
inany_ntop(&tgt->eaddr, estr1, sizeof(estr1)),
tgt->eport);
else if (MAX(state, oldstate) >= FLOW_STATE_INI)
@@ -321,8 +321,8 @@ static void flow_set_state(struct flow_common *f, enum flow_state state)
pif_name(f->pif[INISIDE]),
inany_ntop(&ini->eaddr, estr0, sizeof(estr0)),
ini->eport,
- inany_ntop(&ini->faddr, fstr0, sizeof(fstr0)),
- ini->fport);
+ inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)),
+ ini->oport);
}
/**
@@ -347,7 +347,7 @@ static void flow_initiate_(union flow *flow, uint8_t pif)
* flow_initiate_af() - Move flow to INI, setting INISIDE details
* @flow: Flow to change state
* @pif: pif of the initiating side
- * @af: Address family of @eaddr and @faddr
+ * @af: Address family of @eaddr and @oaddr
* @saddr: Source address (pointer to in_addr or in6_addr)
* @sport: Endpoint port
* @daddr: Destination address (pointer to in_addr or in6_addr)
@@ -384,10 +384,10 @@ const struct flowside *flow_initiate_sa(union flow *flow, uint8_t pif,
inany_from_sockaddr(&ini->eaddr, &ini->eport, ssa);
if (inany_v4(&ini->eaddr))
- ini->faddr = inany_any4;
+ ini->oaddr = inany_any4;
else
- ini->faddr = inany_any6;
- ini->fport = dport;
+ ini->oaddr = inany_any6;
+ ini->oport = dport;
flow_initiate_(flow, pif);
return ini;
}
@@ -432,8 +432,8 @@ const struct flowside *flow_target(const struct ctx *c, union flow *flow,
pif_name(f->pif[INISIDE]),
inany_ntop(&ini->eaddr, estr, sizeof(estr)),
ini->eport,
- inany_ntop(&ini->faddr, fstr, sizeof(fstr)),
- ini->fport);
+ inany_ntop(&ini->oaddr, fstr, sizeof(fstr)),
+ ini->oport);
}
if (tgtpif == PIF_NONE)
@@ -562,17 +562,17 @@ static uint64_t flow_hash(const struct ctx *c, uint8_t proto, uint8_t pif,
struct siphash_state state = SIPHASH_INIT(c->hash_secret);
/* For the hash table to work, we need complete endpoint information,
- * and at least a forwarding port.
+ * and at least our port.
*/
ASSERT(pif != PIF_NONE && !inany_is_unspecified(&side->eaddr) &&
- side->eport != 0 && side->fport != 0);
+ side->eport != 0 && side->oport != 0);
- inany_siphash_feed(&state, &side->faddr);
+ inany_siphash_feed(&state, &side->oaddr);
inany_siphash_feed(&state, &side->eaddr);
return siphash_final(&state, 38, (uint64_t)proto << 40 |
(uint64_t)pif << 32 |
- (uint64_t)side->fport << 16 |
+ (uint64_t)side->oport << 16 |
(uint64_t)side->eport);
}
@@ -707,20 +707,20 @@ static flow_sidx_t flowside_lookup(const struct ctx *c, uint8_t proto,
* @pif: Interface of the flow
* @af: Address family, AF_INET or AF_INET6
* @eaddr: Guest side endpoint address (guest local address)
- * @faddr: Guest side forwarding address (guest remote address)
+ * @oaddr: Our guest side address (guest remote address)
* @eport: Guest side endpoint port (guest local port)
- * @fport: Guest side forwarding port (guest remote port)
+ * @oport: Our guest side port (guest remote port)
*
* Return: sidx of the matching flow & side, FLOW_SIDX_NONE if not found
*/
flow_sidx_t flow_lookup_af(const struct ctx *c,
uint8_t proto, uint8_t pif, sa_family_t af,
- const void *eaddr, const void *faddr,
- in_port_t eport, in_port_t fport)
+ const void *eaddr, const void *oaddr,
+ in_port_t eport, in_port_t oport)
{
struct flowside side;
- flowside_from_af(&side, af, eaddr, eport, faddr, fport);
+ flowside_from_af(&side, af, eaddr, eport, oaddr, oport);
return flowside_lookup(c, proto, pif, &side);
}
@@ -730,22 +730,22 @@ flow_sidx_t flow_lookup_af(const struct ctx *c,
* @proto: Protocol of the flow (IP L4 protocol number)
* @pif: Interface of the flow
* @esa: Socket address of the endpoint
- * @fport: Forwarding port number
+ * @oport: Our port number
*
* Return: sidx of the matching flow & side, FLOW_SIDX_NONE if not found
*/
flow_sidx_t flow_lookup_sa(const struct ctx *c, uint8_t proto, uint8_t pif,
- const void *esa, in_port_t fport)
+ const void *esa, in_port_t oport)
{
struct flowside side = {
- .fport = fport,
+ .oport = oport,
};
inany_from_sockaddr(&side.eaddr, &side.eport, esa);
if (inany_v4(&side.eaddr))
- side.faddr = inany_any4;
+ side.oaddr = inany_any4;
else
- side.faddr = inany_any6;
+ side.oaddr = inany_any6;
return flowside_lookup(c, proto, pif, &side);
}
diff --git a/flow.h b/flow.h
index 078fd605..d167b654 100644
--- a/flow.h
+++ b/flow.h
@@ -140,14 +140,14 @@ extern const uint8_t flow_proto[];
/**
* struct flowside - Address information for one side of a flow
* @eaddr: Endpoint address (remote address from passt's PoV)
- * @faddr: Forwarding address (local address from passt's PoV)
+ * @oaddr: Our address (local address from passt's PoV)
* @eport: Endpoint port
- * @fport: Forwarding port
+ * @oport: Our port
*/
struct flowside {
- union inany_addr faddr;
+ union inany_addr oaddr;
union inany_addr eaddr;
- in_port_t fport;
+ in_port_t oport;
in_port_t eport;
};
@@ -162,8 +162,8 @@ static inline bool flowside_eq(const struct flowside *left,
{
return inany_equals(&left->eaddr, &right->eaddr) &&
left->eport == right->eport &&
- inany_equals(&left->faddr, &right->faddr) &&
- left->fport == right->fport;
+ inany_equals(&left->oaddr, &right->oaddr) &&
+ left->oport == right->oport;
}
int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
@@ -240,10 +240,10 @@ uint64_t flow_hash_insert(const struct ctx *c, flow_sidx_t sidx);
void flow_hash_remove(const struct ctx *c, flow_sidx_t sidx);
flow_sidx_t flow_lookup_af(const struct ctx *c,
uint8_t proto, uint8_t pif, sa_family_t af,
- const void *eaddr, const void *faddr,
- in_port_t eport, in_port_t fport);
+ const void *eaddr, const void *oaddr,
+ in_port_t eport, in_port_t oport);
flow_sidx_t flow_lookup_sa(const struct ctx *c, uint8_t proto, uint8_t pif,
- const void *esa, in_port_t fport);
+ const void *esa, in_port_t oport);
union flow;
diff --git a/fwd.c b/fwd.c
index dea36f6c..b546bc41 100644
--- a/fwd.c
+++ b/fwd.c
@@ -167,7 +167,7 @@ void fwd_scan_ports_init(struct ctx *c)
static bool is_dns_flow(uint8_t proto, const struct flowside *ini)
{
return ((proto == IPPROTO_UDP) || (proto == IPPROTO_TCP)) &&
- ((ini->fport == 53) || (ini->fport == 853));
+ ((ini->oport == 53) || (ini->oport == 853));
}
/**
@@ -184,33 +184,33 @@ uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto,
const struct flowside *ini, struct flowside *tgt)
{
if (is_dns_flow(proto, ini) &&
- inany_equals4(&ini->faddr, &c->ip4.dns_match))
+ inany_equals4(&ini->oaddr, &c->ip4.dns_match))
tgt->eaddr = inany_from_v4(c->ip4.dns_host);
else if (is_dns_flow(proto, ini) &&
- inany_equals6(&ini->faddr, &c->ip6.dns_match))
+ inany_equals6(&ini->oaddr, &c->ip6.dns_match))
tgt->eaddr.a6 = c->ip6.dns_host;
- else if (!c->no_map_gw && inany_equals4(&ini->faddr, &c->ip4.gw))
+ else if (!c->no_map_gw && inany_equals4(&ini->oaddr, &c->ip4.gw))
tgt->eaddr = inany_loopback4;
- else if (!c->no_map_gw && inany_equals6(&ini->faddr, &c->ip6.gw))
+ else if (!c->no_map_gw && inany_equals6(&ini->oaddr, &c->ip6.gw))
tgt->eaddr = inany_loopback6;
else
- tgt->eaddr = ini->faddr;
+ tgt->eaddr = ini->oaddr;
- tgt->eport = ini->fport;
+ tgt->eport = ini->oport;
/* The relevant addr_out controls the host side source address. This
* may be unspecified, which allows the kernel to pick an address.
*/
if (inany_v4(&tgt->eaddr))
- tgt->faddr = inany_from_v4(c->ip4.addr_out);
+ tgt->oaddr = inany_from_v4(c->ip4.addr_out);
else
- tgt->faddr.a6 = c->ip6.addr_out;
+ tgt->oaddr.a6 = c->ip6.addr_out;
/* Let the kernel pick a host side source port */
- tgt->fport = 0;
+ tgt->oport = 0;
if (proto == IPPROTO_UDP) {
/* But for UDP we preserve the source port */
- tgt->fport = ini->eport;
+ tgt->oport = ini->eport;
}
return PIF_HOST;
@@ -230,13 +230,13 @@ uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_t proto,
const struct flowside *ini, struct flowside *tgt)
{
if (!inany_is_loopback(&ini->eaddr) ||
- (!inany_is_loopback(&ini->faddr) && !inany_is_unspecified(&ini->faddr))) {
+ (!inany_is_loopback(&ini->oaddr) && !inany_is_unspecified(&ini->oaddr))) {
char estr[INANY_ADDRSTRLEN], fstr[INANY_ADDRSTRLEN];
debug("Non loopback address on %s: [%s]:%hu -> [%s]:%hu",
pif_name(PIF_SPLICE),
inany_ntop(&ini->eaddr, estr, sizeof(estr)), ini->eport,
- inany_ntop(&ini->faddr, fstr, sizeof(fstr)), ini->fport);
+ inany_ntop(&ini->oaddr, fstr, sizeof(fstr)), ini->oport);
return PIF_NONE;
}
@@ -248,20 +248,20 @@ uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_t proto,
/* Preserve the specific loopback adddress used, but let the kernel pick
* a source port on the target side
*/
- tgt->faddr = ini->eaddr;
- tgt->fport = 0;
+ tgt->oaddr = ini->eaddr;
+ tgt->oport = 0;
- tgt->eport = ini->fport;
+ tgt->eport = ini->oport;
if (proto == IPPROTO_TCP)
tgt->eport += c->tcp.fwd_out.delta[tgt->eport];
else if (proto == IPPROTO_UDP)
tgt->eport += c->udp.fwd_out.delta[tgt->eport];
/* Let the kernel pick a host side source port */
- tgt->fport = 0;
+ tgt->oport = 0;
if (proto == IPPROTO_UDP)
/* But for UDP preserve the source port */
- tgt->fport = ini->eport;
+ tgt->oport = ini->eport;
return PIF_HOST;
}
@@ -280,7 +280,7 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto,
const struct flowside *ini, struct flowside *tgt)
{
/* Common for spliced and non-spliced cases */
- tgt->eport = ini->fport;
+ tgt->eport = ini->oport;
if (proto == IPPROTO_TCP)
tgt->eport += c->tcp.fwd_in.delta[tgt->eport];
else if (proto == IPPROTO_UDP)
@@ -293,11 +293,11 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto,
/* Preserve the specific loopback adddress used, but let the
* kernel pick a source port on the target side
*/
- tgt->faddr = ini->eaddr;
- tgt->fport = 0;
+ tgt->oaddr = ini->eaddr;
+ tgt->oport = 0;
if (proto == IPPROTO_UDP)
/* But for UDP preserve the source port */
- tgt->fport = ini->eport;
+ tgt->oport = ini->eport;
if (inany_v4(&ini->eaddr))
tgt->eaddr = inany_loopback4;
@@ -307,26 +307,26 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto,
return PIF_SPLICE;
}
- tgt->faddr = ini->eaddr;
- tgt->fport = ini->eport;
+ tgt->oaddr = ini->eaddr;
+ tgt->oport = ini->eport;
- if (inany_is_loopback4(&tgt->faddr) ||
- inany_is_unspecified4(&tgt->faddr) ||
- inany_equals4(&tgt->faddr, &c->ip4.addr_seen)) {
- tgt->faddr = inany_from_v4(c->ip4.gw);
- } else if (inany_is_loopback6(&tgt->faddr) ||
- inany_equals6(&tgt->faddr, &c->ip6.addr_seen) ||
- inany_equals6(&tgt->faddr, &c->ip6.addr)) {
+ if (inany_is_loopback4(&tgt->oaddr) ||
+ inany_is_unspecified4(&tgt->oaddr) ||
+ inany_equals4(&tgt->oaddr, &c->ip4.addr_seen)) {
+ tgt->oaddr = inany_from_v4(c->ip4.gw);
+ } else if (inany_is_loopback6(&tgt->oaddr) ||
+ inany_equals6(&tgt->oaddr, &c->ip6.addr_seen) ||
+ inany_equals6(&tgt->oaddr, &c->ip6.addr)) {
if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
- tgt->faddr.a6 = c->ip6.gw;
+ tgt->oaddr.a6 = c->ip6.gw;
else
- tgt->faddr.a6 = c->ip6.addr_ll;
+ tgt->oaddr.a6 = c->ip6.addr_ll;
}
- if (inany_v4(&tgt->faddr)) {
+ if (inany_v4(&tgt->oaddr)) {
tgt->eaddr = inany_from_v4(c->ip4.addr_seen);
} else {
- if (inany_is_linklocal6(&tgt->faddr))
+ if (inany_is_linklocal6(&tgt->oaddr))
tgt->eaddr.a6 = c->ip6.addr_ll_seen;
else
tgt->eaddr.a6 = c->ip6.addr_seen;
diff --git a/icmp.c b/icmp.c
index cb81c768..f514dbc9 100644
--- a/icmp.c
+++ b/icmp.c
@@ -125,13 +125,13 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref)
ini->eport, seq);
if (pingf->f.type == FLOW_PING4) {
- const struct in_addr *saddr = inany_v4(&ini->faddr);
+ const struct in_addr *saddr = inany_v4(&ini->oaddr);
const struct in_addr *daddr = inany_v4(&ini->eaddr);
ASSERT(saddr && daddr); /* Must have IPv4 addresses */
tap_icmp4_send(c, *saddr, *daddr, buf, n);
} else if (pingf->f.type == FLOW_PING6) {
- const struct in6_addr *saddr = &ini->faddr.a6;
+ const struct in6_addr *saddr = &ini->oaddr.a6;
const struct in6_addr *daddr = &ini->eaddr.a6;
tap_icmp6_send(c, saddr, daddr, buf, n);
diff --git a/tcp.c b/tcp.c
index c0820ce7..f01fe8f9 100644
--- a/tcp.c
+++ b/tcp.c
@@ -361,8 +361,8 @@ static const char *tcp_flag_str[] __attribute((__unused__)) = {
static int tcp_sock_init_ext [NUM_PORTS][IP_VERSIONS];
static int tcp_sock_ns [NUM_PORTS][IP_VERSIONS];
-/* Table of guest side forwarding addresses with very low RTT (assumed
- * to be local to the host), LRU
+/* Table of our guest side addresses with very low RTT (assumed to be local to
+ * the host), LRU
*/
static union inany_addr low_rtt_dst[LOW_RTT_TABLE_SIZE];
@@ -663,7 +663,7 @@ static int tcp_rtt_dst_low(const struct tcp_tap_conn *conn)
int i;
for (i = 0; i < LOW_RTT_TABLE_SIZE; i++)
- if (inany_equals(&tapside->faddr, low_rtt_dst + i))
+ if (inany_equals(&tapside->oaddr, low_rtt_dst + i))
return 1;
return 0;
@@ -686,7 +686,7 @@ static void tcp_rtt_dst_check(const struct tcp_tap_conn *conn,
return;
for (i = 0; i < LOW_RTT_TABLE_SIZE; i++) {
- if (inany_equals(&tapside->faddr, low_rtt_dst + i))
+ if (inany_equals(&tapside->oaddr, low_rtt_dst + i))
return;
if (hole == -1 && IN6_IS_ADDR_UNSPECIFIED(low_rtt_dst + i))
hole = i;
@@ -698,7 +698,7 @@ static void tcp_rtt_dst_check(const struct tcp_tap_conn *conn,
if (hole == -1)
return;
- low_rtt_dst[hole++] = tapside->faddr;
+ low_rtt_dst[hole++] = tapside->oaddr;
if (hole == LOW_RTT_TABLE_SIZE)
hole = 0;
inany_from_af(low_rtt_dst + hole, AF_INET6, &in6addr_any);
@@ -881,7 +881,7 @@ static void tcp_fill_header(struct tcphdr *th,
{
const struct flowside *tapside = TAPFLOW(conn);
- th->source = htons(tapside->fport);
+ th->source = htons(tapside->oport);
th->dest = htons(tapside->eport);
th->seq = htonl(seq);
th->ack_seq = htonl(conn->seq_ack_to_tap);
@@ -913,7 +913,7 @@ static size_t tcp_fill_headers4(const struct tcp_tap_conn *conn,
uint32_t seq)
{
const struct flowside *tapside = TAPFLOW(conn);
- const struct in_addr *src4 = inany_v4(&tapside->faddr);
+ const struct in_addr *src4 = inany_v4(&tapside->oaddr);
const struct in_addr *dst4 = inany_v4(&tapside->eaddr);
size_t l4len = dlen + sizeof(*th);
size_t l3len = l4len + sizeof(*iph);
@@ -957,7 +957,7 @@ static size_t tcp_fill_headers6(const struct tcp_tap_conn *conn,
size_t l4len = dlen + sizeof(*th);
ip6h->payload_len = htons(l4len);
- ip6h->saddr = tapside->faddr.a6;
+ ip6h->saddr = tapside->oaddr.a6;
ip6h->daddr = tapside->eaddr.a6;
ip6h->hop_limit = 255;
@@ -992,7 +992,7 @@ size_t tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
const uint16_t *check, uint32_t seq)
{
const struct flowside *tapside = TAPFLOW(conn);
- const struct in_addr *a4 = inany_v4(&tapside->faddr);
+ const struct in_addr *a4 = inany_v4(&tapside->oaddr);
if (a4) {
return tcp_fill_headers4(conn, iov[TCP_IOV_TAP].iov_base,
@@ -1417,15 +1417,15 @@ static void tcp_bind_outbound(const struct ctx *c,
socklen_t sl;
- pif_sockaddr(c, &bind_sa, &sl, PIF_HOST, &tgt->faddr, tgt->fport);
- if (!inany_is_unspecified(&tgt->faddr) || tgt->fport) {
+ pif_sockaddr(c, &bind_sa, &sl, PIF_HOST, &tgt->oaddr, tgt->oport);
+ if (!inany_is_unspecified(&tgt->oaddr) || tgt->oport) {
if (bind(s, &bind_sa.sa, sl)) {
char sstr[INANY_ADDRSTRLEN];
flow_dbg(conn,
"Can't bind TCP outbound socket to %s:%hu: %s",
- inany_ntop(&tgt->faddr, sstr, sizeof(sstr)),
- tgt->fport, strerror(errno));
+ inany_ntop(&tgt->oaddr, sstr, sizeof(sstr)),
+ tgt->oport, strerror(errno));
}
}
@@ -1497,12 +1497,12 @@ static void tcp_conn_from_tap(struct ctx *c, sa_family_t af,
conn = FLOW_SET_TYPE(flow, FLOW_TCP, tcp);
if (!inany_is_unicast(&ini->eaddr) || ini->eport == 0 ||
- !inany_is_unicast(&ini->faddr) || ini->fport == 0) {
+ !inany_is_unicast(&ini->oaddr) || ini->oport == 0) {
char sstr[INANY_ADDRSTRLEN], dstr[INANY_ADDRSTRLEN];
debug("Invalid endpoint in TCP SYN: %s:%hu -> %s:%hu",
inany_ntop(&ini->eaddr, sstr, sizeof(sstr)), ini->eport,
- inany_ntop(&ini->faddr, dstr, sizeof(dstr)), ini->fport);
+ inany_ntop(&ini->oaddr, dstr, sizeof(dstr)), ini->oport);
goto cancel;
}
@@ -2100,7 +2100,8 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
goto cancel;
/* FIXME: When listening port has a specific bound address, record that
- * as the forwarding address */
+ * as our address
+ */
ini = flow_initiate_sa(flow, ref.tcp_listen.pif, &sa,
ref.tcp_listen.port);
diff --git a/tcp_internal.h b/tcp_internal.h
index 8b60aabc..aa8bb64f 100644
--- a/tcp_internal.h
+++ b/tcp_internal.h
@@ -44,7 +44,7 @@
#define TAPFLOW(conn_) (&((conn_)->f.side[TAPSIDE(conn_)]))
#define TAP_SIDX(conn_) (FLOW_SIDX((conn_), TAPSIDE(conn_)))
-#define CONN_V4(conn) (!!inany_v4(&TAPFLOW(conn)->faddr))
+#define CONN_V4(conn) (!!inany_v4(&TAPFLOW(conn)->oaddr))
#define CONN_V6(conn) (!CONN_V4(conn))
/*
diff --git a/udp.c b/udp.c
index 77312572..57dcc667 100644
--- a/udp.c
+++ b/udp.c
@@ -321,7 +321,7 @@ static void udp_splice_send(const struct ctx *c, size_t start, size_t n,
static size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp,
const struct flowside *toside, size_t dlen)
{
- const struct in_addr *src = inany_v4(&toside->faddr);
+ const struct in_addr *src = inany_v4(&toside->oaddr);
const struct in_addr *dst = inany_v4(&toside->eaddr);
size_t l4len = dlen + sizeof(bp->uh);
size_t l3len = l4len + sizeof(*ip4h);
@@ -333,7 +333,7 @@ static size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp,
ip4h->saddr = src->s_addr;
ip4h->check = csum_ip4_header(l3len, IPPROTO_UDP, *src, *dst);
- bp->uh.source = htons(toside->fport);
+ bp->uh.source = htons(toside->oport);
bp->uh.dest = htons(toside->eport);
bp->uh.len = htons(l4len);
csum_udp4(&bp->uh, *src, *dst, bp->data, dlen);
@@ -357,15 +357,15 @@ static size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct udp_payload_t *bp,
ip6h->payload_len = htons(l4len);
ip6h->daddr = toside->eaddr.a6;
- ip6h->saddr = toside->faddr.a6;
+ ip6h->saddr = toside->oaddr.a6;
ip6h->version = 6;
ip6h->nexthdr = IPPROTO_UDP;
ip6h->hop_limit = 255;
- bp->uh.source = htons(toside->fport);
+ bp->uh.source = htons(toside->oport);
bp->uh.dest = htons(toside->eport);
bp->uh.len = ip6h->payload_len;
- csum_udp6(&bp->uh, &toside->faddr.a6, &toside->eaddr.a6, bp->data, dlen);
+ csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, bp->data, dlen);
return l4len;
}
@@ -384,7 +384,7 @@ static void udp_tap_prepare(const struct mmsghdr *mmh, unsigned idx,
struct udp_meta_t *bm = &udp_meta[idx];
size_t l4len;
- if (!inany_v4(&toside->eaddr) || !inany_v4(&toside->faddr)) {
+ if (!inany_v4(&toside->eaddr) || !inany_v4(&toside->oaddr)) {
l4len = udp_update_hdr6(&bm->ip6h, bp, toside, mmh[idx].msg_len);
tap_hdr_update(&bm->taph, l4len + sizeof(bm->ip6h) +
sizeof(udp6_eth_hdr));
--
@@ -321,7 +321,7 @@ static void udp_splice_send(const struct ctx *c, size_t start, size_t n,
static size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp,
const struct flowside *toside, size_t dlen)
{
- const struct in_addr *src = inany_v4(&toside->faddr);
+ const struct in_addr *src = inany_v4(&toside->oaddr);
const struct in_addr *dst = inany_v4(&toside->eaddr);
size_t l4len = dlen + sizeof(bp->uh);
size_t l3len = l4len + sizeof(*ip4h);
@@ -333,7 +333,7 @@ static size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp,
ip4h->saddr = src->s_addr;
ip4h->check = csum_ip4_header(l3len, IPPROTO_UDP, *src, *dst);
- bp->uh.source = htons(toside->fport);
+ bp->uh.source = htons(toside->oport);
bp->uh.dest = htons(toside->eport);
bp->uh.len = htons(l4len);
csum_udp4(&bp->uh, *src, *dst, bp->data, dlen);
@@ -357,15 +357,15 @@ static size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct udp_payload_t *bp,
ip6h->payload_len = htons(l4len);
ip6h->daddr = toside->eaddr.a6;
- ip6h->saddr = toside->faddr.a6;
+ ip6h->saddr = toside->oaddr.a6;
ip6h->version = 6;
ip6h->nexthdr = IPPROTO_UDP;
ip6h->hop_limit = 255;
- bp->uh.source = htons(toside->fport);
+ bp->uh.source = htons(toside->oport);
bp->uh.dest = htons(toside->eport);
bp->uh.len = ip6h->payload_len;
- csum_udp6(&bp->uh, &toside->faddr.a6, &toside->eaddr.a6, bp->data, dlen);
+ csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, bp->data, dlen);
return l4len;
}
@@ -384,7 +384,7 @@ static void udp_tap_prepare(const struct mmsghdr *mmh, unsigned idx,
struct udp_meta_t *bm = &udp_meta[idx];
size_t l4len;
- if (!inany_v4(&toside->eaddr) || !inany_v4(&toside->faddr)) {
+ if (!inany_v4(&toside->eaddr) || !inany_v4(&toside->oaddr)) {
l4len = udp_update_hdr6(&bm->ip6h, bp, toside, mmh[idx].msg_len);
tap_hdr_update(&bm->taph, l4len + sizeof(bm->ip6h) +
sizeof(udp6_eth_hdr));
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 03/16] util: Helper for formatting MAC addresses
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
2024-08-14 4:30 ` [PATCH 01/16] conf: Don't ignore -t and -u options after -D David Gibson
2024-08-14 4:30 ` [PATCH 02/16] treewide: Use "our address" instead of "forwarding address" David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 04/16] treewide: Rename MAC address fields for clarity David Gibson
` (13 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
There are a couple of places where we somewhat messily open code formatting
an Ethernet like MAC address for display. Add an eth_ntop() helper for
this.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 7 +++----
dhcp.c | 5 ++---
util.c | 19 +++++++++++++++++++
util.h | 3 +++
4 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/conf.c b/conf.c
index ed097bdc..830f91a6 100644
--- a/conf.c
+++ b/conf.c
@@ -921,7 +921,8 @@ pasta_opts:
*/
static void conf_print(const struct ctx *c)
{
- char buf4[INET_ADDRSTRLEN], buf6[INET6_ADDRSTRLEN], ifn[IFNAMSIZ];
+ char buf4[INET_ADDRSTRLEN], buf6[INET6_ADDRSTRLEN];
+ char bufmac[ETH_ADDRSTRLEN], ifn[IFNAMSIZ];
int i;
info("Template interface: %s%s%s%s%s",
@@ -955,9 +956,7 @@ static void conf_print(const struct ctx *c)
info("Namespace interface: %s", c->pasta_ifn);
info("MAC:");
- info(" host: %02x:%02x:%02x:%02x:%02x:%02x",
- c->mac[0], c->mac[1], c->mac[2],
- c->mac[3], c->mac[4], c->mac[5]);
+ info(" host: %s", eth_ntop(c->mac, bufmac, sizeof(bufmac)));
if (c->ifi4) {
if (!c->no_dhcp) {
diff --git a/dhcp.c b/dhcp.c
index aa9f59da..acc5b03e 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -276,6 +276,7 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len)
int dhcp(const struct ctx *c, const struct pool *p)
{
size_t mlen, dlen, offset = 0, opt_len, opt_off = 0;
+ char macstr[ETH_ADDRSTRLEN];
const struct ethhdr *eh;
const struct iphdr *iph;
const struct udphdr *uh;
@@ -340,9 +341,7 @@ int dhcp(const struct ctx *c, const struct pool *p)
return -1;
}
- info(" from %02x:%02x:%02x:%02x:%02x:%02x",
- m->chaddr[0], m->chaddr[1], m->chaddr[2],
- m->chaddr[3], m->chaddr[4], m->chaddr[5]);
+ info(" from %s", eth_ntop(m->chaddr, macstr, sizeof(macstr)));
m->yiaddr = c->ip4.addr;
mask.s_addr = htonl(0xffffffff << (32 - c->ip4.prefix_len));
diff --git a/util.c b/util.c
index 0b414045..892358b1 100644
--- a/util.c
+++ b/util.c
@@ -676,6 +676,25 @@ const char *sockaddr_ntop(const void *sa, char *dst, socklen_t size)
return dst;
}
+/** eth_ntop() - Convert an Ethernet MAC address to text format
+ * @mac: MAC address
+ * @dst: output buffer, minimum ETH_ADDRSTRLEN bytes
+ * @size: size of buffer at @dst
+ *
+ * Return: On success, a non-null pointer to @dst, NULL on failure
+ */
+const char *eth_ntop(const unsigned char *mac, char *dst, size_t size)
+{
+ int len;
+
+ len = snprintf(dst, size, "%02x:%02x:%02x:%02x:%02x:%02x",
+ mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
+ if (len < 0 || (size_t)len >= size)
+ return NULL;
+
+ return dst;
+}
+
/** str_ee_origin() - Convert socket extended error origin to a string
* @ee: Socket extended error structure
*
diff --git a/util.h b/util.h
index cb4d181c..c1748074 100644
--- a/util.h
+++ b/util.h
@@ -215,9 +215,12 @@ static inline const char *af_name(sa_family_t af)
#define SOCKADDR_STRLEN MAX(SOCKADDR_INET_STRLEN, SOCKADDR_INET6_STRLEN)
+#define ETH_ADDRSTRLEN (ETH_ALEN * 3)
+
struct sock_extended_err;
const char *sockaddr_ntop(const void *sa, char *dst, socklen_t size);
+const char *eth_ntop(const unsigned char *mac, char *dst, size_t size);
const char *str_ee_origin(const struct sock_extended_err *ee);
/**
--
@@ -215,9 +215,12 @@ static inline const char *af_name(sa_family_t af)
#define SOCKADDR_STRLEN MAX(SOCKADDR_INET_STRLEN, SOCKADDR_INET6_STRLEN)
+#define ETH_ADDRSTRLEN (ETH_ALEN * 3)
+
struct sock_extended_err;
const char *sockaddr_ntop(const void *sa, char *dst, socklen_t size);
+const char *eth_ntop(const unsigned char *mac, char *dst, size_t size);
const char *str_ee_origin(const struct sock_extended_err *ee);
/**
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 04/16] treewide: Rename MAC address fields for clarity
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (2 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 03/16] util: Helper for formatting MAC addresses David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 05/16] treewide: Use struct assignment instead of memcpy() for IP addresses David Gibson
` (12 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
c->mac isn't a great name, because it doesn't say whose mac address it is
and it's not necessarily obvious in all the contexts we use it. Since this
is specifically the address that we (passt/pasta) use on the tap interface,
rename it to "our_tap_mac". Rename the "mac_guest" field to "guest_mac"
to be grammatically consistent.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
arp.c | 4 ++--
conf.c | 10 +++++-----
dhcpv6.c | 6 ++++--
ndp.c | 4 ++--
passt.c | 2 +-
passt.h | 8 ++++----
pasta.c | 8 ++++----
tap.c | 12 ++++++------
8 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/arp.c b/arp.c
index 93b22c5d..53334dac 100644
--- a/arp.c
+++ b/arp.c
@@ -72,7 +72,7 @@ int arp(const struct ctx *c, const struct pool *p)
ah->ar_op = htons(ARPOP_REPLY);
memcpy(am->tha, am->sha, sizeof(am->tha));
- memcpy(am->sha, c->mac, sizeof(am->sha));
+ memcpy(am->sha, c->our_tap_mac, sizeof(am->sha));
memcpy(swap, am->tip, sizeof(am->tip));
memcpy(am->tip, am->sip, sizeof(am->tip));
@@ -80,7 +80,7 @@ int arp(const struct ctx *c, const struct pool *p)
l2len = sizeof(*eh) + sizeof(*ah) + sizeof(*am);
memcpy(eh->h_dest, eh->h_source, sizeof(eh->h_dest));
- memcpy(eh->h_source, c->mac, sizeof(eh->h_source));
+ memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source));
tap_send_single(c, eh, l2len);
diff --git a/conf.c b/conf.c
index 830f91a6..750fdc86 100644
--- a/conf.c
+++ b/conf.c
@@ -956,7 +956,7 @@ static void conf_print(const struct ctx *c)
info("Namespace interface: %s", c->pasta_ifn);
info("MAC:");
- info(" host: %s", eth_ntop(c->mac, bufmac, sizeof(bufmac)));
+ info(" host: %s", eth_ntop(c->our_tap_mac, bufmac, sizeof(bufmac)));
if (c->ifi4) {
if (!c->no_dhcp) {
@@ -1289,7 +1289,7 @@ void conf(struct ctx *c, int argc, char **argv)
if (c->mode != MODE_PASTA)
die("--ns-mac-addr is for pasta mode only");
- parse_mac(c->mac_guest, optarg);
+ parse_mac(c->guest_mac, optarg);
break;
case 5:
if (c->mode != MODE_PASTA)
@@ -1500,7 +1500,7 @@ void conf(struct ctx *c, int argc, char **argv)
break;
case 'M':
- parse_mac(c->mac, optarg);
+ parse_mac(c->our_tap_mac, optarg);
break;
case 'g':
if (inet_pton(AF_INET6, optarg, &c->ip6.gw) &&
@@ -1629,9 +1629,9 @@ void conf(struct ctx *c, int argc, char **argv)
nl_sock_init(c, false);
if (!v6_only)
- c->ifi4 = conf_ip4(ifi4, &c->ip4, c->mac);
+ c->ifi4 = conf_ip4(ifi4, &c->ip4, c->our_tap_mac);
if (!v4_only)
- c->ifi6 = conf_ip6(ifi6, &c->ip6, c->mac);
+ c->ifi6 = conf_ip6(ifi6, &c->ip6, c->our_tap_mac);
if ((!c->ifi4 && !c->ifi6) ||
(*c->ip4.ifname_out && !c->ifi4) ||
(*c->ip6.ifname_out && !c->ifi6))
diff --git a/dhcpv6.c b/dhcpv6.c
index 7dcca2a7..bbed41dc 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -574,8 +574,10 @@ void dhcpv6_init(const struct ctx *c)
resp.server_id.duid_time = duid_time;
resp_not_on_link.server_id.duid_time = duid_time;
- memcpy(resp.server_id.duid_lladdr, c->mac, sizeof(c->mac));
- memcpy(resp_not_on_link.server_id.duid_lladdr, c->mac, sizeof(c->mac));
+ memcpy(resp.server_id.duid_lladdr,
+ c->our_tap_mac, sizeof(c->our_tap_mac));
+ memcpy(resp_not_on_link.server_id.duid_lladdr,
+ c->our_tap_mac, sizeof(c->our_tap_mac));
resp.ia_addr.addr = c->ip6.addr;
}
diff --git a/ndp.c b/ndp.c
index 6dcb4872..9c0fef4a 100644
--- a/ndp.c
+++ b/ndp.c
@@ -247,7 +247,7 @@ int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr,
memcpy(&na.target_addr, &ns->target_addr,
sizeof(na.target_addr));
- memcpy(na.target_l2_addr.mac, c->mac, ETH_ALEN);
+ memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN);
} else if (ih->icmp6_type == RS) {
size_t dns_s_len = 0;
@@ -331,7 +331,7 @@ int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr,
}
dns_done:
- memcpy(&ra.source_ll.mac, c->mac, ETH_ALEN);
+ memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN);
} else {
return 1;
}
diff --git a/passt.c b/passt.c
index 4b3c306e..96374831 100644
--- a/passt.c
+++ b/passt.c
@@ -272,7 +272,7 @@ int main(int argc, char **argv)
if ((!c.no_udp && udp_init(&c)) || (!c.no_tcp && tcp_init(&c)))
exit(EXIT_FAILURE);
- proto_update_l2_buf(c.mac_guest, c.mac);
+ proto_update_l2_buf(c.guest_mac, c.our_tap_mac);
if (c.ifi4 && !c.no_dhcp)
dhcp_init();
diff --git a/passt.h b/passt.h
index ef684037..fe3e47d2 100644
--- a/passt.h
+++ b/passt.h
@@ -172,8 +172,8 @@ struct ip6_ctx {
* @epollfd: File descriptor for epoll instance
* @fd_tap_listen: File descriptor for listening AF_UNIX socket, if any
* @fd_tap: AF_UNIX socket, tuntap device, or pre-opened socket
- * @mac: Host MAC address
- * @mac_guest: MAC address of guest or namespace, seen or configured
+ * @our_tap_mac: Pasta/passt's MAC on the tap link
+ * @guest_mac: MAC address of guest or namespace, seen or configured
* @hash_secret: 128-bit secret for siphash functions
* @ifi4: Index of template interface for IPv4, 0 if IPv4 disabled
* @ip: IPv4 configuration
@@ -226,8 +226,8 @@ struct ctx {
int epollfd;
int fd_tap_listen;
int fd_tap;
- unsigned char mac[ETH_ALEN];
- unsigned char mac_guest[ETH_ALEN];
+ unsigned char our_tap_mac[ETH_ALEN];
+ unsigned char guest_mac[ETH_ALEN];
uint64_t hash_secret[2];
unsigned int ifi4;
diff --git a/pasta.c b/pasta.c
index 615ff7b3..3b4e8ead 100644
--- a/pasta.c
+++ b/pasta.c
@@ -294,10 +294,10 @@ void pasta_ns_conf(struct ctx *c)
strerror(-rc));
/* Get or set MAC in target namespace */
- if (MAC_IS_ZERO(c->mac_guest))
- nl_link_get_mac(nl_sock_ns, c->pasta_ifi, c->mac_guest);
+ if (MAC_IS_ZERO(c->guest_mac))
+ nl_link_get_mac(nl_sock_ns, c->pasta_ifi, c->guest_mac);
else
- rc = nl_link_set_mac(nl_sock_ns, c->pasta_ifi, c->mac_guest);
+ rc = nl_link_set_mac(nl_sock_ns, c->pasta_ifi, c->guest_mac);
if (rc < 0)
die("Couldn't set MAC address in namespace: %s",
strerror(-rc));
@@ -367,7 +367,7 @@ void pasta_ns_conf(struct ctx *c)
}
}
- proto_update_l2_buf(c->mac_guest, NULL);
+ proto_update_l2_buf(c->guest_mac, NULL);
}
/**
diff --git a/tap.c b/tap.c
index 87be3a6b..852d8376 100644
--- a/tap.c
+++ b/tap.c
@@ -118,8 +118,8 @@ static void *tap_push_l2h(const struct ctx *c, void *buf, uint16_t proto)
struct ethhdr *eh = (struct ethhdr *)buf;
/* TODO: ARP table lookup */
- memcpy(eh->h_dest, c->mac_guest, ETH_ALEN);
- memcpy(eh->h_source, c->mac, ETH_ALEN);
+ memcpy(eh->h_dest, c->guest_mac, ETH_ALEN);
+ memcpy(eh->h_source, c->our_tap_mac, ETH_ALEN);
eh->h_proto = ntohs(proto);
return eh + 1;
}
@@ -946,9 +946,9 @@ void tap_add_packet(struct ctx *c, ssize_t l2len, char *p)
eh = (struct ethhdr *)p;
- if (memcmp(c->mac_guest, eh->h_source, ETH_ALEN)) {
- memcpy(c->mac_guest, eh->h_source, ETH_ALEN);
- proto_update_l2_buf(c->mac_guest, NULL);
+ if (memcmp(c->guest_mac, eh->h_source, ETH_ALEN)) {
+ memcpy(c->guest_mac, eh->h_source, ETH_ALEN);
+ proto_update_l2_buf(c->guest_mac, NULL);
}
switch (ntohs(eh->h_proto)) {
@@ -1337,6 +1337,6 @@ void tap_sock_init(struct ctx *c)
* sends us packets. Use the broadcast address so that our
* first packets will reach it.
*/
- memset(&c->mac_guest, 0xff, sizeof(c->mac_guest));
+ memset(&c->guest_mac, 0xff, sizeof(c->guest_mac));
}
}
--
@@ -118,8 +118,8 @@ static void *tap_push_l2h(const struct ctx *c, void *buf, uint16_t proto)
struct ethhdr *eh = (struct ethhdr *)buf;
/* TODO: ARP table lookup */
- memcpy(eh->h_dest, c->mac_guest, ETH_ALEN);
- memcpy(eh->h_source, c->mac, ETH_ALEN);
+ memcpy(eh->h_dest, c->guest_mac, ETH_ALEN);
+ memcpy(eh->h_source, c->our_tap_mac, ETH_ALEN);
eh->h_proto = ntohs(proto);
return eh + 1;
}
@@ -946,9 +946,9 @@ void tap_add_packet(struct ctx *c, ssize_t l2len, char *p)
eh = (struct ethhdr *)p;
- if (memcmp(c->mac_guest, eh->h_source, ETH_ALEN)) {
- memcpy(c->mac_guest, eh->h_source, ETH_ALEN);
- proto_update_l2_buf(c->mac_guest, NULL);
+ if (memcmp(c->guest_mac, eh->h_source, ETH_ALEN)) {
+ memcpy(c->guest_mac, eh->h_source, ETH_ALEN);
+ proto_update_l2_buf(c->guest_mac, NULL);
}
switch (ntohs(eh->h_proto)) {
@@ -1337,6 +1337,6 @@ void tap_sock_init(struct ctx *c)
* sends us packets. Use the broadcast address so that our
* first packets will reach it.
*/
- memset(&c->mac_guest, 0xff, sizeof(c->mac_guest));
+ memset(&c->guest_mac, 0xff, sizeof(c->guest_mac));
}
}
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 05/16] treewide: Use struct assignment instead of memcpy() for IP addresses
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (3 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 04/16] treewide: Rename MAC address fields for clarity David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 06/16] conf: Use array indices rather than pointers for DNS array slots David Gibson
` (11 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
We rely on C11 already, so we can use clearer and more type-checkable
struct assignment instead of mempcy() for copying IP addresses around.
This exposes some "pointer could be const" warnings from cppcheck, so
address those too.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 12 ++++++------
dhcpv6.c | 10 ++++++----
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/conf.c b/conf.c
index 750fdc86..9b05afeb 100644
--- a/conf.c
+++ b/conf.c
@@ -389,14 +389,14 @@ static void add_dns6(struct ctx *c,
/* Guest or container can only access local addresses via redirect */
if (IN6_IS_ADDR_LOOPBACK(addr)) {
if (!c->no_map_gw) {
- memcpy(*conf, &c->ip6.gw, sizeof(**conf));
+ **conf = c->ip6.gw;
(*conf)++;
if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
- memcpy(&c->ip6.dns_match, addr, sizeof(*addr));
+ c->ip6.dns_match = *addr;
}
} else {
- memcpy(*conf, addr, sizeof(**conf));
+ **conf = *addr;
(*conf)++;
}
@@ -632,7 +632,7 @@ static unsigned int conf_ip4(unsigned int ifi,
ip4->prefix_len = 32;
}
- memcpy(&ip4->addr_seen, &ip4->addr, sizeof(ip4->addr_seen));
+ ip4->addr_seen = ip4->addr;
if (MAC_IS_ZERO(mac)) {
int rc = nl_link_get_mac(nl_sock, ifi, mac);
@@ -693,8 +693,8 @@ static unsigned int conf_ip6(unsigned int ifi,
return 0;
}
- memcpy(&ip6->addr_seen, &ip6->addr, sizeof(ip6->addr));
- memcpy(&ip6->addr_ll_seen, &ip6->addr_ll, sizeof(ip6->addr_ll));
+ ip6->addr_seen = ip6->addr;
+ ip6->addr_ll_seen = ip6->addr_ll;
if (MAC_IS_ZERO(mac)) {
rc = nl_link_get_mac(nl_sock, ifi, mac);
diff --git a/dhcpv6.c b/dhcpv6.c
index bbed41dc..87b3c3eb 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -298,7 +298,8 @@ static struct opt_hdr *dhcpv6_ia_notonlink(const struct pool *p,
{
char buf[INET6_ADDRSTRLEN];
struct in6_addr req_addr;
- struct opt_hdr *ia, *h;
+ const struct opt_hdr *h;
+ struct opt_hdr *ia;
size_t offset;
int ia_type;
@@ -312,12 +313,13 @@ ia_ta:
offset += sizeof(struct opt_ia_na);
while ((h = dhcpv6_opt(p, &offset, OPT_IAAADR))) {
- struct opt_ia_addr *opt_addr = (struct opt_ia_addr *)h;
+ const struct opt_ia_addr *opt_addr
+ = (const struct opt_ia_addr *)h;
if (ntohs(h->l) != OPT_VSIZE(ia_addr))
return NULL;
- memcpy(&req_addr, &opt_addr->addr, sizeof(req_addr));
+ req_addr = opt_addr->addr;
if (!IN6_ARE_ADDR_EQUAL(la, &req_addr)) {
info("DHCPv6: requested address %s not on link",
inet_ntop(AF_INET6, &req_addr,
@@ -363,7 +365,7 @@ static size_t dhcpv6_dns_fill(const struct ctx *c, char *buf, int offset)
srv->hdr.l = 0;
}
- memcpy(&srv->addr[i], &c->ip6.dns[i], sizeof(srv->addr[i]));
+ srv->addr[i] = c->ip6.dns[i];
srv->hdr.l += sizeof(srv->addr[i]);
offset += sizeof(srv->addr[i]);
}
--
@@ -298,7 +298,8 @@ static struct opt_hdr *dhcpv6_ia_notonlink(const struct pool *p,
{
char buf[INET6_ADDRSTRLEN];
struct in6_addr req_addr;
- struct opt_hdr *ia, *h;
+ const struct opt_hdr *h;
+ struct opt_hdr *ia;
size_t offset;
int ia_type;
@@ -312,12 +313,13 @@ ia_ta:
offset += sizeof(struct opt_ia_na);
while ((h = dhcpv6_opt(p, &offset, OPT_IAAADR))) {
- struct opt_ia_addr *opt_addr = (struct opt_ia_addr *)h;
+ const struct opt_ia_addr *opt_addr
+ = (const struct opt_ia_addr *)h;
if (ntohs(h->l) != OPT_VSIZE(ia_addr))
return NULL;
- memcpy(&req_addr, &opt_addr->addr, sizeof(req_addr));
+ req_addr = opt_addr->addr;
if (!IN6_ARE_ADDR_EQUAL(la, &req_addr)) {
info("DHCPv6: requested address %s not on link",
inet_ntop(AF_INET6, &req_addr,
@@ -363,7 +365,7 @@ static size_t dhcpv6_dns_fill(const struct ctx *c, char *buf, int offset)
srv->hdr.l = 0;
}
- memcpy(&srv->addr[i], &c->ip6.dns[i], sizeof(srv->addr[i]));
+ srv->addr[i] = c->ip6.dns[i];
srv->hdr.l += sizeof(srv->addr[i]);
offset += sizeof(srv->addr[i]);
}
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 06/16] conf: Use array indices rather than pointers for DNS array slots
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (4 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 05/16] treewide: Use struct assignment instead of memcpy() for IP addresses David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 07/16] conf: More accurately count entries added in get_dns() David Gibson
` (10 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
Currently add_dns[46]() take a somewhat awkward double pointer to the
entry in the c->ip[46].dns array to update. It turns out to be easier to
work with indices into that array instead.
This diff does add some lines, but it's comments, and will allow some
future code reductions.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 73 +++++++++++++++++++++++++++++++++-------------------------
1 file changed, 41 insertions(+), 32 deletions(-)
diff --git a/conf.c b/conf.c
index 9b05afeb..286b3e9f 100644
--- a/conf.c
+++ b/conf.c
@@ -354,54 +354,65 @@ bind_all_fail:
* add_dns4() - Possibly add the IPv4 address of a DNS resolver to configuration
* @c: Execution context
* @addr: Address found in /etc/resolv.conf
- * @conf: Pointer to reference of current entry in array of IPv4 resolvers
+ * @idx: Index of free entry in array of IPv4 resolvers
+ *
+ * Return: Number of entries added (0 or 1)
*/
-static void add_dns4(struct ctx *c, const struct in_addr *addr,
- struct in_addr **conf)
+static unsigned add_dns4(struct ctx *c, const struct in_addr *addr,
+ unsigned idx)
{
+ unsigned added = 0;
+
/* Guest or container can only access local addresses via redirect */
if (IN4_IS_ADDR_LOOPBACK(addr)) {
if (!c->no_map_gw) {
- **conf = c->ip4.gw;
- (*conf)++;
+ c->ip4.dns[idx] = c->ip4.gw;
+ added++;
if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
c->ip4.dns_match = c->ip4.gw;
}
} else {
- **conf = *addr;
- (*conf)++;
+ c->ip4.dns[idx] = *addr;
+ added++;
}
if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
c->ip4.dns_host = *addr;
+
+ return added;
}
/**
* add_dns6() - Possibly add the IPv6 address of a DNS resolver to configuration
* @c: Execution context
* @addr: Address found in /etc/resolv.conf
- * @conf: Pointer to reference of current entry in array of IPv6 resolvers
+ * @idx: Index of free entry in array of IPv4 resolvers
+ *
+ * Return: Number of entries added (0 or 1)
*/
-static void add_dns6(struct ctx *c,
- struct in6_addr *addr, struct in6_addr **conf)
+static unsigned add_dns6(struct ctx *c, struct in6_addr *addr, unsigned idx)
{
+ unsigned added = 0;
+
/* Guest or container can only access local addresses via redirect */
if (IN6_IS_ADDR_LOOPBACK(addr)) {
if (!c->no_map_gw) {
- **conf = c->ip6.gw;
- (*conf)++;
+ c->ip6.dns[idx] = c->ip6.gw;
+ added++;
if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
c->ip6.dns_match = *addr;
}
} else {
- **conf = *addr;
- (*conf)++;
+ c->ip6.dns[idx] = *addr;
+ added++;
}
if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
c->ip6.dns_host = *addr;
+
+ return added;
}
/**
@@ -410,18 +421,19 @@ static void add_dns6(struct ctx *c,
*/
static void get_dns(struct ctx *c)
{
- struct in6_addr *dns6 = &c->ip6.dns[0], dns6_tmp;
- struct in_addr *dns4 = &c->ip4.dns[0], dns4_tmp;
int dns4_set, dns6_set, dnss_set, dns_set, fd;
+ unsigned dns4_idx = 0, dns6_idx = 0;
struct fqdn *s = c->dns_search;
struct lineread resolvconf;
+ struct in6_addr dns6_tmp;
+ struct in_addr dns4_tmp;
unsigned int added = 0;
ssize_t line_len;
char *line, *end;
const char *p;
- dns4_set = !c->ifi4 || !IN4_IS_ADDR_UNSPECIFIED(dns4);
- dns6_set = !c->ifi6 || !IN6_IS_ADDR_UNSPECIFIED(dns6);
+ dns4_set = !c->ifi4 || !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[0]);
+ dns6_set = !c->ifi6 || !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[0]);
dnss_set = !!*s->n || c->no_dns_search;
dns_set = (dns4_set && dns6_set) || c->no_dns;
@@ -442,17 +454,15 @@ static void get_dns(struct ctx *c)
if (end)
*end = 0;
- if (!dns4_set &&
- dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1
+ if (!dns4_set && dns4_idx < ARRAY_SIZE(c->ip4.dns) - 1
&& inet_pton(AF_INET, p + 1, &dns4_tmp)) {
- add_dns4(c, &dns4_tmp, &dns4);
+ dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
added++;
}
- if (!dns6_set &&
- dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) - 1
+ if (!dns6_set && dns6_idx < ARRAY_SIZE(c->ip6.dns) - 1
&& inet_pton(AF_INET6, p + 1, &dns6_tmp)) {
- add_dns6(c, &dns6_tmp, &dns6);
+ dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
added++;
}
} else if (!dnss_set && strstr(line, "search ") == line &&
@@ -1236,8 +1246,7 @@ void conf(struct ctx *c, int argc, char **argv)
bool copy_addrs_opt = false, copy_routes_opt = false;
enum fwd_ports_mode fwd_default = FWD_NONE;
bool v4_only = false, v6_only = false;
- struct in6_addr *dns6 = c->ip6.dns;
- struct in_addr *dns4 = c->ip4.dns;
+ unsigned dns4_idx = 0, dns6_idx = 0;
struct fqdn *dnss = c->dns_search;
unsigned int ifi4 = 0, ifi6 = 0;
const char *logfile = NULL;
@@ -1662,13 +1671,13 @@ void conf(struct ctx *c, int argc, char **argv)
if (!strcmp(optarg, "none")) {
c->no_dns = 1;
- dns4 = &c->ip4.dns[0];
+ dns4_idx = 0;
memset(c->ip4.dns, 0, sizeof(c->ip4.dns));
c->ip4.dns[0] = (struct in_addr){ 0 };
c->ip4.dns_match = (struct in_addr){ 0 };
c->ip4.dns_host = (struct in_addr){ 0 };
- dns6 = &c->ip6.dns[0];
+ dns6_idx = 0;
memset(c->ip6.dns, 0, sizeof(c->ip6.dns));
c->ip6.dns_match = (struct in6_addr){ 0 };
c->ip6.dns_host = (struct in6_addr){ 0 };
@@ -1678,15 +1687,15 @@ void conf(struct ctx *c, int argc, char **argv)
c->no_dns = 0;
- if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) &&
+ if (dns4_idx < ARRAY_SIZE(c->ip4.dns) &&
inet_pton(AF_INET, optarg, &dns4_tmp)) {
- add_dns4(c, &dns4_tmp, &dns4);
+ dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
continue;
}
- if (dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) &&
+ if (dns6_idx < ARRAY_SIZE(c->ip6.dns) &&
inet_pton(AF_INET6, optarg, &dns6_tmp)) {
- add_dns6(c, &dns6_tmp, &dns6);
+ dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
continue;
}
--
@@ -354,54 +354,65 @@ bind_all_fail:
* add_dns4() - Possibly add the IPv4 address of a DNS resolver to configuration
* @c: Execution context
* @addr: Address found in /etc/resolv.conf
- * @conf: Pointer to reference of current entry in array of IPv4 resolvers
+ * @idx: Index of free entry in array of IPv4 resolvers
+ *
+ * Return: Number of entries added (0 or 1)
*/
-static void add_dns4(struct ctx *c, const struct in_addr *addr,
- struct in_addr **conf)
+static unsigned add_dns4(struct ctx *c, const struct in_addr *addr,
+ unsigned idx)
{
+ unsigned added = 0;
+
/* Guest or container can only access local addresses via redirect */
if (IN4_IS_ADDR_LOOPBACK(addr)) {
if (!c->no_map_gw) {
- **conf = c->ip4.gw;
- (*conf)++;
+ c->ip4.dns[idx] = c->ip4.gw;
+ added++;
if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
c->ip4.dns_match = c->ip4.gw;
}
} else {
- **conf = *addr;
- (*conf)++;
+ c->ip4.dns[idx] = *addr;
+ added++;
}
if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
c->ip4.dns_host = *addr;
+
+ return added;
}
/**
* add_dns6() - Possibly add the IPv6 address of a DNS resolver to configuration
* @c: Execution context
* @addr: Address found in /etc/resolv.conf
- * @conf: Pointer to reference of current entry in array of IPv6 resolvers
+ * @idx: Index of free entry in array of IPv4 resolvers
+ *
+ * Return: Number of entries added (0 or 1)
*/
-static void add_dns6(struct ctx *c,
- struct in6_addr *addr, struct in6_addr **conf)
+static unsigned add_dns6(struct ctx *c, struct in6_addr *addr, unsigned idx)
{
+ unsigned added = 0;
+
/* Guest or container can only access local addresses via redirect */
if (IN6_IS_ADDR_LOOPBACK(addr)) {
if (!c->no_map_gw) {
- **conf = c->ip6.gw;
- (*conf)++;
+ c->ip6.dns[idx] = c->ip6.gw;
+ added++;
if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
c->ip6.dns_match = *addr;
}
} else {
- **conf = *addr;
- (*conf)++;
+ c->ip6.dns[idx] = *addr;
+ added++;
}
if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
c->ip6.dns_host = *addr;
+
+ return added;
}
/**
@@ -410,18 +421,19 @@ static void add_dns6(struct ctx *c,
*/
static void get_dns(struct ctx *c)
{
- struct in6_addr *dns6 = &c->ip6.dns[0], dns6_tmp;
- struct in_addr *dns4 = &c->ip4.dns[0], dns4_tmp;
int dns4_set, dns6_set, dnss_set, dns_set, fd;
+ unsigned dns4_idx = 0, dns6_idx = 0;
struct fqdn *s = c->dns_search;
struct lineread resolvconf;
+ struct in6_addr dns6_tmp;
+ struct in_addr dns4_tmp;
unsigned int added = 0;
ssize_t line_len;
char *line, *end;
const char *p;
- dns4_set = !c->ifi4 || !IN4_IS_ADDR_UNSPECIFIED(dns4);
- dns6_set = !c->ifi6 || !IN6_IS_ADDR_UNSPECIFIED(dns6);
+ dns4_set = !c->ifi4 || !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[0]);
+ dns6_set = !c->ifi6 || !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[0]);
dnss_set = !!*s->n || c->no_dns_search;
dns_set = (dns4_set && dns6_set) || c->no_dns;
@@ -442,17 +454,15 @@ static void get_dns(struct ctx *c)
if (end)
*end = 0;
- if (!dns4_set &&
- dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1
+ if (!dns4_set && dns4_idx < ARRAY_SIZE(c->ip4.dns) - 1
&& inet_pton(AF_INET, p + 1, &dns4_tmp)) {
- add_dns4(c, &dns4_tmp, &dns4);
+ dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
added++;
}
- if (!dns6_set &&
- dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) - 1
+ if (!dns6_set && dns6_idx < ARRAY_SIZE(c->ip6.dns) - 1
&& inet_pton(AF_INET6, p + 1, &dns6_tmp)) {
- add_dns6(c, &dns6_tmp, &dns6);
+ dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
added++;
}
} else if (!dnss_set && strstr(line, "search ") == line &&
@@ -1236,8 +1246,7 @@ void conf(struct ctx *c, int argc, char **argv)
bool copy_addrs_opt = false, copy_routes_opt = false;
enum fwd_ports_mode fwd_default = FWD_NONE;
bool v4_only = false, v6_only = false;
- struct in6_addr *dns6 = c->ip6.dns;
- struct in_addr *dns4 = c->ip4.dns;
+ unsigned dns4_idx = 0, dns6_idx = 0;
struct fqdn *dnss = c->dns_search;
unsigned int ifi4 = 0, ifi6 = 0;
const char *logfile = NULL;
@@ -1662,13 +1671,13 @@ void conf(struct ctx *c, int argc, char **argv)
if (!strcmp(optarg, "none")) {
c->no_dns = 1;
- dns4 = &c->ip4.dns[0];
+ dns4_idx = 0;
memset(c->ip4.dns, 0, sizeof(c->ip4.dns));
c->ip4.dns[0] = (struct in_addr){ 0 };
c->ip4.dns_match = (struct in_addr){ 0 };
c->ip4.dns_host = (struct in_addr){ 0 };
- dns6 = &c->ip6.dns[0];
+ dns6_idx = 0;
memset(c->ip6.dns, 0, sizeof(c->ip6.dns));
c->ip6.dns_match = (struct in6_addr){ 0 };
c->ip6.dns_host = (struct in6_addr){ 0 };
@@ -1678,15 +1687,15 @@ void conf(struct ctx *c, int argc, char **argv)
c->no_dns = 0;
- if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) &&
+ if (dns4_idx < ARRAY_SIZE(c->ip4.dns) &&
inet_pton(AF_INET, optarg, &dns4_tmp)) {
- add_dns4(c, &dns4_tmp, &dns4);
+ dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
continue;
}
- if (dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) &&
+ if (dns6_idx < ARRAY_SIZE(c->ip6.dns) &&
inet_pton(AF_INET6, optarg, &dns6_tmp)) {
- add_dns6(c, &dns6_tmp, &dns6);
+ dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
continue;
}
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 07/16] conf: More accurately count entries added in get_dns()
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (5 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 06/16] conf: Use array indices rather than pointers for DNS array slots David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 08/16] conf: Move DNS array bounds checks into add_dns[46] David Gibson
` (9 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
get_dns() counts the number of guest DNS servers it adds, and gives an
error if it couldn't add any. However, this count ignores the fact that
add_dns[46]() may in some cases *not* add an entry. Use the array indices
we're already tracking to get an accurate count.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/conf.c b/conf.c
index 286b3e9f..db036a75 100644
--- a/conf.c
+++ b/conf.c
@@ -427,7 +427,6 @@ static void get_dns(struct ctx *c)
struct lineread resolvconf;
struct in6_addr dns6_tmp;
struct in_addr dns4_tmp;
- unsigned int added = 0;
ssize_t line_len;
char *line, *end;
const char *p;
@@ -455,16 +454,12 @@ static void get_dns(struct ctx *c)
*end = 0;
if (!dns4_set && dns4_idx < ARRAY_SIZE(c->ip4.dns) - 1
- && inet_pton(AF_INET, p + 1, &dns4_tmp)) {
+ && inet_pton(AF_INET, p + 1, &dns4_tmp))
dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
- added++;
- }
if (!dns6_set && dns6_idx < ARRAY_SIZE(c->ip6.dns) - 1
- && inet_pton(AF_INET6, p + 1, &dns6_tmp)) {
+ && inet_pton(AF_INET6, p + 1, &dns6_tmp))
dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
- added++;
- }
} else if (!dnss_set && strstr(line, "search ") == line &&
s == c->dns_search) {
end = strpbrk(line, "\n");
@@ -491,7 +486,7 @@ static void get_dns(struct ctx *c)
out:
if (!dns_set) {
- if (!added)
+ if (!(dns4_idx + dns6_idx))
warn("Couldn't get any nameserver address");
if (c->no_dhcp_dns)
--
@@ -427,7 +427,6 @@ static void get_dns(struct ctx *c)
struct lineread resolvconf;
struct in6_addr dns6_tmp;
struct in_addr dns4_tmp;
- unsigned int added = 0;
ssize_t line_len;
char *line, *end;
const char *p;
@@ -455,16 +454,12 @@ static void get_dns(struct ctx *c)
*end = 0;
if (!dns4_set && dns4_idx < ARRAY_SIZE(c->ip4.dns) - 1
- && inet_pton(AF_INET, p + 1, &dns4_tmp)) {
+ && inet_pton(AF_INET, p + 1, &dns4_tmp))
dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
- added++;
- }
if (!dns6_set && dns6_idx < ARRAY_SIZE(c->ip6.dns) - 1
- && inet_pton(AF_INET6, p + 1, &dns6_tmp)) {
+ && inet_pton(AF_INET6, p + 1, &dns6_tmp))
dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
- added++;
- }
} else if (!dnss_set && strstr(line, "search ") == line &&
s == c->dns_search) {
end = strpbrk(line, "\n");
@@ -491,7 +486,7 @@ static void get_dns(struct ctx *c)
out:
if (!dns_set) {
- if (!added)
+ if (!(dns4_idx + dns6_idx))
warn("Couldn't get any nameserver address");
if (c->no_dhcp_dns)
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 08/16] conf: Move DNS array bounds checks into add_dns[46]
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (6 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 07/16] conf: More accurately count entries added in get_dns() David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 09/16] conf: Move adding of a nameserver from resolv.conf into subfunction David Gibson
` (8 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
Every time we call add_dns[46] we need to first check if there's space in
the c->ip[46].dns array for the new entry. We might as well make that
check in add_dns[46]() itself.
In fact it looks like the calls in get_dns() had an off by one error, not
allowing the last entry of the array to be filled. So, that bug is also
fixed by the change.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/conf.c b/conf.c
index db036a75..a651cb91 100644
--- a/conf.c
+++ b/conf.c
@@ -363,6 +363,9 @@ static unsigned add_dns4(struct ctx *c, const struct in_addr *addr,
{
unsigned added = 0;
+ if (idx >= ARRAY_SIZE(c->ip4.dns))
+ return 0;
+
/* Guest or container can only access local addresses via redirect */
if (IN4_IS_ADDR_LOOPBACK(addr)) {
if (!c->no_map_gw) {
@@ -395,6 +398,9 @@ static unsigned add_dns6(struct ctx *c, struct in6_addr *addr, unsigned idx)
{
unsigned added = 0;
+ if (idx >= ARRAY_SIZE(c->ip6.dns))
+ return 0;
+
/* Guest or container can only access local addresses via redirect */
if (IN6_IS_ADDR_LOOPBACK(addr)) {
if (!c->no_map_gw) {
@@ -453,12 +459,10 @@ static void get_dns(struct ctx *c)
if (end)
*end = 0;
- if (!dns4_set && dns4_idx < ARRAY_SIZE(c->ip4.dns) - 1
- && inet_pton(AF_INET, p + 1, &dns4_tmp))
+ if (!dns4_set && inet_pton(AF_INET, p + 1, &dns4_tmp))
dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
- if (!dns6_set && dns6_idx < ARRAY_SIZE(c->ip6.dns) - 1
- && inet_pton(AF_INET6, p + 1, &dns6_tmp))
+ if (!dns6_set && inet_pton(AF_INET6, p + 1, &dns6_tmp))
dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
} else if (!dnss_set && strstr(line, "search ") == line &&
s == c->dns_search) {
@@ -1682,14 +1686,12 @@ void conf(struct ctx *c, int argc, char **argv)
c->no_dns = 0;
- if (dns4_idx < ARRAY_SIZE(c->ip4.dns) &&
- inet_pton(AF_INET, optarg, &dns4_tmp)) {
+ if (inet_pton(AF_INET, optarg, &dns4_tmp)) {
dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
continue;
}
- if (dns6_idx < ARRAY_SIZE(c->ip6.dns) &&
- inet_pton(AF_INET6, optarg, &dns6_tmp)) {
+ if (inet_pton(AF_INET6, optarg, &dns6_tmp)) {
dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
continue;
}
--
@@ -363,6 +363,9 @@ static unsigned add_dns4(struct ctx *c, const struct in_addr *addr,
{
unsigned added = 0;
+ if (idx >= ARRAY_SIZE(c->ip4.dns))
+ return 0;
+
/* Guest or container can only access local addresses via redirect */
if (IN4_IS_ADDR_LOOPBACK(addr)) {
if (!c->no_map_gw) {
@@ -395,6 +398,9 @@ static unsigned add_dns6(struct ctx *c, struct in6_addr *addr, unsigned idx)
{
unsigned added = 0;
+ if (idx >= ARRAY_SIZE(c->ip6.dns))
+ return 0;
+
/* Guest or container can only access local addresses via redirect */
if (IN6_IS_ADDR_LOOPBACK(addr)) {
if (!c->no_map_gw) {
@@ -453,12 +459,10 @@ static void get_dns(struct ctx *c)
if (end)
*end = 0;
- if (!dns4_set && dns4_idx < ARRAY_SIZE(c->ip4.dns) - 1
- && inet_pton(AF_INET, p + 1, &dns4_tmp))
+ if (!dns4_set && inet_pton(AF_INET, p + 1, &dns4_tmp))
dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
- if (!dns6_set && dns6_idx < ARRAY_SIZE(c->ip6.dns) - 1
- && inet_pton(AF_INET6, p + 1, &dns6_tmp))
+ if (!dns6_set && inet_pton(AF_INET6, p + 1, &dns6_tmp))
dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
} else if (!dnss_set && strstr(line, "search ") == line &&
s == c->dns_search) {
@@ -1682,14 +1686,12 @@ void conf(struct ctx *c, int argc, char **argv)
c->no_dns = 0;
- if (dns4_idx < ARRAY_SIZE(c->ip4.dns) &&
- inet_pton(AF_INET, optarg, &dns4_tmp)) {
+ if (inet_pton(AF_INET, optarg, &dns4_tmp)) {
dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
continue;
}
- if (dns6_idx < ARRAY_SIZE(c->ip6.dns) &&
- inet_pton(AF_INET6, optarg, &dns6_tmp)) {
+ if (inet_pton(AF_INET6, optarg, &dns6_tmp)) {
dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
continue;
}
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 09/16] conf: Move adding of a nameserver from resolv.conf into subfunction
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (7 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 08/16] conf: Move DNS array bounds checks into add_dns[46] David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 10/16] conf: Correct setting of dns_match address in add_dns6() David Gibson
` (7 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
get_dns() is already quite deeply nested, and future changes I have in
mind will add more complexity. Prepare for this by splitting out the
adding of a single nameserver to the configuration into its own function.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/conf.c b/conf.c
index a651cb91..1b57af71 100644
--- a/conf.c
+++ b/conf.c
@@ -421,6 +421,29 @@ static unsigned add_dns6(struct ctx *c, struct in6_addr *addr, unsigned idx)
return added;
}
+/**
+ * add_dns_resolv() - Possibly add ns from host resolv.conf to configuration
+ * @c: Execution context
+ * @nameserver: Nameserver address string from /etc/resolv.conf
+ * @idx4: Pointer to index of current entry in array of IPv4 resolvers
+ * @idx6: Pointer to index of current entry in array of IPv6 resolvers
+ *
+ * @idx4 or @idx6 may be NULL, in which case resolvers of the corresponding type
+ * are ignored.
+ */
+static void add_dns_resolv(struct ctx *c, const char *nameserver,
+ unsigned *idx4, unsigned *idx6)
+{
+ struct in6_addr ns6;
+ struct in_addr ns4;
+
+ if (idx4 && inet_pton(AF_INET, nameserver, &ns4))
+ *idx4 += add_dns4(c, &ns4, *idx4);
+
+ if (idx6 && inet_pton(AF_INET6, nameserver, &ns6))
+ *idx6 += add_dns6(c, &ns6, *idx6);
+}
+
/**
* get_dns() - Get nameserver addresses from local /etc/resolv.conf
* @c: Execution context
@@ -431,8 +454,6 @@ static void get_dns(struct ctx *c)
unsigned dns4_idx = 0, dns6_idx = 0;
struct fqdn *s = c->dns_search;
struct lineread resolvconf;
- struct in6_addr dns6_tmp;
- struct in_addr dns4_tmp;
ssize_t line_len;
char *line, *end;
const char *p;
@@ -459,11 +480,9 @@ static void get_dns(struct ctx *c)
if (end)
*end = 0;
- if (!dns4_set && inet_pton(AF_INET, p + 1, &dns4_tmp))
- dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
-
- if (!dns6_set && inet_pton(AF_INET6, p + 1, &dns6_tmp))
- dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
+ add_dns_resolv(c, p + 1,
+ dns4_set ? NULL : &dns4_idx,
+ dns6_set ? NULL : &dns6_idx);
} else if (!dnss_set && strstr(line, "search ") == line &&
s == c->dns_search) {
end = strpbrk(line, "\n");
--
@@ -421,6 +421,29 @@ static unsigned add_dns6(struct ctx *c, struct in6_addr *addr, unsigned idx)
return added;
}
+/**
+ * add_dns_resolv() - Possibly add ns from host resolv.conf to configuration
+ * @c: Execution context
+ * @nameserver: Nameserver address string from /etc/resolv.conf
+ * @idx4: Pointer to index of current entry in array of IPv4 resolvers
+ * @idx6: Pointer to index of current entry in array of IPv6 resolvers
+ *
+ * @idx4 or @idx6 may be NULL, in which case resolvers of the corresponding type
+ * are ignored.
+ */
+static void add_dns_resolv(struct ctx *c, const char *nameserver,
+ unsigned *idx4, unsigned *idx6)
+{
+ struct in6_addr ns6;
+ struct in_addr ns4;
+
+ if (idx4 && inet_pton(AF_INET, nameserver, &ns4))
+ *idx4 += add_dns4(c, &ns4, *idx4);
+
+ if (idx6 && inet_pton(AF_INET6, nameserver, &ns6))
+ *idx6 += add_dns6(c, &ns6, *idx6);
+}
+
/**
* get_dns() - Get nameserver addresses from local /etc/resolv.conf
* @c: Execution context
@@ -431,8 +454,6 @@ static void get_dns(struct ctx *c)
unsigned dns4_idx = 0, dns6_idx = 0;
struct fqdn *s = c->dns_search;
struct lineread resolvconf;
- struct in6_addr dns6_tmp;
- struct in_addr dns4_tmp;
ssize_t line_len;
char *line, *end;
const char *p;
@@ -459,11 +480,9 @@ static void get_dns(struct ctx *c)
if (end)
*end = 0;
- if (!dns4_set && inet_pton(AF_INET, p + 1, &dns4_tmp))
- dns4_idx += add_dns4(c, &dns4_tmp, dns4_idx);
-
- if (!dns6_set && inet_pton(AF_INET6, p + 1, &dns6_tmp))
- dns6_idx += add_dns6(c, &dns6_tmp, dns6_idx);
+ add_dns_resolv(c, p + 1,
+ dns4_set ? NULL : &dns4_idx,
+ dns6_set ? NULL : &dns6_idx);
} else if (!dnss_set && strstr(line, "search ") == line &&
s == c->dns_search) {
end = strpbrk(line, "\n");
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 10/16] conf: Correct setting of dns_match address in add_dns6()
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (8 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 09/16] conf: Move adding of a nameserver from resolv.conf into subfunction David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 11/16] conf: Treat --dns addresses as guest visible addresses David Gibson
` (6 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
add_dns6() (but not add_dns4()) has a bug setting dns_match: it sets it to
the given address, rather than the gateway address. This is doubly wrong:
- We've just established the given address is a host loopback address
the guest can't access
- We've just set ip6.dns[] to tell the guest to use the gateway address,
so it won't use the dns_match address we're setting
Correct this to use the gateway address, like IPv4.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/conf.c b/conf.c
index 1b57af71..cb462b09 100644
--- a/conf.c
+++ b/conf.c
@@ -408,7 +408,7 @@ static unsigned add_dns6(struct ctx *c, struct in6_addr *addr, unsigned idx)
added++;
if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
- c->ip6.dns_match = *addr;
+ c->ip6.dns_match = c->ip6.gw;
}
} else {
c->ip6.dns[idx] = *addr;
--
@@ -408,7 +408,7 @@ static unsigned add_dns6(struct ctx *c, struct in6_addr *addr, unsigned idx)
added++;
if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
- c->ip6.dns_match = *addr;
+ c->ip6.dns_match = c->ip6.gw;
}
} else {
c->ip6.dns[idx] = *addr;
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 11/16] conf: Treat --dns addresses as guest visible addresses
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (9 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 10/16] conf: Correct setting of dns_match address in add_dns6() David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 12/16] conf: Remove incorrect initialisation of addr_ll_seen David Gibson
` (5 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
Although it's not 100% explicit in the man page, addresses given to the
--dns option are intended to be addresses as seen by the guest. This
differs from addresses taken from the host's /etc/resolv.conf, which must
be translated to to guest accessible versions in some cases.
Our implementation is currently inconsistent on this: when using
--dns-forward, you must usually also give --dns with the matching address,
which is meaningful only in the guest's address view. However if you give
--dns with a loopback addres, it will be translated like a host view
address.
Move the remapping logic for DNS addresses out of add_dns4() and add_dns6()
into add_dns_resolv() so that it is only applied for host nameserver
addresses, not for nameservers given explicitly with --dns.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 84 ++++++++++++++++++++++++++++-----------------------------
passt.1 | 14 ++++++----
2 files changed, 50 insertions(+), 48 deletions(-)
diff --git a/conf.c b/conf.c
index cb462b09..fa6d90ef 100644
--- a/conf.c
+++ b/conf.c
@@ -361,29 +361,11 @@ bind_all_fail:
static unsigned add_dns4(struct ctx *c, const struct in_addr *addr,
unsigned idx)
{
- unsigned added = 0;
-
if (idx >= ARRAY_SIZE(c->ip4.dns))
return 0;
- /* Guest or container can only access local addresses via redirect */
- if (IN4_IS_ADDR_LOOPBACK(addr)) {
- if (!c->no_map_gw) {
- c->ip4.dns[idx] = c->ip4.gw;
- added++;
-
- if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
- c->ip4.dns_match = c->ip4.gw;
- }
- } else {
- c->ip4.dns[idx] = *addr;
- added++;
- }
-
- if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
- c->ip4.dns_host = *addr;
-
- return added;
+ c->ip4.dns[idx] = *addr;
+ return 1;
}
/**
@@ -394,31 +376,14 @@ static unsigned add_dns4(struct ctx *c, const struct in_addr *addr,
*
* Return: Number of entries added (0 or 1)
*/
-static unsigned add_dns6(struct ctx *c, struct in6_addr *addr, unsigned idx)
+static unsigned add_dns6(struct ctx *c, const struct in6_addr *addr,
+ unsigned idx)
{
- unsigned added = 0;
-
if (idx >= ARRAY_SIZE(c->ip6.dns))
return 0;
- /* Guest or container can only access local addresses via redirect */
- if (IN6_IS_ADDR_LOOPBACK(addr)) {
- if (!c->no_map_gw) {
- c->ip6.dns[idx] = c->ip6.gw;
- added++;
-
- if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
- c->ip6.dns_match = c->ip6.gw;
- }
- } else {
- c->ip6.dns[idx] = *addr;
- added++;
- }
-
- if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
- c->ip6.dns_host = *addr;
-
- return added;
+ c->ip6.dns[idx] = *addr;
+ return 1;
}
/**
@@ -437,11 +402,44 @@ static void add_dns_resolv(struct ctx *c, const char *nameserver,
struct in6_addr ns6;
struct in_addr ns4;
- if (idx4 && inet_pton(AF_INET, nameserver, &ns4))
+ if (idx4 && inet_pton(AF_INET, nameserver, &ns4)) {
+ if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
+ c->ip4.dns_host = ns4;
+
+ /* Guest or container can only access local addresses via
+ * redirect
+ */
+ if (IN4_IS_ADDR_LOOPBACK(&ns4)) {
+ if (c->no_map_gw)
+ return;
+
+ ns4 = c->ip4.gw;
+ if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
+ c->ip4.dns_match = c->ip4.gw;
+ }
+
*idx4 += add_dns4(c, &ns4, *idx4);
+ }
+
+ if (idx6 && inet_pton(AF_INET6, nameserver, &ns6)) {
+ if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
+ c->ip6.dns_host = ns6;
+
+ /* Guest or container can only access local addresses via
+ * redirect
+ */
+ if (IN6_IS_ADDR_LOOPBACK(&ns6)) {
+ if (c->no_map_gw)
+ return;
+
+ ns6 = c->ip6.gw;
+
+ if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
+ c->ip6.dns_match = c->ip6.gw;
+ }
- if (idx6 && inet_pton(AF_INET6, nameserver, &ns6))
*idx6 += add_dns6(c, &ns6, *idx6);
+ }
}
/**
diff --git a/passt.1 b/passt.1
index 3062b719..dca433b6 100644
--- a/passt.1
+++ b/passt.1
@@ -236,11 +236,15 @@ interface will be chosen instead.
.TP
.BR \-D ", " \-\-dns " " \fIaddr
-Use \fIaddr\fR (IPv4 or IPv6) for DHCP, DHCPv6, NDP or DNS forwarding, as
-configured (see options \fB--no-dhcp-dns\fR, \fB--dhcp-dns\fR,
-\fB--dns-forward\fR) instead of reading addresses from \fI/etc/resolv.conf\fR.
-This option can be specified multiple times. Specifying \fB-D none\fR disables
-usage of DNS addresses altogether.
+Instruct the guest (via DHCP, DHVPv6 or NDP) to use \fIaddr\fR (IPv4
+or IPv6) as a nameserver, as configured (see options
+\fB--no-dhcp-dns\fR, \fB--dhcp-dns\fR) instead of reading addresses
+from \fI/etc/resolv.conf\fR. This option can be specified multiple
+times. Specifying \fB-D none\fR disables usage of DNS addresses
+altogether. Unlike addresses from \fI/etc/resolv.conf\fR, \fIaddr\fR
+is given to the guest without remapping. For example \fB--dns
+127.0.0.1\fR will instruct the guest to use itself as nameserver, not
+the host.
.TP
.BR \-\-dns-forward " " \fIaddr
--
@@ -236,11 +236,15 @@ interface will be chosen instead.
.TP
.BR \-D ", " \-\-dns " " \fIaddr
-Use \fIaddr\fR (IPv4 or IPv6) for DHCP, DHCPv6, NDP or DNS forwarding, as
-configured (see options \fB--no-dhcp-dns\fR, \fB--dhcp-dns\fR,
-\fB--dns-forward\fR) instead of reading addresses from \fI/etc/resolv.conf\fR.
-This option can be specified multiple times. Specifying \fB-D none\fR disables
-usage of DNS addresses altogether.
+Instruct the guest (via DHCP, DHVPv6 or NDP) to use \fIaddr\fR (IPv4
+or IPv6) as a nameserver, as configured (see options
+\fB--no-dhcp-dns\fR, \fB--dhcp-dns\fR) instead of reading addresses
+from \fI/etc/resolv.conf\fR. This option can be specified multiple
+times. Specifying \fB-D none\fR disables usage of DNS addresses
+altogether. Unlike addresses from \fI/etc/resolv.conf\fR, \fIaddr\fR
+is given to the guest without remapping. For example \fB--dns
+127.0.0.1\fR will instruct the guest to use itself as nameserver, not
+the host.
.TP
.BR \-\-dns-forward " " \fIaddr
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 12/16] conf: Remove incorrect initialisation of addr_ll_seen
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (10 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 11/16] conf: Treat --dns addresses as guest visible addresses David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 13/16] util: Correct sock_l4() binding for link local addresses David Gibson
` (4 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
Despite the names, addr_ll_seen does not relate to addr_ll the same way
addr_see relates to addr. addr_ll_seen is an observed address from the
guest, whereas addr_ll is *our* link-local address for use on the tap link
when we can't use an external endpoint address. It's used both for
passt provided services (DHCPv6, NDP) and in some cases for connections
from addresses the guest can't access.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/conf.c b/conf.c
index fa6d90ef..09e97744 100644
--- a/conf.c
+++ b/conf.c
@@ -720,7 +720,6 @@ static unsigned int conf_ip6(unsigned int ifi,
}
ip6->addr_seen = ip6->addr;
- ip6->addr_ll_seen = ip6->addr_ll;
if (MAC_IS_ZERO(mac)) {
rc = nl_link_get_mac(nl_sock, ifi, mac);
--
@@ -720,7 +720,6 @@ static unsigned int conf_ip6(unsigned int ifi,
}
ip6->addr_seen = ip6->addr;
- ip6->addr_ll_seen = ip6->addr_ll;
if (MAC_IS_ZERO(mac)) {
rc = nl_link_get_mac(nl_sock, ifi, mac);
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 13/16] util: Correct sock_l4() binding for link local addresses
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (11 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 12/16] conf: Remove incorrect initialisation of addr_ll_seen David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 14/16] treewide: Change misleading 'addr_ll' name David Gibson
` (3 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
When binding an IPv6 socket in sock_l4() we need to supply a scope id if
the address is link-local. We check for this by comparing the given
address to c->ip6.addr_ll. This is correct only by accident: while
c->ip6.addr_ll is typically set to the hsot interface's link local
address, the actually purpose of it is to provide a link local address
for passt's private use on the tap interface.
Instead set the scope id for any link-local address we're binding to.
We're going to need something and this is what makes sense for sockets
on the host. It doesn't make sense for PIF_SPLICE sockets, but those
should always have loopback, not link-local addresses.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
util.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/util.c b/util.c
index 892358b1..9682e3ce 100644
--- a/util.c
+++ b/util.c
@@ -199,8 +199,7 @@ int sock_l4(const struct ctx *c, sa_family_t af, enum epoll_type type,
if (bind_addr) {
addr6.sin6_addr = *(struct in6_addr *)bind_addr;
- if (!memcmp(bind_addr, &c->ip6.addr_ll,
- sizeof(c->ip6.addr_ll)))
+ if (IN6_IS_ADDR_LINKLOCAL(bind_addr))
addr6.sin6_scope_id = c->ifi6;
}
return sock_l4_sa(c, type, &addr6, sizeof(addr6), ifname,
--
@@ -199,8 +199,7 @@ int sock_l4(const struct ctx *c, sa_family_t af, enum epoll_type type,
if (bind_addr) {
addr6.sin6_addr = *(struct in6_addr *)bind_addr;
- if (!memcmp(bind_addr, &c->ip6.addr_ll,
- sizeof(c->ip6.addr_ll)))
+ if (IN6_IS_ADDR_LINKLOCAL(bind_addr))
addr6.sin6_scope_id = c->ifi6;
}
return sock_l4_sa(c, type, &addr6, sizeof(addr6), ifname,
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 14/16] treewide: Change misleading 'addr_ll' name
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (12 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 13/16] util: Correct sock_l4() binding for link local addresses David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 15/16] Clarify which addresses in ip[46]_ctx are meaningful where David Gibson
` (2 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
c->ip6.addr_ll is not like c->ip6.addr. The latter is an address for the
guest, but the former is an address for our use on the tap link. Rename it
accordingly, to 'our_tap_ll'.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 7 ++++---
dhcpv6.c | 2 +-
fwd.c | 2 +-
ndp.c | 2 +-
passt.h | 4 ++--
5 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/conf.c b/conf.c
index 09e97744..f1727ade 100644
--- a/conf.c
+++ b/conf.c
@@ -713,7 +713,7 @@ static unsigned int conf_ip6(unsigned int ifi,
rc = nl_addr_get(nl_sock, ifi, AF_INET6,
IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ? &ip6->addr : NULL,
- &prefix_len, &ip6->addr_ll);
+ &prefix_len, &ip6->our_tap_ll);
if (rc < 0) {
err("Couldn't discover IPv6 address: %s", strerror(-rc));
return 0;
@@ -735,7 +735,7 @@ static unsigned int conf_ip6(unsigned int ifi,
}
if (IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ||
- IN6_IS_ADDR_UNSPECIFIED(&ip6->addr_ll))
+ IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll))
return 0;
return ifi;
@@ -1027,7 +1027,8 @@ static void conf_print(const struct ctx *c)
info(" router: %s",
inet_ntop(AF_INET6, &c->ip6.gw, buf6, sizeof(buf6)));
info(" our link-local: %s",
- inet_ntop(AF_INET6, &c->ip6.addr_ll, buf6, sizeof(buf6)));
+ inet_ntop(AF_INET6, &c->ip6.our_tap_ll,
+ buf6, sizeof(buf6)));
dns6:
for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) {
diff --git a/dhcpv6.c b/dhcpv6.c
index 87b3c3eb..44e954e7 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -456,7 +456,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
src = &c->ip6.gw;
else
- src = &c->ip6.addr_ll;
+ src = &c->ip6.our_tap_ll;
mh = packet_get(p, 0, sizeof(*uh), sizeof(*mh), NULL);
if (!mh)
diff --git a/fwd.c b/fwd.c
index b546bc41..dccc947d 100644
--- a/fwd.c
+++ b/fwd.c
@@ -320,7 +320,7 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto,
if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
tgt->oaddr.a6 = c->ip6.gw;
else
- tgt->oaddr.a6 = c->ip6.addr_ll;
+ tgt->oaddr.a6 = c->ip6.our_tap_ll;
}
if (inany_v4(&tgt->oaddr)) {
diff --git a/ndp.c b/ndp.c
index 9c0fef4a..3a76b00a 100644
--- a/ndp.c
+++ b/ndp.c
@@ -344,7 +344,7 @@ dns_done:
if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
rsaddr = &c->ip6.gw;
else
- rsaddr = &c->ip6.addr_ll;
+ rsaddr = &c->ip6.our_tap_ll;
if (ih->icmp6_type == NS) {
dlen = sizeof(struct ndp_na);
diff --git a/passt.h b/passt.h
index fe3e47d2..5e7e6a04 100644
--- a/passt.h
+++ b/passt.h
@@ -122,7 +122,7 @@ struct ip4_ctx {
/**
* struct ip6_ctx - IPv6 execution context
* @addr: IPv6 address assigned to guest
- * @addr_ll: Link-local IPv6 address on external, routable interface
+ * @our_tap_ll: Link-local IPv6 address for passt's use on tap
* @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
@@ -136,7 +136,7 @@ struct ip4_ctx {
*/
struct ip6_ctx {
struct in6_addr addr;
- struct in6_addr addr_ll;
+ struct in6_addr our_tap_ll;
struct in6_addr addr_seen;
struct in6_addr addr_ll_seen;
struct in6_addr gw;
--
@@ -122,7 +122,7 @@ struct ip4_ctx {
/**
* struct ip6_ctx - IPv6 execution context
* @addr: IPv6 address assigned to guest
- * @addr_ll: Link-local IPv6 address on external, routable interface
+ * @our_tap_ll: Link-local IPv6 address for passt's use on tap
* @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
@@ -136,7 +136,7 @@ struct ip4_ctx {
*/
struct ip6_ctx {
struct in6_addr addr;
- struct in6_addr addr_ll;
+ struct in6_addr our_tap_ll;
struct in6_addr addr_seen;
struct in6_addr addr_ll_seen;
struct in6_addr gw;
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 15/16] Clarify which addresses in ip[46]_ctx are meaningful where
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (13 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 14/16] treewide: Change misleading 'addr_ll' name David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:30 ` [PATCH 16/16] Initialise our_tap_ll to ip6.gw when suitable David Gibson
2024-08-14 5:03 ` [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
Some are guest visible addresses and may not be valid on the host, others
are host visible addresses and may not be valid on the guest.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
passt.h | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/passt.h b/passt.h
index 5e7e6a04..06754bcc 100644
--- a/passt.h
+++ b/passt.h
@@ -104,15 +104,18 @@ enum passt_modes {
* @no_copy_addrs: Don't copy all addresses when configuring namespace
*/
struct ip4_ctx {
+ /* PIF_TAP addresses */
struct in_addr addr;
struct in_addr addr_seen;
int prefix_len;
struct in_addr gw;
struct in_addr dns[MAXNS + 1];
struct in_addr dns_match;
- struct in_addr dns_host;
+ /* PIF_HOST addresses */
+ struct in_addr dns_host;
struct in_addr addr_out;
+
char ifname_out[IFNAMSIZ];
bool no_copy_routes;
@@ -122,12 +125,12 @@ struct ip4_ctx {
/**
* struct ip6_ctx - IPv6 execution context
* @addr: IPv6 address assigned to guest
- * @our_tap_ll: Link-local IPv6 address for passt's use on tap
* @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: DNS addresses for DHCPv6 and NDP, zero-terminated
* @dns_match: Forward DNS query if sent to this address
+ * @our_tap_ll: Link-local IPv6 address for passt's use on tap
* @dns_host: Use this DNS on the host for forwarding
* @addr_out: Optional source address for outbound traffic
* @ifname_out: Optional interface name to bind outbound sockets to
@@ -135,16 +138,19 @@ struct ip4_ctx {
* @no_copy_addrs: Don't copy all addresses when configuring namespace
*/
struct ip6_ctx {
+ /* PIF_TAP addresses */
struct in6_addr addr;
- struct in6_addr our_tap_ll;
struct in6_addr addr_seen;
struct in6_addr addr_ll_seen;
struct in6_addr gw;
struct in6_addr dns[MAXNS + 1];
struct in6_addr dns_match;
- struct in6_addr dns_host;
+ /* PIF_HOST addresses */
+ struct in6_addr our_tap_ll;
+ struct in6_addr dns_host;
struct in6_addr addr_out;
+
char ifname_out[IFNAMSIZ];
bool no_copy_routes;
--
@@ -104,15 +104,18 @@ enum passt_modes {
* @no_copy_addrs: Don't copy all addresses when configuring namespace
*/
struct ip4_ctx {
+ /* PIF_TAP addresses */
struct in_addr addr;
struct in_addr addr_seen;
int prefix_len;
struct in_addr gw;
struct in_addr dns[MAXNS + 1];
struct in_addr dns_match;
- struct in_addr dns_host;
+ /* PIF_HOST addresses */
+ struct in_addr dns_host;
struct in_addr addr_out;
+
char ifname_out[IFNAMSIZ];
bool no_copy_routes;
@@ -122,12 +125,12 @@ struct ip4_ctx {
/**
* struct ip6_ctx - IPv6 execution context
* @addr: IPv6 address assigned to guest
- * @our_tap_ll: Link-local IPv6 address for passt's use on tap
* @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: DNS addresses for DHCPv6 and NDP, zero-terminated
* @dns_match: Forward DNS query if sent to this address
+ * @our_tap_ll: Link-local IPv6 address for passt's use on tap
* @dns_host: Use this DNS on the host for forwarding
* @addr_out: Optional source address for outbound traffic
* @ifname_out: Optional interface name to bind outbound sockets to
@@ -135,16 +138,19 @@ struct ip4_ctx {
* @no_copy_addrs: Don't copy all addresses when configuring namespace
*/
struct ip6_ctx {
+ /* PIF_TAP addresses */
struct in6_addr addr;
- struct in6_addr our_tap_ll;
struct in6_addr addr_seen;
struct in6_addr addr_ll_seen;
struct in6_addr gw;
struct in6_addr dns[MAXNS + 1];
struct in6_addr dns_match;
- struct in6_addr dns_host;
+ /* PIF_HOST addresses */
+ struct in6_addr our_tap_ll;
+ struct in6_addr dns_host;
struct in6_addr addr_out;
+
char ifname_out[IFNAMSIZ];
bool no_copy_routes;
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 16/16] Initialise our_tap_ll to ip6.gw when suitable
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (14 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 15/16] Clarify which addresses in ip[46]_ctx are meaningful where David Gibson
@ 2024-08-14 4:30 ` David Gibson
2024-08-14 4:39 ` David Gibson
2024-08-14 5:03 ` [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
16 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:30 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
In every place we use our_tap_ll, we only use it as a fallback if the
IPv6 gateway address is not link-local. We can avoid that conditional at
use time by doing it at initialisation of our_tap_ll instead.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 3 +++
dhcpv6.c | 5 +----
ndp.c | 5 +----
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/conf.c b/conf.c
index f1727ade..30769474 100644
--- a/conf.c
+++ b/conf.c
@@ -721,6 +721,9 @@ static unsigned int conf_ip6(unsigned int ifi,
ip6->addr_seen = ip6->addr;
+ if (IN6_IS_ADDR_LINKLOCAL(&ip6->gw))
+ ip6->our_tap_ll = ip6->gw;
+
if (MAC_IS_ZERO(mac)) {
rc = nl_link_get_mac(nl_sock, ifi, mac);
if (rc < 0) {
diff --git a/dhcpv6.c b/dhcpv6.c
index 44e954e7..69841abc 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -453,10 +453,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
c->ip6.addr_ll_seen = *saddr;
- if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
- src = &c->ip6.gw;
- else
- src = &c->ip6.our_tap_ll;
+ src = &c->ip6.our_tap_ll;
mh = packet_get(p, 0, sizeof(*uh), sizeof(*mh), NULL);
if (!mh)
diff --git a/ndp.c b/ndp.c
index 3a76b00a..a1ee8349 100644
--- a/ndp.c
+++ b/ndp.c
@@ -341,10 +341,7 @@ dns_done:
else
c->ip6.addr_seen = *saddr;
- if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
- rsaddr = &c->ip6.gw;
- else
- rsaddr = &c->ip6.our_tap_ll;
+ rsaddr = &c->ip6.our_tap_ll;
if (ih->icmp6_type == NS) {
dlen = sizeof(struct ndp_na);
--
@@ -341,10 +341,7 @@ dns_done:
else
c->ip6.addr_seen = *saddr;
- if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
- rsaddr = &c->ip6.gw;
- else
- rsaddr = &c->ip6.our_tap_ll;
+ rsaddr = &c->ip6.our_tap_ll;
if (ih->icmp6_type == NS) {
dlen = sizeof(struct ndp_na);
--
2.46.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 16/16] Initialise our_tap_ll to ip6.gw when suitable
2024-08-14 4:30 ` [PATCH 16/16] Initialise our_tap_ll to ip6.gw when suitable David Gibson
@ 2024-08-14 4:39 ` David Gibson
0 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 4:39 UTC (permalink / raw)
To: passt-dev, Stefano Brivio
[-- Attachment #1: Type: text/plain, Size: 1969 bytes --]
On Wed, Aug 14, 2024 at 02:30:50PM +1000, David Gibson wrote:
> In every place we use our_tap_ll, we only use it as a fallback if the
> IPv6 gateway address is not link-local. We can avoid that conditional at
> use time by doing it at initialisation of our_tap_ll instead.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Ugh, sorry. Please don't apply this one, I missed a case.
> ---
> conf.c | 3 +++
> dhcpv6.c | 5 +----
> ndp.c | 5 +----
> 3 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/conf.c b/conf.c
> index f1727ade..30769474 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -721,6 +721,9 @@ static unsigned int conf_ip6(unsigned int ifi,
>
> ip6->addr_seen = ip6->addr;
>
> + if (IN6_IS_ADDR_LINKLOCAL(&ip6->gw))
> + ip6->our_tap_ll = ip6->gw;
> +
> if (MAC_IS_ZERO(mac)) {
> rc = nl_link_get_mac(nl_sock, ifi, mac);
> if (rc < 0) {
> diff --git a/dhcpv6.c b/dhcpv6.c
> index 44e954e7..69841abc 100644
> --- a/dhcpv6.c
> +++ b/dhcpv6.c
> @@ -453,10 +453,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
>
> c->ip6.addr_ll_seen = *saddr;
>
> - if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
> - src = &c->ip6.gw;
> - else
> - src = &c->ip6.our_tap_ll;
> + src = &c->ip6.our_tap_ll;
>
> mh = packet_get(p, 0, sizeof(*uh), sizeof(*mh), NULL);
> if (!mh)
> diff --git a/ndp.c b/ndp.c
> index 3a76b00a..a1ee8349 100644
> --- a/ndp.c
> +++ b/ndp.c
> @@ -341,10 +341,7 @@ dns_done:
> else
> c->ip6.addr_seen = *saddr;
>
> - if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
> - rsaddr = &c->ip6.gw;
> - else
> - rsaddr = &c->ip6.our_tap_ll;
> + rsaddr = &c->ip6.our_tap_ll;
>
> if (ih->icmp6_type == NS) {
> dlen = sizeof(struct ndp_na);
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 00/16] Cleanups and fixes related to "our" addresses
2024-08-14 4:30 [PATCH 00/16] Cleanups and fixes related to "our" addresses David Gibson
` (15 preceding siblings ...)
2024-08-14 4:30 ` [PATCH 16/16] Initialise our_tap_ll to ip6.gw when suitable David Gibson
@ 2024-08-14 5:03 ` David Gibson
16 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2024-08-14 5:03 UTC (permalink / raw)
To: passt-dev, Stefano Brivio
[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]
On Wed, Aug 14, 2024 at 02:30:34PM +1000, David Gibson wrote:
> There are some places where we have addresses that are "ours" in the
> sense that they're local to passt on at least one interface. But in
> some cases it wasn't clear which addresses those were or how to use
> them. Make a number of renames, cleanups and small fixes related to
> that.
>
> ..and also an assortment of slightly related things that I encountered
> along the way.
>
> Note that 1/16 is an important fix for a bug I introduced in the last
> series I sent. For the rest, apply as many as you're happy with and
> I'll respin what's left as necessary.
Ugh. I've found several more problems in the series (though mostly
just incorrect comments). Review if you want, but don't apply please.
I'll send a new spin soon.
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread