[PATCH v6 07/13] ip: refactor function pasta_ns_conf()

[Jon Maloy <jmaloy@redhat.com>]

After the previous changes in this series it becomes possible
to simplify the pasta_ns_conf() function.

We extract address and route configuration into helper functions
pasta_conf_addrs() and pasta_conf_routes(), reducing nesting
and improving readability.

To allow pasta_conf_addrs() to handle both address families
uniformly, we change nl_addr_set() to take a union inany_addr pointer
instead of void pointer, moving the address family handling into
the function itself.

We also fix a bug where the IPv6 code path incorrectly wrote to
req.set.a4.rta_l.rta_type instead of req.set.a6.rta_l.rta_type.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
 netlink.c |  13 +--
 netlink.h |   2 +-
 pasta.c   | 232 +++++++++++++++++++++++++++---------------------------
 3 files changed, 124 insertions(+), 123 deletions(-)

diff --git a/netlink.c b/netlink.c
index 2727eec..fcdc983 100644
--- a/netlink.c
+++ b/netlink.c
@@ -870,7 +870,7 @@ int nl_addr_get_ll(int s, unsigned int ifi, struct in6_addr *addr)
  * Return: 0 on success, negative error code on failure
  */
 int nl_addr_set(int s, unsigned int ifi, sa_family_t af,
-		const void *addr, int prefix_len)
+		const union inany_addr *addr, int prefix_len)
 {
 	struct req_t {
 		struct nlmsghdr nlh;
@@ -905,21 +905,22 @@ int nl_addr_set(int s, unsigned int ifi, sa_family_t af,
 
 		len = offsetof(struct req_t, set.a6) + sizeof(req.set.a6);
 
-		memcpy(&req.set.a6.l, addr, sizeof(req.set.a6.l));
+		memcpy(&req.set.a6.l, &addr->a6, sizeof(req.set.a6.l));
 		req.set.a6.rta_l.rta_len = rta_len;
-		req.set.a4.rta_l.rta_type = IFA_LOCAL;
-		memcpy(&req.set.a6.a, addr, sizeof(req.set.a6.a));
+		req.set.a6.rta_l.rta_type = IFA_LOCAL;
+		memcpy(&req.set.a6.a, &addr->a6, sizeof(req.set.a6.a));
 		req.set.a6.rta_a.rta_len = rta_len;
 		req.set.a6.rta_a.rta_type = IFA_ADDRESS;
 	} else {
+		const struct in_addr *v4 = inany_v4(addr);
 		size_t rta_len = RTA_LENGTH(sizeof(req.set.a4.l));
 
 		len = offsetof(struct req_t, set.a4) + sizeof(req.set.a4);
 
-		memcpy(&req.set.a4.l, addr, sizeof(req.set.a4.l));
+		memcpy(&req.set.a4.l, v4, sizeof(req.set.a4.l));
 		req.set.a4.rta_l.rta_len = rta_len;
 		req.set.a4.rta_l.rta_type = IFA_LOCAL;
-		memcpy(&req.set.a4.a, addr, sizeof(req.set.a4.a));
+		memcpy(&req.set.a4.a, v4, sizeof(req.set.a4.a));
 		req.set.a4.rta_a.rta_len = rta_len;
 		req.set.a4.rta_a.rta_type = IFA_ADDRESS;
 	}
diff --git a/netlink.h b/netlink.h
index 3af6d58..26f5ef7 100644
--- a/netlink.h
+++ b/netlink.h
@@ -23,7 +23,7 @@ int nl_addr_get_all(struct ctx *c, int s, unsigned int ifi, sa_family_t af);
 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,
-		const void *addr, int prefix_len);
+		const union inany_addr *addr, int prefix_len);
 int nl_addr_get_ll(int s, unsigned int ifi, struct in6_addr *addr);
 int nl_addr_set_ll_nodad(int s, unsigned int ifi);
 int nl_addr_dup(int s_src, unsigned int ifi_src,
diff --git a/pasta.c b/pasta.c
index 6307c65..b8d7cf4 100644
--- a/pasta.c
+++ b/pasta.c
@@ -46,6 +46,7 @@
 
 #include "util.h"
 #include "passt.h"
+#include "conf.h"
 #include "isolation.h"
 #include "netlink.h"
 #include "log.h"
@@ -303,13 +304,73 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
 		die_perror("Failed to join network namespace");
 }
 
+/**
+ * pasta_conf_addrs() - Configure addresses for one address family in namespace
+ * @c:		Execution context
+ * @af:		Address family (AF_INET or AF_INET6)
+ * @ifi:	Host interface index for this address family
+ * @no_copy:	If true, set addresses from c->addrs; if false, copy from host
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int pasta_conf_addrs(struct ctx *c, sa_family_t af,
+			    int ifi, bool no_copy)
+{
+	const struct guest_addr *a;
+
+	if (!ifi)
+		return 0;
+
+	if (!no_copy)
+		return nl_addr_dup(nl_sock, ifi, nl_sock_ns, c->pasta_ifi, af);
+
+	for_each_addr(a, c, af) {
+		int rc;
+
+		/* Skip link-local, kernel auto-configures */
+		if (a->flags & CONF_ADDR_LINKLOCAL)
+			continue;
+
+		rc = nl_addr_set(nl_sock_ns, c->pasta_ifi, af, &a->addr,
+				 inany_prefix_len(a));
+		if (rc < 0)
+			return rc;
+	}
+	return 0;
+}
+
+/**
+ * pasta_conf_routes() - Configure routes for one address family in namespace
+ * @c:		Execution context
+ * @af:		Address family (AF_INET or AF_INET6)
+ * @ifi:	Host interface index for this address family
+ * @no_copy:	If true, set default route; if false, copy routes from host
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int pasta_conf_routes(struct ctx *c, sa_family_t af, int ifi,
+			     bool no_copy)
+{
+	const void *gw = (af == AF_INET) ?
+		(const void *)&c->ip4.guest_gw : (const void *)&c->ip6.guest_gw;
+
+	if (!ifi)
+		return 0;
+
+	if (no_copy)
+		return nl_route_set_def(nl_sock_ns, c->pasta_ifi, af, gw);
+
+	return nl_route_dup(nl_sock, ifi, nl_sock_ns, c->pasta_ifi, af);
+}
+
 /**
  * pasta_ns_conf() - Set up loopback and tap interfaces in namespace as needed
  * @c:		Execution context
  */
 void pasta_ns_conf(struct ctx *c)
 {
-	int rc = 0;
+	unsigned int flags = IFF_UP;
+	int rc;
 
 	rc = nl_link_set_flags(nl_sock_ns, 1 /* lo */, IFF_UP, IFF_UP);
 	if (rc < 0)
@@ -328,123 +389,62 @@ void pasta_ns_conf(struct ctx *c)
 		die("Couldn't set MAC address in namespace: %s",
 		    strerror_(-rc));
 
-	if (c->pasta_conf_ns) {
-		unsigned int flags = IFF_UP;
-		const struct guest_addr *a;
-
-		if (c->mtu)
-			nl_link_set_mtu(nl_sock_ns, c->pasta_ifi, c->mtu);
-
-		if (c->ifi6) /* Avoid duplicate address detection on link up */
-			flags |= IFF_NOARP;
-
-		nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags);
-
-		if (c->ifi4) {
-			if (c->ip4.no_copy_addrs) {
-				for_each_addr(a, c, AF_INET) {
-					rc = nl_addr_set(nl_sock_ns,
-							 c->pasta_ifi, AF_INET,
-							 inany_v4(&a->addr),
-							 inany_prefix_len(a));
-					if (rc < 0)
-						break;
-				}
-			} else {
-				rc = nl_addr_dup(nl_sock, c->ifi4,
-						 nl_sock_ns, c->pasta_ifi,
-						 AF_INET);
-			}
-
-			if (c->ifi4 == -1 && rc == -ENOTSUP) {
-				warn("IPv4 not supported, disabling");
-				c->ifi4 = 0;
-				goto ipv4_done;
-			}
-
-			if (rc < 0) {
-				die("Couldn't set IPv4 address(es) in namespace: %s",
-				    strerror_(-rc));
-			}
-
-			if (c->ip4.no_copy_routes) {
-				rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
-						      AF_INET,
-						      &c->ip4.guest_gw);
-			} else {
-				rc = nl_route_dup(nl_sock, c->ifi4, nl_sock_ns,
-						  c->pasta_ifi, AF_INET);
-			}
-
-			if (rc < 0) {
-				die("Couldn't set IPv4 route(s) in guest: %s",
-				    strerror_(-rc));
-			}
-		}
-ipv4_done:
-
-		if (c->ifi6) {
-			rc = nl_addr_get_ll(nl_sock_ns, c->pasta_ifi,
-					    &c->ip6.addr_ll_seen);
-			if (rc < 0) {
-				warn("Can't get LL address from namespace: %s",
-				    strerror_(-rc));
-			}
-
-			rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi);
-			if (rc < 0) {
-				warn("Can't set nodad for LL in namespace: %s",
-				    strerror_(-rc));
-			}
-
-			/* We dodged DAD: re-enable neighbour solicitations */
-			nl_link_set_flags(nl_sock_ns, c->pasta_ifi,
-					  0, IFF_NOARP);
-
-			if (c->ip6.no_copy_addrs) {
-				for_each_addr(a, c, AF_INET6) {
-					rc = nl_addr_set(nl_sock_ns,
-							 c->pasta_ifi,
-							 AF_INET6, &a->addr.a6,
-							 a->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) {
-				die("Couldn't set IPv6 address(es) in namespace: %s",
-				    strerror_(-rc));
-			}
-
-			if (c->ip6.no_copy_routes) {
-				rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
-						      AF_INET6,
-						      &c->ip6.guest_gw);
-			} else {
-				rc = nl_route_dup(nl_sock, c->ifi6,
-						  nl_sock_ns, c->pasta_ifi,
-						  AF_INET6);
-			}
-
-			if (c->ifi6 == -1 && rc == -ENOTSUP) {
-				warn("IPv6 not supported, disabling");
-				c->ifi6 = 0;
-				goto ipv6_done;
-			}
-
-			if (rc < 0) {
-				die("Couldn't set IPv6 route(s) in guest: %s",
-				    strerror_(-rc));
-			}
-		}
+	if (!c->pasta_conf_ns)
+		goto done;
+
+	if (c->mtu)
+		nl_link_set_mtu(nl_sock_ns, c->pasta_ifi, c->mtu);
+
+	if (c->ifi6) /* Avoid duplicate address detection on link up */
+		flags |= IFF_NOARP;
+
+	nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags);
+
+	/* IPv4 configuration */
+	rc = pasta_conf_addrs(c, AF_INET, c->ifi4, c->ip4.no_copy_addrs);
+	if (c->ifi4 == -1 && rc == -ENOTSUP) {
+		warn("IPv4 not supported, disabling");
+		c->ifi4 = 0;
+	} else if (rc < 0) {
+		die("Couldn't set IPv4 address(es): %s", strerror_(-rc));
+	} else if (c->ifi4) {
+		rc = pasta_conf_routes(c, AF_INET, c->ifi4,
+				       c->ip4.no_copy_routes);
+		if (rc < 0)
+			die("Couldn't set IPv4 route(s): %s", strerror_(-rc));
+	}
+
+	if (!c->ifi6)
+		goto done;
+
+	rc = nl_addr_get_ll(nl_sock_ns, c->pasta_ifi,
+			    &c->ip6.addr_ll_seen);
+	if (rc < 0)
+		warn("Can't get LL address from namespace: %s",
+		     strerror_(-rc));
+
+	rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi);
+	if (rc < 0)
+		warn("Can't set nodad for LL in namespace: %s",
+		     strerror_(-rc));
+
+	/* We dodged DAD: re-enable neighbour solicitations */
+	nl_link_set_flags(nl_sock_ns, c->pasta_ifi, 0, IFF_NOARP);
+
+	rc = pasta_conf_addrs(c, AF_INET6, c->ifi6, c->ip6.no_copy_addrs);
+	if (c->ifi6 == -1 && rc == -ENOTSUP) {
+		warn("IPv6 not supported, disabling");
+		c->ifi6 = 0;
+	} else if (rc < 0) {
+		die("Couldn't set IPv6 address(es): %s", strerror_(-rc));
+	} else {
+		rc = pasta_conf_routes(c, AF_INET6, c->ifi6,
+				       c->ip6.no_copy_routes);
+		if (rc < 0)
+			die("Couldn't set IPv6 route(s): %s", strerror_(-rc));
 	}
-ipv6_done:
 
+done:
 	proto_update_l2_buf(c->guest_mac);
 }
 
-- 
2.52.0