* [PATCH] conf, tcp, udp: Exit if we fail to bind sockets for all given ports
@ 2023-02-16 1:07 Stefano Brivio
2023-02-16 4:42 ` David Gibson
0 siblings, 1 reply; 2+ messages in thread
From: Stefano Brivio @ 2023-02-16 1:07 UTC (permalink / raw)
To: passt-dev; +Cc: Yalan Zhang
passt supports ranges of forwarded ports as well as 'all' for TCP and
UDP, so it might be convenient to proceed if we fail to bind only
some of the desired ports.
But if we fail to bind even a single port for a given specification,
we're clearly, unexpectedly, conflicting with another network
service. In that case, report failure and exit.
Reported-by: Yalan Zhang <yalzhang@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
conf.c | 47 ++++++++++++++++++++++++++++++++++-------------
tcp.c | 25 ++++++++++++++++++-------
tcp.h | 4 ++--
udp.c | 16 +++++++++++++---
udp.h | 4 ++--
5 files changed, 69 insertions(+), 27 deletions(-)
diff --git a/conf.c b/conf.c
index f175405..675d961 100644
--- a/conf.c
+++ b/conf.c
@@ -179,9 +179,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
{
char addr_buf[sizeof(struct in6_addr)] = { 0 }, *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;
- bool exclude_only = true;
if (!strcmp(optarg, "none")) {
if (fwd->mode)
@@ -215,12 +215,19 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
memset(fwd->map, 0xff, PORT_EPHEMERAL_MIN / 8);
for (i = 0; i < PORT_EPHEMERAL_MIN; i++) {
- if (optname == 't')
- tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i);
- else if (optname == 'u')
- udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, i);
+ if (optname == 't') {
+ if (!tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i))
+ bound_one = true;
+ } else if (optname == 'u') {
+ if (!udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL,
+ i))
+ bound_one = true;
+ }
}
+ if (!bound_one)
+ goto bind_fail;
+
return;
}
@@ -293,12 +300,18 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
bitmap_set(fwd->map, i);
- if (optname == 't')
- tcp_sock_init(c, af, addr, ifname, i);
- else if (optname == 'u')
- udp_sock_init(c, 0, af, addr, ifname, i);
+ if (optname == 't') {
+ if (!tcp_sock_init(c, af, addr, ifname, i))
+ bound_one = true;
+ } else if (optname == 'u') {
+ if (!udp_sock_init(c, 0, af, addr, ifname, i))
+ bound_one = true;
+ }
}
+ if (!bound_one)
+ goto bind_fail;
+
return;
}
@@ -339,13 +352,19 @@ 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')
- tcp_sock_init(c, af, addr, ifname, i);
- else if (optname == 'u')
- udp_sock_init(c, 0, af, addr, ifname, i);
+ if (optname == 't') {
+ if (!tcp_sock_init(c, af, addr, ifname, i))
+ bound_one = true;
+ } else if (optname == 'u') {
+ if (udp_sock_init(c, 0, af, addr, ifname, i))
+ bound_one = true;
+ }
}
} while ((p = next_chunk(p, ',')));
+ if (!bound_one)
+ goto bind_fail;
+
return;
bad:
die("Invalid port specifier %s", optarg);
@@ -353,6 +372,8 @@ overlap:
die("Overlapping port specifier %s", optarg);
mode_conflict:
die("Port forwarding mode '%s' conflicts with previous mode", optarg);
+bind_fail:
+ die("Failed to bind any port for '-%c %s', exiting", optname, optarg);
}
/**
diff --git a/tcp.c b/tcp.c
index f028e01..32ce1e9 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2916,20 +2916,31 @@ static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
* @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, -1 on (complete) failure
*/
-void tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
- const char *ifname, 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;
+
if (af == AF_UNSPEC && c->ifi4 && c->ifi6)
/* Attempt to get a dual stack socket */
if (tcp_sock_init_af(c, AF_UNSPEC, port, addr, ifname) >= 0)
- return;
+ return 0;
/* Otherwise create a socket per IP version */
- if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4)
- tcp_sock_init_af(c, AF_INET, port, addr, ifname);
- if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6)
- tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
+ if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
+ if (tcp_sock_init_af(c, AF_INET, port, addr, ifname) < 0)
+ ret = -1;
+ }
+
+ if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
+ if (tcp_sock_init_af(c, AF_INET6, port, addr, ifname) < 0)
+ ret = -1;
+ }
+
+ return ret;
}
/**
diff --git a/tcp.h b/tcp.h
index 236038f..5527c5b 100644
--- a/tcp.h
+++ b/tcp.h
@@ -17,8 +17,8 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
const struct timespec *now);
int tcp_tap_handler(struct ctx *c, int af, const void *addr,
const struct pool *p, const struct timespec *now);
-void tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
- const char *ifname, 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 tcp_init(struct ctx *c);
void tcp_timer(struct ctx *c, const struct timespec *ts);
void tcp_defer_handler(struct ctx *c);
diff --git a/udp.c b/udp.c
index 2e9b7e6..c913d27 100644
--- a/udp.c
+++ b/udp.c
@@ -955,12 +955,14 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
* @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, -1 on (complete) failure
*/
-void 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, sa_family_t af,
+ const void *addr, const char *ifname, in_port_t port)
{
union udp_epoll_ref uref = { .u32 = 0 };
- int s;
+ int s, ret = 0;
if (ns) {
uref.udp.port = (in_port_t)(port +
@@ -989,6 +991,9 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
ifname, port, uref.u32);
udp_splice_ns[V4][port].sock = s;
}
+
+ if (s < 0)
+ ret = -1;
}
if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
@@ -1009,7 +1014,12 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
ifname, port, uref.u32);
udp_splice_ns[V6][port].sock = s;
}
+
+ if (s < 0)
+ ret = -1;
}
+
+ return ret;
}
/**
diff --git a/udp.h b/udp.h
index 68082ea..0e81be8 100644
--- a/udp.h
+++ b/udp.h
@@ -12,8 +12,8 @@ void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
const struct timespec *now);
int udp_tap_handler(struct ctx *c, int af, const void *addr,
const struct pool *p, const struct timespec *now);
-void 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, sa_family_t af,
+ const void *addr, const char *ifname, in_port_t port);
int udp_init(struct ctx *c);
void udp_timer(struct ctx *c, const struct timespec *ts);
void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s,
--
@@ -12,8 +12,8 @@ void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
const struct timespec *now);
int udp_tap_handler(struct ctx *c, int af, const void *addr,
const struct pool *p, const struct timespec *now);
-void 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, sa_family_t af,
+ const void *addr, const char *ifname, in_port_t port);
int udp_init(struct ctx *c);
void udp_timer(struct ctx *c, const struct timespec *ts);
void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s,
--
2.35.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] conf, tcp, udp: Exit if we fail to bind sockets for all given ports
2023-02-16 1:07 [PATCH] conf, tcp, udp: Exit if we fail to bind sockets for all given ports Stefano Brivio
@ 2023-02-16 4:42 ` David Gibson
0 siblings, 0 replies; 2+ messages in thread
From: David Gibson @ 2023-02-16 4:42 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev, Yalan Zhang
[-- Attachment #1: Type: text/plain, Size: 8411 bytes --]
On Thu, Feb 16, 2023 at 02:07:20AM +0100, Stefano Brivio wrote:
> passt supports ranges of forwarded ports as well as 'all' for TCP and
> UDP, so it might be convenient to proceed if we fail to bind only
> some of the desired ports.
>
> But if we fail to bind even a single port for a given specification,
> we're clearly, unexpectedly, conflicting with another network
> service. In that case, report failure and exit.
>
> Reported-by: Yalan Zhang <yalzhang@redhat.com>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> conf.c | 47 ++++++++++++++++++++++++++++++++++-------------
> tcp.c | 25 ++++++++++++++++++-------
> tcp.h | 4 ++--
> udp.c | 16 +++++++++++++---
> udp.h | 4 ++--
> 5 files changed, 69 insertions(+), 27 deletions(-)
>
> diff --git a/conf.c b/conf.c
> index f175405..675d961 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -179,9 +179,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
> {
> char addr_buf[sizeof(struct in6_addr)] = { 0 }, *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;
> - bool exclude_only = true;
>
> if (!strcmp(optarg, "none")) {
> if (fwd->mode)
> @@ -215,12 +215,19 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
> memset(fwd->map, 0xff, PORT_EPHEMERAL_MIN / 8);
>
> for (i = 0; i < PORT_EPHEMERAL_MIN; i++) {
> - if (optname == 't')
> - tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i);
> - else if (optname == 'u')
> - udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, i);
> + if (optname == 't') {
> + if (!tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i))
> + bound_one = true;
> + } else if (optname == 'u') {
> + if (!udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL,
> + i))
> + bound_one = true;
> + }
> }
>
> + if (!bound_one)
> + goto bind_fail;
> +
> return;
> }
>
> @@ -293,12 +300,18 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>
> bitmap_set(fwd->map, i);
>
> - if (optname == 't')
> - tcp_sock_init(c, af, addr, ifname, i);
> - else if (optname == 'u')
> - udp_sock_init(c, 0, af, addr, ifname, i);
> + if (optname == 't') {
> + if (!tcp_sock_init(c, af, addr, ifname, i))
> + bound_one = true;
> + } else if (optname == 'u') {
> + if (!udp_sock_init(c, 0, af, addr, ifname, i))
> + bound_one = true;
> + }
> }
>
> + if (!bound_one)
> + goto bind_fail;
> +
> return;
> }
>
> @@ -339,13 +352,19 @@ 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')
> - tcp_sock_init(c, af, addr, ifname, i);
> - else if (optname == 'u')
> - udp_sock_init(c, 0, af, addr, ifname, i);
> + if (optname == 't') {
> + if (!tcp_sock_init(c, af, addr, ifname, i))
> + bound_one = true;
> + } else if (optname == 'u') {
> + if (udp_sock_init(c, 0, af, addr, ifname, i))
> + bound_one = true;
> + }
> }
> } while ((p = next_chunk(p, ',')));
>
> + if (!bound_one)
> + goto bind_fail;
> +
> return;
> bad:
> die("Invalid port specifier %s", optarg);
> @@ -353,6 +372,8 @@ overlap:
> die("Overlapping port specifier %s", optarg);
> mode_conflict:
> die("Port forwarding mode '%s' conflicts with previous mode", optarg);
> +bind_fail:
> + die("Failed to bind any port for '-%c %s', exiting", optname, optarg);
> }
>
> /**
> diff --git a/tcp.c b/tcp.c
> index f028e01..32ce1e9 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2916,20 +2916,31 @@ static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
> * @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, -1 on (complete) failure
> */
> -void tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
> - const char *ifname, 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;
> +
> if (af == AF_UNSPEC && c->ifi4 && c->ifi6)
> /* Attempt to get a dual stack socket */
> if (tcp_sock_init_af(c, AF_UNSPEC, port, addr, ifname) >= 0)
> - return;
> + return 0;
>
> /* Otherwise create a socket per IP version */
> - if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4)
> - tcp_sock_init_af(c, AF_INET, port, addr, ifname);
> - if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6)
> - tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
> + if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
> + if (tcp_sock_init_af(c, AF_INET, port, addr, ifname) < 0)
> + ret = -1;
> + }
> +
> + if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
> + if (tcp_sock_init_af(c, AF_INET6, port, addr, ifname) < 0)
> + ret = -1;
> + }
> +
> + return ret;
> }
>
> /**
> diff --git a/tcp.h b/tcp.h
> index 236038f..5527c5b 100644
> --- a/tcp.h
> +++ b/tcp.h
> @@ -17,8 +17,8 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
> const struct timespec *now);
> int tcp_tap_handler(struct ctx *c, int af, const void *addr,
> const struct pool *p, const struct timespec *now);
> -void tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
> - const char *ifname, 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 tcp_init(struct ctx *c);
> void tcp_timer(struct ctx *c, const struct timespec *ts);
> void tcp_defer_handler(struct ctx *c);
> diff --git a/udp.c b/udp.c
> index 2e9b7e6..c913d27 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -955,12 +955,14 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
> * @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, -1 on (complete) failure
> */
> -void 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, sa_family_t af,
> + const void *addr, const char *ifname, in_port_t port)
> {
> union udp_epoll_ref uref = { .u32 = 0 };
> - int s;
> + int s, ret = 0;
>
> if (ns) {
> uref.udp.port = (in_port_t)(port +
> @@ -989,6 +991,9 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
> ifname, port, uref.u32);
> udp_splice_ns[V4][port].sock = s;
> }
> +
> + if (s < 0)
> + ret = -1;
> }
>
> if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
> @@ -1009,7 +1014,12 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
> ifname, port, uref.u32);
> udp_splice_ns[V6][port].sock = s;
> }
> +
> + if (s < 0)
> + ret = -1;
> }
> +
> + return ret;
> }
>
> /**
> diff --git a/udp.h b/udp.h
> index 68082ea..0e81be8 100644
> --- a/udp.h
> +++ b/udp.h
> @@ -12,8 +12,8 @@ void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
> const struct timespec *now);
> int udp_tap_handler(struct ctx *c, int af, const void *addr,
> const struct pool *p, const struct timespec *now);
> -void 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, sa_family_t af,
> + const void *addr, const char *ifname, in_port_t port);
> int udp_init(struct ctx *c);
> void udp_timer(struct ctx *c, const struct timespec *ts);
> void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s,
--
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] 2+ messages in thread
end of thread, other threads:[~2023-02-16 5:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-16 1:07 [PATCH] conf, tcp, udp: Exit if we fail to bind sockets for all given ports Stefano Brivio
2023-02-16 4:42 ` David Gibson
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).