To save some kernel memory we try to use "dual stack" sockets (that listen to both IPv4 and IPv6 traffic) when possible. However udp_sock_init() attempts to do this in some cases that can't work. Specifically we can only do this when listening on any address. That's never true for the ns (splicing) case, because we always listen on loopback. For the !ns case and AF_UNSPEC case, addr should always be NULL, but add an assert to verify. This is harmless: if addr is non-NULL, sock_l4() will just fail and we'll fall back to the other path. But, it's messy and makes some upcoming changes harder, so avoid attempting this in cases we know can't work. Signed-off-by: David Gibson --- udp.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/udp.c b/udp.c index 7b283138..8cea80cd 100644 --- a/udp.c +++ b/udp.c @@ -803,21 +803,16 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, ASSERT(!c->no_udp); - if (af == AF_UNSPEC && c->ifi4 && c->ifi6) { + if (af == AF_UNSPEC && c->ifi4 && c->ifi6 && !ns) { int s; + ASSERT(!addr); + /* Attempt to get a dual stack socket */ - if (!ns) { - s = sock_l4(c, AF_UNSPEC, EPOLL_TYPE_UDP_LISTEN, - addr, ifname, port, uref.u32); - udp_splice_init[V4][port] = s < 0 ? -1 : s; - udp_splice_init[V6][port] = s < 0 ? -1 : s; - } else { - s = sock_l4(c, AF_UNSPEC, EPOLL_TYPE_UDP_LISTEN, - &in4addr_loopback, ifname, port, uref.u32); - udp_splice_ns[V4][port] = s < 0 ? -1 : s; - udp_splice_ns[V6][port] = s < 0 ? -1 : s; - } + s = sock_l4(c, AF_UNSPEC, EPOLL_TYPE_UDP_LISTEN, + NULL, ifname, port, uref.u32); + udp_splice_init[V4][port] = s < 0 ? -1 : s; + udp_splice_init[V6][port] = s < 0 ? -1 : s; if (IN_INTERVAL(0, FD_REF_MAX, s)) return 0; } -- 2.46.1

tcp_sock_init() and udp_sock_init() take an address to bind to as an address family and void * pair. Use an inany instead. Formerly AF_UNSPEC was used to indicate that we want to listen on both 0.0.0.0 and ::, now use a NULL inany to indicate that. Signed-off-by: David Gibson --- conf.c | 28 ++++++++++++++-------------- tcp.c | 47 ++++++++++++++++++----------------------------- tcp.h | 2 +- udp.c | 31 ++++++++++--------------------- udp.h | 4 ++-- 5 files changed, 45 insertions(+), 67 deletions(-) diff --git a/conf.c b/conf.c index b2758864..9f1cd835 100644 --- a/conf.c +++ b/conf.c @@ -116,11 +116,10 @@ static int parse_port_range(const char *s, char **endptr, static void conf_ports(const struct ctx *c, char optname, const char *optarg, struct fwd_ports *fwd) { - char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf; + union inany_addr addr_buf = inany_any6, *addr = &addr_buf; char buf[BUFSIZ], *spec, *ifname = NULL, *p; bool exclude_only = true, bound_one = false; uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; - sa_family_t af = AF_UNSPEC; unsigned i; int ret; @@ -166,15 +165,13 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, bitmap_set(fwd->map, i); if (optname == 't') { - ret = tcp_sock_init(c, AF_UNSPEC, NULL, NULL, - i); + ret = tcp_sock_init(c, NULL, NULL, i); if (ret == -ENFILE || ret == -EMFILE) goto enfile; if (!ret) bound_one = true; } else if (optname == 'u') { - ret = udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, - i); + ret = udp_sock_init(c, 0, NULL, NULL, i); if (ret == -ENFILE || ret == -EMFILE) goto enfile; if (!ret) @@ -218,6 +215,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, if (ifname == buf + 1) { /* Interface without address */ addr = NULL; } else { + struct in6_addr a6; + struct in_addr a4; + p = buf; /* Allow square brackets for IPv4 too for convenience */ @@ -226,10 +226,10 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, p++; } - if (inet_pton(AF_INET, p, addr)) - af = AF_INET; - else if (inet_pton(AF_INET6, p, addr)) - af = AF_INET6; + if (inet_pton(AF_INET, p, &a4)) + inany_from_af(addr, AF_INET, &a4); + else if (inet_pton(AF_INET6, p, &a6)) + inany_from_af(addr, AF_INET6, &a6); else goto bad; } @@ -276,13 +276,13 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, bitmap_set(fwd->map, i); if (optname == 't') { - ret = tcp_sock_init(c, af, addr, ifname, i); + ret = tcp_sock_init(c, addr, ifname, i); if (ret == -ENFILE || ret == -EMFILE) goto enfile; if (!ret) bound_one = true; } else if (optname == 'u') { - ret = udp_sock_init(c, 0, af, addr, ifname, i); + ret = udp_sock_init(c, 0, addr, ifname, i); if (ret == -ENFILE || ret == -EMFILE) goto enfile; if (!ret) @@ -338,9 +338,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, ret = 0; if (optname == 't') - ret = tcp_sock_init(c, af, addr, ifname, i); + ret = tcp_sock_init(c, addr, ifname, i); else if (optname == 'u') - ret = udp_sock_init(c, 0, af, addr, ifname, i); + ret = udp_sock_init(c, 0, addr, ifname, i); if (ret) goto bind_fail; } diff --git a/tcp.c b/tcp.c index 49e0cfe3..6ca3700e 100644 --- a/tcp.c +++ b/tcp.c @@ -2273,17 +2273,16 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref, } /** - * tcp_sock_init_af() - Initialise listening socket for a given af and port + * tcp_sock_init_one() - Initialise listening socket for address and port * @c: Execution context - * @af: Address family to listen on - * @port: Port, host order - * @addr: Pointer to address for binding, NULL if not configured + * @addr: Pointer to address for binding, NULL for dual stack any * @ifname: Name of interface to bind to, NULL if not configured + * @port: Port, host order * * Return: fd for the new listening socket, negative error code on failure */ -static int tcp_sock_init_af(const struct ctx *c, sa_family_t af, in_port_t port, - const void *addr, const char *ifname) +static int tcp_sock_init_one(const struct ctx *c, const union inany_addr *addr, + const char *ifname, in_port_t port) { union tcp_listen_epoll_ref tref = { .port = port, @@ -2291,24 +2290,13 @@ static int tcp_sock_init_af(const struct ctx *c, sa_family_t af, in_port_t port, }; int s; - if (af == AF_UNSPEC) { - ASSERT(!addr); - s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_HOST, NULL, - ifname, port, tref.u32); - } else { - union inany_addr aany = af == AF_INET ? inany_any4 : inany_any6; - - if (addr) - inany_from_af(&aany, af, addr); - - s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_HOST, &aany, + s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_HOST, addr, ifname, port, tref.u32); - } if (c->tcp.fwd_in.mode == FWD_AUTO) { - if (af == AF_INET || af == AF_UNSPEC) + if (!addr || inany_v4(addr)) tcp_sock_init_ext[port][V4] = s < 0 ? -1 : s; - if (af == AF_INET6 || af == AF_UNSPEC) + if (!addr || !inany_v4(addr)) tcp_sock_init_ext[port][V6] = s < 0 ? -1 : s; } @@ -2322,31 +2310,32 @@ static int tcp_sock_init_af(const struct ctx *c, sa_family_t af, in_port_t port, /** * tcp_sock_init() - Create listening sockets for a given host ("inbound") port * @c: Execution context - * @af: Address family to select a specific IP version, or AF_UNSPEC * @addr: Pointer to address for binding, NULL if not configured * @ifname: Name of interface to bind to, NULL if not configured * @port: Port, host order * * Return: 0 on (partial) success, negative error code on (complete) failure */ -int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, +int tcp_sock_init(const struct ctx *c, const union inany_addr *addr, const char *ifname, in_port_t port) { int r4 = FD_REF_MAX + 1, r6 = FD_REF_MAX + 1; ASSERT(!c->no_tcp); - if (af == AF_UNSPEC && c->ifi4 && c->ifi6) + if (!addr && c->ifi4 && c->ifi6) /* Attempt to get a dual stack socket */ - if (tcp_sock_init_af(c, AF_UNSPEC, port, addr, ifname) >= 0) + if (tcp_sock_init_one(c, NULL, ifname, port) >= 0) return 0; /* Otherwise create a socket per IP version */ - if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) - r4 = tcp_sock_init_af(c, AF_INET, port, addr, ifname); + if ((!addr || inany_v4(addr)) && c->ifi4) + r4 = tcp_sock_init_one(c, addr ? addr : &inany_any4, + ifname, port); - if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) - r6 = tcp_sock_init_af(c, AF_INET6, port, addr, ifname); + if ((!addr || !inany_v4(addr)) && c->ifi6) + r6 = tcp_sock_init_one(c, addr ? addr : &inany_any6, + ifname, port); if (IN_INTERVAL(0, FD_REF_MAX, r4) || IN_INTERVAL(0, FD_REF_MAX, r6)) return 0; @@ -2629,7 +2618,7 @@ static void tcp_port_rebind(struct ctx *c, bool outbound) if (outbound) tcp_ns_sock_init(c, port); else - tcp_sock_init(c, AF_UNSPEC, NULL, NULL, port); + tcp_sock_init(c, NULL, NULL, port); } } } diff --git a/tcp.h b/tcp.h index 5585924f..cf30744d 100644 --- a/tcp.h +++ b/tcp.h @@ -18,7 +18,7 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref, int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, const struct pool *p, int idx, const struct timespec *now); -int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, +int tcp_sock_init(const struct ctx *c, const union inany_addr *addr, const char *ifname, in_port_t port); int tcp_init(struct ctx *c); void tcp_timer(struct ctx *c, const struct timespec *now); diff --git a/udp.c b/udp.c index b3d4a64c..08faaecf 100644 --- a/udp.c +++ b/udp.c @@ -785,15 +785,14 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, * udp_sock_init() - Initialise listening sockets for a given port * @c: Execution context * @ns: In pasta mode, if set, bind with loopback address in namespace - * @af: Address family to select a specific IP version, or AF_UNSPEC * @addr: Pointer to address for binding, NULL if not configured * @ifname: Name of interface to bind to, NULL if not configured * @port: Port, host order * * Return: 0 on (partial) success, negative error code on (complete) failure */ -int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, const char *ifname, in_port_t port) +int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr, + const char *ifname, in_port_t port) { union udp_listen_epoll_ref uref = { .pif = ns ? PIF_SPLICE : PIF_HOST, @@ -803,11 +802,9 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, ASSERT(!c->no_udp); - if (af == AF_UNSPEC && c->ifi4 && c->ifi6 && !ns) { + if (!addr && c->ifi4 && c->ifi6 && !ns) { int s; - ASSERT(!addr); - /* Attempt to get a dual stack socket */ s = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST, NULL, ifname, port, uref.u32); @@ -817,15 +814,11 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, return 0; } - if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) { + if ((!addr || inany_v4(addr)) && c->ifi4) { if (!ns) { - union inany_addr aany = inany_any4; - - if (addr) - inany_from_af(&aany, AF_INET, addr); - r4 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST, - &aany, ifname, port, uref.u32); + addr ? addr : &inany_any4, ifname, + port, uref.u32); udp_splice_init[V4][port] = r4 < 0 ? -1 : r4; } else { @@ -836,15 +829,11 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, } } - if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) { + if ((!addr || !inany_v4(addr)) && c->ifi6) { if (!ns) { - union inany_addr aany = inany_any6; - - if (addr) - inany_from_af(&aany, AF_INET6, addr); - r6 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST, - &aany, ifname, port, uref.u32); + addr ? addr : &inany_any6, ifname, + port, uref.u32); udp_splice_init[V6][port] = r6 < 0 ? -1 : r6; } else { @@ -918,7 +907,7 @@ static void udp_port_rebind(struct ctx *c, bool outbound) if ((c->ifi4 && socks[V4][port] == -1) || (c->ifi6 && socks[V6][port] == -1)) - udp_sock_init(c, outbound, AF_UNSPEC, NULL, NULL, port); + udp_sock_init(c, outbound, NULL, NULL, port); } } diff --git a/udp.h b/udp.h index a8e76bfe..de2df6de 100644 --- a/udp.h +++ b/udp.h @@ -16,8 +16,8 @@ void udp_reply_sock_handler(const struct ctx *c, union epoll_ref ref, int udp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, const struct pool *p, int idx, const struct timespec *now); -int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, const char *ifname, in_port_t port); +int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr, + const char *ifname, in_port_t port); int udp_init(struct ctx *c); void udp_timer(struct ctx *c, const struct timespec *now); void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s); -- 2.46.1