[PATCH v2 7/9] pasta: Unify address configuration paths using address array

[Jon Maloy <jmaloy@redhat.com>]

Until now, pasta has maintained two separate code paths for
setting up addresses in the guest namespace:

- "copy addrs" path: When --config-net is used without -a, pasta
   would call nl_addr_dup() to copy addresses directly from the host
   template interface to the guest.

- "no_copy_addrs" path: When -a is specified, pasta would iterate
   through the addrs[] array and call nl_addr_set() for each entry.

This commit unifies these paths by adding a new nl_addr_get_all()
function that retrieves all addresses from an interface into the
addrs[] array, marking them with INANY_ADDR_HOST flag.

We modify conf_ip4() and conf_ip6() to always populate the address
array, either from command-line -a options or from host interface
discovery.

This gives us a single code path for all address configuration scenarios
and a more consistent address handling regardless of source.

Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
 conf.c    | 82 ++++++++++++++++++++++++++++-------------------------
 netlink.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 netlink.h |  5 ++++
 passt.h   |  4 ---
 pasta.c   | 36 ++++++++++--------------
 5 files changed, 147 insertions(+), 64 deletions(-)

diff --git a/conf.c b/conf.c
index 32a754d..22c2222 100644
--- a/conf.c
+++ b/conf.c
@@ -745,6 +745,8 @@ static int conf_addr_prefix_len(char *arg, union inany_addr *addr,
  */
 static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
 {
+	int i;
+
 	if (!ifi)
 		ifi = nl_get_ext_if(nl_sock, AF_INET);
 
@@ -764,25 +766,25 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
 	}
 
 	if (!ip4->addr_count) {
-		struct in_addr addr;
-		int prefix_len = 0;
-		int rc = nl_addr_get(nl_sock, ifi, AF_INET,
-				     &addr, &prefix_len, NULL);
-		if (rc < 0) {
+		int count = nl_addr_get_all(nl_sock, ifi, AF_INET,
+					    ip4->addrs, IP4_MAX_ADDRS, NULL);
+		if (count < 0) {
 			debug("Couldn't discover IPv4 address: %s",
-			      strerror_(-rc));
+			      strerror_(-count));
+			return 0;
+		}
+		if (count == 0) {
+			debug("No IPv4 address on interface");
 			return 0;
 		}
-		ip4->addrs[0].addr = inany_from_v4(addr);
-		ip4->addrs[0].prefix_len = prefix_len;
-		ip4->addrs[0].flags = INANY_ADDR_HOST;
-		ip4->addr_count = 1;
+		ip4->addr_count = count;
 	}
 
-	if (!ip4->addrs[0].prefix_len) {
-		const struct in_addr *a4 = inany_v4(&ip4->addrs[0].addr);
+	for (i = 0; i < (int)ip4->addr_count; i++) {
+		const struct in_addr *a4 = inany_v4(&ip4->addrs[i].addr);
 
-		ip4->addrs[0].prefix_len = ip4_default_prefix_len(a4);
+		if (!ip4->addrs[i].prefix_len)
+			ip4->addrs[i].prefix_len = ip4_default_prefix_len(a4);
 	}
 
 	ip4->addr_seen = *inany_v4(&ip4->addrs[0].addr);
@@ -807,7 +809,7 @@ static void conf_ip4_local(struct ip4_ctx *ip4)
 	ip4->addrs[0].prefix_len = IP4_LL_PREFIX_LEN;
 	ip4->addr_count = 1;
 
-	ip4->no_copy_addrs = ip4->no_copy_routes = true;
+	ip4->no_copy_routes = true;
 }
 
 /**
@@ -819,7 +821,6 @@ static void conf_ip4_local(struct ip4_ctx *ip4)
  */
 static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6)
 {
-	int prefix_len = 0;
 	int rc;
 
 	if (!ifi)
@@ -839,18 +840,29 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6)
 		}
 	}
 
-	rc = nl_addr_get(nl_sock, ifi, AF_INET6,
-			 ip6->addr_count ? NULL : &ip6->addrs[0].addr.a6,
-			 &prefix_len, &ip6->our_tap_ll);
-	if (rc < 0) {
-		debug("Couldn't discover IPv6 address: %s", strerror_(-rc));
-		return 0;
-	}
-
 	if (!ip6->addr_count) {
-		ip6->addrs[0].prefix_len = prefix_len ? prefix_len : 64;
-		ip6->addrs[0].flags = INANY_ADDR_HOST;
-		ip6->addr_count = 1;
+		int count = nl_addr_get_all(nl_sock, ifi, AF_INET6,
+					    ip6->addrs, IP6_MAX_ADDRS,
+					    &ip6->our_tap_ll);
+		if (count < 0) {
+			debug("Couldn't discover IPv6 address: %s",
+			      strerror_(-count));
+			return 0;
+		}
+		if (count == 0) {
+			debug("No IPv6 address on interface");
+			return 0;
+		}
+		ip6->addr_count = count;
+	} else {
+		/* Even with -a, we still need the link-local for our_tap_ll */
+		rc = nl_addr_get(nl_sock, ifi, AF_INET6,
+				 NULL, NULL, &ip6->our_tap_ll);
+		if (rc < 0) {
+			debug("Couldn't discover IPv6 link-local: %s",
+			      strerror_(-rc));
+			return 0;
+		}
 	}
 
 	ip6->addr_seen = ip6->addrs[0].addr.a6;
@@ -872,7 +884,7 @@ static void conf_ip6_local(struct ip6_ctx *ip6)
 {
 	ip6->our_tap_ll = ip6->guest_gw = IP6_LL_GUEST_GW;
 
-	ip6->no_copy_addrs = ip6->no_copy_routes = true;
+	ip6->no_copy_routes = true;
 }
 
 /**
@@ -1723,8 +1735,7 @@ void conf(struct ctx *c, int argc, char **argv)
 			if (c->mode != MODE_PASTA)
 				die("--no-copy-addrs is for pasta mode only");
 
-			warn("--no-copy-addrs will be dropped soon");
-			c->ip4.no_copy_addrs = c->ip6.no_copy_addrs = true;
+			warn("--no-copy-addrs is deprecated and has no effect");
 			copy_addrs_opt = true;
 			break;
 		case 20:
@@ -1883,8 +1894,7 @@ void conf(struct ctx *c, int argc, char **argv)
 			union inany_addr addr;
 			const struct in_addr *a4;
 			int prefix_len = 0;
-			unsigned int i;
-			int af;
+			int af, i;
 
 			af = conf_addr_prefix_len(optarg, &addr, &prefix_len);
 
@@ -1901,11 +1911,9 @@ void conf(struct ctx *c, int argc, char **argv)
 					die("Too many IPv6 addresses");
 
 				c->ip6.addrs[i].addr.a6 = addr.a6;
-				c->ip6.addrs[i].prefix_len = prefix_len;
+				c->ip6.addrs[i].prefix_len = prefix_len ? prefix_len : 64;
 				c->ip6.addrs[i].flags = INANY_ADDR_CONFIGURED;
 				c->ip6.addr_count++;
-				if (c->mode == MODE_PASTA)
-					c->ip6.no_copy_addrs = true;
 				break;
 			}
 
@@ -1918,19 +1926,17 @@ void conf(struct ctx *c, int argc, char **argv)
 					die("Too many IPv4 addresses");
 
 				c->ip4.addrs[i].addr = inany_from_v4(*a4);
-				c->ip4.addrs[i].prefix_len = prefix_len;
 				c->ip4.addrs[i].flags = INANY_ADDR_CONFIGURED;
-				if (i == 0 && prefix_len) {
+				if (prefix_len) {
 					if (prefix_from_opt)
 						die("Can't mix CIDR with -n");
+					c->ip4.addrs[i].prefix_len = prefix_len;
 					prefix_from_cidr = true;
 				} else {
 					prefix_len = ip4_default_prefix_len(a4);
 				}
 				c->ip4.addrs[0].prefix_len = prefix_len;
 				c->ip4.addr_count++;
-				if (c->mode == MODE_PASTA)
-					c->ip4.no_copy_addrs = true;
 				break;
 			}
 
diff --git a/netlink.c b/netlink.c
index 82a2f0c..8c4412c 100644
--- a/netlink.c
+++ b/netlink.c
@@ -35,6 +35,7 @@
 #include "passt.h"
 #include "log.h"
 #include "ip.h"
+#include "inany.h"
 #include "netlink.h"
 #include "epoll_ctl.h"
 
@@ -812,6 +813,89 @@ int nl_addr_get(int s, unsigned int ifi, sa_family_t af,
 	return status;
 }
 
+/**
+ * nl_addr_get_all() - Read all addresses for a given interface into an array
+ * @s:		Netlink socket
+ * @ifi:	Interface index
+ * @af:	Address family
+ * @addrs:	Array of address entries to fill
+ * @max_addrs:	Maximum number of addresses to store
+ * @addr_ll:	Fill with link-local address if non-NULL
+ *
+ * Return: number of addresses found on success, negative error code on failure
+ */
+int nl_addr_get_all(int s, unsigned int ifi, sa_family_t af,
+		    struct inany_addr_entry *addrs, int max_addrs,
+		    void *addr_ll)
+{
+	struct req_t {
+		struct nlmsghdr nlh;
+		struct ifaddrmsg ifa;
+	} req = {
+		.ifa.ifa_family = af,
+		.ifa.ifa_index  = ifi,
+	};
+	struct nlmsghdr *nh;
+	char buf[NLBUFSIZ];
+	ssize_t status;
+	int count = 0;
+	uint32_t seq;
+
+	seq = nl_send(s, &req, RTM_GETADDR, NLM_F_DUMP, sizeof(req));
+	nl_foreach_oftype(nh, status, s, buf, seq, RTM_NEWADDR) {
+		struct ifaddrmsg *ifa = (struct ifaddrmsg *)NLMSG_DATA(nh);
+		struct rtattr *rta;
+		size_t na;
+
+		if (ifa->ifa_index != ifi || ifa->ifa_flags & IFA_F_DEPRECATED)
+			continue;
+
+		/* Add link-local address if requested */
+		if (ifa->ifa_scope == RT_SCOPE_LINK &&
+		    addr_ll && af == AF_INET6) {
+			for (rta = IFA_RTA(ifa), na = IFA_PAYLOAD(nh);
+			     RTA_OK(rta, na); rta = RTA_NEXT(rta, na)) {
+				if (rta->rta_type == IFA_ADDRESS) {
+					memcpy(addr_ll, RTA_DATA(rta),
+					       RTA_PAYLOAD(rta));
+					break;
+				}
+			}
+			continue;
+		}
+
+		for (rta = IFA_RTA(ifa), na = IFA_PAYLOAD(nh); RTA_OK(rta, na);
+		     rta = RTA_NEXT(rta, na)) {
+			if (af == AF_INET  && rta->rta_type != IFA_LOCAL)
+				continue;
+
+			if (af == AF_INET6 && rta->rta_type != IFA_ADDRESS)
+				continue;
+
+			if (count >= max_addrs)
+				break;
+
+			if (af == AF_INET) {
+				struct in_addr a4;
+
+				memcpy(&a4, RTA_DATA(rta), sizeof(a4));
+				addrs[count].addr = inany_from_v4(a4);
+			} else {
+				memcpy(&addrs[count].addr.a6, RTA_DATA(rta),
+				       sizeof(addrs[count].addr.a6));
+			}
+			addrs[count].prefix_len = ifa->ifa_prefixlen;
+			addrs[count].flags = INANY_ADDR_HOST;
+			count++;
+		}
+	}
+
+	if (status < 0)
+		return status;
+
+	return count;
+}
+
 /**
  * nl_addr_get_ll() - Get first IPv6 link-local address for a given interface
  * @s:		Netlink socket
diff --git a/netlink.h b/netlink.h
index 8f1e9b9..c983922 100644
--- a/netlink.h
+++ b/netlink.h
@@ -6,6 +6,8 @@
 #ifndef NETLINK_H
 #define NETLINK_H
 
+struct inany_addr_entry;
+
 extern int nl_sock;
 extern int nl_sock_ns;
 
@@ -17,6 +19,9 @@ int nl_route_dup(int s_src, unsigned int ifi_src,
 		 int s_dst, unsigned int ifi_dst, sa_family_t af);
 int nl_addr_get(int s, unsigned int ifi, sa_family_t af,
 		void *addr, int *prefix_len, void *addr_l);
+int nl_addr_get_all(int s, unsigned int ifi, sa_family_t af,
+		    struct inany_addr_entry *addrs, int max_addrs,
+		    void *addr_ll);
 bool nl_neigh_mac_get(int s, const union inany_addr *addr, int ifi,
 		      unsigned char *mac);
 int nl_addr_set(int s, unsigned int ifi, sa_family_t af,
diff --git a/passt.h b/passt.h
index 9c0c3fe..929b474 100644
--- a/passt.h
+++ b/passt.h
@@ -81,7 +81,6 @@ enum passt_modes {
  * @addr_out:		Optional source address for outbound traffic
  * @ifname_out:		Optional interface name to bind outbound sockets to
  * @no_copy_routes:	Don't copy all routes when configuring target namespace
- * @no_copy_addrs:	Don't copy all addresses when configuring namespace
  */
 struct ip4_ctx {
 	/* PIF_TAP addresses */
@@ -103,7 +102,6 @@ struct ip4_ctx {
 	char ifname_out[IFNAMSIZ];
 
 	bool no_copy_routes;
-	bool no_copy_addrs;
 };
 
 /**
@@ -124,7 +122,6 @@ struct ip4_ctx {
  * @addr_out:		Optional source address for outbound traffic
  * @ifname_out:		Optional interface name to bind outbound sockets to
  * @no_copy_routes:	Don't copy all routes when configuring target namespace
- * @no_copy_addrs:	Don't copy all addresses when configuring namespace
  */
 struct ip6_ctx {
 	/* PIF_TAP addresses */
@@ -147,7 +144,6 @@ struct ip6_ctx {
 	char ifname_out[IFNAMSIZ];
 
 	bool no_copy_routes;
-	bool no_copy_addrs;
 };
 
 #include <netinet/if_ether.h>
diff --git a/pasta.c b/pasta.c
index 27ce6a7..faf1a73 100644
--- a/pasta.c
+++ b/pasta.c
@@ -337,21 +337,15 @@ void pasta_ns_conf(struct ctx *c)
 		nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags);
 
 		if (c->ifi4) {
-			if (c->ip4.no_copy_addrs) {
-				int i;
-
-				for (i = 0; i < c->ip4.addr_count; i++) {
-					rc = nl_addr_set(nl_sock_ns,
-							 c->pasta_ifi, AF_INET,
-							 inany_v4(&c->ip4.addrs[i].addr),
-							 c->ip4.addrs[i].prefix_len);
-					if (rc < 0)
-						break;
-				}
-			} else {
-				rc = nl_addr_dup(nl_sock, c->ifi4,
-						 nl_sock_ns, c->pasta_ifi,
-						 AF_INET);
+			int i;
+
+			for (i = 0; i < c->ip4.addr_count; i++) {
+				rc = nl_addr_set(nl_sock_ns,
+						 c->pasta_ifi, AF_INET,
+						 inany_v4(&c->ip4.addrs[i].addr),
+						 c->ip4.addrs[i].prefix_len);
+				if (rc < 0)
+					break;
 			}
 
 			if (rc < 0) {
@@ -392,7 +386,7 @@ void pasta_ns_conf(struct ctx *c)
 			nl_link_set_flags(nl_sock_ns, c->pasta_ifi,
 					  0, IFF_NOARP);
 
-			if (c->ip6.no_copy_addrs) {
+		{
 				struct in6_addr *a;
 				int i;
 
@@ -400,16 +394,14 @@ void pasta_ns_conf(struct ctx *c)
 					a = &c->ip6.addrs[i].addr.a6;
 					if (IN6_IS_ADDR_UNSPECIFIED(a))
 						continue;
+
 					rc = nl_addr_set(nl_sock_ns,
-							 c->pasta_ifi,
-							 AF_INET6, a, 64);
+							 c->pasta_ifi, AF_INET6,
+							 a,
+							 c->ip6.addrs[i].prefix_len);
 					if (rc < 0)
 						break;
 				}
-			} else {
-				rc = nl_addr_dup(nl_sock, c->ifi6,
-						 nl_sock_ns, c->pasta_ifi,
-						 AF_INET6);
 			}
 
 			if (rc < 0) {
-- 
2.52.0