From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v2 14/17] netlink: Propagate errors for "set" operations
Date: Thu, 3 Aug 2023 17:19:53 +1000 [thread overview]
Message-ID: <20230803071956.3198452-15-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20230803071956.3198452-1-david@gibson.dropbear.id.au>
Currently if anything goes wrong while we're configuring the namespace
network with --config-net, we'll just ignore it and carry on. This might
lead to a silently unconfigured or misconfigured namespace environment.
For simple "set" operations based on nl_do() we can now detect failures
reported via netlink. Propagate those errors up to pasta_ns_conf() and
report them usefully.
Link: https://bugs.passt.top/show_bug.cgi?id=60
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
netlink.c | 26 +++++++++++++++++---------
netlink.h | 10 +++++-----
pasta.c | 42 ++++++++++++++++++++++++++++++++----------
3 files changed, 54 insertions(+), 24 deletions(-)
diff --git a/netlink.c b/netlink.c
index 6a2bab2..09ee518 100644
--- a/netlink.c
+++ b/netlink.c
@@ -354,8 +354,10 @@ void nl_route_get_def(int s, unsigned int ifi, sa_family_t af, void *gw)
* @ifi: Interface index in target namespace
* @af: Address family
* @gw: Default gateway to set
+ *
+ * Return: 0 on success, negative error code on failure
*/
-void nl_route_set_def(int s, unsigned int ifi, sa_family_t af, void *gw)
+int nl_route_set_def(int s, unsigned int ifi, sa_family_t af, void *gw)
{
struct req_t {
struct nlmsghdr nlh;
@@ -413,7 +415,7 @@ void nl_route_set_def(int s, unsigned int ifi, sa_family_t af, void *gw)
req.set.r4.rta_gw.rta_len = rta_len;
}
- nl_do(s, &req, RTM_NEWROUTE, NLM_F_CREATE | NLM_F_EXCL, len);
+ return nl_do(s, &req, RTM_NEWROUTE, NLM_F_CREATE | NLM_F_EXCL, len);
}
/**
@@ -558,9 +560,11 @@ void nl_addr_get(int s, unsigned int ifi, sa_family_t af,
* @af: Address family
* @addr: Global address to set
* @prefix_len: Mask or prefix length to set
+ *
+ * Return: 0 on success, negative error code on failure
*/
-void nl_addr_set(int s, unsigned int ifi, sa_family_t af,
- void *addr, int prefix_len)
+int nl_addr_set(int s, unsigned int ifi, sa_family_t af,
+ void *addr, int prefix_len)
{
struct req_t {
struct nlmsghdr nlh;
@@ -613,7 +617,7 @@ void nl_addr_set(int s, unsigned int ifi, sa_family_t af,
req.set.a4.rta_a.rta_type = IFA_ADDRESS;
}
- nl_do(s, &req, RTM_NEWADDR, NLM_F_CREATE | NLM_F_EXCL, len);
+ return nl_do(s, &req, RTM_NEWADDR, NLM_F_CREATE | NLM_F_EXCL, len);
}
/**
@@ -713,8 +717,10 @@ void nl_link_get_mac(int s, unsigned int ifi, void *mac)
* @ns: Use netlink socket in namespace
* @ifi: Interface index
* @mac: MAC address to set
+ *
+ * Return: 0 on success, negative error code on failure
*/
-void nl_link_set_mac(int s, unsigned int ifi, void *mac)
+int nl_link_set_mac(int s, unsigned int ifi, void *mac)
{
struct req_t {
struct nlmsghdr nlh;
@@ -730,7 +736,7 @@ void nl_link_set_mac(int s, unsigned int ifi, void *mac)
memcpy(req.mac, mac, ETH_ALEN);
- nl_do(s, &req, RTM_NEWLINK, 0, sizeof(req));
+ return nl_do(s, &req, RTM_NEWLINK, 0, sizeof(req));
}
/**
@@ -738,8 +744,10 @@ void nl_link_set_mac(int s, unsigned int ifi, void *mac)
* @s: Netlink socket
* @ifi: Interface index
* @mtu: If non-zero, set interface MTU
+ *
+ * Return: 0 on success, negative error code on failure
*/
-void nl_link_up(int s, unsigned int ifi, int mtu)
+int nl_link_up(int s, unsigned int ifi, int mtu)
{
struct req_t {
struct nlmsghdr nlh;
@@ -761,5 +769,5 @@ void nl_link_up(int s, unsigned int ifi, int mtu)
/* Shorten request to drop MTU attribute */
len = offsetof(struct req_t, rta);
- nl_do(s, &req, RTM_NEWLINK, 0, len);
+ return nl_do(s, &req, RTM_NEWLINK, 0, len);
}
diff --git a/netlink.h b/netlink.h
index 5ca17c6..977244b 100644
--- a/netlink.h
+++ b/netlink.h
@@ -12,17 +12,17 @@ extern int nl_sock_ns;
void nl_sock_init(const struct ctx *c, bool ns);
unsigned int nl_get_ext_if(int s, sa_family_t af);
void nl_route_get_def(int s, unsigned int ifi, sa_family_t af, void *gw);
-void nl_route_set_def(int s, unsigned int ifi, sa_family_t af, void *gw);
+int nl_route_set_def(int s, unsigned int ifi, sa_family_t af, void *gw);
void nl_route_dup(int s_src, unsigned int ifi_src,
int s_dst, unsigned int ifi_dst, sa_family_t af);
void nl_addr_get(int s, unsigned int ifi, sa_family_t af,
void *addr, int *prefix_len, void *addr_l);
-void nl_addr_set(int s, unsigned int ifi, sa_family_t af,
- void *addr, int prefix_len);
+int nl_addr_set(int s, unsigned int ifi, sa_family_t af,
+ void *addr, int prefix_len);
void nl_addr_dup(int s_src, unsigned int ifi_src,
int s_dst, unsigned int ifi_dst, sa_family_t af);
void nl_link_get_mac(int s, unsigned int ifi, void *mac);
-void nl_link_set_mac(int s, unsigned int ifi, void *mac);
-void nl_link_up(int s, unsigned int ifi, int mtu);
+int nl_link_set_mac(int s, unsigned int ifi, void *mac);
+int nl_link_up(int s, unsigned int ifi, int mtu);
#endif /* NETLINK_H */
diff --git a/pasta.c b/pasta.c
index 5a1bc36..54c2afa 100644
--- a/pasta.c
+++ b/pasta.c
@@ -272,49 +272,71 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
*/
void pasta_ns_conf(struct ctx *c)
{
- nl_link_up(nl_sock_ns, 1 /* lo */, 0);
+ int rc = 0;
+
+ rc = nl_link_up(nl_sock_ns, 1 /* lo */, 0);
+ if (rc < 0)
+ die("Couldn't bring up loopback interface in namespace: %s",
+ strerror(-rc));
/* Get or set MAC in target namespace */
if (MAC_IS_ZERO(c->mac_guest))
nl_link_get_mac(nl_sock_ns, c->pasta_ifi, c->mac_guest);
else
- nl_link_set_mac(nl_sock_ns, c->pasta_ifi, c->mac_guest);
+ rc = nl_link_set_mac(nl_sock_ns, c->pasta_ifi, c->mac_guest);
+ if (rc < 0)
+ die("Couldn't set MAC address in namespace: %s",
+ strerror(-rc));
if (c->pasta_conf_ns) {
nl_link_up(nl_sock_ns, c->pasta_ifi, c->mtu);
if (c->ifi4) {
if (c->no_copy_addrs)
- nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET,
- &c->ip4.addr, c->ip4.prefix_len);
+ rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
+ AF_INET,
+ &c->ip4.addr,
+ c->ip4.prefix_len);
else
nl_addr_dup(nl_sock, c->ifi4,
nl_sock_ns, c->pasta_ifi, AF_INET);
+ if (rc < 0)
+ die("Couldn't set IPv4 address(es) in namespace: %s",
+ strerror(-rc));
if (c->no_copy_routes)
- nl_route_set_def(nl_sock_ns, c->pasta_ifi,
- AF_INET, &c->ip4.gw);
+ rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
+ AF_INET, &c->ip4.gw);
else
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));
}
if (c->ifi6) {
if (c->no_copy_addrs)
- nl_addr_set(nl_sock_ns, c->pasta_ifi,
- AF_INET6, &c->ip6.addr, 64);
+ rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
+ AF_INET6, &c->ip6.addr, 64);
else
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->no_copy_routes)
- nl_route_set_def(nl_sock_ns, c->pasta_ifi,
- AF_INET6, &c->ip6.gw);
+ rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
+ AF_INET6, &c->ip6.gw);
else
nl_route_dup(nl_sock, c->ifi6,
nl_sock_ns, c->pasta_ifi,
AF_INET6);
+ if (rc < 0)
+ die("Couldn't set IPv6 route(s) in guest: %s",
+ strerror(-rc));
}
}
--
@@ -272,49 +272,71 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
*/
void pasta_ns_conf(struct ctx *c)
{
- nl_link_up(nl_sock_ns, 1 /* lo */, 0);
+ int rc = 0;
+
+ rc = nl_link_up(nl_sock_ns, 1 /* lo */, 0);
+ if (rc < 0)
+ die("Couldn't bring up loopback interface in namespace: %s",
+ strerror(-rc));
/* Get or set MAC in target namespace */
if (MAC_IS_ZERO(c->mac_guest))
nl_link_get_mac(nl_sock_ns, c->pasta_ifi, c->mac_guest);
else
- nl_link_set_mac(nl_sock_ns, c->pasta_ifi, c->mac_guest);
+ rc = nl_link_set_mac(nl_sock_ns, c->pasta_ifi, c->mac_guest);
+ if (rc < 0)
+ die("Couldn't set MAC address in namespace: %s",
+ strerror(-rc));
if (c->pasta_conf_ns) {
nl_link_up(nl_sock_ns, c->pasta_ifi, c->mtu);
if (c->ifi4) {
if (c->no_copy_addrs)
- nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET,
- &c->ip4.addr, c->ip4.prefix_len);
+ rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
+ AF_INET,
+ &c->ip4.addr,
+ c->ip4.prefix_len);
else
nl_addr_dup(nl_sock, c->ifi4,
nl_sock_ns, c->pasta_ifi, AF_INET);
+ if (rc < 0)
+ die("Couldn't set IPv4 address(es) in namespace: %s",
+ strerror(-rc));
if (c->no_copy_routes)
- nl_route_set_def(nl_sock_ns, c->pasta_ifi,
- AF_INET, &c->ip4.gw);
+ rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
+ AF_INET, &c->ip4.gw);
else
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));
}
if (c->ifi6) {
if (c->no_copy_addrs)
- nl_addr_set(nl_sock_ns, c->pasta_ifi,
- AF_INET6, &c->ip6.addr, 64);
+ rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
+ AF_INET6, &c->ip6.addr, 64);
else
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->no_copy_routes)
- nl_route_set_def(nl_sock_ns, c->pasta_ifi,
- AF_INET6, &c->ip6.gw);
+ rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
+ AF_INET6, &c->ip6.gw);
else
nl_route_dup(nl_sock, c->ifi6,
nl_sock_ns, c->pasta_ifi,
AF_INET6);
+ if (rc < 0)
+ die("Couldn't set IPv6 route(s) in guest: %s",
+ strerror(-rc));
}
}
--
2.41.0
next prev parent reply other threads:[~2023-08-03 7:20 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-03 7:19 [PATCH v2 00/17] netlink fixes and cleanups David Gibson
2023-08-03 7:19 ` [PATCH v2 01/17] netlink: Split up functionality of nl_link() David Gibson
2023-08-03 7:19 ` [PATCH v2 02/17] netlink: Split nl_addr() into separate operation functions David Gibson
2023-08-03 7:19 ` [PATCH v2 03/17] netlink: Split nl_route() " David Gibson
2023-08-03 7:19 ` [PATCH v2 04/17] netlink: Use struct in_addr for IPv4 addresses, not bare uint32_t David Gibson
2023-08-03 7:19 ` [PATCH v2 05/17] netlink: Explicitly pass netlink sockets to operations David Gibson
2023-08-03 7:19 ` [PATCH v2 06/17] netlink: Make nl_*_dup() use a separate datagram for each request David Gibson
2023-08-03 7:19 ` [PATCH v2 07/17] netlink: Start sequence number from 1 instead of 0 David Gibson
2023-08-03 7:19 ` [PATCH v2 08/17] netlink: Treat send() or recv() errors as fatal David Gibson
2023-08-03 7:19 ` [PATCH v2 09/17] netlink: Fill in netlink header fields from nl_req() David Gibson
2023-08-03 7:19 ` [PATCH v2 10/17] netlink: Add nl_do() helper for simple operations with error checking David Gibson
2023-08-03 7:19 ` [PATCH v2 11/17] netlink: Clearer reasoning about the netlink response buffer size David Gibson
2023-08-03 7:19 ` [PATCH v2 12/17] netlink: Split nl_req() to allow processing multiple response datagrams David Gibson
2023-08-03 7:19 ` [PATCH v2 13/17] netlink: Add nl_foreach_oftype to filter response message types David Gibson
2023-08-03 7:19 ` David Gibson [this message]
2023-08-03 7:19 ` [PATCH v2 15/17] netlink: Always process all responses to a netlink request David Gibson
2023-08-03 7:19 ` [PATCH v2 16/17] netlink: Propagate errors for "dump" operations David Gibson
2023-08-03 7:19 ` [PATCH v2 17/17] netlink: Propagate errors for "dup" operations David Gibson
2023-08-04 7:04 ` [PATCH v2 00/17] netlink fixes and cleanups Stefano Brivio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230803071956.3198452-15-david@gibson.dropbear.id.au \
--to=david@gibson.dropbear.id.au \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).