[libvirt] [PATCHv3 0/2] Support IPv6 in port allocator

v1: Support IPv6 in port allocator https://www.redhat.com/archives/libvir-list/2013-October/msg00007.html v2: https://www.redhat.com/archives/libvir-list/2013-October/msg01313.html bind to v4 and v6 separately v3: fix the embarrasing bug of hardcoding AF_INET anyway added a test that mocks a v4-only system even on systems with IPv6 compiled in Ján Tomko (2): Split out bind() from virPortAllocatorAcquire Support IPv6 in port allocator src/util/virportallocator.c | 106 +++++++++++++++++++++++++++++++------------ tests/virportallocatortest.c | 68 +++++++++++++++++++++++++-- 2 files changed, 143 insertions(+), 31 deletions(-) -- 1.8.3.2

--- src/util/virportallocator.c | 72 ++++++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 27 deletions(-) diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 694f191..5d51434 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -99,19 +99,59 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name, return pa; } +static int virPortAllocatorBindToPort(bool *used, + unsigned short port) +{ + struct sockaddr_in addr = { + .sin_family = AF_INET, + .sin_port = htons(port), + .sin_addr.s_addr = htonl(INADDR_ANY) + }; + int reuse = 1; + int ret = -1; + int fd = -1; + + *used = false; + + fd = socket(PF_INET, SOCK_STREAM, 0); + if (fd < 0) { + virReportSystemError(errno, "%s", _("Unable to open test socket")); + goto cleanup; + } + + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)&reuse, + sizeof(reuse)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set socket reuse addr flag")); + goto cleanup; + } + + if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { + if (errno == EADDRINUSE) { + *used = true; + ret = 0; + } else { + virReportSystemError(errno, _("Unable to bind to port %d"), port); + } + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} + int virPortAllocatorAcquire(virPortAllocatorPtr pa, unsigned short *port) { int ret = -1; size_t i; - int fd = -1; *port = 0; virObjectLock(pa); for (i = pa->start; i <= pa->end && !*port; i++) { - int reuse = 1; - struct sockaddr_in addr; bool used = false; if (virBitmapGetBit(pa->bitmap, @@ -124,31 +164,10 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, if (used) continue; - addr.sin_family = AF_INET; - addr.sin_port = htons(i); - addr.sin_addr.s_addr = htonl(INADDR_ANY); - fd = socket(PF_INET, SOCK_STREAM, 0); - if (fd < 0) { - virReportSystemError(errno, "%s", - _("Unable to open test socket")); + if (virPortAllocatorBindToPort(&used, i) < 0) goto cleanup; - } - - if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)&reuse, sizeof(reuse)) < 0) { - virReportSystemError(errno, "%s", - _("Unable to set socket reuse addr flag")); - goto cleanup; - } - if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { - if (errno != EADDRINUSE) { - virReportSystemError(errno, - _("Unable to bind to port %zu"), i); - goto cleanup; - } - /* In use, try next */ - VIR_FORCE_CLOSE(fd); - } else { + if (!used) { /* Add port to bitmap of reserved ports */ if (virBitmapSetBit(pa->bitmap, i - pa->start) < 0) { @@ -168,7 +187,6 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, } cleanup: virObjectUnlock(pa); - VIR_FORCE_CLOSE(fd); return ret; } -- 1.8.3.2

On Thu, Feb 06, 2014 at 05:43:13PM +0100, Ján Tomko wrote:
--- src/util/virportallocator.c | 72 ++++++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 27 deletions(-)
ACK, Martin

Also try to bind on IPv6 to check if the port is occupied. Change the mocked bind in the test to return EADDRINUSE for some ports only for the IPv4/IPv6 socket if we're testing on a host with IPv6 compiled in. Also mock socket() to make it fail with EAFNOTSUPPORTED if LIBVIRT_TEST_IPV4ONLY is set in the environment, to simulate a host without IPv6 support in the kernel. The tests are repeated again with this variable set. https://bugzilla.redhat.com/show_bug.cgi?id=1025407 --- src/util/virportallocator.c | 46 +++++++++++++++++++++++++----- tests/virportallocatortest.c | 68 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 10 deletions(-) diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 5d51434..06174b0 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -100,21 +100,45 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name, } static int virPortAllocatorBindToPort(bool *used, - unsigned short port) + unsigned short port, + int family) { - struct sockaddr_in addr = { + struct sockaddr_in6 addr6 = { + .sin6_family = AF_INET6, + .sin6_port = htons(port), + .sin6_addr = IN6ADDR_ANY_INIT, + }; + struct sockaddr_in addr4 = { .sin_family = AF_INET, .sin_port = htons(port), .sin_addr.s_addr = htonl(INADDR_ANY) }; + struct sockaddr* addr; + size_t addrlen; + int v6only = 1; int reuse = 1; int ret = -1; int fd = -1; + bool ipv6 = false; + + if (family == AF_INET6) { + addr = (struct sockaddr*)&addr6; + addrlen = sizeof(addr6); + ipv6 = true; + } else if (family == AF_INET) { + addr = (struct sockaddr*)&addr4; + addrlen = sizeof(addr4); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown family %d"), family); + return -1; + } *used = false; - fd = socket(PF_INET, SOCK_STREAM, 0); + fd = socket(family, SOCK_STREAM, 0); if (fd < 0) { + if (errno == EAFNOSUPPORT) + return 0; virReportSystemError(errno, "%s", _("Unable to open test socket")); goto cleanup; } @@ -126,7 +150,14 @@ static int virPortAllocatorBindToPort(bool *used, goto cleanup; } - if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { + if (ipv6 && setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (void*)&v6only, + sizeof(v6only)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set IPV6_V6ONLY flag")); + goto cleanup; + } + + if (bind(fd, addr, addrlen) < 0) { if (errno == EADDRINUSE) { *used = true; ret = 0; @@ -152,7 +183,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, virObjectLock(pa); for (i = pa->start; i <= pa->end && !*port; i++) { - bool used = false; + bool used = false, v6used = false; if (virBitmapGetBit(pa->bitmap, i - pa->start, &used) < 0) { @@ -164,10 +195,11 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, if (used) continue; - if (virPortAllocatorBindToPort(&used, i) < 0) + if (virPortAllocatorBindToPort(&v6used, i, AF_INET6) < 0 || + virPortAllocatorBindToPort(&used, i, AF_INET) < 0) goto cleanup; - if (!used) { + if (!used && !v6used) { /* Add port to bitmap of reserved ports */ if (virBitmapSetBit(pa->bitmap, i - pa->start) < 0) { diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c index 721356e..7fe18df 100644 --- a/tests/virportallocatortest.c +++ b/tests/virportallocatortest.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013 Red Hat, Inc. + * Copyright (C) 2013-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -19,6 +19,8 @@ */ #include <config.h> +#include <stdlib.h> +#include "virfile.h" #ifdef MOCK_HELPER # include "internal.h" @@ -26,6 +28,47 @@ # include <errno.h> # include <arpa/inet.h> # include <netinet/in.h> +# include <stdio.h> +# include <dlfcn.h> + +static bool host_has_ipv6 = false; +static int (*realsocket)(int domain, int type, int protocol); + +static void init_syms(void) +{ + int fd; + + if (realsocket) + return; + + realsocket = dlsym(RTLD_NEXT, "socket"); + + if (!realsocket) { + fprintf(stderr, "Unable to find 'socket' symbol\n"); + abort(); + } + + fd = realsocket(AF_INET6, SOCK_STREAM, 0); + if (fd < 0) + return; + + host_has_ipv6 = true; + VIR_FORCE_CLOSE(fd); +} + +int socket(int domain, + int type, + int protocol) +{ + init_syms(); + + if (getenv("LIBVIRT_TEST_IPV4ONLY") && domain == AF_INET6) { + errno = EAFNOSUPPORT; + return -1; + } + + return realsocket(domain, type, protocol); +} int bind(int sockfd ATTRIBUTE_UNUSED, const struct sockaddr *addr, @@ -35,6 +78,19 @@ int bind(int sockfd ATTRIBUTE_UNUSED, memcpy(&saddr, addr, sizeof(saddr)); + if (host_has_ipv6 && !getenv("LIBVIRT_TEST_IPV4ONLY")) { + if (saddr.sin_port == htons(5900) || + (saddr.sin_family == AF_INET && + saddr.sin_port == htons(5904)) || + (saddr.sin_family == AF_INET6 && + (saddr.sin_port == htons(5905) || + saddr.sin_port == htons(5906)))) { + errno = EADDRINUSE; + return -1; + } + return 0; + } + if (saddr.sin_port == htons(5900) || saddr.sin_port == htons(5904) || saddr.sin_port == htons(5905) || @@ -47,13 +103,11 @@ int bind(int sockfd ATTRIBUTE_UNUSED, } #else -# include <stdlib.h> # include "testutils.h" # include "virutil.h" # include "virerror.h" # include "viralloc.h" -# include "virfile.h" # include "virlog.h" # include "virportallocator.h" # include "virstring.h" @@ -195,6 +249,14 @@ mymain(void) if (virtTestRun("Test alloc reuse", testAllocReuse, NULL) < 0) ret = -1; + setenv("LIBVIRT_TEST_IPV4ONLY", "really", 1); + + if (virtTestRun("Test IPv4-only alloc all", testAllocAll, NULL) < 0) + ret = -1; + + if (virtTestRun("Test IPv4-only alloc reuse", testAllocReuse, NULL) < 0) + ret = -1; + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.3.2

On 02/06/2014 09:43 AM, Ján Tomko wrote:
Also try to bind on IPv6 to check if the port is occupied.
Change the mocked bind in the test to return EADDRINUSE for some ports only for the IPv4/IPv6 socket if we're testing on a host with IPv6 compiled in.
Also mock socket() to make it fail with EAFNOTSUPPORTED if LIBVIRT_TEST_IPV4ONLY is set in the environment, to simulate a host without IPv6 support in the kernel. The tests are repeated again with this variable set.
https://bugzilla.redhat.com/show_bug.cgi?id=1025407 --- src/util/virportallocator.c | 46 +++++++++++++++++++++++++----- tests/virportallocatortest.c | 68 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 10 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 02/06/2014 05:43 PM, Ján Tomko wrote:
v1: Support IPv6 in port allocator https://www.redhat.com/archives/libvir-list/2013-October/msg00007.html
v2: https://www.redhat.com/archives/libvir-list/2013-October/msg01313.html bind to v4 and v6 separately
v3: fix the embarrasing bug of hardcoding AF_INET anyway added a test that mocks a v4-only system even on systems with IPv6 compiled in
Ján Tomko (2): Split out bind() from virPortAllocatorAcquire Support IPv6 in port allocator
src/util/virportallocator.c | 106 +++++++++++++++++++++++++++++++------------ tests/virportallocatortest.c | 68 +++++++++++++++++++++++++-- 2 files changed, 143 insertions(+), 31 deletions(-)
I've pushed the series, thank you for the reviews! Jan
participants (3)
-
Eric Blake
-
Ján Tomko
-
Martin Kletzander