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
Reviewed-by: David Gibson
---
v7: -Removed redundant argument 'af' in nl_addr_set()
-Removed redundant label and 'goto's in pasta_ns_conf()
-Since I excluded addition of all LINKLOCAL addresses from host
address array in a previous commit I can now omit this test
in pasta_conf_addrs() as suggested by David.
-Here is the example of agnostic usage of inany_prefix_len()
I referred to in a previous commit.
v8: -Do prefix_len conversion inside nl_addr_set() instead of in
the calling function.
-In nl_addr_set(), branch on inany_prefix_v4() return value instead
of inany_af() to avoid redundant AF check and potential static
checker false positives. (David)
-Swap branch order in pasta_conf_routes() for consistency with
pasta_conf_addrs(). (David)
-Defer investigation of pre-existing IFF_NOARP / DAD interaction
oddity noted by David.
---
netlink.c | 46 ++++++-----
netlink.h | 4 +-
pasta.c | 240 ++++++++++++++++++++++++++----------------------------
3 files changed, 141 insertions(+), 149 deletions(-)
diff --git a/netlink.c b/netlink.c
index 9aa64e5a..201dd255 100644
--- a/netlink.c
+++ b/netlink.c
@@ -866,15 +866,16 @@ int nl_addr_get_ll(int s, unsigned int ifi, struct in6_addr *addr)
* nl_addr_set() - Set IP addresses for given interface and address family
* @s: Netlink socket
* @ifi: Interface index
- * @af: Address family
* @addr: Global address to set
- * @prefix_len: Mask or prefix length to set
+ * @prefix_len: Prefix length (IPv6 format, 96-128 for IPv4-mapped)
*
* 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)
+int nl_addr_set(int s, unsigned int ifi, const union inany_addr *addr,
+ int prefix_len)
{
+ int plen;
+ const struct in_addr *v4 = inany_prefix_v4(addr, prefix_len, &plen);
struct req_t {
struct nlmsghdr nlh;
struct ifaddrmsg ifa;
@@ -893,38 +894,41 @@ int nl_addr_set(int s, unsigned int ifi, sa_family_t af,
} a6;
} set;
} req = {
- .ifa.ifa_family = af,
.ifa.ifa_index = ifi,
- .ifa.ifa_prefixlen = prefix_len,
+ .ifa.ifa_prefixlen = v4 ? plen : prefix_len,
.ifa.ifa_scope = RT_SCOPE_UNIVERSE,
};
ssize_t len;
- if (af == AF_INET6) {
+ if (v4) {
+ size_t rta_len = RTA_LENGTH(sizeof(req.set.a4.l));
+
+ req.ifa.ifa_family = AF_INET;
+
+ len = offsetof(struct req_t, set.a4) + sizeof(req.set.a4);
+
+ 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, v4, sizeof(req.set.a4.a));
+ req.set.a4.rta_a.rta_len = rta_len;
+ req.set.a4.rta_a.rta_type = IFA_ADDRESS;
+ } else {
size_t rta_len = RTA_LENGTH(sizeof(req.set.a6.l));
+ req.ifa.ifa_family = AF_INET6;
+
/* By default, strictly speaking, it's duplicated */
req.ifa.ifa_flags = IFA_F_NODAD;
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 {
- 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));
- 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));
- req.set.a4.rta_a.rta_len = rta_len;
- req.set.a4.rta_a.rta_type = IFA_ADDRESS;
}
return nl_do(s, &req, RTM_NEWADDR, NLM_F_CREATE | NLM_F_EXCL, len);
diff --git a/netlink.h b/netlink.h
index 3af6d58b..ec859b74 100644
--- a/netlink.h
+++ b/netlink.h
@@ -22,8 +22,8 @@ int nl_route_dup(int s_src, unsigned int ifi_src,
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);
+int nl_addr_set(int s, unsigned int ifi, 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 aea52001..56df2fdc 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,69 @@ 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->addrs, c->addr_count, af) {
+ int rc;
+
+ rc = nl_addr_set(nl_sock_ns, c->pasta_ifi, &a->addr,
+ a->prefix_len);
+ 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_dup(nl_sock, ifi, nl_sock_ns, c->pasta_ifi, af);
+
+ return nl_route_set_def(nl_sock_ns, c->pasta_ifi, af, gw);
+}
+
/**
* 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,131 +385,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;
- int plen = 0;
-
- 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) {
- int ac = c->addr_count;
-
- for_each_addr(a, c->addrs, ac, AF_INET) {
- const struct in_addr *v4;
-
- v4 = inany_prefix_v4(&a->addr,
- a->prefix_len,
- &plen);
- rc = nl_addr_set(nl_sock_ns,
- c->pasta_ifi, AF_INET,
- v4, plen);
- 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->addrs, c->addr_count, 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));
- }
- }
+ proto_update_l2_buf(c->guest_mac);
+
+ if (!c->pasta_conf_ns)
+ return;
+
+ 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));
}
-ipv6_done:
- proto_update_l2_buf(c->guest_mac);
+ if (!c->ifi6)
+ return;
+
+ 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));
+ }
}
/**
@@ -536,7 +524,7 @@ void pasta_netns_quit_init(const struct ctx *c)
* @c: Execution context
* @inotify_fd: inotify file descriptor with watch on namespace directory
*/
-void pasta_netns_quit_inotify_handler(const struct ctx *c, int inotify_fd)
+void pasta_netns_quit_inotify_handler(struct ctx *c, int inotify_fd)
{
char buf[sizeof(struct inotify_event) + NAME_MAX + 1]
__attribute__ ((aligned(__alignof__(struct inotify_event))));
@@ -562,7 +550,7 @@ void pasta_netns_quit_inotify_handler(const struct ctx *c, int inotify_fd)
* @c: Execution context
* @ref: epoll reference for timer descriptor
*/
-void pasta_netns_quit_timer_handler(const struct ctx *c, union epoll_ref ref)
+void pasta_netns_quit_timer_handler(struct ctx *c, union epoll_ref ref)
{
uint64_t expirations;
ssize_t n;
--
2.52.0