[PATCH 0/7] Improved selection of external interface and IP configuration

public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed

* [PATCH 0/7] Improved selection of external interface and IP configuration
@ 2022-07-22  5:31 David Gibson
  2022-07-22  5:31 ` [PATCH 1/7] Allow different external interfaces for IPv4 and IPv6 connectivity David Gibson
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: David Gibson @ 2022-07-22  5:31 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 1701 bytes --]

The current semantics for selecting an external interface are quite
confusing - depending on details it can pick either the interface
associated with the first default route, or the lowest numbered
interface with a default route, which might not be the same.The logic
for checking the interface in the tests isn't quite identical which
can lead to test failures when there are multiple external routes.

This series fixes that bug and makes a number of follow on clean ups
to the detection / configuration of IP parameters from the host.

David Gibson (7):
  Allow different external interfaces for IPv4 and IPv6 connectivity
  Separately locate external interfaces for IPv4 and IPv6
  Initialize host side MAC when in IPv6 only mode
  Move passt mac_guest init to be more symmetric with pasta
  Clarify semantics of c->v4 and c->v6 variables
  Separate IPv4 and IPv6 configuration
  Make substructures for IPv4 and IPv6 specific context information

 arp.c                 |   2 +-
 conf.c                | 326 ++++++++++++++++++++++--------------------
 dhcp.c                |  22 +--
 dhcpv6.c              |  18 +--
 ndp.c                 |  16 +--
 netlink.c             |  79 +---------
 netlink.h             |   2 +-
 passt.c               |   6 +-
 passt.h               |  78 +++++-----
 pasta.c               |  14 +-
 tap.c                 |  32 +++--
 tcp.c                 |  56 ++++----
 test/dhcp/passt       |   3 +-
 test/dhcp/pasta       |   3 +-
 test/ndp/passt        |   4 +-
 test/two_guests/basic |   3 +-
 udp.c                 |  70 ++++-----
 util.c                |   6 +-
 util.h                |   6 -
 19 files changed, 357 insertions(+), 389 deletions(-)

-- 
2.37.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/7] Allow different external interfaces for IPv4 and IPv6 connectivity
  2022-07-22  5:31 [PATCH 0/7] Improved selection of external interface and IP configuration David Gibson
@ 2022-07-22  5:31 ` David Gibson
  2022-07-22  5:31 ` [PATCH 2/7] Separately locate external interfaces for IPv4 and IPv6 David Gibson
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: David Gibson @ 2022-07-22  5:31 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 6554 bytes --]

It's quite plausible for a host to have both IPv4 and IPv6 connectivity,
but only via different interfaces.  For example, this will happen in the
case that IPv6 connectivity is via a tunnel (e.g. 6in4 or 6rd).  It would
also happen in the case that IPv4 access is via a tunnel on an otherwise
IPv6 only local network, which is a setup that might become more common in
the post IPv4 address exhaustion world.

In turns out there's no real need for passt/pasta to get its IPv4 and IPv6
connectivity via the same interface, so we can handle this situation fairly
easily.  Change the core to allow eparate external interfaces for IPv4 and
IPv6.  We don't actually set these separately for now.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c  | 38 +++++++++++++++++++++-----------------
 passt.h |  6 ++++--
 tcp.c   |  2 +-
 util.c  |  2 +-
 4 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/conf.c b/conf.c
index cddc769..943526d 100644
--- a/conf.c
+++ b/conf.c
@@ -630,17 +630,17 @@ static void conf_ip(struct ctx *c)
 		v4 = v6		= IP_VERSION_PROBE;
 	}
 
-	if (!c->ifi)
-		c->ifi = nl_get_ext_if(&v4, &v6);
+	if (!c->ifi4 && !c->ifi6)
+		c->ifi4 = c->ifi6 = nl_get_ext_if(&v4, &v6);
 
 	if (v4 != IP_VERSION_DISABLED) {
 		if (!c->gw4)
-			nl_route(0, c->ifi, AF_INET, &c->gw4);
+			nl_route(0, c->ifi4, AF_INET, &c->gw4);
 
 		if (!c->addr4) {
 			int mask_len = 0;
 
-			nl_addr(0, c->ifi, AF_INET, &c->addr4, &mask_len, NULL);
+			nl_addr(0, c->ifi4, AF_INET, &c->addr4, &mask_len, NULL);
 			c->mask4 = htonl(0xffffffff << (32 - mask_len));
 		}
 
@@ -658,7 +658,7 @@ static void conf_ip(struct ctx *c)
 		memcpy(&c->addr4_seen, &c->addr4, sizeof(c->addr4_seen));
 
 		if (!memcmp(c->mac, MAC_ZERO, ETH_ALEN))
-			nl_link(0, c->ifi, c->mac, 0, 0);
+			nl_link(0, c->ifi4, c->mac, 0, 0);
 	}
 
 	if (c->mode == MODE_PASST)
@@ -668,9 +668,9 @@ static void conf_ip(struct ctx *c)
 		int prefix_len = 0;
 
 		if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6))
-			nl_route(0, c->ifi, AF_INET6, &c->gw6);
+			nl_route(0, c->ifi6, AF_INET6, &c->gw6);
 
-		nl_addr(0, c->ifi, AF_INET6,
+		nl_addr(0, c->ifi6, AF_INET6,
 			IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ? &c->addr6 : NULL,
 			&prefix_len, &c->addr6_ll);
 
@@ -883,12 +883,12 @@ static void conf_print(const struct ctx *c)
 	char buf4[INET_ADDRSTRLEN], ifn[IFNAMSIZ];
 	int i;
 
-	if (c->mode == MODE_PASTA) {
-		info("Outbound interface: %s, namespace interface: %s",
-		     if_indextoname(c->ifi, ifn), c->pasta_ifn);
-	} else {
-		info("Outbound interface: %s", if_indextoname(c->ifi, ifn));
-	}
+	if (c->ifi4)
+		info("Outbound interface (IPv4): %s", if_indextoname(c->ifi4, ifn));
+	if (c->ifi6)
+		info("Outbound interface (IPv6): %s", if_indextoname(c->ifi6, ifn));
+	if (c->mode == MODE_PASTA)
+		info("Namespace interface: %s", c->pasta_ifn);
 
 	if (c->v4) {
 		info("ARP:");
@@ -1395,12 +1395,12 @@ void conf(struct ctx *c, int argc, char **argv)
 			usage(argv[0]);
 			break;
 		case 'i':
-			if (c->ifi) {
+			if (c->ifi4 || c->ifi6) {
 				err("Redundant interface: %s", optarg);
 				usage(argv[0]);
 			}
 
-			if (!(c->ifi = if_nametoindex(optarg))) {
+			if (!(c->ifi4 = c->ifi6 = if_nametoindex(optarg))) {
 				err("Invalid interface name %s: %s", optarg,
 				    strerror(errno));
 				usage(argv[0]);
@@ -1559,8 +1559,12 @@ void conf(struct ctx *c, int argc, char **argv)
 
 	get_dns(c);
 
-	if (!*c->pasta_ifn)
-		if_indextoname(c->ifi, c->pasta_ifn);
+	if (!*c->pasta_ifn) {
+		if (c->ifi4)
+			if_indextoname(c->ifi4, c->pasta_ifn);
+		else
+			if_indextoname(c->ifi6, c->pasta_ifn);
+	}
 
 	c->tcp.ns_detect_ports   = c->udp.ns_detect_ports   = 0;
 	c->tcp.init_detect_ports = c->udp.init_detect_ports = 0;
diff --git a/passt.h b/passt.h
index e541341..23145c6 100644
--- a/passt.h
+++ b/passt.h
@@ -122,6 +122,7 @@ enum passt_modes {
  * @mac:		Host MAC address
  * @mac_guest:		MAC address of guest or namespace, seen or configured
  * @v4:			Enable IPv4 transport
+ * @ifi4:		Index of routable interface for IPv4
  * @addr4:		IPv4 address for external, routable interface
  * @addr4_seen:		Latest IPv4 address seen as source from tap
  * @mask4:		IPv4 netmask, network order
@@ -130,6 +131,7 @@ enum passt_modes {
  * @dns4_fwd:		Address forwarded (UDP) to first IPv4 DNS, network order
  * @dns_search:		DNS search list
  * @v6:			Enable IPv6 transport
+ * @ifi6:		Index of routable interface for IPv6
  * @addr6:		IPv6 address for external, routable interface
  * @addr6_ll:		Link-local IPv6 address on external, routable interface
  * @addr6_seen:		Latest IPv6 global/site address seen as source from tap
@@ -137,7 +139,6 @@ enum passt_modes {
  * @gw6:		Default IPv6 gateway
  * @dns6:		IPv6 DNS addresses, zero-terminated
  * @dns6_fwd:		Address forwarded (UDP) to first IPv6 DNS, network order
- * @ifi:		Index of routable interface
  * @pasta_ifn:		Name of namespace interface for pasta
  * @pasta_ifn:		Index of namespace interface for pasta
  * @pasta_conf_ns:	Configure namespace interface after creating it
@@ -193,6 +194,7 @@ struct ctx {
 	unsigned char mac_guest[ETH_ALEN];
 
 	int v4;
+	unsigned int ifi4;
 	uint32_t addr4;
 	uint32_t addr4_seen;
 	uint32_t mask4;
@@ -203,6 +205,7 @@ struct ctx {
 	struct fqdn dns_search[MAXDNSRCH];
 
 	int v6;
+	unsigned int ifi6;
 	struct in6_addr addr6;
 	struct in6_addr addr6_ll;
 	struct in6_addr addr6_seen;
@@ -211,7 +214,6 @@ struct ctx {
 	struct in6_addr dns6[MAXNS + 1];
 	struct in6_addr dns6_fwd;
 
-	unsigned int ifi;
 	char pasta_ifn[IF_NAMESIZE];
 	unsigned int pasta_ifi;
 	int pasta_conf_ns;
diff --git a/tcp.c b/tcp.c
index 5b84110..3b27f11 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2207,7 +2207,7 @@ static void tcp_conn_from_tap(struct ctx *c, int af, const void *addr,
 		struct sockaddr_in6 addr6_ll = {
 			.sin6_family = AF_INET6,
 			.sin6_addr = c->addr6_ll,
-			.sin6_scope_id = c->ifi,
+			.sin6_scope_id = c->ifi6,
 		};
 		if (bind(s, (struct sockaddr *)&addr6_ll, sizeof(addr6_ll))) {
 			close(s);
diff --git a/util.c b/util.c
index 2eea0ef..f4ec102 100644
--- a/util.c
+++ b/util.c
@@ -280,7 +280,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
 
 			if (!memcmp(bind_addr, &c->addr6_ll,
 			    sizeof(c->addr6_ll)))
-				addr6.sin6_scope_id = c->ifi;
+				addr6.sin6_scope_id = c->ifi6;
 		} else {
 			addr6.sin6_addr = in6addr_any;
 		}
-- 
@@ -280,7 +280,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
 
 			if (!memcmp(bind_addr, &c->addr6_ll,
 			    sizeof(c->addr6_ll)))
-				addr6.sin6_scope_id = c->ifi;
+				addr6.sin6_scope_id = c->ifi6;
 		} else {
 			addr6.sin6_addr = in6addr_any;
 		}
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/7] Separately locate external interfaces for IPv4 and IPv6
  2022-07-22  5:31 [PATCH 0/7] Improved selection of external interface and IP configuration David Gibson
  2022-07-22  5:31 ` [PATCH 1/7] Allow different external interfaces for IPv4 and IPv6 connectivity David Gibson
@ 2022-07-22  5:31 ` David Gibson
  2022-08-01 10:23   ` Stefano Brivio
  2022-07-22  5:31 ` [PATCH 3/7] Initialize host side MAC when in IPv6 only mode David Gibson
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: David Gibson @ 2022-07-22  5:31 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 9947 bytes --]

Now that the back end allows passt/pasta to use different external
interfaces for IPv4 and IPv6, use that to do the right thing in the case
that the host has IPv4 and IPv6 connectivity via different interfaces.
If the user hasn't explicitly chosen an interface, separately search for
a suitable external interface for each protocol.

As a bonus, this substantially simplifies the external interface probe.  It
also eliminates a subtle confusing case where in some circumstances we
would pick the first interface in interface index order, and sometimes in
order of routes returned from netlink.  On some network configurations that
could cause tests to fail, because the logic in the tests was subtly
different (it always used route order).

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c                | 19 +++++++++--
 netlink.c             | 79 ++++---------------------------------------
 netlink.h             |  2 +-
 test/dhcp/passt       |  3 +-
 test/dhcp/pasta       |  3 +-
 test/ndp/passt        |  4 +--
 test/two_guests/basic |  3 +-
 7 files changed, 33 insertions(+), 80 deletions(-)

diff --git a/conf.c b/conf.c
index 943526d..cc08de3 100644
--- a/conf.c
+++ b/conf.c
@@ -630,8 +630,23 @@ static void conf_ip(struct ctx *c)
 		v4 = v6		= IP_VERSION_PROBE;
 	}
 
-	if (!c->ifi4 && !c->ifi6)
-		c->ifi4 = c->ifi6 = nl_get_ext_if(&v4, &v6);
+	if (v4 != IP_VERSION_DISABLED) {
+		if (!c->ifi4)
+			c->ifi4 = nl_get_ext_if(AF_INET);
+		if (!c->ifi4) {
+			warn("No external routable interface for IPv4");
+			v4 = IP_VERSION_DISABLED;
+		}
+	}
+
+	if (v6 != IP_VERSION_DISABLED) {
+		if (!c->ifi6)
+			c->ifi6 = nl_get_ext_if(AF_INET6);
+		if (!c->ifi6) {
+			warn("No external routable interface for IPv6");
+			v6 = IP_VERSION_DISABLED;
+		}
+	}
 
 	if (v4 != IP_VERSION_DISABLED) {
 		if (!c->gw4)
diff --git a/netlink.c b/netlink.c
index 66a95e4..8ad6a0c 100644
--- a/netlink.c
+++ b/netlink.c
@@ -126,13 +126,13 @@ static int nl_req(int ns, char *buf, const void *req, ssize_t len)
 }
 
 /**
- * nl_get_ext_if() - Get interface index supporting IP versions being probed
- * @v4:		Probe IPv4 support, set to ENABLED or DISABLED on return
- * @v6:		Probe IPv4 support, set to ENABLED or DISABLED on return
+ * nl_get_ext_if() - Get interface index supporting IP version being probed
+ * @af:	Address family (AF_INET or AF_INET6) to look for connectivity
+ *      for.
  *
  * Return: interface index, 0 if not found
  */
-unsigned int nl_get_ext_if(int *v4, int *v6)
+unsigned int nl_get_ext_if(sa_family_t af)
 {
 	struct { struct nlmsghdr nlh; struct rtmsg rtm; } req = {
 		.nlh.nlmsg_type	 = RTM_GETROUTE,
@@ -143,32 +143,14 @@ unsigned int nl_get_ext_if(int *v4, int *v6)
 		.rtm.rtm_table	 = RT_TABLE_MAIN,
 		.rtm.rtm_scope	 = RT_SCOPE_UNIVERSE,
 		.rtm.rtm_type	 = RTN_UNICAST,
+		.rtm.rtm_family	 = af,
 	};
-	unsigned int i, first_v4 = 0, first_v6 = 0;
-	uint8_t has_v4[PAGE_SIZE * 8 / 8] = { 0 }; /* See __dev_alloc_name() */
-	uint8_t has_v6[PAGE_SIZE * 8 / 8] = { 0 }; /* in kernel */
 	struct nlmsghdr *nh;
 	struct rtattr *rta;
 	struct rtmsg *rtm;
 	char buf[BUFSIZ];
-	long *word, tmp;
-	uint8_t *vmap;
 	ssize_t n;
 	size_t na;
-	int *v;
-
-	if (*v4 == IP_VERSION_PROBE) {
-		v = v4;
-		req.rtm.rtm_family = AF_INET;
-		vmap = has_v4;
-	} else if (*v6 == IP_VERSION_PROBE) {
-v6:
-		v = v6;
-		req.rtm.rtm_family = AF_INET6;
-		vmap = has_v6;
-	} else {
-		return 0;
-	}
 
 	if ((n = nl_req(0, buf, &req, sizeof(req))) < 0)
 		return 0;
@@ -178,7 +160,7 @@ v6:
 	for ( ; NLMSG_OK(nh, n); nh = NLMSG_NEXT(nh, n)) {
 		rtm = (struct rtmsg *)NLMSG_DATA(nh);
 
-		if (rtm->rtm_dst_len || rtm->rtm_family != req.rtm.rtm_family)
+		if (rtm->rtm_dst_len || rtm->rtm_family != af)
 			continue;
 
 		for (rta = RTM_RTA(rtm), na = RTM_PAYLOAD(nh); RTA_OK(rta, na);
@@ -190,57 +172,10 @@ v6:
 
 			ifi = *(unsigned int *)RTA_DATA(rta);
 
-			if (*v4 == IP_VERSION_DISABLED ||
-			    *v6 == IP_VERSION_DISABLED) {
-				*v = IP_VERSION_ENABLED;
-				return ifi;
-			}
-
-			if (v == v4 && !first_v4)
-				first_v4 = ifi;
-
-			if (v == v6 && !first_v6)
-				first_v6 = ifi;
-
-			bitmap_set(vmap, ifi);
+			return ifi;
 		}
 	}
 
-	if (v == v4 && *v6 == IP_VERSION_PROBE) {
-		req.nlh.nlmsg_seq = nl_seq++;
-		goto v6;
-	}
-
-	word = (long *)has_v4;
-	for (i = 0; i < ARRAY_SIZE(has_v4) / sizeof(long); i++, word++) {
-		tmp = *word;
-		while ((n = ffsl(tmp))) {
-			int ifi = i * sizeof(long) * 8 + n - 1;
-
-			if (!first_v4)
-				first_v4 = ifi;
-
-			tmp &= ~(1UL << (n - 1));
-			if (bitmap_isset(has_v6, ifi)) {
-				*v4 = *v6 = IP_VERSION_ENABLED;
-				return ifi;
-			}
-		}
-	}
-
-	if (first_v4) {
-		*v4 = IP_VERSION_ENABLED;
-		*v6 = IP_VERSION_DISABLED;
-		return first_v4;
-	}
-
-	if (first_v6) {
-		*v4 = IP_VERSION_DISABLED;
-		*v6 = IP_VERSION_ENABLED;
-		return first_v6;
-	}
-
-	err("No external routable interface for any IP protocol");
 	return 0;
 }
 
diff --git a/netlink.h b/netlink.h
index 261904e..5ce5037 100644
--- a/netlink.h
+++ b/netlink.h
@@ -7,7 +7,7 @@
 #define NETLINK_H
 
 int nl_sock_init(const struct ctx *c);
-unsigned int nl_get_ext_if(int *v4, int *v6);
+unsigned int nl_get_ext_if(sa_family_t af);
 void nl_route(int ns, unsigned int ifi, sa_family_t af, void *gw);
 void nl_addr(int ns, unsigned int ifi, sa_family_t af,
 	     void *addr, int *prefix_len, void *addr_l);
diff --git a/test/dhcp/passt b/test/dhcp/passt
index f45227a..11e0eb3 100644
--- a/test/dhcp/passt
+++ b/test/dhcp/passt
@@ -17,6 +17,7 @@ htools	ip jq sed tr head
 test	Interface name
 gout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
 hout	HOST_IFNAME ip -j -4 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
+hout	HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
 check	[ -n "__IFNAME__" ]
 
 test	DHCP: address
@@ -49,7 +50,7 @@ check	[ "__SEARCH__" = "__HOST_SEARCH__" ]
 test	DHCPv6: address
 guest	/sbin/dhclient -6 __IFNAME__
 gout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.prefixlen == 128).local'
-hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME__").addr_info[] | select(.scope == "global").local'
+hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global").local'
 check	[ "__ADDR6__" = "__HOST_ADDR6__" ]
 
 test	DHCPv6: route
diff --git a/test/dhcp/pasta b/test/dhcp/pasta
index 65b1d42..076ec8d 100644
--- a/test/dhcp/pasta
+++ b/test/dhcp/pasta
@@ -35,8 +35,9 @@ check	[ __MTU__ = 65520 ]
 
 test	DHCPv6: address
 ns	/sbin/dhclient -6 --no-pid __IFNAME__
+hout	HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
 nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.prefixlen == 128).local'
-hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global").local'
+hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global").local'
 check	[ __ADDR6__ = __HOST_ADDR6__ ]
 
 test	DHCPv6: route
diff --git a/test/ndp/passt b/test/ndp/passt
index d6b4c40..8ef15e7 100644
--- a/test/ndp/passt
+++ b/test/ndp/passt
@@ -17,13 +17,13 @@ htools	ip jq sipcalc grep cut
 test	Interface name
 gout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
 guest	ip link set dev __IFNAME__ up && sleep 2
-hout	HOST_IFNAME ip -j -4 route show|jq -rM '.[] | select(.dst == "default").dev'
+hout	HOST_IFNAME6 ip -j -6 route show|jq -rM '.[] | select(.dst == "default").dev'
 check	[ -n "__IFNAME__" ]
 
 test	SLAAC: prefix
 gout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .prefixlen == 64).local'
 gout	PREFIX6 sipcalc __ADDR6__/64 | grep prefix | cut -d' ' -f4
-hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME__").addr_info[] | select(.scope == "global").local'
+hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global").local'
 hout	HOST_PREFIX6 sipcalc __HOST_ADDR6__/64 | grep prefix | cut -d' ' -f4
 check	[ "__PREFIX6__" = "__HOST_PREFIX6__" ]
 
diff --git a/test/two_guests/basic b/test/two_guests/basic
index f7c016d..e226178 100644
--- a/test/two_guests/basic
+++ b/test/two_guests/basic
@@ -19,6 +19,7 @@ test	Interface names
 g1out	IFNAME1 ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
 g2out	IFNAME2 ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
 hout	HOST_IFNAME ip -j -4 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
+hout	HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
 check	[ -n "__IFNAME1__" ]
 check	[ -n "__IFNAME2__" ]
 
@@ -40,7 +41,7 @@ guest1	/sbin/dhclient -6 __IFNAME1__
 guest2	/sbin/dhclient -6 __IFNAME2__
 g1out	ADDR1_6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME1__").addr_info[] | select(.prefixlen == 128).local'
 g2out	ADDR2_6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME2__").addr_info[] | select(.prefixlen == 128).local'
-hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME__").addr_info[] | select(.scope == "global").local'
+hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global").local'
 check	[ "__ADDR1_6__" = "__HOST_ADDR6__" ]
 check	[ "__ADDR2_6__" = "__HOST_ADDR6__" ]
 
-- 
@@ -19,6 +19,7 @@ test	Interface names
 g1out	IFNAME1 ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
 g2out	IFNAME2 ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
 hout	HOST_IFNAME ip -j -4 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
+hout	HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
 check	[ -n "__IFNAME1__" ]
 check	[ -n "__IFNAME2__" ]
 
@@ -40,7 +41,7 @@ guest1	/sbin/dhclient -6 __IFNAME1__
 guest2	/sbin/dhclient -6 __IFNAME2__
 g1out	ADDR1_6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME1__").addr_info[] | select(.prefixlen == 128).local'
 g2out	ADDR2_6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME2__").addr_info[] | select(.prefixlen == 128).local'
-hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME__").addr_info[] | select(.scope == "global").local'
+hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global").local'
 check	[ "__ADDR1_6__" = "__HOST_ADDR6__" ]
 check	[ "__ADDR2_6__" = "__HOST_ADDR6__" ]
 
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/7] Initialize host side MAC when in IPv6 only mode
  2022-07-22  5:31 [PATCH 0/7] Improved selection of external interface and IP configuration David Gibson
  2022-07-22  5:31 ` [PATCH 1/7] Allow different external interfaces for IPv4 and IPv6 connectivity David Gibson
  2022-07-22  5:31 ` [PATCH 2/7] Separately locate external interfaces for IPv4 and IPv6 David Gibson
@ 2022-07-22  5:31 ` David Gibson
  2022-07-22  5:31 ` [PATCH 4/7] Move passt mac_guest init to be more symmetric with pasta David Gibson
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: David Gibson @ 2022-07-22  5:31 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 2437 bytes --]

When sending packets to the guest we need a source MAC address, which we
currently take from the host side interface we're using (though it's
basically arbitrary).  However if not given on the command line this MAC
is initialized in an IPv4 specific path, and will end up as
00:00:00:00:00:00 when running "passt 6".  The MAC address is also used
for IPv6 packets, though.

Interestingly, we largely seem to get away with using an all-zero MAC, but
it's probably not a good idea.  Make the IPv6 path pick the MAC address
from its interface if the IPv4 path hasn't already done so.

While we're there, use the existing MAC_IS_ZERO macro to make the code a
little clearer.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/conf.c b/conf.c
index cc08de3..bc8851f 100644
--- a/conf.c
+++ b/conf.c
@@ -672,7 +672,7 @@ static void conf_ip(struct ctx *c)
 
 		memcpy(&c->addr4_seen, &c->addr4, sizeof(c->addr4_seen));
 
-		if (!memcmp(c->mac, MAC_ZERO, ETH_ALEN))
+		if (MAC_IS_ZERO(c->mac))
 			nl_link(0, c->ifi4, c->mac, 0, 0);
 	}
 
@@ -691,17 +691,20 @@ static void conf_ip(struct ctx *c)
 
 		memcpy(&c->addr6_seen, &c->addr6, sizeof(c->addr6));
 		memcpy(&c->addr6_ll_seen, &c->addr6_ll, sizeof(c->addr6_ll));
+
+		if (MAC_IS_ZERO(c->mac))
+			nl_link(0, c->ifi6, c->mac, 0, 0);
 	}
 
-	if (!c->gw4 || !c->addr4 ||
-	    !memcmp(c->mac, ((uint8_t [ETH_ALEN]){ 0 }), ETH_ALEN))
+	if (!c->gw4 || !c->addr4 || MAC_IS_ZERO(c->mac))
 		v4 = IP_VERSION_DISABLED;
 	else
 		v4 = IP_VERSION_ENABLED;
 
 	if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6) ||
 	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ||
-	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6_ll))
+	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6_ll) ||
+	    MAC_IS_ZERO(c->mac))
 		v6 = IP_VERSION_DISABLED;
 	else
 		v6 = IP_VERSION_ENABLED;
@@ -905,12 +908,12 @@ static void conf_print(const struct ctx *c)
 	if (c->mode == MODE_PASTA)
 		info("Namespace interface: %s", c->pasta_ifn);
 
-	if (c->v4) {
-		info("ARP:");
-		info("    address: %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("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]);
 
+	if (c->v4) {
 		if (!c->no_dhcp) {
 			info("DHCP:");
 			info("    assign: %s",
-- 
@@ -672,7 +672,7 @@ static void conf_ip(struct ctx *c)
 
 		memcpy(&c->addr4_seen, &c->addr4, sizeof(c->addr4_seen));
 
-		if (!memcmp(c->mac, MAC_ZERO, ETH_ALEN))
+		if (MAC_IS_ZERO(c->mac))
 			nl_link(0, c->ifi4, c->mac, 0, 0);
 	}
 
@@ -691,17 +691,20 @@ static void conf_ip(struct ctx *c)
 
 		memcpy(&c->addr6_seen, &c->addr6, sizeof(c->addr6));
 		memcpy(&c->addr6_ll_seen, &c->addr6_ll, sizeof(c->addr6_ll));
+
+		if (MAC_IS_ZERO(c->mac))
+			nl_link(0, c->ifi6, c->mac, 0, 0);
 	}
 
-	if (!c->gw4 || !c->addr4 ||
-	    !memcmp(c->mac, ((uint8_t [ETH_ALEN]){ 0 }), ETH_ALEN))
+	if (!c->gw4 || !c->addr4 || MAC_IS_ZERO(c->mac))
 		v4 = IP_VERSION_DISABLED;
 	else
 		v4 = IP_VERSION_ENABLED;
 
 	if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6) ||
 	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ||
-	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6_ll))
+	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6_ll) ||
+	    MAC_IS_ZERO(c->mac))
 		v6 = IP_VERSION_DISABLED;
 	else
 		v6 = IP_VERSION_ENABLED;
@@ -905,12 +908,12 @@ static void conf_print(const struct ctx *c)
 	if (c->mode == MODE_PASTA)
 		info("Namespace interface: %s", c->pasta_ifn);
 
-	if (c->v4) {
-		info("ARP:");
-		info("    address: %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("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]);
 
+	if (c->v4) {
 		if (!c->no_dhcp) {
 			info("DHCP:");
 			info("    assign: %s",
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/7] Move passt mac_guest init to be more symmetric with pasta
  2022-07-22  5:31 [PATCH 0/7] Improved selection of external interface and IP configuration David Gibson
                   ` (2 preceding siblings ...)
  2022-07-22  5:31 ` [PATCH 3/7] Initialize host side MAC when in IPv6 only mode David Gibson
@ 2022-07-22  5:31 ` David Gibson
  2022-07-22  5:31 ` [PATCH 5/7] Clarify semantics of c->v4 and c->v6 variables David Gibson
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: David Gibson @ 2022-07-22  5:31 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 1592 bytes --]

In pasta mode, the guest's MAC address is set up in pasta_ns_cobf() called
from tap_sock_tun_init().  If we have a guest MAC configured with
--ns-mac-addr, this will set the given MAC on the kernel tuntap device, or
if we haven't configured one it will update our record of the guest MAC to
the kernel assigned one from the device.

For passt, we don't initially know the guest's MAC until we receive packets
from it, so we have to initially use a broadcast address.  This is - oddly
- set up in an entirely different place, in conf_ip() conditional on the
mode.

Move it to the logically matching place for passt - tap_sock_unix_init().

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c | 3 ---
 tap.c  | 6 ++++++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/conf.c b/conf.c
index bc8851f..f5b761f 100644
--- a/conf.c
+++ b/conf.c
@@ -676,9 +676,6 @@ static void conf_ip(struct ctx *c)
 			nl_link(0, c->ifi4, c->mac, 0, 0);
 	}
 
-	if (c->mode == MODE_PASST)
-		memset(&c->mac_guest, 0xff, sizeof(c->mac_guest));
-
 	if (v6 != IP_VERSION_DISABLED) {
 		int prefix_len = 0;
 
diff --git a/tap.c b/tap.c
index a3c711c..43a7372 100644
--- a/tap.c
+++ b/tap.c
@@ -794,6 +794,12 @@ static void tap_sock_unix_init(struct ctx *c)
 		exit(EXIT_FAILURE);
 	}
 
+	/* In passt mode, we don't know the guest's MAC until it sends
+	 * us packets.  Use the broadcast address so our first packets
+	 * will reach it.
+	 */
+	memset(&c->mac_guest, 0xff, sizeof(c->mac_guest));
+
 	for (i = 1; i < UNIX_SOCK_MAX; i++) {
 		char *path = addr.sun_path;
 
-- 
@@ -794,6 +794,12 @@ static void tap_sock_unix_init(struct ctx *c)
 		exit(EXIT_FAILURE);
 	}
 
+	/* In passt mode, we don't know the guest's MAC until it sends
+	 * us packets.  Use the broadcast address so our first packets
+	 * will reach it.
+	 */
+	memset(&c->mac_guest, 0xff, sizeof(c->mac_guest));
+
 	for (i = 1; i < UNIX_SOCK_MAX; i++) {
 		char *path = addr.sun_path;
 
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/7] Clarify semantics of c->v4 and c->v6 variables
  2022-07-22  5:31 [PATCH 0/7] Improved selection of external interface and IP configuration David Gibson
                   ` (3 preceding siblings ...)
  2022-07-22  5:31 ` [PATCH 4/7] Move passt mac_guest init to be more symmetric with pasta David Gibson
@ 2022-07-22  5:31 ` David Gibson
  2022-08-01 10:24   ` Stefano Brivio
  2022-07-22  5:31 ` [PATCH 6/7] Separate IPv4 and IPv6 configuration David Gibson
  2022-07-22  5:31 ` [PATCH 7/7] Make substructures for IPv4 and IPv6 specific context information David Gibson
  6 siblings, 1 reply; 11+ messages in thread
From: David Gibson @ 2022-07-22  5:31 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 5239 bytes --]

The v4 and v6 fields of the context structure can be confusing, because
they change meaning part way through the code:  Before conf_ip(), they are
booleans which indicate whether the -4 or -6 options have been given.
After conf_ip() they are DISABLED|ENABLED|PROBE enums which indicate
whether the IP version is available (which means both that it was allowed
on the command line and we were able to configure it).  The PROBE variant
of the enum is only used locally within conf_ip() and since recent changes
there it no longer has a real purpose different from ENABLED.

Simplify this all by making the context fields always just a boolean
indicating the availability of the IP version.  They both default to 1, but
can be set to 0 by either command line options or configuration failures.
We use some local variables in conf() for tracking the state of the command
line options on their own.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c | 59 ++++++++++++++++++++--------------------------------------
 util.h |  6 ------
 2 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/conf.c b/conf.c
index f5b761f..63f0f3d 100644
--- a/conf.c
+++ b/conf.c
@@ -26,6 +26,7 @@
 #include <pwd.h>
 #include <stdlib.h>
 #include <stdint.h>
+#include <stdbool.h>
 #include <unistd.h>
 #include <syslog.h>
 #include <time.h>
@@ -615,40 +616,25 @@ static int conf_ns_opt(struct ctx *c,
  */
 static void conf_ip(struct ctx *c)
 {
-	int v4, v6;
-
 	if (c->v4) {
-		c->v4		= IP_VERSION_ENABLED;
-		v4		= IP_VERSION_PROBE;
-		v6 = c->v6	= IP_VERSION_DISABLED;
-	} else if (c->v6) {
-		c->v6		= IP_VERSION_ENABLED;
-		v6		= IP_VERSION_PROBE;
-		v4 = c->v4	= IP_VERSION_DISABLED;
-	} else {
-		c->v4 = c->v6	= IP_VERSION_ENABLED;
-		v4 = v6		= IP_VERSION_PROBE;
-	}
-
-	if (v4 != IP_VERSION_DISABLED) {
 		if (!c->ifi4)
 			c->ifi4 = nl_get_ext_if(AF_INET);
 		if (!c->ifi4) {
 			warn("No external routable interface for IPv4");
-			v4 = IP_VERSION_DISABLED;
+			c->v4 = 0;
 		}
 	}
 
-	if (v6 != IP_VERSION_DISABLED) {
+	if (c->v6) {
 		if (!c->ifi6)
 			c->ifi6 = nl_get_ext_if(AF_INET6);
 		if (!c->ifi6) {
 			warn("No external routable interface for IPv6");
-			v6 = IP_VERSION_DISABLED;
+			c->v6 = 0;
 		}
 	}
 
-	if (v4 != IP_VERSION_DISABLED) {
+	if (c->v4) {
 		if (!c->gw4)
 			nl_route(0, c->ifi4, AF_INET, &c->gw4);
 
@@ -676,7 +662,7 @@ static void conf_ip(struct ctx *c)
 			nl_link(0, c->ifi4, c->mac, 0, 0);
 	}
 
-	if (v6 != IP_VERSION_DISABLED) {
+	if (c->v6) {
 		int prefix_len = 0;
 
 		if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6))
@@ -694,25 +680,18 @@ static void conf_ip(struct ctx *c)
 	}
 
 	if (!c->gw4 || !c->addr4 || MAC_IS_ZERO(c->mac))
-		v4 = IP_VERSION_DISABLED;
-	else
-		v4 = IP_VERSION_ENABLED;
+		c->v4 = 0;
 
 	if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6) ||
 	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ||
 	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6_ll) ||
 	    MAC_IS_ZERO(c->mac))
-		v6 = IP_VERSION_DISABLED;
-	else
-		v6 = IP_VERSION_ENABLED;
+		c->v6 = 0;
 
-	if ((v4 == IP_VERSION_DISABLED) && (v6 == IP_VERSION_DISABLED)) {
+	if (!c->v4 && !c->v6) {
 		err("External interface not usable");
 		exit(EXIT_FAILURE);
 	}
-
-	c->v4 = v4;
-	c->v6 = v6;
 }
 
 /**
@@ -1054,8 +1033,8 @@ void conf(struct ctx *c, int argc, char **argv)
 		{"no-ndp",	no_argument,		&c->no_ndp,	1 },
 		{"no-ra",	no_argument,		&c->no_ra,	1 },
 		{"no-map-gw",	no_argument,		&c->no_map_gw,	1 },
-		{"ipv4-only",	no_argument,		&c->v4,		'4' },
-		{"ipv6-only",	no_argument,		&c->v6,		'6' },
+		{"ipv4-only",	no_argument,		NULL,		'4' },
+		{"ipv6-only",	no_argument,		NULL,		'6' },
 		{"tcp-ports",	required_argument,	NULL,		't' },
 		{"udp-ports",	required_argument,	NULL,		'u' },
 		{"tcp-ns",	required_argument,	NULL,		'T' },
@@ -1083,6 +1062,7 @@ void conf(struct ctx *c, int argc, char **argv)
 	struct in6_addr *dns6 = c->dns6;
 	int name, ret, mask, b, i;
 	uint32_t *dns4 = c->dns4;
+	bool v4_only = false, v6_only = false;
 
 	if (c->mode == MODE_PASTA)
 		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
@@ -1474,10 +1454,10 @@ void conf(struct ctx *c, int argc, char **argv)
 			usage(argv[0]);
 			break;
 		case '4':
-			c->v4 = 1;
+			v4_only = true;
 			break;
 		case '6':
-			c->v6 = 1;
+			v6_only = true;
 			break;
 		case 't':
 		case 'u':
@@ -1508,11 +1488,6 @@ void conf(struct ctx *c, int argc, char **argv)
 		usage(argv[0]);
 	}
 
-	if (c->v4 && c->v6) {
-		err("Options ipv4-only and ipv6-only are mutually exclusive");
-		usage(argv[0]);
-	}
-
 	if (c->pasta_conf_ns)
 		c->no_ra = 1;
 
@@ -1524,6 +1499,12 @@ void conf(struct ctx *c, int argc, char **argv)
 		exit(EXIT_FAILURE);
 	}
 
+	if (v4_only && v6_only) {
+		err("Options ipv4-only and ipv6-only are mutually exclusive");
+		usage(argv[0]);
+	}
+	c->v4 = !v6_only;
+	c->v6 = !v4_only;
 	conf_ip(c);
 
 	/* Now we can process port configuration options */
diff --git a/util.h b/util.h
index ae4cc54..8297bec 100644
--- a/util.h
+++ b/util.h
@@ -74,12 +74,6 @@ void trace_init(int enable);
 #define V6		1
 #define IP_VERSIONS	2
 
-enum {
-	IP_VERSION_DISABLED = 0,
-	IP_VERSION_ENABLED,
-	IP_VERSION_PROBE,
-};
-
 #define ARRAY_SIZE(a)		((int)(sizeof(a) / sizeof((a)[0])))
 
 #define IN_INTERVAL(a, b, x)	((x) >= (a) && (x) <= (b))
-- 
@@ -74,12 +74,6 @@ void trace_init(int enable);
 #define V6		1
 #define IP_VERSIONS	2
 
-enum {
-	IP_VERSION_DISABLED = 0,
-	IP_VERSION_ENABLED,
-	IP_VERSION_PROBE,
-};
-
 #define ARRAY_SIZE(a)		((int)(sizeof(a) / sizeof((a)[0])))
 
 #define IN_INTERVAL(a, b, x)	((x) >= (a) && (x) <= (b))
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/7] Separate IPv4 and IPv6 configuration
  2022-07-22  5:31 [PATCH 0/7] Improved selection of external interface and IP configuration David Gibson
                   ` (4 preceding siblings ...)
  2022-07-22  5:31 ` [PATCH 5/7] Clarify semantics of c->v4 and c->v6 variables David Gibson
@ 2022-07-22  5:31 ` David Gibson
  2022-08-01 10:24   ` Stefano Brivio
  2022-07-22  5:31 ` [PATCH 7/7] Make substructures for IPv4 and IPv6 specific context information David Gibson
  6 siblings, 1 reply; 11+ messages in thread
From: David Gibson @ 2022-07-22  5:31 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 13520 bytes --]

After recent changes, conf_ip() now has essentially entirely disjoint paths
for IPv4 and IPv6 configuration.  So, it's cleaner to split them out into
different functions conf_ip4() and conf_ip6().

Splitting these out also lets us make the interface a bit nicer, having
them return success or failure directly, rather than manipulating c->v4
and c->v6 to indicate success/failure of the two versions.

Since these functions may also initialize the interface index for each
protocol, it turns out we can then drop c->v4 and c->v6 entirely, replacing
tests on those with tests on whether c->ifi4 or c->ifi6 is non-zero (since
a 0 interface index is never valid).

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c  | 150 ++++++++++++++++++++++++++++++--------------------------
 passt.c |   4 +-
 passt.h |   8 +--
 pasta.c |   4 +-
 tap.c   |   4 +-
 tcp.c   |  20 ++++----
 udp.c   |   8 +--
 7 files changed, 103 insertions(+), 95 deletions(-)

diff --git a/conf.c b/conf.c
index 63f0f3d..2a4ef23 100644
--- a/conf.c
+++ b/conf.c
@@ -411,8 +411,8 @@ static void get_dns(struct ctx *c)
 	int line_len;
 	char *line, *p, *end;
 
-	dns4_set = !c->v4  || !!*dns4;
-	dns6_set = !c->v6  || !IN6_IS_ADDR_UNSPECIFIED(dns6);
+	dns4_set = !c->ifi4  || !!*dns4;
+	dns6_set = !c->ifi6  || !IN6_IS_ADDR_UNSPECIFIED(dns6);
 	dnss_set = !!*s->n || c->no_dns_search;
 	dns_set = (dns4_set && dns6_set) || c->no_dns;
 
@@ -611,87 +611,93 @@ static int conf_ns_opt(struct ctx *c,
 }
 
 /**
- * conf_ip() - Verify or detect IPv4/IPv6 support, get relevant addresses
+ * conf_ip4() - Verify or detect IPv4 support, get relevant addresses
  * @c:		Execution context
+ * @ifi:	Host interface to attempt (0 to determine one)
+ *
+ * Return:	Interface index for IPv4, or 0 on failure.
  */
-static void conf_ip(struct ctx *c)
+static unsigned int conf_ip4(struct ctx *c, unsigned int ifi)
 {
-	if (c->v4) {
-		if (!c->ifi4)
-			c->ifi4 = nl_get_ext_if(AF_INET);
-		if (!c->ifi4) {
-			warn("No external routable interface for IPv4");
-			c->v4 = 0;
-		}
-	}
+	if (!ifi)
+		ifi = nl_get_ext_if(AF_INET);
 
-	if (c->v6) {
-		if (!c->ifi6)
-			c->ifi6 = nl_get_ext_if(AF_INET6);
-		if (!c->ifi6) {
-			warn("No external routable interface for IPv6");
-			c->v6 = 0;
-		}
+	if (!ifi) {
+		warn("No external routable interface for IPv4");
+		return 0;
 	}
 
-	if (c->v4) {
-		if (!c->gw4)
-			nl_route(0, c->ifi4, AF_INET, &c->gw4);
+	if (!c->gw4)
+		nl_route(0, ifi, AF_INET, &c->gw4);
 
-		if (!c->addr4) {
-			int mask_len = 0;
+	if (!c->addr4) {
+		int mask_len = 0;
 
-			nl_addr(0, c->ifi4, AF_INET, &c->addr4, &mask_len, NULL);
-			c->mask4 = htonl(0xffffffff << (32 - mask_len));
-		}
+		nl_addr(0, ifi, AF_INET, &c->addr4, &mask_len, NULL);
+		c->mask4 = htonl(0xffffffff << (32 - mask_len));
+	}
 
-		if (!c->mask4) {
-			if (IN_CLASSA(ntohl(c->addr4)))
-				c->mask4 = htonl(IN_CLASSA_NET);
-			else if (IN_CLASSB(ntohl(c->addr4)))
-				c->mask4 = htonl(IN_CLASSB_NET);
-			else if (IN_CLASSC(ntohl(c->addr4)))
-				c->mask4 = htonl(IN_CLASSC_NET);
-			else
-				c->mask4 = 0xffffffff;
-		}
+	if (!c->mask4) {
+		if (IN_CLASSA(ntohl(c->addr4)))
+			c->mask4 = htonl(IN_CLASSA_NET);
+		else if (IN_CLASSB(ntohl(c->addr4)))
+			c->mask4 = htonl(IN_CLASSB_NET);
+		else if (IN_CLASSC(ntohl(c->addr4)))
+			c->mask4 = htonl(IN_CLASSC_NET);
+		else
+			c->mask4 = 0xffffffff;
+	}
 
-		memcpy(&c->addr4_seen, &c->addr4, sizeof(c->addr4_seen));
+	memcpy(&c->addr4_seen, &c->addr4, sizeof(c->addr4_seen));
 
-		if (MAC_IS_ZERO(c->mac))
-			nl_link(0, c->ifi4, c->mac, 0, 0);
-	}
+	if (MAC_IS_ZERO(c->mac))
+		nl_link(0, ifi, c->mac, 0, 0);
 
-	if (c->v6) {
-		int prefix_len = 0;
+	if (!c->gw4 || !c->addr4 || MAC_IS_ZERO(c->mac))
+		return 0;
 
-		if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6))
-			nl_route(0, c->ifi6, AF_INET6, &c->gw6);
+	return ifi;
+}
 
-		nl_addr(0, c->ifi6, AF_INET6,
-			IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ? &c->addr6 : NULL,
-			&prefix_len, &c->addr6_ll);
+/**
+ * conf_ip6() - Verify or detect IPv6 support, get relevant addresses
+ * @c:		Execution context
+ * @ifi:	Host interface to attempt (0 to determine one)
+ *
+ * Return:	Interface index for IPv6, or 0 on failure.
+ */
+static unsigned int conf_ip6(struct ctx *c, unsigned int ifi)
+{
+	int prefix_len = 0;
 
-		memcpy(&c->addr6_seen, &c->addr6, sizeof(c->addr6));
-		memcpy(&c->addr6_ll_seen, &c->addr6_ll, sizeof(c->addr6_ll));
+	if (!ifi)
+		ifi = nl_get_ext_if(AF_INET6);
 
-		if (MAC_IS_ZERO(c->mac))
-			nl_link(0, c->ifi6, c->mac, 0, 0);
+	if (!ifi) {
+		warn("No external routable interface for IPv6");
+		return 0;
 	}
 
-	if (!c->gw4 || !c->addr4 || MAC_IS_ZERO(c->mac))
-		c->v4 = 0;
+	if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6))
+		nl_route(0, ifi, AF_INET6, &c->gw6);
+
+	nl_addr(0, ifi, AF_INET6,
+		IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ? &c->addr6 : NULL,
+		&prefix_len, &c->addr6_ll);
+
+	memcpy(&c->addr6_seen, &c->addr6, sizeof(c->addr6));
+	memcpy(&c->addr6_ll_seen, &c->addr6_ll, sizeof(c->addr6_ll));
+
+	if (MAC_IS_ZERO(c->mac))
+		nl_link(0, ifi, c->mac, 0, 0);
 
 	if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6) ||
 	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ||
 	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6_ll) ||
 	    MAC_IS_ZERO(c->mac))
-		c->v6 = 0;
+		return 0;
 
-	if (!c->v4 && !c->v6) {
-		err("External interface not usable");
-		exit(EXIT_FAILURE);
-	}
+	return ifi;
 }
 
 /**
@@ -889,7 +895,7 @@ static void conf_print(const struct ctx *c)
 	     c->mac[0], c->mac[1], c->mac[2],
 	     c->mac[3], c->mac[4], c->mac[5]);
 
-	if (c->v4) {
+	if (c->ifi4) {
 		if (!c->no_dhcp) {
 			info("DHCP:");
 			info("    assign: %s",
@@ -914,7 +920,7 @@ static void conf_print(const struct ctx *c)
 		}
 	}
 
-	if (c->v6) {
+	if (c->ifi6) {
 		char buf6[INET6_ADDRSTRLEN];
 
 		if (!c->no_ndp && !c->no_dhcpv6)
@@ -1063,6 +1069,7 @@ void conf(struct ctx *c, int argc, char **argv)
 	int name, ret, mask, b, i;
 	uint32_t *dns4 = c->dns4;
 	bool v4_only = false, v6_only = false;
+	unsigned int ifi = 0;
 
 	if (c->mode == MODE_PASTA)
 		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
@@ -1390,12 +1397,12 @@ void conf(struct ctx *c, int argc, char **argv)
 			usage(argv[0]);
 			break;
 		case 'i':
-			if (c->ifi4 || c->ifi6) {
+			if (ifi) {
 				err("Redundant interface: %s", optarg);
 				usage(argv[0]);
 			}
 
-			if (!(c->ifi4 = c->ifi6 = if_nametoindex(optarg))) {
+			if (!(ifi = if_nametoindex(optarg))) {
 				err("Invalid interface name %s: %s", optarg,
 				    strerror(errno));
 				usage(argv[0]);
@@ -1503,10 +1510,15 @@ void conf(struct ctx *c, int argc, char **argv)
 		err("Options ipv4-only and ipv6-only are mutually exclusive");
 		usage(argv[0]);
 	}
-	c->v4 = !v6_only;
-	c->v6 = !v4_only;
-	conf_ip(c);
-
+	if (!v6_only)
+		c->ifi4 = conf_ip4(c, ifi);
+	if (!v4_only)
+		c->ifi6 = conf_ip6(c, ifi);
+	if (!c->ifi4 && !c->ifi6) {
+		err("External interface not usable");
+		exit(EXIT_FAILURE);
+	}
+	
 	/* Now we can process port configuration options */
 	optind = 1;
 	do {
@@ -1542,10 +1554,10 @@ void conf(struct ctx *c, int argc, char **argv)
 		}
 	} while (name != -1);
 
-	if (!c->v4)
+	if (!c->ifi4)
 		c->no_dhcp = 1;
 
-	if (!c->v6) {
+	if (!c->ifi6) {
 		c->no_ndp = 1;
 		c->no_dhcpv6 = 1;
 	}
diff --git a/passt.c b/passt.c
index a8d94b4..18c3512 100644
--- a/passt.c
+++ b/passt.c
@@ -365,10 +365,10 @@ int main(int argc, char **argv)
 
 	proto_update_l2_buf(c.mac_guest, c.mac, &c.addr4);
 
-	if (c.v4 && !c.no_dhcp)
+	if (c.ifi4 && !c.no_dhcp)
 		dhcp_init();
 
-	if (c.v6 && !c.no_dhcpv6)
+	if (c.ifi6 && !c.no_dhcpv6)
 		dhcpv6_init(&c);
 
 	if (c.debug)
diff --git a/passt.h b/passt.h
index 23145c6..a8d5992 100644
--- a/passt.h
+++ b/passt.h
@@ -121,8 +121,7 @@ enum passt_modes {
  * @fd_tap:		File descriptor for AF_UNIX socket or tuntap device
  * @mac:		Host MAC address
  * @mac_guest:		MAC address of guest or namespace, seen or configured
- * @v4:			Enable IPv4 transport
- * @ifi4:		Index of routable interface for IPv4
+ * @ifi4:		Index of routable interface for IPv4, 0 if IPv4 disabled
  * @addr4:		IPv4 address for external, routable interface
  * @addr4_seen:		Latest IPv4 address seen as source from tap
  * @mask4:		IPv4 netmask, network order
@@ -130,8 +129,7 @@ enum passt_modes {
  * @dns4:		IPv4 DNS addresses, zero-terminated, network order
  * @dns4_fwd:		Address forwarded (UDP) to first IPv4 DNS, network order
  * @dns_search:		DNS search list
- * @v6:			Enable IPv6 transport
- * @ifi6:		Index of routable interface for IPv6
+ * @ifi6:		Index of routable interface for IPv6, 0 if IPv6 disabled
  * @addr6:		IPv6 address for external, routable interface
  * @addr6_ll:		Link-local IPv6 address on external, routable interface
  * @addr6_seen:		Latest IPv6 global/site address seen as source from tap
@@ -193,7 +191,6 @@ struct ctx {
 	unsigned char mac[ETH_ALEN];
 	unsigned char mac_guest[ETH_ALEN];
 
-	int v4;
 	unsigned int ifi4;
 	uint32_t addr4;
 	uint32_t addr4_seen;
@@ -204,7 +201,6 @@ struct ctx {
 
 	struct fqdn dns_search[MAXDNSRCH];
 
-	int v6;
 	unsigned int ifi6;
 	struct in6_addr addr6;
 	struct in6_addr addr6_ll;
diff --git a/pasta.c b/pasta.c
index cd37d16..2d7b5a1 100644
--- a/pasta.c
+++ b/pasta.c
@@ -195,14 +195,14 @@ void pasta_ns_conf(struct ctx *c)
 
 		nl_link(1, c->pasta_ifi, c->mac_guest, 1, c->mtu);
 
-		if (c->v4) {
+		if (c->ifi4) {
 			prefix_len = __builtin_popcount(c->mask4);
 			nl_addr(1, c->pasta_ifi, AF_INET, &c->addr4,
 				&prefix_len, NULL);
 			nl_route(1, c->pasta_ifi, AF_INET, &c->gw4);
 		}
 
-		if (c->v6) {
+		if (c->ifi6) {
 			prefix_len = 64;
 			nl_addr(1, c->pasta_ifi, AF_INET6, &c->addr6,
 				&prefix_len, NULL);
diff --git a/tap.c b/tap.c
index 43a7372..8d552e9 100644
--- a/tap.c
+++ b/tap.c
@@ -318,7 +318,7 @@ static int tap4_handler(struct ctx *c, const struct pool *in,
 	unsigned int i, j, seq_count;
 	struct tap4_l4_t *seq;
 
-	if (!c->v4 || !in->count)
+	if (!c->ifi4 || !in->count)
 		return in->count;
 
 	i = 0;
@@ -471,7 +471,7 @@ static int tap6_handler(struct ctx *c, const struct pool *in,
 	unsigned int i, j, seq_count = 0;
 	struct tap6_l4_t *seq;
 
-	if (!c->v6 || !in->count)
+	if (!c->ifi6 || !in->count)
 		return in->count;
 
 	i = 0;
diff --git a/tcp.c b/tcp.c
index 3b27f11..3382dfd 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3252,7 +3252,7 @@ static int tcp_sock_refill(void *arg)
 		p6 = init_sock_pool6;
 	}
 
-	for (i = 0; a->c->v4 && i < TCP_SOCK_POOL_SIZE; i++, p4++) {
+	for (i = 0; a->c->ifi4 && i < TCP_SOCK_POOL_SIZE; i++, p4++) {
 		if (*p4 >= 0)
 			break;
 
@@ -3267,7 +3267,7 @@ static int tcp_sock_refill(void *arg)
 			tcp_sock_set_bufsize(a->c, *p4);
 	}
 
-	for (i = 0; a->c->v6 && i < TCP_SOCK_POOL_SIZE; i++, p6++) {
+	for (i = 0; a->c->ifi6 && i < TCP_SOCK_POOL_SIZE; i++, p6++) {
 		if (*p6 >= 0)
 			break;
 
@@ -3327,10 +3327,10 @@ int tcp_init(struct ctx *c)
 	for (i = 0; i < ARRAY_SIZE(tcp_l2_mh); i++)
 		tcp_l2_mh[i] = (struct mmsghdr) { .msg_hdr.msg_iovlen = 1 };
 
-	if (c->v4)
+	if (c->ifi4)
 		tcp_sock4_iov_init();
 
-	if (c->v6)
+	if (c->ifi6)
 		tcp_sock6_iov_init();
 
 	memset(init_sock_pool4,		0xff,	sizeof(init_sock_pool4));
@@ -3431,8 +3431,8 @@ static int tcp_port_rebind(void *arg)
 			if (bitmap_isset(a->c->tcp.port_to_tap, port))
 				continue;
 
-			if ((a->c->v4 && tcp_sock_ns[port][V4] == -1) ||
-			    (a->c->v6 && tcp_sock_ns[port][V6] == -1))
+			if ((a->c->ifi4 && tcp_sock_ns[port][V4] == -1) ||
+			    (a->c->ifi6 && tcp_sock_ns[port][V6] == -1))
 				tcp_sock_init(a->c, 1, AF_UNSPEC, NULL, port);
 		}
 	} else {
@@ -3464,8 +3464,8 @@ static int tcp_port_rebind(void *arg)
 			if (bitmap_isset(a->c->tcp.port_to_init, port))
 				continue;
 
-			if ((a->c->v4 && tcp_sock_init_ext[port][V4] == -1) ||
-			    (a->c->v6 && tcp_sock_init_ext[port][V6] == -1))
+			if ((a->c->ifi4 && tcp_sock_init_ext[port][V4] == -1) ||
+			    (a->c->ifi6 && tcp_sock_init_ext[port][V6] == -1))
 				tcp_sock_init(a->c, 0, AF_UNSPEC, NULL, port);
 		}
 	}
@@ -3512,8 +3512,8 @@ void tcp_timer(struct ctx *c, const struct timespec *ts)
 	tcp_sock_refill(&refill_arg);
 	if (c->mode == MODE_PASTA) {
 		refill_arg.ns = 1;
-		if ((c->v4 && ns_sock_pool4[TCP_SOCK_POOL_TSH] < 0) ||
-		    (c->v6 && ns_sock_pool6[TCP_SOCK_POOL_TSH] < 0))
+		if ((c->ifi4 && ns_sock_pool4[TCP_SOCK_POOL_TSH] < 0) ||
+		    (c->ifi6 && ns_sock_pool6[TCP_SOCK_POOL_TSH] < 0))
 			NS_CALL(tcp_sock_refill, &refill_arg);
 
 		tcp_splice_timer(c);
diff --git a/udp.c b/udp.c
index 98e3eaa..856429f 100644
--- a/udp.c
+++ b/udp.c
@@ -1266,10 +1266,10 @@ static void udp_splice_iov_init(void)
  */
 int udp_init(const struct ctx *c)
 {
-	if (c->v4)
+	if (c->ifi4)
 		udp_sock4_iov_init();
 
-	if (c->v6)
+	if (c->ifi6)
 		udp_sock6_iov_init();
 
 	if (c->mode == MODE_PASTA) {
@@ -1341,7 +1341,7 @@ void udp_timer(struct ctx *c, const struct timespec *ts)
 	unsigned int i;
 	long *word, tmp;
 
-	if (!c->v4)
+	if (!c->ifi4)
 		v6 = 1;
 v6:
 	for (t = 0; t < UDP_ACT_TYPE_MAX; t++) {
@@ -1356,7 +1356,7 @@ v6:
 		}
 	}
 
-	if (!v6 && c->v6) {
+	if (!v6 && c->ifi6) {
 		v6 = 1;
 		goto v6;
 	}
-- 
@@ -1266,10 +1266,10 @@ static void udp_splice_iov_init(void)
  */
 int udp_init(const struct ctx *c)
 {
-	if (c->v4)
+	if (c->ifi4)
 		udp_sock4_iov_init();
 
-	if (c->v6)
+	if (c->ifi6)
 		udp_sock6_iov_init();
 
 	if (c->mode == MODE_PASTA) {
@@ -1341,7 +1341,7 @@ void udp_timer(struct ctx *c, const struct timespec *ts)
 	unsigned int i;
 	long *word, tmp;
 
-	if (!c->v4)
+	if (!c->ifi4)
 		v6 = 1;
 v6:
 	for (t = 0; t < UDP_ACT_TYPE_MAX; t++) {
@@ -1356,7 +1356,7 @@ v6:
 		}
 	}
 
-	if (!v6 && c->v6) {
+	if (!v6 && c->ifi6) {
 		v6 = 1;
 		goto v6;
 	}
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/7] Make substructures for IPv4 and IPv6 specific context information
  2022-07-22  5:31 [PATCH 0/7] Improved selection of external interface and IP configuration David Gibson
                   ` (5 preceding siblings ...)
  2022-07-22  5:31 ` [PATCH 6/7] Separate IPv4 and IPv6 configuration David Gibson
@ 2022-07-22  5:31 ` David Gibson
  6 siblings, 0 replies; 11+ messages in thread
From: David Gibson @ 2022-07-22  5:31 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 36325 bytes --]

The context structure contains a batch of fields specific to IPv4 and to
IPv6 connectivity.  Split those out into a sub-structure.

This allows the conf_ip4() and conf_ip6() functions, which take the
entire context but touch very little of it, to be given more specific
parameters, making it clearer what it affects without stepping through the
code.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 arp.c    |   2 +-
 conf.c   | 184 ++++++++++++++++++++++++++++---------------------------
 dhcp.c   |  22 +++----
 dhcpv6.c |  18 +++---
 ndp.c    |  16 ++---
 passt.c  |   2 +-
 passt.h  |  68 ++++++++++++--------
 pasta.c  |  10 +--
 tap.c    |  22 +++----
 tcp.c    |  34 +++++-----
 udp.c    |  62 +++++++++----------
 util.c   |   4 +-
 12 files changed, 232 insertions(+), 212 deletions(-)

diff --git a/arp.c b/arp.c
index e8f21b5..0ad97af 100644
--- a/arp.c
+++ b/arp.c
@@ -66,7 +66,7 @@ int arp(const struct ctx *c, const struct pool *p)
 		return 1;
 
 	/* Don't resolve our own address, either. */
-	if (!memcmp(am->tip, &c->addr4, sizeof(am->tip)))
+	if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip)))
 		return 1;
 
 	ah->ar_op = htons(ARPOP_REPLY);
diff --git a/conf.c b/conf.c
index 2a4ef23..f46b195 100644
--- a/conf.c
+++ b/conf.c
@@ -404,9 +404,9 @@ overlap:
 static void get_dns(struct ctx *c)
 {
 	int dns4_set, dns6_set, dnss_set, dns_set, fd;
-	struct in6_addr *dns6 = &c->dns6[0];
+	struct in6_addr *dns6 = &c->ip6.dns[0];
 	struct fqdn *s = c->dns_search;
-	uint32_t *dns4 = &c->dns4[0];
+	uint32_t *dns4 = &c->ip4.dns[0];
 	struct lineread resolvconf;
 	int line_len;
 	char *line, *p, *end;
@@ -434,7 +434,7 @@ static void get_dns(struct ctx *c)
 				*end = 0;
 
 			if (!dns4_set &&
-			    dns4 - &c->dns4[0] < ARRAY_SIZE(c->dns4) - 1 &&
+			    dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1 &&
 			    inet_pton(AF_INET, p + 1, dns4)) {
 				/* We can only access local addresses via the gw redirect */
 				if (ntohl(*dns4) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET) {
@@ -442,14 +442,14 @@ static void get_dns(struct ctx *c)
 						*dns4 = 0;
 						continue;
 					}
-					*dns4 = c->gw4;
+					*dns4 = c->ip4.gw;
 				}
 				dns4++;
 				*dns4 = 0;
 			}
 
 			if (!dns6_set &&
-			    dns6 - &c->dns6[0] < ARRAY_SIZE(c->dns6) - 1 &&
+			    dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) - 1 &&
 			    inet_pton(AF_INET6, p + 1, dns6)) {
 				/* We can only access local addresses via the gw redirect */
 				if (IN6_IS_ADDR_LOOPBACK(dns6)) {
@@ -457,7 +457,7 @@ static void get_dns(struct ctx *c)
 						memset(dns6, 0, sizeof(*dns6));
 						continue;
 					}
-					memcpy(dns6, &c->gw6, sizeof(*dns6));
+					memcpy(dns6, &c->ip6.gw, sizeof(*dns6));
 				}
 				dns6++;
 				memset(dns6, 0, sizeof(*dns6));
@@ -485,7 +485,7 @@ static void get_dns(struct ctx *c)
 	close(fd);
 
 out:
-	if (!dns_set && dns4 == c->dns4 && dns6 == c->dns6)
+	if (!dns_set && dns4 == c->ip4.dns && dns6 == c->ip6.dns)
 		warn("Couldn't get any nameserver address");
 }
 
@@ -612,12 +612,14 @@ static int conf_ns_opt(struct ctx *c,
 
 /**
  * conf_ip4() - Verify or detect IPv4 support, get relevant addresses
- * @c:		Execution context
  * @ifi:	Host interface to attempt (0 to determine one)
+ * @ip4:	IPv4 context (will be written)
+ * @mac:	MAC address to use (written if unset)
  *
  * Return:	Interface index for IPv4, or 0 on failure.
  */
-static unsigned int conf_ip4(struct ctx *c, unsigned int ifi)
+static unsigned int conf_ip4(unsigned int ifi,
+			     struct ip4_ctx *ip4, unsigned char *mac)
 {
 	if (!ifi)
 		ifi = nl_get_ext_if(AF_INET);
@@ -627,33 +629,33 @@ static unsigned int conf_ip4(struct ctx *c, unsigned int ifi)
 		return 0;
 	}
 
-	if (!c->gw4)
-		nl_route(0, ifi, AF_INET, &c->gw4);
+	if (!ip4->gw)
+		nl_route(0, ifi, AF_INET, &ip4->gw);
 
-	if (!c->addr4) {
+	if (!ip4->addr) {
 		int mask_len = 0;
 
-		nl_addr(0, ifi, AF_INET, &c->addr4, &mask_len, NULL);
-		c->mask4 = htonl(0xffffffff << (32 - mask_len));
+		nl_addr(0, ifi, AF_INET, &ip4->addr, &mask_len, NULL);
+		ip4->mask = htonl(0xffffffff << (32 - mask_len));
 	}
 
-	if (!c->mask4) {
-		if (IN_CLASSA(ntohl(c->addr4)))
-			c->mask4 = htonl(IN_CLASSA_NET);
-		else if (IN_CLASSB(ntohl(c->addr4)))
-			c->mask4 = htonl(IN_CLASSB_NET);
-		else if (IN_CLASSC(ntohl(c->addr4)))
-			c->mask4 = htonl(IN_CLASSC_NET);
+	if (!ip4->mask) {
+		if (IN_CLASSA(ntohl(ip4->addr)))
+			ip4->mask = htonl(IN_CLASSA_NET);
+		else if (IN_CLASSB(ntohl(ip4->addr)))
+			ip4->mask = htonl(IN_CLASSB_NET);
+		else if (IN_CLASSC(ntohl(ip4->addr)))
+			ip4->mask = htonl(IN_CLASSC_NET);
 		else
-			c->mask4 = 0xffffffff;
+			ip4->mask = 0xffffffff;
 	}
 
-	memcpy(&c->addr4_seen, &c->addr4, sizeof(c->addr4_seen));
+	memcpy(&ip4->addr_seen, &ip4->addr, sizeof(ip4->addr_seen));
 
-	if (MAC_IS_ZERO(c->mac))
-		nl_link(0, ifi, c->mac, 0, 0);
+	if (MAC_IS_ZERO(mac))
+		nl_link(0, ifi, mac, 0, 0);
 
-	if (!c->gw4 || !c->addr4 || MAC_IS_ZERO(c->mac))
+	if (!ip4->gw || !ip4->addr || MAC_IS_ZERO(mac))
 		return 0;
 
 	return ifi;
@@ -661,12 +663,14 @@ static unsigned int conf_ip4(struct ctx *c, unsigned int ifi)
 
 /**
  * conf_ip6() - Verify or detect IPv6 support, get relevant addresses
- * @c:		Execution context
  * @ifi:	Host interface to attempt (0 to determine one)
+ * @ip6:	IPv6 context (will be written)
+ * @mac:	MAC address to use (written if unset)
  *
  * Return:	Interface index for IPv6, or 0 on failure.
  */
-static unsigned int conf_ip6(struct ctx *c, unsigned int ifi)
+static unsigned int conf_ip6(unsigned int ifi,
+			     struct ip6_ctx *ip6, unsigned char *mac)
 {
 	int prefix_len = 0;
 
@@ -678,23 +682,23 @@ static unsigned int conf_ip6(struct ctx *c, unsigned int ifi)
 		return 0;
 	}
 
-	if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6))
-		nl_route(0, ifi, AF_INET6, &c->gw6);
+	if (IN6_IS_ADDR_UNSPECIFIED(&ip6->gw))
+		nl_route(0, ifi, AF_INET6, &ip6->gw);
 
 	nl_addr(0, ifi, AF_INET6,
-		IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ? &c->addr6 : NULL,
-		&prefix_len, &c->addr6_ll);
+		IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ? &ip6->addr : NULL,
+		&prefix_len, &ip6->addr_ll);
 
-	memcpy(&c->addr6_seen, &c->addr6, sizeof(c->addr6));
-	memcpy(&c->addr6_ll_seen, &c->addr6_ll, sizeof(c->addr6_ll));
+	memcpy(&ip6->addr_seen, &ip6->addr, sizeof(ip6->addr));
+	memcpy(&ip6->addr_ll_seen, &ip6->addr_ll, sizeof(ip6->addr_ll));
 
-	if (MAC_IS_ZERO(c->mac))
-		nl_link(0, ifi, c->mac, 0, 0);
+	if (MAC_IS_ZERO(mac))
+		nl_link(0, ifi, mac, 0, 0);
 
-	if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6) ||
-	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ||
-	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6_ll) ||
-	    MAC_IS_ZERO(c->mac))
+	if (IN6_IS_ADDR_UNSPECIFIED(&ip6->gw) ||
+	    IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ||
+	    IN6_IS_ADDR_UNSPECIFIED(&ip6->addr_ll) ||
+	    MAC_IS_ZERO(mac))
 		return 0;
 
 	return ifi;
@@ -899,17 +903,17 @@ static void conf_print(const struct ctx *c)
 		if (!c->no_dhcp) {
 			info("DHCP:");
 			info("    assign: %s",
-			     inet_ntop(AF_INET, &c->addr4, buf4, sizeof(buf4)));
+			     inet_ntop(AF_INET, &c->ip4.addr, buf4, sizeof(buf4)));
 			info("    mask: %s",
-			     inet_ntop(AF_INET, &c->mask4, buf4, sizeof(buf4)));
+			     inet_ntop(AF_INET, &c->ip4.mask, buf4, sizeof(buf4)));
 			info("    router: %s",
-			     inet_ntop(AF_INET, &c->gw4,   buf4, sizeof(buf4)));
+			     inet_ntop(AF_INET, &c->ip4.gw,   buf4, sizeof(buf4)));
 		}
 
-		for (i = 0; c->dns4[i]; i++) {
+		for (i = 0; c->ip4.dns[i]; i++) {
 			if (!i)
 				info("DNS:");
-			inet_ntop(AF_INET, &c->dns4[i], buf4, sizeof(buf4));
+			inet_ntop(AF_INET, &c->ip4.dns[i], buf4, sizeof(buf4));
 			info("    %s", buf4);
 		}
 
@@ -933,17 +937,17 @@ static void conf_print(const struct ctx *c)
 			goto dns6;
 
 		info("    assign: %s",
-		     inet_ntop(AF_INET6, &c->addr6, buf6, sizeof(buf6)));
+		     inet_ntop(AF_INET6, &c->ip6.addr, buf6, sizeof(buf6)));
 		info("    router: %s",
-		     inet_ntop(AF_INET6, &c->gw6,   buf6, sizeof(buf6)));
+		     inet_ntop(AF_INET6, &c->ip6.gw,   buf6, sizeof(buf6)));
 		info("    our link-local: %s",
-		     inet_ntop(AF_INET6, &c->addr6_ll, buf6, sizeof(buf6)));
+		     inet_ntop(AF_INET6, &c->ip6.addr_ll, buf6, sizeof(buf6)));
 
 dns6:
-		for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->dns6[i]); i++) {
+		for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) {
 			if (!i)
 				info("DNS:");
-			inet_ntop(AF_INET6, &c->dns6[i], buf6, sizeof(buf6));
+			inet_ntop(AF_INET6, &c->ip6.dns[i], buf6, sizeof(buf6));
 			info("    %s", buf6);
 		}
 
@@ -1065,9 +1069,9 @@ void conf(struct ctx *c, int argc, char **argv)
 	enum conf_port_type tcp_tap = 0, tcp_init = 0;
 	enum conf_port_type udp_tap = 0, udp_init = 0;
 	struct fqdn *dnss = c->dns_search;
-	struct in6_addr *dns6 = c->dns6;
+	struct in6_addr *dns6 = c->ip6.dns;
 	int name, ret, mask, b, i;
-	uint32_t *dns4 = c->dns4;
+	uint32_t *dns4 = c->ip4.dns;
 	bool v4_only = false, v6_only = false;
 	unsigned int ifi = 0;
 
@@ -1167,17 +1171,17 @@ void conf(struct ctx *c, int argc, char **argv)
 			c->no_dhcp_dns_search = 1;
 			break;
 		case 9:
-			if (IN6_IS_ADDR_UNSPECIFIED(&c->dns6_fwd)	&&
-			    inet_pton(AF_INET6, optarg, &c->dns6_fwd)	&&
-			    !IN6_IS_ADDR_UNSPECIFIED(&c->dns6_fwd)	&&
-			    !IN6_IS_ADDR_LOOPBACK(&c->dns6_fwd))
+			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_fwd)	&&
+			    inet_pton(AF_INET6, optarg, &c->ip6.dns_fwd) &&
+			    !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_fwd)	&&
+			    !IN6_IS_ADDR_LOOPBACK(&c->ip6.dns_fwd))
 				break;
 
-			if (c->dns4_fwd == INADDR_ANY			&&
-			    inet_pton(AF_INET, optarg, &c->dns4_fwd)	&&
-			    c->dns4_fwd != INADDR_ANY			&&
-			    c->dns4_fwd != INADDR_BROADCAST		&&
-			    c->dns4_fwd != INADDR_LOOPBACK)
+			if (c->ip4.dns_fwd == INADDR_ANY		&&
+			    inet_pton(AF_INET, optarg, &c->ip4.dns_fwd)	&&
+			    c->ip4.dns_fwd != INADDR_ANY		&&
+			    c->ip4.dns_fwd != INADDR_BROADCAST		&&
+			    c->ip4.dns_fwd != INADDR_LOOPBACK)
 				break;
 
 			err("Invalid DNS forwarding address: %s", optarg);
@@ -1334,34 +1338,34 @@ void conf(struct ctx *c, int argc, char **argv)
 			}
 			break;
 		case 'a':
-			if (IN6_IS_ADDR_UNSPECIFIED(&c->addr6)		&&
-			    inet_pton(AF_INET6, optarg, &c->addr6)	&&
-			    !IN6_IS_ADDR_UNSPECIFIED(&c->addr6)		&&
-			    !IN6_IS_ADDR_LOOPBACK(&c->addr6)		&&
-			    !IN6_IS_ADDR_V4MAPPED(&c->addr6)		&&
-			    !IN6_IS_ADDR_V4COMPAT(&c->addr6)		&&
-			    !IN6_IS_ADDR_MULTICAST(&c->addr6))
+			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)	&&
+			    inet_pton(AF_INET6, optarg, &c->ip6.addr)	&&
+			    !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)	&&
+			    !IN6_IS_ADDR_LOOPBACK(&c->ip6.addr)		&&
+			    !IN6_IS_ADDR_V4MAPPED(&c->ip6.addr)		&&
+			    !IN6_IS_ADDR_V4COMPAT(&c->ip6.addr)		&&
+			    !IN6_IS_ADDR_MULTICAST(&c->ip6.addr))
 				break;
 
-			if (c->addr4 == INADDR_ANY			&&
-			    inet_pton(AF_INET, optarg, &c->addr4)	&&
-			    c->addr4 != INADDR_ANY			&&
-			    c->addr4 != INADDR_BROADCAST		&&
-			    c->addr4 != INADDR_LOOPBACK			&&
-			    !IN_MULTICAST(c->addr4))
+			if (c->ip4.addr == INADDR_ANY			&&
+			    inet_pton(AF_INET, optarg, &c->ip4.addr)	&&
+			    c->ip4.addr != INADDR_ANY			&&
+			    c->ip4.addr != INADDR_BROADCAST		&&
+			    c->ip4.addr != INADDR_LOOPBACK		&&
+			    !IN_MULTICAST(c->ip4.addr))
 				break;
 
 			err("Invalid address: %s", optarg);
 			usage(argv[0]);
 			break;
 		case 'n':
-			if (inet_pton(AF_INET, optarg, &c->mask4))
+			if (inet_pton(AF_INET, optarg, &c->ip4.mask))
 				break;
 
 			errno = 0;
 			mask = strtol(optarg, NULL, 0);
 			if (mask > 0 && mask <= 32 && !errno) {
-				c->mask4 = htonl(0xffffffff << (32 - mask));
+				c->ip4.mask = htonl(0xffffffff << (32 - mask));
 				break;
 			}
 
@@ -1380,17 +1384,17 @@ void conf(struct ctx *c, int argc, char **argv)
 			}
 			break;
 		case 'g':
-			if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6)		&&
-			    inet_pton(AF_INET6, optarg, &c->gw6)	&&
-			    !IN6_IS_ADDR_UNSPECIFIED(&c->gw6)		&&
-			    !IN6_IS_ADDR_LOOPBACK(&c->gw6))
+			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.gw)		&&
+			    inet_pton(AF_INET6, optarg, &c->ip6.gw)	&&
+			    !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.gw)	&&
+			    !IN6_IS_ADDR_LOOPBACK(&c->ip6.gw))
 				break;
 
-			if (c->gw4 == INADDR_ANY			&&
-			    inet_pton(AF_INET, optarg, &c->gw4)		&&
-			    c->gw4 != INADDR_ANY			&&
-			    c->gw4 != INADDR_BROADCAST			&&
-			    c->gw4 != INADDR_LOOPBACK)
+			if (c->ip4.gw == INADDR_ANY			&&
+			    inet_pton(AF_INET, optarg, &c->ip4.gw)	&&
+			    c->ip4.gw != INADDR_ANY			&&
+			    c->ip4.gw != INADDR_BROADCAST		&&
+			    c->ip4.gw != INADDR_LOOPBACK)
 				break;
 
 			err("Invalid gateway address: %s", optarg);
@@ -1410,7 +1414,7 @@ void conf(struct ctx *c, int argc, char **argv)
 			break;
 		case 'D':
 			if (c->no_dns ||
-			    (!optarg && (dns4 - c->dns4 || dns6 - c->dns6))) {
+			    (!optarg && (dns4 - c->ip4.dns || dns6 - c->ip6.dns))) {
 				err("Empty and non-empty DNS options given");
 				usage(argv[0]);
 			}
@@ -1420,13 +1424,13 @@ void conf(struct ctx *c, int argc, char **argv)
 				break;
 			}
 
-			if (dns4 - &c->dns4[0] < ARRAY_SIZE(c->dns4) &&
+			if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) &&
 			    inet_pton(AF_INET, optarg, dns4)) {
 				dns4++;
 				break;
 			}
 
-			if (dns6 - &c->dns6[0] < ARRAY_SIZE(c->dns6) &&
+			if (dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) &&
 			    inet_pton(AF_INET6, optarg, dns6)) {
 				dns6++;
 				break;
@@ -1511,9 +1515,9 @@ void conf(struct ctx *c, int argc, char **argv)
 		usage(argv[0]);
 	}
 	if (!v6_only)
-		c->ifi4 = conf_ip4(c, ifi);
+		c->ifi4 = conf_ip4(ifi, &c->ip4, c->mac);
 	if (!v4_only)
-		c->ifi6 = conf_ip6(c, ifi);
+		c->ifi6 = conf_ip6(ifi, &c->ip6, c->mac);
 	if (!c->ifi4 && !c->ifi6) {
 		err("External interface not usable");
 		exit(EXIT_FAILURE);
diff --git a/dhcp.c b/dhcp.c
index 32dee56..7ad1319 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -332,20 +332,20 @@ int dhcp(const struct ctx *c, const struct pool *p)
 	     m->chaddr[0], m->chaddr[1], m->chaddr[2],
 	     m->chaddr[3], m->chaddr[4], m->chaddr[5]);
 
-	m->yiaddr = c->addr4;
-	memcpy(opts[1].s,  &c->mask4, sizeof(c->mask4));
-	memcpy(opts[3].s,  &c->gw4,   sizeof(c->gw4));
-	memcpy(opts[54].s, &c->gw4,   sizeof(c->gw4));
+	m->yiaddr = c->ip4.addr;
+	memcpy(opts[1].s,  &c->ip4.mask, sizeof(c->ip4.mask));
+	memcpy(opts[3].s,  &c->ip4.gw,   sizeof(c->ip4.gw));
+	memcpy(opts[54].s, &c->ip4.gw,   sizeof(c->ip4.gw));
 
 	/* If the gateway is not on the assigned subnet, send an option 121
 	 * (Classless Static Routing) adding a dummy route to it.
 	 */
-	if ((c->addr4 & c->mask4) != (c->gw4 & c->mask4)) {
+	if ((c->ip4.addr & c->ip4.mask) != (c->ip4.gw & c->ip4.mask)) {
 		/* a.b.c.d/32:0.0.0.0, 0:a.b.c.d */
 		opts[121].slen = 14;
 		opts[121].s[0] = 32;
-		memcpy(opts[121].s + 1,  &c->gw4, sizeof(c->gw4));
-		memcpy(opts[121].s + 10, &c->gw4, sizeof(c->gw4));
+		memcpy(opts[121].s + 1,  &c->ip4.gw, sizeof(c->ip4.gw));
+		memcpy(opts[121].s + 10, &c->ip4.gw, sizeof(c->ip4.gw));
 	}
 
 	if (c->mtu != -1) {
@@ -354,8 +354,8 @@ int dhcp(const struct ctx *c, const struct pool *p)
 		opts[26].s[1] = c->mtu % 256;
 	}
 
-	for (i = 0, opts[6].slen = 0; !c->no_dhcp_dns && c->dns4[i]; i++) {
-		((uint32_t *)opts[6].s)[i] = c->dns4[i];
+	for (i = 0, opts[6].slen = 0; !c->no_dhcp_dns && c->ip4.dns[i]; i++) {
+		((uint32_t *)opts[6].s)[i] = c->ip4.dns[i];
 		opts[6].slen += sizeof(uint32_t);
 	}
 
@@ -368,8 +368,8 @@ int dhcp(const struct ctx *c, const struct pool *p)
 	uh->dest = htons(68);
 
 	iph->tot_len = htons(len += sizeof(*iph));
-	iph->daddr = c->addr4;
-	iph->saddr = c->gw4;
+	iph->daddr = c->ip4.addr;
+	iph->saddr = c->ip4.gw;
 	iph->check = 0;
 	iph->check = csum_unaligned(iph, (intptr_t)(iph->ihl * 4), 0);
 
diff --git a/dhcpv6.c b/dhcpv6.c
index 4124a3e..fbae88d 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -390,7 +390,7 @@ static size_t dhcpv6_dns_fill(const struct ctx *c, char *buf, int offset)
 	if (c->no_dhcp_dns)
 		goto search;
 
-	for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->dns6[i]); i++) {
+	for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) {
 		if (!i) {
 			srv = (struct opt_dns_servers *)(buf + offset);
 			offset += sizeof(struct opt_hdr);
@@ -398,7 +398,7 @@ static size_t dhcpv6_dns_fill(const struct ctx *c, char *buf, int offset)
 			srv->hdr.l = 0;
 		}
 
-		memcpy(&srv->addr[i], &c->dns6[i], sizeof(srv->addr[i]));
+		memcpy(&srv->addr[i], &c->ip6.dns[i], sizeof(srv->addr[i]));
 		srv->hdr.l += sizeof(srv->addr[i]);
 		offset += sizeof(srv->addr[i]);
 	}
@@ -473,12 +473,12 @@ int dhcpv6(struct ctx *c, const struct pool *p,
 	if (mlen + sizeof(*uh) != ntohs(uh->len) || mlen < sizeof(*mh))
 		return -1;
 
-	c->addr6_ll_seen = *saddr;
+	c->ip6.addr_ll_seen = *saddr;
 
-	if (IN6_IS_ADDR_LINKLOCAL(&c->gw6))
-		src = &c->gw6;
+	if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
+		src = &c->ip6.gw;
 	else
-		src = &c->addr6_ll;
+		src = &c->ip6.addr_ll;
 
 	mh = packet_get(p, 0, sizeof(*uh), sizeof(*mh), NULL);
 	if (!mh)
@@ -508,7 +508,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
 		if (mh->type == TYPE_CONFIRM && server_id)
 			return -1;
 
-		if ((bad_ia = dhcpv6_ia_notonlink(p, &c->addr6))) {
+		if ((bad_ia = dhcpv6_ia_notonlink(p, &c->ip6.addr))) {
 			info("DHCPv6: received CONFIRM with inappropriate IA,"
 			     " sending NotOnLink status in REPLY");
 
@@ -580,7 +580,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
 	resp.hdr.xid = mh->xid;
 
 	tap_ip_send(c, src, IPPROTO_UDP, (char *)&resp, n, mh->xid);
-	c->addr6_seen = c->addr6;
+	c->ip6.addr_seen = c->ip6.addr;
 
 	return 1;
 }
@@ -602,5 +602,5 @@ void dhcpv6_init(const struct ctx *c)
 	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));
 
-	resp.ia_addr.addr	= c->addr6;
+	resp.ia_addr.addr	= c->ip6.addr;
 }
diff --git a/ndp.c b/ndp.c
index 4d13be3..29c4b14 100644
--- a/ndp.c
+++ b/ndp.c
@@ -107,7 +107,7 @@ int ndp(struct ctx *c, const struct icmp6hdr *ih,
 		p += 4;
 		*(uint32_t *)p = htonl(3600);	/* preferred lifetime */
 		p += 8;
-		memcpy(p, &c->addr6, 8);	/* prefix */
+		memcpy(p, &c->ip6.addr, 8);	/* prefix */
 		p += 16;
 
 		if (c->mtu != -1) {
@@ -121,7 +121,7 @@ int ndp(struct ctx *c, const struct icmp6hdr *ih,
 		if (c->no_dhcp_dns)
 			goto dns_done;
 
-		for (n = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->dns6[n]); n++);
+		for (n = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]); n++);
 		if (n) {
 			*p++ = 25;				/* RDNSS */
 			*p++ = 1 + 2 * n;			/* length */
@@ -130,7 +130,7 @@ int ndp(struct ctx *c, const struct icmp6hdr *ih,
 			p += 4;
 
 			for (i = 0; i < n; i++) {
-				memcpy(p, &c->dns6[i], 16);	/* address */
+				memcpy(p, &c->ip6.dns[i], 16);	/* address */
 				p += 16;
 			}
 
@@ -177,15 +177,15 @@ dns_done:
 	len = (uintptr_t)p - (uintptr_t)ihr - sizeof(*ihr);
 
 	if (IN6_IS_ADDR_LINKLOCAL(saddr))
-		c->addr6_ll_seen = *saddr;
+		c->ip6.addr_ll_seen = *saddr;
 	else
-		c->addr6_seen = *saddr;
+		c->ip6.addr_seen = *saddr;
 
 	ip6hr->daddr = *saddr;
-	if (IN6_IS_ADDR_LINKLOCAL(&c->gw6))
-		ip6hr->saddr = c->gw6;
+	if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
+		ip6hr->saddr = c->ip6.gw;
 	else
-		ip6hr->saddr = c->addr6_ll;
+		ip6hr->saddr = c->ip6.addr_ll;
 
 	ip6hr->payload_len = htons(sizeof(*ihr) + len);
 	ip6hr->hop_limit = IPPROTO_ICMPV6;
diff --git a/passt.c b/passt.c
index 18c3512..686d483 100644
--- a/passt.c
+++ b/passt.c
@@ -363,7 +363,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, &c.addr4);
+	proto_update_l2_buf(c.mac_guest, c.mac, &c.ip4.addr);
 
 	if (c.ifi4 && !c.no_dhcp)
 		dhcp_init();
diff --git a/passt.h b/passt.h
index a8d5992..347e7c1 100644
--- a/passt.h
+++ b/passt.h
@@ -94,6 +94,44 @@ enum passt_modes {
 	MODE_PASTA,
 };
 
+/**
+ * struct ip4_ctx - IPv4 execution context
+ * @addr:		IPv4 address for external, routable interface
+ * @addr_seen:		Latest IPv4 address seen as source from tap
+ * @mask:		IPv4 netmask, network order
+ * @gw:			Default IPv4 gateway, network order
+ * @dns:		IPv4 DNS addresses, zero-terminated, network order
+ * @dns_fwd:		Address forwarded (UDP) to first IPv4 DNS, network order
+ */
+struct ip4_ctx {
+	uint32_t addr;
+	uint32_t addr_seen;
+	uint32_t mask;
+	uint32_t gw;
+	uint32_t dns[MAXNS + 1];
+	uint32_t dns_fwd;
+};
+
+/**
+ * struct ip6_ctx - IPv6 execution context
+ * @addr:		IPv6 address for external, routable interface
+ * @addr_ll:		Link-local IPv6 address on external, routable interface
+ * @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:		IPv6 DNS addresses, zero-terminated
+ * @dns_fwd:		Address forwarded (UDP) to first IPv6 DNS, network order
+ */
+struct ip6_ctx {
+	struct in6_addr addr;
+	struct in6_addr addr_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_fwd;
+};
+
 /**
  * struct ctx - Execution context
  * @mode:		Operation mode, qemu/UNIX domain socket or namespace/tap
@@ -122,21 +160,10 @@ enum passt_modes {
  * @mac:		Host MAC address
  * @mac_guest:		MAC address of guest or namespace, seen or configured
  * @ifi4:		Index of routable interface for IPv4, 0 if IPv4 disabled
- * @addr4:		IPv4 address for external, routable interface
- * @addr4_seen:		Latest IPv4 address seen as source from tap
- * @mask4:		IPv4 netmask, network order
- * @gw4:		Default IPv4 gateway, network order
- * @dns4:		IPv4 DNS addresses, zero-terminated, network order
- * @dns4_fwd:		Address forwarded (UDP) to first IPv4 DNS, network order
+ * @ip:			IPv4 configuration
  * @dns_search:		DNS search list
  * @ifi6:		Index of routable interface for IPv6, 0 if IPv6 disabled
- * @addr6:		IPv6 address for external, routable interface
- * @addr6_ll:		Link-local IPv6 address on external, routable interface
- * @addr6_seen:		Latest IPv6 global/site address seen as source from tap
- * @addr6_ll_seen:	Latest IPv6 link-local address seen as source from tap
- * @gw6:		Default IPv6 gateway
- * @dns6:		IPv6 DNS addresses, zero-terminated
- * @dns6_fwd:		Address forwarded (UDP) to first IPv6 DNS, network order
+ * @ip6:		IPv6 configuration
  * @pasta_ifn:		Name of namespace interface for pasta
  * @pasta_ifn:		Index of namespace interface for pasta
  * @pasta_conf_ns:	Configure namespace interface after creating it
@@ -192,23 +219,12 @@ struct ctx {
 	unsigned char mac_guest[ETH_ALEN];
 
 	unsigned int ifi4;
-	uint32_t addr4;
-	uint32_t addr4_seen;
-	uint32_t mask4;
-	uint32_t gw4;
-	uint32_t dns4[MAXNS + 1];
-	uint32_t dns4_fwd;
+	struct ip4_ctx ip4;
 
 	struct fqdn dns_search[MAXDNSRCH];
 
 	unsigned int ifi6;
-	struct in6_addr addr6;
-	struct in6_addr addr6_ll;
-	struct in6_addr addr6_seen;
-	struct in6_addr addr6_ll_seen;
-	struct in6_addr gw6;
-	struct in6_addr dns6[MAXNS + 1];
-	struct in6_addr dns6_fwd;
+	struct ip6_ctx ip6;
 
 	char pasta_ifn[IF_NAMESIZE];
 	unsigned int pasta_ifi;
diff --git a/pasta.c b/pasta.c
index 2d7b5a1..5a78065 100644
--- a/pasta.c
+++ b/pasta.c
@@ -196,17 +196,17 @@ void pasta_ns_conf(struct ctx *c)
 		nl_link(1, c->pasta_ifi, c->mac_guest, 1, c->mtu);
 
 		if (c->ifi4) {
-			prefix_len = __builtin_popcount(c->mask4);
-			nl_addr(1, c->pasta_ifi, AF_INET, &c->addr4,
+			prefix_len = __builtin_popcount(c->ip4.mask);
+			nl_addr(1, c->pasta_ifi, AF_INET, &c->ip4.addr,
 				&prefix_len, NULL);
-			nl_route(1, c->pasta_ifi, AF_INET, &c->gw4);
+			nl_route(1, c->pasta_ifi, AF_INET, &c->ip4.gw);
 		}
 
 		if (c->ifi6) {
 			prefix_len = 64;
-			nl_addr(1, c->pasta_ifi, AF_INET6, &c->addr6,
+			nl_addr(1, c->pasta_ifi, AF_INET6, &c->ip6.addr,
 				&prefix_len, NULL);
-			nl_route(1, c->pasta_ifi, AF_INET6, &c->gw6);
+			nl_route(1, c->pasta_ifi, AF_INET6, &c->ip6.gw);
 		}
 	} else {
 		nl_link(1, c->pasta_ifi, c->mac_guest, 0, 0);
diff --git a/tap.c b/tap.c
index 8d552e9..3231da7 100644
--- a/tap.c
+++ b/tap.c
@@ -130,7 +130,7 @@ void tap_ip_send(const struct ctx *c, const struct in6_addr *src, uint8_t proto,
 		iph->frag_off = 0;
 		iph->ttl = 255;
 		iph->protocol = proto;
-		iph->daddr = c->addr4_seen;
+		iph->daddr = c->ip4.addr_seen;
 		memcpy(&iph->saddr, &src->s6_addr[12], 4);
 
 		iph->check = 0;
@@ -165,9 +165,9 @@ void tap_ip_send(const struct ctx *c, const struct in6_addr *src, uint8_t proto,
 
 		ip6h->saddr = *src;
 		if (IN6_IS_ADDR_LINKLOCAL(src))
-			ip6h->daddr = c->addr6_ll_seen;
+			ip6h->daddr = c->ip6.addr_ll_seen;
 		else
-			ip6h->daddr = c->addr6_seen;
+			ip6h->daddr = c->ip6.addr_seen;
 
 		memcpy(data, in, len);
 
@@ -354,9 +354,9 @@ resume:
 
 		l4_len = l3_len - hlen;
 
-		if (iph->saddr && c->addr4_seen != iph->saddr) {
-			c->addr4_seen = iph->saddr;
-			proto_update_l2_buf(NULL, NULL, &c->addr4_seen);
+		if (iph->saddr && c->ip4.addr_seen != iph->saddr) {
+			c->ip4.addr_seen = iph->saddr;
+			proto_update_l2_buf(NULL, NULL, &c->ip4.addr_seen);
 		}
 
 		l4h = packet_get(in, i, sizeof(*eh) + hlen, l4_len, NULL);
@@ -504,13 +504,13 @@ resume:
 			continue;
 
 		if (IN6_IS_ADDR_LINKLOCAL(saddr)) {
-			c->addr6_ll_seen = *saddr;
+			c->ip6.addr_ll_seen = *saddr;
 
-			if (IN6_IS_ADDR_UNSPECIFIED(&c->addr6_seen)) {
-				c->addr6_seen = *saddr;
+			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_seen)) {
+				c->ip6.addr_seen = *saddr;
 			}
 		} else {
-			c->addr6_seen = *saddr;
+			c->ip6.addr_seen = *saddr;
 		}
 
 		if (proto == IPPROTO_ICMPV6) {
@@ -545,7 +545,7 @@ resume:
 				continue;
 		}
 
-		*saddr = c->addr6;
+		*saddr = c->ip6.addr;
 
 		if (proto != IPPROTO_TCP && proto != IPPROTO_UDP) {
 			tap_packet_debug(NULL, ip6h, NULL, proto, NULL, 1);
diff --git a/tcp.c b/tcp.c
index 3382dfd..ec8c32e 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1700,9 +1700,9 @@ do {									\
 		b->ip6h.payload_len = htons(plen + sizeof(struct tcphdr));
 		b->ip6h.saddr = conn->a.a6;
 		if (IN6_IS_ADDR_LINKLOCAL(&b->ip6h.saddr))
-			b->ip6h.daddr = c->addr6_ll_seen;
+			b->ip6h.daddr = c->ip6.addr_ll_seen;
 		else
-			b->ip6h.daddr = c->addr6_seen;
+			b->ip6h.daddr = c->ip6.addr_seen;
 
 		memset(b->ip6h.flow_lbl, 0, 3);
 
@@ -1723,7 +1723,7 @@ do {									\
 		ip_len = plen + sizeof(struct iphdr) + sizeof(struct tcphdr);
 		b->iph.tot_len = htons(ip_len);
 		b->iph.saddr = conn->a.a4.a.s_addr;
-		b->iph.daddr = c->addr4_seen;
+		b->iph.daddr = c->ip4.addr_seen;
 
 		if (check)
 			b->iph.check = *check;
@@ -2069,7 +2069,7 @@ static uint32_t tcp_seq_init(const struct ctx *c, int af, const void *addr,
 		} __attribute__((__packed__)) in = {
 			.src = *(struct in_addr *)addr,
 			.srcport = srcport,
-			.dst = { c->addr4 },
+			.dst = { c->ip4.addr },
 			.dstport = dstport,
 		};
 
@@ -2083,7 +2083,7 @@ static uint32_t tcp_seq_init(const struct ctx *c, int af, const void *addr,
 		} __attribute__((__packed__)) in = {
 			.src = *(struct in6_addr *)addr,
 			.srcport = srcport,
-			.dst = c->addr6,
+			.dst = c->ip6.addr,
 			.dstport = dstport,
 		};
 
@@ -2197,16 +2197,16 @@ static void tcp_conn_from_tap(struct ctx *c, int af, const void *addr,
 		return;
 
 	if (!c->no_map_gw) {
-		if (af == AF_INET && addr4.sin_addr.s_addr == c->gw4)
+		if (af == AF_INET && addr4.sin_addr.s_addr == c->ip4.gw)
 			addr4.sin_addr.s_addr	= htonl(INADDR_LOOPBACK);
-		if (af == AF_INET6 && IN6_ARE_ADDR_EQUAL(addr, &c->gw6))
+		if (af == AF_INET6 && IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw))
 			addr6.sin6_addr		= in6addr_loopback;
 	}
 
 	if (af == AF_INET6 && IN6_IS_ADDR_LINKLOCAL(&addr6.sin6_addr)) {
 		struct sockaddr_in6 addr6_ll = {
 			.sin6_family = AF_INET6,
-			.sin6_addr = c->addr6_ll,
+			.sin6_addr = c->ip6.addr_ll,
 			.sin6_scope_id = c->ifi6,
 		};
 		if (bind(s, (struct sockaddr *)&addr6_ll, sizeof(addr6_ll))) {
@@ -2894,14 +2894,14 @@ static void tcp_conn_from_sock(struct ctx *c, union epoll_ref ref,
 		memcpy(&sa6, &sa, sizeof(sa6));
 
 		if (IN6_IS_ADDR_LOOPBACK(&sa6.sin6_addr) ||
-		    IN6_ARE_ADDR_EQUAL(&sa6.sin6_addr, &c->addr6_seen) ||
-		    IN6_ARE_ADDR_EQUAL(&sa6.sin6_addr, &c->addr6)) {
+		    IN6_ARE_ADDR_EQUAL(&sa6.sin6_addr, &c->ip6.addr_seen) ||
+		    IN6_ARE_ADDR_EQUAL(&sa6.sin6_addr, &c->ip6.addr)) {
 			struct in6_addr *src;
 
-			if (IN6_IS_ADDR_LINKLOCAL(&c->gw6))
-				src = &c->gw6;
+			if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
+				src = &c->ip6.gw;
 			else
-				src = &c->addr6_ll;
+				src = &c->ip6.addr_ll;
 
 			memcpy(&sa6.sin6_addr, src, sizeof(*src));
 		}
@@ -2928,8 +2928,8 @@ static void tcp_conn_from_sock(struct ctx *c, union epoll_ref ref,
 		memset(&conn->a.a4.one, 0xff, sizeof(conn->a.a4.one));
 
 		if (s_addr >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET ||
-		    s_addr == INADDR_ANY || htonl(s_addr) == c->addr4_seen)
-			s_addr = ntohl(c->gw4);
+		    s_addr == INADDR_ANY || htonl(s_addr) == c->ip4.addr_seen)
+			s_addr = ntohl(c->ip4.gw);
 
 		s_addr = htonl(s_addr);
 		memcpy(&conn->a.a4.a, &s_addr, sizeof(conn->a.a4.a));
@@ -3118,7 +3118,7 @@ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 
 	if (af == AF_INET || af == AF_UNSPEC) {
 		if (!addr && c->mode == MODE_PASTA)
-			bind_addr = &c->addr4;
+			bind_addr = &c->ip4.addr;
 		else
 			bind_addr = addr;
 
@@ -3159,7 +3159,7 @@ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 
 	if (af == AF_INET6 || af == AF_UNSPEC) {
 		if (!addr && c->mode == MODE_PASTA)
-			bind_addr = &c->addr6;
+			bind_addr = &c->ip6.addr;
 		else
 			bind_addr = addr;
 
diff --git a/udp.c b/udp.c
index 856429f..c4ebecc 100644
--- a/udp.c
+++ b/udp.c
@@ -690,20 +690,20 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
 	src_port = htons(b->s_in.sin_port);
 
 	if (src >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET ||
-	    src == INADDR_ANY || src == ntohl(c->addr4_seen)) {
-		b->iph.saddr = c->gw4;
+	    src == INADDR_ANY || src == ntohl(c->ip4.addr_seen)) {
+		b->iph.saddr = c->ip4.gw;
 		udp_tap_map[V4][src_port].ts = now->tv_sec;
 		udp_tap_map[V4][src_port].flags |= PORT_LOCAL;
 
-		if (b->s_in.sin_addr.s_addr == c->addr4_seen)
+		if (b->s_in.sin_addr.s_addr == c->ip4.addr_seen)
 			udp_tap_map[V4][src_port].flags &= ~PORT_LOOPBACK;
 		else
 			udp_tap_map[V4][src_port].flags |= PORT_LOOPBACK;
 
 		bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port);
-	} else if (c->dns4_fwd &&
-		   src == ntohl(c->dns4[0]) && ntohs(src_port) == 53) {
-		b->iph.saddr = c->dns4_fwd;
+	} else if (c->ip4.dns_fwd &&
+		   src == ntohl(c->ip4.dns[0]) && ntohs(src_port) == 53) {
+		b->iph.saddr = c->ip4.dns_fwd;
 	} else {
 		b->iph.saddr = b->s_in.sin_addr.s_addr;
 	}
@@ -768,17 +768,17 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n,
 	b->ip6h.payload_len = htons(udp6_l2_mh_sock[n].msg_len + sizeof(b->uh));
 
 	if (IN6_IS_ADDR_LINKLOCAL(src)) {
-		b->ip6h.daddr = c->addr6_ll_seen;
+		b->ip6h.daddr = c->ip6.addr_ll_seen;
 		b->ip6h.saddr = b->s_in6.sin6_addr;
 	} else if (IN6_IS_ADDR_LOOPBACK(src)			||
-		   IN6_ARE_ADDR_EQUAL(src, &c->addr6_seen)	||
-		   IN6_ARE_ADDR_EQUAL(src, &c->addr6)) {
-		b->ip6h.daddr = c->addr6_ll_seen;
+		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen)	||
+		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr)) {
+		b->ip6h.daddr = c->ip6.addr_ll_seen;
 
-		if (IN6_IS_ADDR_LINKLOCAL(&c->gw6))
-			b->ip6h.saddr = c->gw6;
+		if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
+			b->ip6h.saddr = c->ip6.gw;
 		else
-			b->ip6h.saddr = c->addr6_ll;
+			b->ip6h.saddr = c->ip6.addr_ll;
 
 		udp_tap_map[V6][src_port].ts = now->tv_sec;
 		udp_tap_map[V6][src_port].flags |= PORT_LOCAL;
@@ -788,18 +788,18 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n,
 		else
 			udp_tap_map[V6][src_port].flags &= ~PORT_LOOPBACK;
 
-		if (IN6_ARE_ADDR_EQUAL(src, &c->addr6))
+		if (IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr))
 			udp_tap_map[V6][src_port].flags |= PORT_GUA;
 		else
 			udp_tap_map[V6][src_port].flags &= ~PORT_GUA;
 
 		bitmap_set(udp_act[V6][UDP_ACT_TAP], src_port);
-	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->dns6_fwd) &&
-		   IN6_ARE_ADDR_EQUAL(src, &c->dns6_fwd) && src_port == 53) {
-		b->ip6h.daddr = c->addr6_seen;
-		b->ip6h.saddr = c->dns6_fwd;
+	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_fwd) &&
+		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_fwd) && src_port == 53) {
+		b->ip6h.daddr = c->ip6.addr_seen;
+		b->ip6h.saddr = c->ip6.dns_fwd;
 	} else {
-		b->ip6h.daddr = c->addr6_seen;
+		b->ip6h.daddr = c->ip6.addr_seen;
 		b->ip6h.saddr = b->s_in6.sin6_addr;
 	}
 
@@ -1015,15 +1015,15 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 
 		udp_tap_map[V4][src].ts = now->tv_sec;
 
-		if (s_in.sin_addr.s_addr == c->gw4 && !c->no_map_gw) {
+		if (s_in.sin_addr.s_addr == c->ip4.gw && !c->no_map_gw) {
 			if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) ||
 			    (udp_tap_map[V4][dst].flags & PORT_LOOPBACK))
 				s_in.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
 			else
-				s_in.sin_addr.s_addr = c->addr4_seen;
-		} else if (s_in.sin_addr.s_addr == c->dns4_fwd &&
+				s_in.sin_addr.s_addr = c->ip4.addr_seen;
+		} else if (s_in.sin_addr.s_addr == c->ip4.dns_fwd &&
 			   ntohs(s_in.sin_port) == 53) {
-			s_in.sin_addr.s_addr = c->dns4[0];
+			s_in.sin_addr.s_addr = c->ip4.dns[0];
 		}
 	} else {
 		s_in6 = (struct sockaddr_in6) {
@@ -1036,19 +1036,19 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 		sa = (struct sockaddr *)&s_in6;
 		sl = sizeof(s_in6);
 
-		if (IN6_ARE_ADDR_EQUAL(addr, &c->gw6) && !c->no_map_gw) {
+		if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw) && !c->no_map_gw) {
 			if (!(udp_tap_map[V6][dst].flags & PORT_LOCAL) ||
 			    (udp_tap_map[V6][dst].flags & PORT_LOOPBACK))
 				s_in6.sin6_addr = in6addr_loopback;
 			else if (udp_tap_map[V6][dst].flags & PORT_GUA)
-				s_in6.sin6_addr = c->addr6;
+				s_in6.sin6_addr = c->ip6.addr;
 			else
-				s_in6.sin6_addr = c->addr6_seen;
-		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->dns6_fwd) &&
+				s_in6.sin6_addr = c->ip6.addr_seen;
+		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_fwd) &&
 			   ntohs(s_in6.sin6_port) == 53) {
-			s_in6.sin6_addr = c->dns6[0];
+			s_in6.sin6_addr = c->ip6.dns[0];
 		} else if (IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) {
-			bind_addr = &c->addr6_ll;
+			bind_addr = &c->ip6.addr_ll;
 		}
 
 		if (!(s = udp_tap_map[V6][src].sock)) {
@@ -1122,7 +1122,7 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 
 	if (af == AF_INET || af == AF_UNSPEC) {
 		if (!addr && c->mode == MODE_PASTA)
-			bind_addr = &c->addr4;
+			bind_addr = &c->ip4.addr;
 		else
 			bind_addr = addr;
 
@@ -1155,7 +1155,7 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 
 	if (af == AF_INET6 || af == AF_UNSPEC) {
 		if (!addr && c->mode == MODE_PASTA)
-			bind_addr = &c->addr6;
+			bind_addr = &c->ip6.addr;
 		else
 			bind_addr = addr;
 
diff --git a/util.c b/util.c
index f4ec102..9b87b65 100644
--- a/util.c
+++ b/util.c
@@ -278,8 +278,8 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
 		if (bind_addr) {
 			addr6.sin6_addr = *(struct in6_addr *)bind_addr;
 
-			if (!memcmp(bind_addr, &c->addr6_ll,
-			    sizeof(c->addr6_ll)))
+			if (!memcmp(bind_addr, &c->ip6.addr_ll,
+			    sizeof(c->ip6.addr_ll)))
 				addr6.sin6_scope_id = c->ifi6;
 		} else {
 			addr6.sin6_addr = in6addr_any;
-- 
@@ -278,8 +278,8 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
 		if (bind_addr) {
 			addr6.sin6_addr = *(struct in6_addr *)bind_addr;
 
-			if (!memcmp(bind_addr, &c->addr6_ll,
-			    sizeof(c->addr6_ll)))
+			if (!memcmp(bind_addr, &c->ip6.addr_ll,
+			    sizeof(c->ip6.addr_ll)))
 				addr6.sin6_scope_id = c->ifi6;
 		} else {
 			addr6.sin6_addr = in6addr_any;
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/7] Separately locate external interfaces for IPv4 and IPv6
  2022-07-22  5:31 ` [PATCH 2/7] Separately locate external interfaces for IPv4 and IPv6 David Gibson
@ 2022-08-01 10:23   ` Stefano Brivio
  0 siblings, 0 replies; 11+ messages in thread
From: Stefano Brivio @ 2022-08-01 10:23 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 4097 bytes --]

On Fri, 22 Jul 2022 15:31:13 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> Now that the back end allows passt/pasta to use different external
> interfaces for IPv4 and IPv6, use that to do the right thing in the case
> that the host has IPv4 and IPv6 connectivity via different interfaces.
> If the user hasn't explicitly chosen an interface, separately search for
> a suitable external interface for each protocol.
> 
> As a bonus, this substantially simplifies the external interface probe.  It
> also eliminates a subtle confusing case where in some circumstances we
> would pick the first interface in interface index order, and sometimes in
> order of routes returned from netlink.  On some network configurations that
> could cause tests to fail, because the logic in the tests was subtly
> different (it always used route order).
> 
> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
> ---
>  conf.c                | 19 +++++++++--
>  netlink.c             | 79 ++++---------------------------------------
>  netlink.h             |  2 +-
>  test/dhcp/passt       |  3 +-
>  test/dhcp/pasta       |  3 +-
>  test/ndp/passt        |  4 +--
>  test/two_guests/basic |  3 +-
>  7 files changed, 33 insertions(+), 80 deletions(-)
> 
> [...]
>
> diff --git a/test/dhcp/passt b/test/dhcp/passt
> index f45227a..11e0eb3 100644
> --- a/test/dhcp/passt
> +++ b/test/dhcp/passt
> @@ -17,6 +17,7 @@ htools	ip jq sed tr head
>  test	Interface name
>  gout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
>  hout	HOST_IFNAME ip -j -4 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
> +hout	HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
>  check	[ -n "__IFNAME__" ]
>  
>  test	DHCP: address
> @@ -49,7 +50,7 @@ check	[ "__SEARCH__" = "__HOST_SEARCH__" ]
>  test	DHCPv6: address
>  guest	/sbin/dhclient -6 __IFNAME__
>  gout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.prefixlen == 128).local'
> -hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME__").addr_info[] | select(.scope == "global").local'
> +hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global").local'
>  check	[ "__ADDR6__" = "__HOST_ADDR6__" ]
>  
>  test	DHCPv6: route

This,

> [...]
> 
> diff --git a/test/two_guests/basic b/test/two_guests/basic
> index f7c016d..e226178 100644
> --- a/test/two_guests/basic
> +++ b/test/two_guests/basic
> @@ -19,6 +19,7 @@ test	Interface names
>  g1out	IFNAME1 ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
>  g2out	IFNAME2 ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
>  hout	HOST_IFNAME ip -j -4 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
> +hout	HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
>  check	[ -n "__IFNAME1__" ]
>  check	[ -n "__IFNAME2__" ]
>  
> @@ -40,7 +41,7 @@ guest1	/sbin/dhclient -6 __IFNAME1__
>  guest2	/sbin/dhclient -6 __IFNAME2__
>  g1out	ADDR1_6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME1__").addr_info[] | select(.prefixlen == 128).local'
>  g2out	ADDR2_6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME2__").addr_info[] | select(.prefixlen == 128).local'
> -hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME__").addr_info[] | select(.scope == "global").local'
> +hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global").local'
>  check	[ "__ADDR1_6__" = "__HOST_ADDR6__" ]
>  check	[ "__ADDR2_6__" = "__HOST_ADDR6__" ]
>  

and this are based on patch 17/18 from your previous series ("tests:
Correct determination of host interface name in tests"). I went ahead
and applied that patch (which I skipped previously) before this one,
anyway it makes sense now.

-- 
Stefano


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 5/7] Clarify semantics of c->v4 and c->v6 variables
  2022-07-22  5:31 ` [PATCH 5/7] Clarify semantics of c->v4 and c->v6 variables David Gibson
@ 2022-08-01 10:24   ` Stefano Brivio
  0 siblings, 0 replies; 11+ messages in thread
From: Stefano Brivio @ 2022-08-01 10:24 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 4650 bytes --]

On Fri, 22 Jul 2022 15:31:16 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> The v4 and v6 fields of the context structure can be confusing, because
> they change meaning part way through the code:  Before conf_ip(), they are
> booleans which indicate whether the -4 or -6 options have been given.
> After conf_ip() they are DISABLED|ENABLED|PROBE enums which indicate
> whether the IP version is available (which means both that it was allowed
> on the command line and we were able to configure it).  The PROBE variant
> of the enum is only used locally within conf_ip() and since recent changes
> there it no longer has a real purpose different from ENABLED.
> 
> Simplify this all by making the context fields always just a boolean
> indicating the availability of the IP version.  They both default to 1, but
> can be set to 0 by either command line options or configuration failures.
> We use some local variables in conf() for tracking the state of the command
> line options on their own.
> 
> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
> ---
>  conf.c | 59 ++++++++++++++++++++--------------------------------------
>  util.h |  6 ------
>  2 files changed, 20 insertions(+), 45 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index f5b761f..63f0f3d 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -26,6 +26,7 @@
>  #include <pwd.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> +#include <stdbool.h>
>  #include <unistd.h>
>  #include <syslog.h>
>  #include <time.h>
> @@ -615,40 +616,25 @@ static int conf_ns_opt(struct ctx *c,
>   */
>  static void conf_ip(struct ctx *c)
>  {
> -	int v4, v6;
> -
>  	if (c->v4) {
> -		c->v4		= IP_VERSION_ENABLED;
> -		v4		= IP_VERSION_PROBE;
> -		v6 = c->v6	= IP_VERSION_DISABLED;
> -	} else if (c->v6) {
> -		c->v6		= IP_VERSION_ENABLED;
> -		v6		= IP_VERSION_PROBE;
> -		v4 = c->v4	= IP_VERSION_DISABLED;
> -	} else {
> -		c->v4 = c->v6	= IP_VERSION_ENABLED;
> -		v4 = v6		= IP_VERSION_PROBE;
> -	}
> -
> -	if (v4 != IP_VERSION_DISABLED) {
>  		if (!c->ifi4)
>  			c->ifi4 = nl_get_ext_if(AF_INET);
>  		if (!c->ifi4) {
>  			warn("No external routable interface for IPv4");
> -			v4 = IP_VERSION_DISABLED;
> +			c->v4 = 0;
>  		}
>  	}
>  
> -	if (v6 != IP_VERSION_DISABLED) {
> +	if (c->v6) {
>  		if (!c->ifi6)
>  			c->ifi6 = nl_get_ext_if(AF_INET6);
>  		if (!c->ifi6) {
>  			warn("No external routable interface for IPv6");
> -			v6 = IP_VERSION_DISABLED;
> +			c->v6 = 0;
>  		}
>  	}
>  
> -	if (v4 != IP_VERSION_DISABLED) {
> +	if (c->v4) {
>  		if (!c->gw4)
>  			nl_route(0, c->ifi4, AF_INET, &c->gw4);
>  
> @@ -676,7 +662,7 @@ static void conf_ip(struct ctx *c)
>  			nl_link(0, c->ifi4, c->mac, 0, 0);
>  	}
>  
> -	if (v6 != IP_VERSION_DISABLED) {
> +	if (c->v6) {
>  		int prefix_len = 0;
>  
>  		if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6))
> @@ -694,25 +680,18 @@ static void conf_ip(struct ctx *c)
>  	}
>  
>  	if (!c->gw4 || !c->addr4 || MAC_IS_ZERO(c->mac))
> -		v4 = IP_VERSION_DISABLED;
> -	else
> -		v4 = IP_VERSION_ENABLED;
> +		c->v4 = 0;
>  
>  	if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6) ||
>  	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ||
>  	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6_ll) ||
>  	    MAC_IS_ZERO(c->mac))
> -		v6 = IP_VERSION_DISABLED;
> -	else
> -		v6 = IP_VERSION_ENABLED;
> +		c->v6 = 0;
>  
> -	if ((v4 == IP_VERSION_DISABLED) && (v6 == IP_VERSION_DISABLED)) {
> +	if (!c->v4 && !c->v6) {
>  		err("External interface not usable");
>  		exit(EXIT_FAILURE);
>  	}
> -
> -	c->v4 = v4;
> -	c->v6 = v6;
>  }
>  
>  /**
> @@ -1054,8 +1033,8 @@ void conf(struct ctx *c, int argc, char **argv)
>  		{"no-ndp",	no_argument,		&c->no_ndp,	1 },
>  		{"no-ra",	no_argument,		&c->no_ra,	1 },
>  		{"no-map-gw",	no_argument,		&c->no_map_gw,	1 },
> -		{"ipv4-only",	no_argument,		&c->v4,		'4' },
> -		{"ipv6-only",	no_argument,		&c->v6,		'6' },
> +		{"ipv4-only",	no_argument,		NULL,		'4' },
> +		{"ipv6-only",	no_argument,		NULL,		'6' },
>  		{"tcp-ports",	required_argument,	NULL,		't' },
>  		{"udp-ports",	required_argument,	NULL,		'u' },
>  		{"tcp-ns",	required_argument,	NULL,		'T' },
> @@ -1083,6 +1062,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  	struct in6_addr *dns6 = c->dns6;
>  	int name, ret, mask, b, i;
>  	uint32_t *dns4 = c->dns4;
> +	bool v4_only = false, v6_only = false;

Moved before declaration of *dnss on merge, for coding style
consistency, to keep declarations from longest to shortest (what they
call reverse Christmas tree in the kernel networking area, which is
pretty much the style I followed here). Rationale similar to:

  https://hisham.hm/2018/06/16/when-listing-repeated-things-make-pyramids/

-- 
Stefano


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/7] Separate IPv4 and IPv6 configuration
  2022-07-22  5:31 ` [PATCH 6/7] Separate IPv4 and IPv6 configuration David Gibson
@ 2022-08-01 10:24   ` Stefano Brivio
  0 siblings, 0 replies; 11+ messages in thread
From: Stefano Brivio @ 2022-08-01 10:24 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 6617 bytes --]

On merge:

On Fri, 22 Jul 2022 15:31:17 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> [...]
> 
> diff --git a/conf.c b/conf.c
> index 63f0f3d..2a4ef23 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -411,8 +411,8 @@ static void get_dns(struct ctx *c)
>  	int line_len;
>  	char *line, *p, *end;
>  
> -	dns4_set = !c->v4  || !!*dns4;
> -	dns6_set = !c->v6  || !IN6_IS_ADDR_UNSPECIFIED(dns6);
> +	dns4_set = !c->ifi4  || !!*dns4;
> +	dns6_set = !c->ifi6  || !IN6_IS_ADDR_UNSPECIFIED(dns6);

I dropped the extra whitespace here (the original purpose was to align
things), and...

>  	dnss_set = !!*s->n || c->no_dns_search;
>  	dns_set = (dns4_set && dns6_set) || c->no_dns;
>  
> @@ -611,87 +611,93 @@ static int conf_ns_opt(struct ctx *c,
>  }
>  
>  /**
> - * conf_ip() - Verify or detect IPv4/IPv6 support, get relevant addresses
> + * conf_ip4() - Verify or detect IPv4 support, get relevant addresses
>   * @c:		Execution context
> + * @ifi:	Host interface to attempt (0 to determine one)
> + *
> + * Return:	Interface index for IPv4, or 0 on failure.
>   */
> -static void conf_ip(struct ctx *c)
> +static unsigned int conf_ip4(struct ctx *c, unsigned int ifi)
>  {
> -	if (c->v4) {
> -		if (!c->ifi4)
> -			c->ifi4 = nl_get_ext_if(AF_INET);
> -		if (!c->ifi4) {
> -			warn("No external routable interface for IPv4");
> -			c->v4 = 0;
> -		}
> -	}
> +	if (!ifi)
> +		ifi = nl_get_ext_if(AF_INET);
>  
> -	if (c->v6) {
> -		if (!c->ifi6)
> -			c->ifi6 = nl_get_ext_if(AF_INET6);
> -		if (!c->ifi6) {
> -			warn("No external routable interface for IPv6");
> -			c->v6 = 0;
> -		}
> +	if (!ifi) {
> +		warn("No external routable interface for IPv4");
> +		return 0;
>  	}
>  
> -	if (c->v4) {
> -		if (!c->gw4)
> -			nl_route(0, c->ifi4, AF_INET, &c->gw4);
> +	if (!c->gw4)
> +		nl_route(0, ifi, AF_INET, &c->gw4);
>  
> -		if (!c->addr4) {
> -			int mask_len = 0;
> +	if (!c->addr4) {
> +		int mask_len = 0;
>  
> -			nl_addr(0, c->ifi4, AF_INET, &c->addr4, &mask_len, NULL);
> -			c->mask4 = htonl(0xffffffff << (32 - mask_len));
> -		}
> +		nl_addr(0, ifi, AF_INET, &c->addr4, &mask_len, NULL);
> +		c->mask4 = htonl(0xffffffff << (32 - mask_len));
> +	}
>  
> -		if (!c->mask4) {
> -			if (IN_CLASSA(ntohl(c->addr4)))
> -				c->mask4 = htonl(IN_CLASSA_NET);
> -			else if (IN_CLASSB(ntohl(c->addr4)))
> -				c->mask4 = htonl(IN_CLASSB_NET);
> -			else if (IN_CLASSC(ntohl(c->addr4)))
> -				c->mask4 = htonl(IN_CLASSC_NET);
> -			else
> -				c->mask4 = 0xffffffff;
> -		}
> +	if (!c->mask4) {
> +		if (IN_CLASSA(ntohl(c->addr4)))
> +			c->mask4 = htonl(IN_CLASSA_NET);
> +		else if (IN_CLASSB(ntohl(c->addr4)))
> +			c->mask4 = htonl(IN_CLASSB_NET);
> +		else if (IN_CLASSC(ntohl(c->addr4)))
> +			c->mask4 = htonl(IN_CLASSC_NET);
> +		else
> +			c->mask4 = 0xffffffff;
> +	}
>  
> -		memcpy(&c->addr4_seen, &c->addr4, sizeof(c->addr4_seen));
> +	memcpy(&c->addr4_seen, &c->addr4, sizeof(c->addr4_seen));
>  
> -		if (MAC_IS_ZERO(c->mac))
> -			nl_link(0, c->ifi4, c->mac, 0, 0);
> -	}
> +	if (MAC_IS_ZERO(c->mac))
> +		nl_link(0, ifi, c->mac, 0, 0);
>  
> -	if (c->v6) {
> -		int prefix_len = 0;
> +	if (!c->gw4 || !c->addr4 || MAC_IS_ZERO(c->mac))
> +		return 0;
>  
> -		if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6))
> -			nl_route(0, c->ifi6, AF_INET6, &c->gw6);
> +	return ifi;
> +}
>  
> -		nl_addr(0, c->ifi6, AF_INET6,
> -			IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ? &c->addr6 : NULL,
> -			&prefix_len, &c->addr6_ll);
> +/**
> + * conf_ip6() - Verify or detect IPv6 support, get relevant addresses
> + * @c:		Execution context
> + * @ifi:	Host interface to attempt (0 to determine one)
> + *
> + * Return:	Interface index for IPv6, or 0 on failure.
> + */
> +static unsigned int conf_ip6(struct ctx *c, unsigned int ifi)
> +{
> +	int prefix_len = 0;
>  
> -		memcpy(&c->addr6_seen, &c->addr6, sizeof(c->addr6));
> -		memcpy(&c->addr6_ll_seen, &c->addr6_ll, sizeof(c->addr6_ll));
> +	if (!ifi)
> +		ifi = nl_get_ext_if(AF_INET6);
>  
> -		if (MAC_IS_ZERO(c->mac))
> -			nl_link(0, c->ifi6, c->mac, 0, 0);
> +	if (!ifi) {
> +		warn("No external routable interface for IPv6");
> +		return 0;
>  	}
>  
> -	if (!c->gw4 || !c->addr4 || MAC_IS_ZERO(c->mac))
> -		c->v4 = 0;
> +	if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6))
> +		nl_route(0, ifi, AF_INET6, &c->gw6);
> +
> +	nl_addr(0, ifi, AF_INET6,
> +		IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ? &c->addr6 : NULL,
> +		&prefix_len, &c->addr6_ll);
> +
> +	memcpy(&c->addr6_seen, &c->addr6, sizeof(c->addr6));
> +	memcpy(&c->addr6_ll_seen, &c->addr6_ll, sizeof(c->addr6_ll));
> +
> +	if (MAC_IS_ZERO(c->mac))
> +		nl_link(0, ifi, c->mac, 0, 0);
>  
>  	if (IN6_IS_ADDR_UNSPECIFIED(&c->gw6) ||
>  	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6) ||
>  	    IN6_IS_ADDR_UNSPECIFIED(&c->addr6_ll) ||
>  	    MAC_IS_ZERO(c->mac))
> -		c->v6 = 0;
> +		return 0;
>  
> -	if (!c->v4 && !c->v6) {
> -		err("External interface not usable");
> -		exit(EXIT_FAILURE);
> -	}
> +	return ifi;
>  }
>  
>  /**
> @@ -889,7 +895,7 @@ static void conf_print(const struct ctx *c)
>  	     c->mac[0], c->mac[1], c->mac[2],
>  	     c->mac[3], c->mac[4], c->mac[5]);
>  
> -	if (c->v4) {
> +	if (c->ifi4) {
>  		if (!c->no_dhcp) {
>  			info("DHCP:");
>  			info("    assign: %s",
> @@ -914,7 +920,7 @@ static void conf_print(const struct ctx *c)
>  		}
>  	}
>  
> -	if (c->v6) {
> +	if (c->ifi6) {
>  		char buf6[INET6_ADDRSTRLEN];
>  
>  		if (!c->no_ndp && !c->no_dhcpv6)
> @@ -1063,6 +1069,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  	int name, ret, mask, b, i;
>  	uint32_t *dns4 = c->dns4;
>  	bool v4_only = false, v6_only = false;
> +	unsigned int ifi = 0;
>  
>  	if (c->mode == MODE_PASTA)
>  		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
> @@ -1390,12 +1397,12 @@ void conf(struct ctx *c, int argc, char **argv)
>  			usage(argv[0]);
>  			break;
>  		case 'i':
> -			if (c->ifi4 || c->ifi6) {
> +			if (ifi) {
>  				err("Redundant interface: %s", optarg);
>  				usage(argv[0]);
>  			}
>  
> -			if (!(c->ifi4 = c->ifi6 = if_nametoindex(optarg))) {
> +			if (!(ifi = if_nametoindex(optarg))) {
>  				err("Invalid interface name %s: %s", optarg,
>  				    strerror(errno));
>  				usage(argv[0]);
> @@ -1503,10 +1510,15 @@ void conf(struct ctx *c, int argc, char **argv)
>  		err("Options ipv4-only and ipv6-only are mutually exclusive");
>  		usage(argv[0]);
>  	}
> -	c->v4 = !v6_only;
> -	c->v6 = !v4_only;
> -	conf_ip(c);
> -
> +	if (!v6_only)
> +		c->ifi4 = conf_ip4(c, ifi);
> +	if (!v4_only)
> +		c->ifi6 = conf_ip6(c, ifi);
> +	if (!c->ifi4 && !c->ifi6) {
> +		err("External interface not usable");
> +		exit(EXIT_FAILURE);
> +	}
> +	

I dropped this tab.

-- 
Stefano


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-08-01 10:24 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-22  5:31 [PATCH 0/7] Improved selection of external interface and IP configuration David Gibson
2022-07-22  5:31 ` [PATCH 1/7] Allow different external interfaces for IPv4 and IPv6 connectivity David Gibson
2022-07-22  5:31 ` [PATCH 2/7] Separately locate external interfaces for IPv4 and IPv6 David Gibson
2022-08-01 10:23   ` Stefano Brivio
2022-07-22  5:31 ` [PATCH 3/7] Initialize host side MAC when in IPv6 only mode David Gibson
2022-07-22  5:31 ` [PATCH 4/7] Move passt mac_guest init to be more symmetric with pasta David Gibson
2022-07-22  5:31 ` [PATCH 5/7] Clarify semantics of c->v4 and c->v6 variables David Gibson
2022-08-01 10:24   ` Stefano Brivio
2022-07-22  5:31 ` [PATCH 6/7] Separate IPv4 and IPv6 configuration David Gibson
2022-08-01 10:24   ` Stefano Brivio
2022-07-22  5:31 ` [PATCH 7/7] Make substructures for IPv4 and IPv6 specific context information David Gibson

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).