Allow to reuse as much as possible from virNetlinkCommand(). This comment prepares for the introduction of virNetlindDumpCommand() only differing by how it handles the responses. --- src/util/virnetlink.c | 90 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 35 deletions(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index a5d10fa8e..5fb49251c 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -209,61 +209,38 @@ virNetlinkCreateSocket(int protocol) goto cleanup; } - -/** - * virNetlinkCommand: - * @nlmsg: pointer to netlink message - * @respbuf: pointer to pointer where response buffer will be allocated - * @respbuflen: pointer to integer holding the size of the response buffer - * on return of the function. - * @src_pid: the pid of the process to send a message - * @dst_pid: the pid of the process to talk to, i.e., pid = 0 for kernel - * @protocol: netlink protocol - * @groups: the group identifier - * - * Send the given message to the netlink layer and receive response. - * Returns 0 on success, -1 on error. In case of error, no response - * buffer will be returned. - */ -int virNetlinkCommand(struct nl_msg *nl_msg, - struct nlmsghdr **resp, unsigned int *respbuflen, - uint32_t src_pid, uint32_t dst_pid, - unsigned int protocol, unsigned int groups) +static virNetlinkHandle * +virNetlinkDoCommand(struct nl_msg *nl_msg, uint32_t src_pid, + struct sockaddr_nl nladdr, + unsigned int protocol, unsigned int groups) { - int ret = -1; - struct sockaddr_nl nladdr = { - .nl_family = AF_NETLINK, - .nl_pid = dst_pid, - .nl_groups = 0, - }; ssize_t nbytes; - struct pollfd fds[1]; int fd; int n; - struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg); virNetlinkHandle *nlhandle = NULL; - int len = 0; + struct pollfd fds[1]; + struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg); if (protocol >= MAX_LINKS) { virReportSystemError(EINVAL, _("invalid protocol argument: %d"), protocol); - goto cleanup; + goto error; } if (!(nlhandle = virNetlinkCreateSocket(protocol))) - goto cleanup; + goto error; fd = nl_socket_get_fd(nlhandle); if (fd < 0) { virReportSystemError(errno, "%s", _("cannot get netlink socket fd")); - goto cleanup; + goto error; } if (groups && nl_socket_add_membership(nlhandle, groups) < 0) { virReportSystemError(errno, "%s", _("cannot add netlink membership")); - goto cleanup; + goto error; } nlmsg_set_dst(nl_msg, &nladdr); @@ -274,10 +251,11 @@ int virNetlinkCommand(struct nl_msg *nl_msg, if (nbytes < 0) { virReportSystemError(errno, "%s", _("cannot send to netlink socket")); - goto cleanup; + goto error; } memset(fds, 0, sizeof(fds)); + fds[0].fd = fd; fds[0].events = POLLIN; @@ -289,9 +267,51 @@ int virNetlinkCommand(struct nl_msg *nl_msg, if (n == 0) virReportSystemError(ETIMEDOUT, "%s", _("no valid netlink response was received")); - goto cleanup; } + return nlhandle; + + error: + virNetlinkFree(nlhandle); + return NULL; +} + +/** + * virNetlinkCommand: + * @nlmsg: pointer to netlink message + * @respbuf: pointer to pointer where response buffer will be allocated + * @respbuflen: pointer to integer holding the size of the response buffer + * on return of the function. + * @src_pid: the pid of the process to send a message + * @dst_pid: the pid of the process to talk to, i.e., pid = 0 for kernel + * @protocol: netlink protocol + * @groups: the group identifier + * + * Send the given message to the netlink layer and receive response. + * Returns 0 on success, -1 on error. In case of error, no response + * buffer will be returned. + */ +int virNetlinkCommand(struct nl_msg *nl_msg, + struct nlmsghdr **resp, unsigned int *respbuflen, + uint32_t src_pid, uint32_t dst_pid, + unsigned int protocol, unsigned int groups) +{ + int ret = -1; + struct sockaddr_nl nladdr = { + .nl_family = AF_NETLINK, + .nl_pid = dst_pid, + .nl_groups = 0, + }; + struct pollfd fds[1]; + virNetlinkHandle *nlhandle = NULL; + int len = 0; + + memset(fds, 0, sizeof(fds)); + + if (!(nlhandle = virNetlinkDoCommand(nl_msg, src_pid, nladdr, + protocol, groups))) + goto cleanup; + len = nl_recv(nlhandle, &nladdr, (unsigned char **)resp, NULL); if (len == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", -- 2.11.0

virNetlinkCommand() processes only one response message, while some netlink commands like routes dumping need to process several ones. Add virNetlinkDumpCommand() as a virNetlinkCommand() sister. --- src/libvirt_private.syms | 1 + src/util/virnetlink.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnetlink.h | 9 ++++++++ 3 files changed, 65 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bce0487ab..71143851c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2117,6 +2117,7 @@ virNetDevVPortProfileOpTypeToString; # util/virnetlink.h virNetlinkCommand; virNetlinkDelLink; +virNetlinkDumpCommand; virNetlinkDumpLink; virNetlinkEventAddClient; virNetlinkEventRemoveClient; diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 5fb49251c..4747ba5a4 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -335,6 +335,49 @@ int virNetlinkCommand(struct nl_msg *nl_msg, return ret; } +int +virNetlinkDumpCommand(struct nl_msg *nl_msg, + virNetlinkDumpCallback callback, + uint32_t src_pid, uint32_t dst_pid, + unsigned int protocol, unsigned int groups, + void *opaque) +{ + int ret = -1; + bool end = false; + int len = 0; + struct nlmsghdr *resp = NULL; + struct nlmsghdr *msg = NULL; + + struct sockaddr_nl nladdr = { + .nl_family = AF_NETLINK, + .nl_pid = dst_pid, + .nl_groups = 0, + }; + virNetlinkHandle *nlhandle = NULL; + + if (!(nlhandle = virNetlinkDoCommand(nl_msg, src_pid, nladdr, + protocol, groups))) + goto cleanup; + + while (!end) { + len = nl_recv(nlhandle, &nladdr, (unsigned char **)&resp, NULL); + + for (msg = resp; NLMSG_OK(msg, len); msg = NLMSG_NEXT(msg, len)) { + if (msg->nlmsg_type == NLMSG_DONE) + end = true; + + if (callback(msg, opaque) < 0) + goto cleanup; + } + } + + ret = 0; + + cleanup: + virNetlinkFree(nlhandle); + return ret; +} + /** * virNetlinkDumpLink: @@ -1062,6 +1105,18 @@ int virNetlinkCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED, return -1; } +int +virNetlinkDumpCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED, + virNetlinkDumpCallback callback ATTRIBUTE_UNUSED, + uint32_t src_pid ATTRIBUTE_UNUSED, + uint32_t dst_pid ATTRIBUTE_UNUSED, + unsigned int protocol ATTRIBUTE_UNUSED, + unsigned int groups ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); + return -1; +} int virNetlinkDumpLink(const char *ifname ATTRIBUTE_UNUSED, diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h index 11e817c82..088b01343 100644 --- a/src/util/virnetlink.h +++ b/src/util/virnetlink.h @@ -52,6 +52,15 @@ int virNetlinkCommand(struct nl_msg *nl_msg, uint32_t src_pid, uint32_t dst_pid, unsigned int protocol, unsigned int groups); +typedef int (*virNetlinkDumpCallback)(const struct nlmsghdr *resp, + void *data); + +int virNetlinkDumpCommand(struct nl_msg *nl_msg, + virNetlinkDumpCallback callback, + uint32_t src_pid, uint32_t dst_pid, + unsigned int protocol, unsigned int groups, + void *opaque); + typedef int (*virNetlinkDelLinkFallback)(const char *ifname); int virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback); -- 2.11.0

Replace a few occurences of /proc/sys by the corresponding macro defined a few lines after: SYSCTL_PATH --- src/network/bridge_driver.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c5ec2823d..3f6561055 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -85,6 +85,8 @@ */ #define VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX (32 * 1024 * 1024) +#define SYSCTL_PATH "/proc/sys" + VIR_LOG_INIT("network.bridge_driver"); static virNetworkDriverStatePtr network_driver; @@ -2080,15 +2082,14 @@ networkEnableIPForwarding(bool enableIPv4, bool enableIPv6) &enabled, sizeof(enabled)); #else if (enableIPv4) - ret = virFileWriteStr("/proc/sys/net/ipv4/ip_forward", "1\n", 0); + ret = virFileWriteStr(SYSCTL_PATH "/net/ipv4/ip_forward", "1\n", 0); if (enableIPv6 && ret == 0) - ret = virFileWriteStr("/proc/sys/net/ipv6/conf/all/forwarding", "1\n", 0); + ret = virFileWriteStr(SYSCTL_PATH "/net/ipv6/conf/all/forwarding", "1\n", 0); + #endif return ret; } -#define SYSCTL_PATH "/proc/sys" - static int networkSetIPv6Sysctls(virNetworkObjPtr network) { -- 2.11.0

When enabling IPv6 on all interfaces, we may get the host Router Advertisement routes discarded. To avoid this, the user needs to set accept_ra to 2 for the interfaces with such routes. See https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt on this topic. To avoid user mistakenly loosing routes on their hosts, check accept_ra values before enabling IPv6 forwarding. If a RA route is detected, but neither the corresponding device nor global accept_ra is set to 2, the network will fail to start. --- src/network/bridge_driver.c | 178 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 173 insertions(+), 5 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3f6561055..1ac837f7f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -61,6 +61,7 @@ #include "virlog.h" #include "virdnsmasq.h" #include "configmake.h" +#include "virnetlink.h" #include "virnetdev.h" #include "virnetdevip.h" #include "virnetdevbridge.h" @@ -2067,6 +2068,168 @@ networkReloadFirewallRules(virNetworkDriverStatePtr driver) NULL); } +static int +networkGetAcceptRA(const char *ifname) +{ + char *path = NULL; + char *buf = NULL; + char *suffix; + int accept_ra = -1; + + if (virAsprintf(&path, SYSCTL_PATH "/net/ipv6/conf/%s/accept_ra", + ifname ? ifname : "all") < 0) + goto cleanup; + + if ((virFileReadAll(path, 512, &buf) < 0) || + (virStrToLong_i(buf, &suffix, 10, &accept_ra) < 0)) + goto cleanup; + + cleanup: + VIR_FREE(path); + VIR_FREE(buf); + + return accept_ra; +} + +#if defined(__linux__) && defined(HAVE_LIBNL) +struct networkIPv6CheckData { + bool hasRARoutes; + + /* Devices with conflicting accept_ra */ + char **devices; + size_t ndevices; +}; + +static int +networkCheckIPv6ForwardingCallback(const struct nlmsghdr *resp, + void *opaque) +{ + struct rtmsg *rtmsg = NLMSG_DATA(resp); + int accept_ra = -1; + struct rtattr *rta; + char *ifname = NULL; + char name[IFNAMSIZ]; + struct networkIPv6CheckData *data = opaque; + int ret = 0; + int len = RTM_PAYLOAD(resp); + int oif = -1; + + /* Ignore messages other than route ones */ + if (resp->nlmsg_type != RTM_NEWROUTE) + return ret; + + memset(&name, 0, sizeof(name)); + + /* Extract a few attributes */ + for (rta = RTM_RTA(rtmsg); RTA_OK(rta, len); rta = RTA_NEXT(rta, len)) { + switch (rta->rta_type) { + case RTA_OIF: + oif = *(int *)RTA_DATA(rta); + if (!if_indextoname(oif, name) || VIR_STRDUP(ifname, name) < 0) + VIR_DEBUG("Failed to convert OIF to a device name"); + break; + } + } + + /* No need to do anything else for non RA routes */ + if (rtmsg->rtm_protocol != RTPROT_RA) + goto cleanup; + + data->hasRARoutes = true; + + /* Check the accept_ra value for the interface */ + accept_ra = networkGetAcceptRA(ifname); + VIR_DEBUG("Checking route for device %s, accept_ra: %d", ifname, accept_ra); + + if (accept_ra != 2 && VIR_APPEND_ELEMENT(data->devices, data->ndevices, ifname) < 0) + ret = -1; + + cleanup: + VIR_FREE(ifname); + return ret; +} + +static bool +networkCheckIPv6Forwarding(void) +{ + struct nl_msg *nlmsg = NULL; + bool valid = false; + struct rtgenmsg genmsg; + size_t i; + struct networkIPv6CheckData data = { + .hasRARoutes = false, + .devices = NULL, + .ndevices = 0 + }; + + + /* Prepare the request message */ + if (!(nlmsg = nlmsg_alloc_simple(RTM_GETROUTE, + NLM_F_REQUEST | NLM_F_DUMP))) { + virReportOOMError(); + goto cleanup; + } + + memset(&genmsg, 0, sizeof(genmsg)); + genmsg.rtgen_family = AF_INET6; + + if (nlmsg_append(nlmsg, &genmsg, sizeof(genmsg), NLMSG_ALIGNTO) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("allocated netlink buffer is too small")); + goto cleanup; + } + + /* Send the request and loop over the responses */ + if (virNetlinkDumpCommand(nlmsg, networkCheckIPv6ForwardingCallback, 0, 0, + NETLINK_ROUTE, 0, &data) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to loop over routes")); + goto cleanup; + } + + valid = !data.hasRARoutes || data.ndevices == 0; + + /* Check the global accept_ra if at least one isn't set on a + per-device basis */ + if (!valid && data.hasRARoutes) { + int accept_ra = networkGetAcceptRA(NULL); + valid = accept_ra == 2; + VIR_DEBUG("Checked global accept_ra: %d", accept_ra); + } + + if (!valid) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + for (i = 0; i < data.ndevices; i++) { + virBufferAdd(&buf, data.devices[i], -1); + if (i < data.ndevices - 1) + virBufferAddLit(&buf, ", "); + } + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Check the host setup: enabling IPv6 forwarding with " + "RA routes without accept_ra set to 2 is likely to cause " + "routes loss. Interfaces to look at: %s"), + virBufferCurrentContent(&buf)); + virBufferFreeAndReset(&buf); + } + + cleanup: + nlmsg_free(nlmsg); + for (i = 0; i < data.ndevices; i++) + VIR_FREE(data.devices[i]); + return valid; +} + +#else /* defined(__linux__) && defined(HAVE_LIBNL) */ + +static bool +networkCheckIPv6Forwarding(void) +{ + VIR_WARN("built without libnl: unable to check if IPv6 forwarding can be safely enabled"); + return true; +} +#endif /* defined(__linux__) && defined(HAVE_LIBNL) */ + /* Enable IP Forwarding. Return 0 for success, -1 for failure. */ static int networkEnableIPForwarding(bool enableIPv4, bool enableIPv6) @@ -2377,11 +2540,16 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, } /* If forward.type != NONE, turn on global IP forwarding */ - if (network->def->forward.type != VIR_NETWORK_FORWARD_NONE && - networkEnableIPForwarding(v4present, v6present) < 0) { - virReportSystemError(errno, "%s", - _("failed to enable IP forwarding")); - goto err3; + if (network->def->forward.type != VIR_NETWORK_FORWARD_NONE) { + if (!networkCheckIPv6Forwarding()) + goto err3; /* Precise error message already provided */ + + + if (networkEnableIPForwarding(v4present, v6present) < 0) { + virReportSystemError(errno, "%s", + _("failed to enable IP forwarding")); + goto err3; + } } -- 2.11.0