* [PATCH 1/3] tcp, udp, util: Pass socket creation errors all the way up
2023-03-08 12:33 [PATCH 0/3] Fail gracefully on too many open files Stefano Brivio
@ 2023-03-08 12:33 ` Stefano Brivio
2023-03-08 22:16 ` David Gibson
2023-03-08 12:33 ` [PATCH 2/3] tcp, udp: Fix partial success return codes in {tcp,udp}_sock_init() Stefano Brivio
2023-03-08 12:33 ` [PATCH 3/3] conf: Terminate on EMFILE or ENFILE on sockets for port mapping Stefano Brivio
2 siblings, 1 reply; 7+ messages in thread
From: Stefano Brivio @ 2023-03-08 12:33 UTC (permalink / raw)
To: passt-dev
...starting from sock_l4(), pass negative error (errno) codes instead
of -1. They will only be used in two commits from now, no functional
changes intended here.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
tcp.c | 22 ++++++++++++----------
udp.c | 18 +++++++++---------
util.c | 31 ++++++++++++++++++-------------
3 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/tcp.c b/tcp.c
index 96ca5c7..e209483 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2955,7 +2955,7 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
* @addr: Pointer to address for binding, NULL if not configured
* @ifname: Name of interface to bind to, NULL if not configured
*
- * Return: fd for the new listening socket, or -1 on failure
+ * Return: fd for the new listening socket, negative error code on failure
*/
static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
const struct in_addr *addr, const char *ifname)
@@ -2968,13 +2968,13 @@ static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
if (c->tcp.fwd_in.mode == FWD_AUTO) {
if (af == AF_INET || af == AF_UNSPEC)
- tcp_sock_init_ext[port][V4] = s;
+ tcp_sock_init_ext[port][V4] = s < 0 ? -1 : s;
if (af == AF_INET6 || af == AF_UNSPEC)
- tcp_sock_init_ext[port][V6] = s;
+ tcp_sock_init_ext[port][V6] = s < 0 ? -1 : s;
}
if (s < 0)
- return -1;
+ return s;
tcp_sock_set_bufsize(c, s);
return s;
@@ -2988,12 +2988,12 @@ static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
* @ifname: Name of interface to bind to, NULL if not configured
* @port: Port, host order
*
- * Return: 0 on (partial) success, -1 on (complete) failure
+ * 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,
const char *ifname, in_port_t port)
{
- int ret = 0;
+ int ret = 0, af_ret;
if (af == AF_UNSPEC && c->ifi4 && c->ifi6)
/* Attempt to get a dual stack socket */
@@ -3002,13 +3002,15 @@ int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
/* Otherwise create a socket per IP version */
if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
- if (tcp_sock_init_af(c, AF_INET, port, addr, ifname) < 0)
- ret = -1;
+ af_ret = tcp_sock_init_af(c, AF_INET, port, addr, ifname);
+ if (af_ret < 0)
+ ret = af_ret;
}
if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
- if (tcp_sock_init_af(c, AF_INET6, port, addr, ifname) < 0)
- ret = -1;
+ af_ret = tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
+ if (af_ret < 0)
+ ret = af_ret;
}
return ret;
diff --git a/udp.c b/udp.c
index 1077cde..41e0afd 100644
--- a/udp.c
+++ b/udp.c
@@ -977,7 +977,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
* @ifname: Name of interface to bind to, NULL if not configured
* @port: Port, host order
*
- * Return: 0 on (partial) success, -1 on (complete) failure
+ * 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)
@@ -1002,19 +1002,19 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
s = sock_l4(c, AF_INET, IPPROTO_UDP, addr, ifname,
port, uref.u32);
- udp_tap_map[V4][uref.udp.port].sock = s;
- udp_splice_init[V4][port].sock = s;
+ udp_tap_map[V4][uref.udp.port].sock = s < 0 ? -1 : s;
+ udp_splice_init[V4][port].sock = s < 0 ? -1 : s;
} else {
struct in_addr loopback = { htonl(INADDR_LOOPBACK) };
uref.udp.ns = true;
s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
ifname, port, uref.u32);
- udp_splice_ns[V4][port].sock = s;
+ udp_splice_ns[V4][port].sock = s < 0 ? -1 : s;
}
if (s < 0)
- ret = -1;
+ ret = s;
}
if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
@@ -1026,18 +1026,18 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr, ifname,
port, uref.u32);
- udp_tap_map[V6][uref.udp.port].sock = s;
- udp_splice_init[V6][port].sock = s;
+ udp_tap_map[V6][uref.udp.port].sock = s < 0 ? -1 : s;
+ udp_splice_init[V6][port].sock = s < 0 ? -1 : s;
} else {
uref.udp.ns = true;
s = sock_l4(c, AF_INET6, IPPROTO_UDP, &in6addr_loopback,
ifname, port, uref.u32);
- udp_splice_ns[V6][port].sock = s;
+ udp_splice_ns[V6][port].sock = s < 0 ? -1 : s;
}
if (s < 0)
- ret = -1;
+ ret = s;
}
return ret;
diff --git a/util.c b/util.c
index 484889b..fddb5ed 100644
--- a/util.c
+++ b/util.c
@@ -96,7 +96,7 @@ found:
* @port: Port, host order
* @data: epoll reference portion for protocol handlers
*
- * Return: newly created socket, -1 on error
+ * Return: newly created socket, negative error code on failure
*/
int sock_l4(const struct ctx *c, int af, uint8_t proto,
const void *bind_addr, const char *ifname, uint16_t port,
@@ -115,16 +115,16 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
};
const struct sockaddr *sa;
bool dual_stack = false;
+ int fd, sl, y = 1, ret;
struct epoll_event ev;
- int fd, sl, y = 1;
if (proto != IPPROTO_TCP && proto != IPPROTO_UDP &&
proto != IPPROTO_ICMP && proto != IPPROTO_ICMPV6)
- return -1; /* Not implemented. */
+ return -EPFNOSUPPORT; /* Not implemented. */
if (af == AF_UNSPEC) {
if (!DUAL_STACK_SOCKETS || bind_addr)
- return -1;
+ return -EINVAL;
dual_stack = true;
af = AF_INET6;
}
@@ -134,14 +134,15 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
else
fd = socket(af, SOCK_DGRAM | SOCK_NONBLOCK, proto);
+ ret = -errno;
if (fd < 0) {
- warn("L4 socket: %s", strerror(errno));
- return -1;
+ warn("L4 socket: %s", strerror(-ret));
+ return ret;
}
if (fd > SOCKET_MAX) {
close(fd);
- return -1;
+ return -EBADF;
}
ref.r.s = fd;
@@ -186,10 +187,11 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
*/
if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE,
ifname, strlen(ifname))) {
+ ret = -errno;
warn("Can't bind socket for %s port %u to %s, closing",
ip_proto_str[proto], port, ifname);
close(fd);
- return -1;
+ return ret;
}
}
@@ -200,22 +202,25 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
* broken SELinux policy, see icmp_tap_handler().
*/
if (proto != IPPROTO_ICMP && proto != IPPROTO_ICMPV6) {
+ ret = -errno;
close(fd);
- return -1;
+ return ret;
}
}
if (proto == IPPROTO_TCP && listen(fd, 128) < 0) {
- warn("TCP socket listen: %s", strerror(errno));
+ ret = -errno;
+ warn("TCP socket listen: %s", strerror(-ret));
close(fd);
- return -1;
+ return ret;
}
ev.events = EPOLLIN;
ev.data.u64 = ref.u64;
if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
- warn("L4 epoll_ctl: %s", strerror(errno));
- return -1;
+ ret = -errno;
+ warn("L4 epoll_ctl: %s", strerror(-ret));
+ return ret;
}
return fd;
--
@@ -96,7 +96,7 @@ found:
* @port: Port, host order
* @data: epoll reference portion for protocol handlers
*
- * Return: newly created socket, -1 on error
+ * Return: newly created socket, negative error code on failure
*/
int sock_l4(const struct ctx *c, int af, uint8_t proto,
const void *bind_addr, const char *ifname, uint16_t port,
@@ -115,16 +115,16 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
};
const struct sockaddr *sa;
bool dual_stack = false;
+ int fd, sl, y = 1, ret;
struct epoll_event ev;
- int fd, sl, y = 1;
if (proto != IPPROTO_TCP && proto != IPPROTO_UDP &&
proto != IPPROTO_ICMP && proto != IPPROTO_ICMPV6)
- return -1; /* Not implemented. */
+ return -EPFNOSUPPORT; /* Not implemented. */
if (af == AF_UNSPEC) {
if (!DUAL_STACK_SOCKETS || bind_addr)
- return -1;
+ return -EINVAL;
dual_stack = true;
af = AF_INET6;
}
@@ -134,14 +134,15 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
else
fd = socket(af, SOCK_DGRAM | SOCK_NONBLOCK, proto);
+ ret = -errno;
if (fd < 0) {
- warn("L4 socket: %s", strerror(errno));
- return -1;
+ warn("L4 socket: %s", strerror(-ret));
+ return ret;
}
if (fd > SOCKET_MAX) {
close(fd);
- return -1;
+ return -EBADF;
}
ref.r.s = fd;
@@ -186,10 +187,11 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
*/
if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE,
ifname, strlen(ifname))) {
+ ret = -errno;
warn("Can't bind socket for %s port %u to %s, closing",
ip_proto_str[proto], port, ifname);
close(fd);
- return -1;
+ return ret;
}
}
@@ -200,22 +202,25 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
* broken SELinux policy, see icmp_tap_handler().
*/
if (proto != IPPROTO_ICMP && proto != IPPROTO_ICMPV6) {
+ ret = -errno;
close(fd);
- return -1;
+ return ret;
}
}
if (proto == IPPROTO_TCP && listen(fd, 128) < 0) {
- warn("TCP socket listen: %s", strerror(errno));
+ ret = -errno;
+ warn("TCP socket listen: %s", strerror(-ret));
close(fd);
- return -1;
+ return ret;
}
ev.events = EPOLLIN;
ev.data.u64 = ref.u64;
if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
- warn("L4 epoll_ctl: %s", strerror(errno));
- return -1;
+ ret = -errno;
+ warn("L4 epoll_ctl: %s", strerror(-ret));
+ return ret;
}
return fd;
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] tcp, udp, util: Pass socket creation errors all the way up
2023-03-08 12:33 ` [PATCH 1/3] tcp, udp, util: Pass socket creation errors all the way up Stefano Brivio
@ 2023-03-08 22:16 ` David Gibson
0 siblings, 0 replies; 7+ messages in thread
From: David Gibson @ 2023-03-08 22:16 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 8118 bytes --]
On Wed, Mar 08, 2023 at 01:33:46PM +0100, Stefano Brivio wrote:
> ...starting from sock_l4(), pass negative error (errno) codes instead
> of -1. They will only be used in two commits from now, no functional
> changes intended here.
>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> tcp.c | 22 ++++++++++++----------
> udp.c | 18 +++++++++---------
> util.c | 31 ++++++++++++++++++-------------
> 3 files changed, 39 insertions(+), 32 deletions(-)
>
> diff --git a/tcp.c b/tcp.c
> index 96ca5c7..e209483 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2955,7 +2955,7 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
> * @addr: Pointer to address for binding, NULL if not configured
> * @ifname: Name of interface to bind to, NULL if not configured
> *
> - * Return: fd for the new listening socket, or -1 on failure
> + * Return: fd for the new listening socket, negative error code on failure
> */
> static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
> const struct in_addr *addr, const char *ifname)
> @@ -2968,13 +2968,13 @@ static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
>
> if (c->tcp.fwd_in.mode == FWD_AUTO) {
> if (af == AF_INET || af == AF_UNSPEC)
> - tcp_sock_init_ext[port][V4] = s;
> + tcp_sock_init_ext[port][V4] = s < 0 ? -1 : s;
> if (af == AF_INET6 || af == AF_UNSPEC)
> - tcp_sock_init_ext[port][V6] = s;
> + tcp_sock_init_ext[port][V6] = s < 0 ? -1 : s;
> }
>
> if (s < 0)
> - return -1;
> + return s;
>
> tcp_sock_set_bufsize(c, s);
> return s;
> @@ -2988,12 +2988,12 @@ static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
> * @ifname: Name of interface to bind to, NULL if not configured
> * @port: Port, host order
> *
> - * Return: 0 on (partial) success, -1 on (complete) failure
> + * 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,
> const char *ifname, in_port_t port)
> {
> - int ret = 0;
> + int ret = 0, af_ret;
>
> if (af == AF_UNSPEC && c->ifi4 && c->ifi6)
> /* Attempt to get a dual stack socket */
> @@ -3002,13 +3002,15 @@ int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
>
> /* Otherwise create a socket per IP version */
> if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
> - if (tcp_sock_init_af(c, AF_INET, port, addr, ifname) < 0)
> - ret = -1;
> + af_ret = tcp_sock_init_af(c, AF_INET, port, addr, ifname);
> + if (af_ret < 0)
> + ret = af_ret;
> }
>
> if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
> - if (tcp_sock_init_af(c, AF_INET6, port, addr, ifname) < 0)
> - ret = -1;
> + af_ret = tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
> + if (af_ret < 0)
> + ret = af_ret;
> }
>
> return ret;
> diff --git a/udp.c b/udp.c
> index 1077cde..41e0afd 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -977,7 +977,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
> * @ifname: Name of interface to bind to, NULL if not configured
> * @port: Port, host order
> *
> - * Return: 0 on (partial) success, -1 on (complete) failure
> + * 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)
> @@ -1002,19 +1002,19 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
> s = sock_l4(c, AF_INET, IPPROTO_UDP, addr, ifname,
> port, uref.u32);
>
> - udp_tap_map[V4][uref.udp.port].sock = s;
> - udp_splice_init[V4][port].sock = s;
> + udp_tap_map[V4][uref.udp.port].sock = s < 0 ? -1 : s;
> + udp_splice_init[V4][port].sock = s < 0 ? -1 : s;
> } else {
> struct in_addr loopback = { htonl(INADDR_LOOPBACK) };
> uref.udp.ns = true;
>
> s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
> ifname, port, uref.u32);
> - udp_splice_ns[V4][port].sock = s;
> + udp_splice_ns[V4][port].sock = s < 0 ? -1 : s;
> }
>
> if (s < 0)
> - ret = -1;
> + ret = s;
> }
>
> if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
> @@ -1026,18 +1026,18 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
> s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr, ifname,
> port, uref.u32);
>
> - udp_tap_map[V6][uref.udp.port].sock = s;
> - udp_splice_init[V6][port].sock = s;
> + udp_tap_map[V6][uref.udp.port].sock = s < 0 ? -1 : s;
> + udp_splice_init[V6][port].sock = s < 0 ? -1 : s;
> } else {
> uref.udp.ns = true;
>
> s = sock_l4(c, AF_INET6, IPPROTO_UDP, &in6addr_loopback,
> ifname, port, uref.u32);
> - udp_splice_ns[V6][port].sock = s;
> + udp_splice_ns[V6][port].sock = s < 0 ? -1 : s;
> }
>
> if (s < 0)
> - ret = -1;
> + ret = s;
> }
>
> return ret;
> diff --git a/util.c b/util.c
> index 484889b..fddb5ed 100644
> --- a/util.c
> +++ b/util.c
> @@ -96,7 +96,7 @@ found:
> * @port: Port, host order
> * @data: epoll reference portion for protocol handlers
> *
> - * Return: newly created socket, -1 on error
> + * Return: newly created socket, negative error code on failure
> */
> int sock_l4(const struct ctx *c, int af, uint8_t proto,
> const void *bind_addr, const char *ifname, uint16_t port,
> @@ -115,16 +115,16 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
> };
> const struct sockaddr *sa;
> bool dual_stack = false;
> + int fd, sl, y = 1, ret;
> struct epoll_event ev;
> - int fd, sl, y = 1;
>
> if (proto != IPPROTO_TCP && proto != IPPROTO_UDP &&
> proto != IPPROTO_ICMP && proto != IPPROTO_ICMPV6)
> - return -1; /* Not implemented. */
> + return -EPFNOSUPPORT; /* Not implemented. */
>
> if (af == AF_UNSPEC) {
> if (!DUAL_STACK_SOCKETS || bind_addr)
> - return -1;
> + return -EINVAL;
> dual_stack = true;
> af = AF_INET6;
> }
> @@ -134,14 +134,15 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
> else
> fd = socket(af, SOCK_DGRAM | SOCK_NONBLOCK, proto);
>
> + ret = -errno;
> if (fd < 0) {
> - warn("L4 socket: %s", strerror(errno));
> - return -1;
> + warn("L4 socket: %s", strerror(-ret));
> + return ret;
> }
>
> if (fd > SOCKET_MAX) {
> close(fd);
> - return -1;
> + return -EBADF;
> }
>
> ref.r.s = fd;
> @@ -186,10 +187,11 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
> */
> if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE,
> ifname, strlen(ifname))) {
> + ret = -errno;
> warn("Can't bind socket for %s port %u to %s, closing",
> ip_proto_str[proto], port, ifname);
> close(fd);
> - return -1;
> + return ret;
> }
> }
>
> @@ -200,22 +202,25 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
> * broken SELinux policy, see icmp_tap_handler().
> */
> if (proto != IPPROTO_ICMP && proto != IPPROTO_ICMPV6) {
> + ret = -errno;
> close(fd);
> - return -1;
> + return ret;
> }
> }
>
> if (proto == IPPROTO_TCP && listen(fd, 128) < 0) {
> - warn("TCP socket listen: %s", strerror(errno));
> + ret = -errno;
> + warn("TCP socket listen: %s", strerror(-ret));
> close(fd);
> - return -1;
> + return ret;
> }
>
> ev.events = EPOLLIN;
> ev.data.u64 = ref.u64;
> if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
> - warn("L4 epoll_ctl: %s", strerror(errno));
> - return -1;
> + ret = -errno;
> + warn("L4 epoll_ctl: %s", strerror(-ret));
> + return ret;
> }
>
> return fd;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] tcp, udp: Fix partial success return codes in {tcp,udp}_sock_init()
2023-03-08 12:33 [PATCH 0/3] Fail gracefully on too many open files Stefano Brivio
2023-03-08 12:33 ` [PATCH 1/3] tcp, udp, util: Pass socket creation errors all the way up Stefano Brivio
@ 2023-03-08 12:33 ` Stefano Brivio
2023-03-08 22:43 ` David Gibson
2023-03-08 12:33 ` [PATCH 3/3] conf: Terminate on EMFILE or ENFILE on sockets for port mapping Stefano Brivio
2 siblings, 1 reply; 7+ messages in thread
From: Stefano Brivio @ 2023-03-08 12:33 UTC (permalink / raw)
To: passt-dev
The comments say we should return 0 on partial success, and an error
code on complete failure. Rationale: if the user configures a port
forwarding, and we succeed to bind that port for IPv4 or IPv6 only,
that might actually be what the user intended.
Adjust the two functions to reflect the comments.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
tcp.c | 21 +++++++++------------
udp.c | 30 ++++++++++++++----------------
2 files changed, 23 insertions(+), 28 deletions(-)
diff --git a/tcp.c b/tcp.c
index e209483..a29e387 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2993,7 +2993,7 @@ static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
const char *ifname, in_port_t port)
{
- int ret = 0, af_ret;
+ int r4 = SOCKET_MAX + 1, r6 = SOCKET_MAX + 1;
if (af == AF_UNSPEC && c->ifi4 && c->ifi6)
/* Attempt to get a dual stack socket */
@@ -3001,19 +3001,16 @@ int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
return 0;
/* Otherwise create a socket per IP version */
- if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
- af_ret = tcp_sock_init_af(c, AF_INET, port, addr, ifname);
- if (af_ret < 0)
- ret = af_ret;
- }
+ if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4)
+ r4 = tcp_sock_init_af(c, AF_INET, port, addr, ifname);
- if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
- af_ret = tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
- if (af_ret < 0)
- ret = af_ret;
- }
+ if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6)
+ r6 = tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
- return ret;
+ if (IN_INTERVAL(0, SOCKET_MAX, r4) || IN_INTERVAL(0, SOCKET_MAX, r6))
+ return 0;
+
+ return r4 < 0 ? r4 : r6;
}
/**
diff --git a/udp.c b/udp.c
index 41e0afd..a5f7537 100644
--- a/udp.c
+++ b/udp.c
@@ -983,7 +983,7 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
const void *addr, const char *ifname, in_port_t port)
{
union udp_epoll_ref uref = { .u32 = 0 };
- int s, ret = 0;
+ int s, r4 = SOCKET_MAX + 1, r6 = SOCKET_MAX + 1;
if (ns) {
uref.udp.port = (in_port_t)(port +
@@ -999,8 +999,8 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
uref.udp.orig = true;
if (!ns) {
- s = sock_l4(c, AF_INET, IPPROTO_UDP, addr, ifname,
- port, uref.u32);
+ r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, addr,
+ ifname, port, uref.u32);
udp_tap_map[V4][uref.udp.port].sock = s < 0 ? -1 : s;
udp_splice_init[V4][port].sock = s < 0 ? -1 : s;
@@ -1008,13 +1008,10 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
struct in_addr loopback = { htonl(INADDR_LOOPBACK) };
uref.udp.ns = true;
- s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
- ifname, port, uref.u32);
+ r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
+ ifname, port, uref.u32);
udp_splice_ns[V4][port].sock = s < 0 ? -1 : s;
}
-
- if (s < 0)
- ret = s;
}
if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
@@ -1023,24 +1020,25 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
uref.udp.orig = true;
if (!ns) {
- s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr, ifname,
- port, uref.u32);
+ r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr,
+ ifname, port, uref.u32);
udp_tap_map[V6][uref.udp.port].sock = s < 0 ? -1 : s;
udp_splice_init[V6][port].sock = s < 0 ? -1 : s;
} else {
uref.udp.ns = true;
- s = sock_l4(c, AF_INET6, IPPROTO_UDP, &in6addr_loopback,
- ifname, port, uref.u32);
+ r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP,
+ &in6addr_loopback,
+ ifname, port, uref.u32);
udp_splice_ns[V6][port].sock = s < 0 ? -1 : s;
}
-
- if (s < 0)
- ret = s;
}
- return ret;
+ if (IN_INTERVAL(0, SOCKET_MAX, r4) || IN_INTERVAL(0, SOCKET_MAX, r6))
+ return 0;
+
+ return r4 < 0 ? r4 : r6;
}
/**
--
@@ -983,7 +983,7 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
const void *addr, const char *ifname, in_port_t port)
{
union udp_epoll_ref uref = { .u32 = 0 };
- int s, ret = 0;
+ int s, r4 = SOCKET_MAX + 1, r6 = SOCKET_MAX + 1;
if (ns) {
uref.udp.port = (in_port_t)(port +
@@ -999,8 +999,8 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
uref.udp.orig = true;
if (!ns) {
- s = sock_l4(c, AF_INET, IPPROTO_UDP, addr, ifname,
- port, uref.u32);
+ r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, addr,
+ ifname, port, uref.u32);
udp_tap_map[V4][uref.udp.port].sock = s < 0 ? -1 : s;
udp_splice_init[V4][port].sock = s < 0 ? -1 : s;
@@ -1008,13 +1008,10 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
struct in_addr loopback = { htonl(INADDR_LOOPBACK) };
uref.udp.ns = true;
- s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
- ifname, port, uref.u32);
+ r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
+ ifname, port, uref.u32);
udp_splice_ns[V4][port].sock = s < 0 ? -1 : s;
}
-
- if (s < 0)
- ret = s;
}
if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
@@ -1023,24 +1020,25 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
uref.udp.orig = true;
if (!ns) {
- s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr, ifname,
- port, uref.u32);
+ r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr,
+ ifname, port, uref.u32);
udp_tap_map[V6][uref.udp.port].sock = s < 0 ? -1 : s;
udp_splice_init[V6][port].sock = s < 0 ? -1 : s;
} else {
uref.udp.ns = true;
- s = sock_l4(c, AF_INET6, IPPROTO_UDP, &in6addr_loopback,
- ifname, port, uref.u32);
+ r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP,
+ &in6addr_loopback,
+ ifname, port, uref.u32);
udp_splice_ns[V6][port].sock = s < 0 ? -1 : s;
}
-
- if (s < 0)
- ret = s;
}
- return ret;
+ if (IN_INTERVAL(0, SOCKET_MAX, r4) || IN_INTERVAL(0, SOCKET_MAX, r6))
+ return 0;
+
+ return r4 < 0 ? r4 : r6;
}
/**
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] tcp, udp: Fix partial success return codes in {tcp,udp}_sock_init()
2023-03-08 12:33 ` [PATCH 2/3] tcp, udp: Fix partial success return codes in {tcp,udp}_sock_init() Stefano Brivio
@ 2023-03-08 22:43 ` David Gibson
0 siblings, 0 replies; 7+ messages in thread
From: David Gibson @ 2023-03-08 22:43 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 4713 bytes --]
On Wed, Mar 08, 2023 at 01:33:47PM +0100, Stefano Brivio wrote:
> The comments say we should return 0 on partial success, and an error
> code on complete failure. Rationale: if the user configures a port
> forwarding, and we succeed to bind that port for IPv4 or IPv6 only,
> that might actually be what the user intended.
>
> Adjust the two functions to reflect the comments.
>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> tcp.c | 21 +++++++++------------
> udp.c | 30 ++++++++++++++----------------
> 2 files changed, 23 insertions(+), 28 deletions(-)
>
> diff --git a/tcp.c b/tcp.c
> index e209483..a29e387 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2993,7 +2993,7 @@ static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
> int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
> const char *ifname, in_port_t port)
> {
> - int ret = 0, af_ret;
> + int r4 = SOCKET_MAX + 1, r6 = SOCKET_MAX + 1;
>
> if (af == AF_UNSPEC && c->ifi4 && c->ifi6)
> /* Attempt to get a dual stack socket */
> @@ -3001,19 +3001,16 @@ int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
> return 0;
>
> /* Otherwise create a socket per IP version */
> - if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
> - af_ret = tcp_sock_init_af(c, AF_INET, port, addr, ifname);
> - if (af_ret < 0)
> - ret = af_ret;
> - }
> + if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4)
> + r4 = tcp_sock_init_af(c, AF_INET, port, addr, ifname);
>
> - if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
> - af_ret = tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
> - if (af_ret < 0)
> - ret = af_ret;
> - }
> + if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6)
> + r6 = tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
>
> - return ret;
> + if (IN_INTERVAL(0, SOCKET_MAX, r4) || IN_INTERVAL(0, SOCKET_MAX, r6))
> + return 0;
> +
> + return r4 < 0 ? r4 : r6;
> }
>
> /**
> diff --git a/udp.c b/udp.c
> index 41e0afd..a5f7537 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -983,7 +983,7 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
> const void *addr, const char *ifname, in_port_t port)
> {
> union udp_epoll_ref uref = { .u32 = 0 };
> - int s, ret = 0;
> + int s, r4 = SOCKET_MAX + 1, r6 = SOCKET_MAX + 1;
>
> if (ns) {
> uref.udp.port = (in_port_t)(port +
> @@ -999,8 +999,8 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
> uref.udp.orig = true;
>
> if (!ns) {
> - s = sock_l4(c, AF_INET, IPPROTO_UDP, addr, ifname,
> - port, uref.u32);
> + r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, addr,
> + ifname, port, uref.u32);
>
> udp_tap_map[V4][uref.udp.port].sock = s < 0 ? -1 : s;
> udp_splice_init[V4][port].sock = s < 0 ? -1 : s;
> @@ -1008,13 +1008,10 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
> struct in_addr loopback = { htonl(INADDR_LOOPBACK) };
> uref.udp.ns = true;
>
> - s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
> - ifname, port, uref.u32);
> + r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
> + ifname, port, uref.u32);
> udp_splice_ns[V4][port].sock = s < 0 ? -1 : s;
> }
> -
> - if (s < 0)
> - ret = s;
> }
>
> if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
> @@ -1023,24 +1020,25 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
> uref.udp.orig = true;
>
> if (!ns) {
> - s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr, ifname,
> - port, uref.u32);
> + r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr,
> + ifname, port, uref.u32);
>
> udp_tap_map[V6][uref.udp.port].sock = s < 0 ? -1 : s;
> udp_splice_init[V6][port].sock = s < 0 ? -1 : s;
> } else {
> uref.udp.ns = true;
>
> - s = sock_l4(c, AF_INET6, IPPROTO_UDP, &in6addr_loopback,
> - ifname, port, uref.u32);
> + r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP,
> + &in6addr_loopback,
> + ifname, port, uref.u32);
> udp_splice_ns[V6][port].sock = s < 0 ? -1 : s;
> }
> -
> - if (s < 0)
> - ret = s;
> }
>
> - return ret;
> + if (IN_INTERVAL(0, SOCKET_MAX, r4) || IN_INTERVAL(0, SOCKET_MAX, r6))
> + return 0;
> +
> + return r4 < 0 ? r4 : r6;
Same comment (unless sock_l4() already looks for
> }
>
> /**
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] conf: Terminate on EMFILE or ENFILE on sockets for port mapping
2023-03-08 12:33 [PATCH 0/3] Fail gracefully on too many open files Stefano Brivio
2023-03-08 12:33 ` [PATCH 1/3] tcp, udp, util: Pass socket creation errors all the way up Stefano Brivio
2023-03-08 12:33 ` [PATCH 2/3] tcp, udp: Fix partial success return codes in {tcp,udp}_sock_init() Stefano Brivio
@ 2023-03-08 12:33 ` Stefano Brivio
2023-03-08 22:45 ` David Gibson
2 siblings, 1 reply; 7+ messages in thread
From: Stefano Brivio @ 2023-03-08 12:33 UTC (permalink / raw)
To: passt-dev
In general, we don't terminate or report failures if we fail to bind
to some ports out of a given port range specifier, to allow users to
conveniently specify big port ranges (or "all") without having to
care about ports that might already be in use.
However, running out of the open file descriptors quota is a
different story: we can't do what the user requested in a very
substantial way.
For example, if the user specifies '-t all' and we can only bind
1024 sockets, the behaviour is rather unexpected.
Fail whenever socket creation returns -ENFILE or -EMFILE.
Link: https://bugs.passt.top/show_bug.cgi?id=27
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
conf.c | 36 +++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/conf.c b/conf.c
index 582c391..ee56501 100644
--- a/conf.c
+++ b/conf.c
@@ -184,6 +184,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
bool exclude_only = true, bound_one = false;
uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
sa_family_t af = AF_UNSPEC;
+ int ret;
if (!strcmp(optarg, "none")) {
if (fwd->mode)
@@ -218,11 +219,18 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
for (i = 0; i < PORT_EPHEMERAL_MIN; i++) {
if (optname == 't') {
- if (!tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i))
+ ret = tcp_sock_init(c, AF_UNSPEC, NULL, NULL,
+ i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
} else if (optname == 'u') {
- if (!udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL,
- i))
+ ret = udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL,
+ i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
}
}
@@ -303,10 +311,16 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
bitmap_set(fwd->map, i);
if (optname == 't') {
- if (!tcp_sock_init(c, af, addr, ifname, i))
+ ret = tcp_sock_init(c, af, addr, ifname, i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
} else if (optname == 'u') {
- if (!udp_sock_init(c, 0, af, addr, ifname, i))
+ ret = udp_sock_init(c, 0, af, addr, ifname, i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
} else {
/* No way to check in advance for -T and -U */
@@ -358,10 +372,16 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
fwd->delta[i] = mapped_range.first - orig_range.first;
if (optname == 't') {
- if (!tcp_sock_init(c, af, addr, ifname, i))
+ ret = tcp_sock_init(c, af, addr, ifname, i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
} else if (optname == 'u') {
- if (!udp_sock_init(c, 0, af, addr, ifname, i))
+ ret = udp_sock_init(c, 0, af, addr, ifname, i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
} else {
/* No way to check in advance for -T and -U */
@@ -374,6 +394,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
goto bind_fail;
return;
+enfile:
+ die("Can't open enough sockets for port specifier: %s", optarg);
bad:
die("Invalid port specifier %s", optarg);
overlap:
--
@@ -184,6 +184,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
bool exclude_only = true, bound_one = false;
uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
sa_family_t af = AF_UNSPEC;
+ int ret;
if (!strcmp(optarg, "none")) {
if (fwd->mode)
@@ -218,11 +219,18 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
for (i = 0; i < PORT_EPHEMERAL_MIN; i++) {
if (optname == 't') {
- if (!tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i))
+ ret = tcp_sock_init(c, AF_UNSPEC, NULL, NULL,
+ i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
} else if (optname == 'u') {
- if (!udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL,
- i))
+ ret = udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL,
+ i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
}
}
@@ -303,10 +311,16 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
bitmap_set(fwd->map, i);
if (optname == 't') {
- if (!tcp_sock_init(c, af, addr, ifname, i))
+ ret = tcp_sock_init(c, af, addr, ifname, i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
} else if (optname == 'u') {
- if (!udp_sock_init(c, 0, af, addr, ifname, i))
+ ret = udp_sock_init(c, 0, af, addr, ifname, i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
} else {
/* No way to check in advance for -T and -U */
@@ -358,10 +372,16 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
fwd->delta[i] = mapped_range.first - orig_range.first;
if (optname == 't') {
- if (!tcp_sock_init(c, af, addr, ifname, i))
+ ret = tcp_sock_init(c, af, addr, ifname, i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
} else if (optname == 'u') {
- if (!udp_sock_init(c, 0, af, addr, ifname, i))
+ ret = udp_sock_init(c, 0, af, addr, ifname, i);
+ if (ret == -ENFILE || ret == -EMFILE)
+ goto enfile;
+ if (!ret)
bound_one = true;
} else {
/* No way to check in advance for -T and -U */
@@ -374,6 +394,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
goto bind_fail;
return;
+enfile:
+ die("Can't open enough sockets for port specifier: %s", optarg);
bad:
die("Invalid port specifier %s", optarg);
overlap:
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] conf: Terminate on EMFILE or ENFILE on sockets for port mapping
2023-03-08 12:33 ` [PATCH 3/3] conf: Terminate on EMFILE or ENFILE on sockets for port mapping Stefano Brivio
@ 2023-03-08 22:45 ` David Gibson
0 siblings, 0 replies; 7+ messages in thread
From: David Gibson @ 2023-03-08 22:45 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 4060 bytes --]
On Wed, Mar 08, 2023 at 01:33:48PM +0100, Stefano Brivio wrote:
> In general, we don't terminate or report failures if we fail to bind
> to some ports out of a given port range specifier, to allow users to
> conveniently specify big port ranges (or "all") without having to
> care about ports that might already be in use.
>
> However, running out of the open file descriptors quota is a
> different story: we can't do what the user requested in a very
> substantial way.
>
> For example, if the user specifies '-t all' and we can only bind
> 1024 sockets, the behaviour is rather unexpected.
>
> Fail whenever socket creation returns -ENFILE or -EMFILE.
>
> Link: https://bugs.passt.top/show_bug.cgi?id=27
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> conf.c | 36 +++++++++++++++++++++++++++++-------
> 1 file changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/conf.c b/conf.c
> index 582c391..ee56501 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -184,6 +184,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
> bool exclude_only = true, bound_one = false;
> uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
> sa_family_t af = AF_UNSPEC;
> + int ret;
>
> if (!strcmp(optarg, "none")) {
> if (fwd->mode)
> @@ -218,11 +219,18 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>
> for (i = 0; i < PORT_EPHEMERAL_MIN; i++) {
> if (optname == 't') {
> - if (!tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i))
> + ret = tcp_sock_init(c, AF_UNSPEC, NULL, NULL,
> + i);
> + if (ret == -ENFILE || ret == -EMFILE)
> + goto enfile;
> + if (!ret)
> bound_one = true;
> } else if (optname == 'u') {
> - if (!udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL,
> - i))
> + ret = udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL,
> + i);
> + if (ret == -ENFILE || ret == -EMFILE)
> + goto enfile;
> + if (!ret)
> bound_one = true;
> }
> }
> @@ -303,10 +311,16 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
> bitmap_set(fwd->map, i);
>
> if (optname == 't') {
> - if (!tcp_sock_init(c, af, addr, ifname, i))
> + ret = tcp_sock_init(c, af, addr, ifname, i);
> + if (ret == -ENFILE || ret == -EMFILE)
> + goto enfile;
> + if (!ret)
> bound_one = true;
> } else if (optname == 'u') {
> - if (!udp_sock_init(c, 0, af, addr, ifname, i))
> + ret = udp_sock_init(c, 0, af, addr, ifname, i);
> + if (ret == -ENFILE || ret == -EMFILE)
> + goto enfile;
> + if (!ret)
> bound_one = true;
> } else {
> /* No way to check in advance for -T and -U */
> @@ -358,10 +372,16 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
> fwd->delta[i] = mapped_range.first - orig_range.first;
>
> if (optname == 't') {
> - if (!tcp_sock_init(c, af, addr, ifname, i))
> + ret = tcp_sock_init(c, af, addr, ifname, i);
> + if (ret == -ENFILE || ret == -EMFILE)
> + goto enfile;
> + if (!ret)
> bound_one = true;
> } else if (optname == 'u') {
> - if (!udp_sock_init(c, 0, af, addr, ifname, i))
> + ret = udp_sock_init(c, 0, af, addr, ifname, i);
> + if (ret == -ENFILE || ret == -EMFILE)
> + goto enfile;
> + if (!ret)
> bound_one = true;
> } else {
> /* No way to check in advance for -T and -U */
> @@ -374,6 +394,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
> goto bind_fail;
>
> return;
> +enfile:
> + die("Can't open enough sockets for port specifier: %s", optarg);
> bad:
> die("Invalid port specifier %s", optarg);
> overlap:
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread