[PATCH 0/9] Clean ups to automatic port forwarding

older
[PATCH] log: Don't define logging...

David Gibson

5 Oct 2023 5 Oct '23

5:44 a.m.

This contains an assortment of cleanups to the code supporting the automatic port forwarding more for past - specifically get_bound_ports() and related functions. This shouldn't mae any functional changes. Based on the series with fixes for new cppcheck-2.12 warnings. David Gibson (9): conf: Cleaner initialisation of default forwarding modes port_fwd: Move automatic port forwarding code to port_fwd.[ch] port_fwd: Better parameterise procfs_scan_listen() util: Add open_in_ns() helper port_fwd: Pre-open /proc/net/* files rather than on-demand port_fwd: Don't NS_CALL get_bound_ports() port_fwd: Split TCP and UDP cases for get_bound_ports() port_fwd: Move port scanning /proc fds into struct port_fwd port_fwd: Simplify get_bound_ports_*() to port_fwd_scan_*() Makefile | 2 +- conf.c | 113 +++++-------------------------------------- conf.h | 1 - passt.h | 5 -- port_fwd.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++++ port_fwd.h | 9 ++++ tcp.c | 39 +-------------- util.c | 104 +++++++++++++++------------------------ util.h | 3 +- 9 files changed, 204 insertions(+), 211 deletions(-) create mode 100644 port_fwd.c -- 2.41.0

Show replies by date

David Gibson

5 Oct 5 Oct

5:44 a.m.

New subject: [PATCH 1/9] conf: Cleaner initialisation of default forwarding modes

Initialisation of the forwarding mode variables is complicated a bit by the fact that we have different defaults for passt and pasta modes. This leads to some debateably duplicated code between those two cases. More significantly, however, currently the final setting of the mode variable is interleaved with the code to actually start automatic scanning when that's selected. This essentially mingles "policy" code (setting the default mode), with implementation code (making that happen). That's a bit inflexible if we want to change policies in future. Disentangle these two pieces, and use a slightly different construction to make things briefer as well. Signed-off-by: David Gibson --- conf.c | 60 ++++++++++++++++++++++++++-------------------------------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/conf.c b/conf.c index a235b31..4d37af1 100644 --- a/conf.c +++ b/conf.c @@ -1238,6 +1238,7 @@ void conf(struct ctx *c, int argc, char **argv) struct get_bound_ports_ns_arg ns_ports_arg = { .c = c }; char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 }; bool copy_addrs_opt = false, copy_routes_opt = false; + enum port_fwd_mode fwd_default = FWD_NONE; bool v4_only = false, v6_only = false; char *runas = NULL, *logfile = NULL; struct in6_addr *dns6 = c->ip6.dns; @@ -1252,6 +1253,7 @@ void conf(struct ctx *c, int argc, char **argv) if (c->mode == MODE_PASTA) { c->no_dhcp_dns = c->no_dhcp_dns_search = 1; + fwd_default = FWD_AUTO; optstring = "dqfel:hF:I:p:P:m:a:n:M:g:i:o:D:S:46t:u:T:U:"; } else { optstring = "dqfel:hs:F:p:P:m:a:n:M:g:i:o:D:S:461t:u:"; @@ -1803,40 +1805,32 @@ void conf(struct ctx *c, int argc, char **argv) if_indextoname(c->ifi6, c->pasta_ifn); } - if (c->mode == MODE_PASTA) { - c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1; - c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1; - c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1; - c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1; - - if (!c->tcp.fwd_in.mode || c->tcp.fwd_in.mode == FWD_AUTO) { - c->tcp.fwd_in.mode = FWD_AUTO; - ns_ports_arg.proto = IPPROTO_TCP; - NS_CALL(get_bound_ports_ns, &ns_ports_arg); - } - if (!c->udp.fwd_in.f.mode || c->udp.fwd_in.f.mode == FWD_AUTO) { - c->udp.fwd_in.f.mode = FWD_AUTO; - ns_ports_arg.proto = IPPROTO_UDP; - NS_CALL(get_bound_ports_ns, &ns_ports_arg); - } - if (!c->tcp.fwd_out.mode || c->tcp.fwd_out.mode == FWD_AUTO) { - c->tcp.fwd_out.mode = FWD_AUTO; - get_bound_ports(c, 0, IPPROTO_TCP); - } - if (!c->udp.fwd_out.f.mode || c->udp.fwd_out.f.mode == FWD_AUTO) { - c->udp.fwd_out.f.mode = FWD_AUTO; - get_bound_ports(c, 0, IPPROTO_UDP); - } - } else { - if (!c->tcp.fwd_in.mode) - c->tcp.fwd_in.mode = FWD_NONE; - if (!c->tcp.fwd_out.mode) - c->tcp.fwd_out.mode = FWD_NONE; - if (!c->udp.fwd_in.f.mode) - c->udp.fwd_in.f.mode = FWD_NONE; - if (!c->udp.fwd_out.f.mode) - c->udp.fwd_out.f.mode = FWD_NONE; + if (!c->tcp.fwd_in.mode) + c->tcp.fwd_in.mode = fwd_default; + if (!c->tcp.fwd_out.mode) + c->tcp.fwd_out.mode = fwd_default; + if (!c->udp.fwd_in.f.mode) + c->udp.fwd_in.f.mode = fwd_default; + if (!c->udp.fwd_out.f.mode) + c->udp.fwd_out.f.mode = fwd_default; + + c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1; + c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1; + c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1; + c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1; + + if (c->tcp.fwd_in.mode == FWD_AUTO) { + ns_ports_arg.proto = IPPROTO_TCP; + NS_CALL(get_bound_ports_ns, &ns_ports_arg); + } + if (c->udp.fwd_in.f.mode == FWD_AUTO) { + ns_ports_arg.proto = IPPROTO_UDP; + NS_CALL(get_bound_ports_ns, &ns_ports_arg); } + if (c->tcp.fwd_out.mode == FWD_AUTO) + get_bound_ports(c, 0, IPPROTO_TCP); + if (c->udp.fwd_out.f.mode == FWD_AUTO) + get_bound_ports(c, 0, IPPROTO_UDP); if (!c->quiet) conf_print(c); -- 2.41.0

David Gibson

5:44 a.m.

New subject: [PATCH 2/9] port_fwd: Move automatic port forwarding code to port_fwd.[ch]

The implementation of scanning /proc files to do automatic port forwarding is a bit awkwardly split between procfs_scan_listen() in util.c, get_bound_ports() and related functions in conf.c and the initial setup code in conf(). Consolidate all of this into port_fwd.h, which already has some related definitions, and a new port_fwd.c. Signed-off-by: David Gibson --- Makefile | 2 +- conf.c | 85 +------------------------ conf.h | 1 - port_fwd.c | 179 +++++++++++++++++++++++++++++++++++++++++++++++++++++ port_fwd.h | 3 + tcp.c | 1 - util.c | 65 +------------------ util.h | 2 - 8 files changed, 185 insertions(+), 153 deletions(-) create mode 100644 port_fwd.c diff --git a/Makefile b/Makefile index 941086a..0324fdd 100644 --- a/Makefile +++ b/Makefile @@ -45,7 +45,7 @@ FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \ isolation.c lineread.c log.c mld.c ndp.c netlink.c packet.c passt.c \ - pasta.c pcap.c tap.c tcp.c tcp_splice.c udp.c util.c + pasta.c pcap.c port_fwd.c tap.c tcp.c tcp_splice.c udp.c util.c QRAP_SRCS = qrap.c SRCS = $(PASST_SRCS) $(QRAP_SRCS) diff --git a/conf.c b/conf.c index 4d37af1..d3e6eb2 100644 --- a/conf.c +++ b/conf.c @@ -44,72 +44,6 @@ #include "isolation.h" #include "log.h" -/** - * get_bound_ports() - Get maps of ports with bound sockets - * @c: Execution context - * @ns: If set, set bitmaps for ports to tap/ns -- to init otherwise - * @proto: Protocol number (IPPROTO_TCP or IPPROTO_UDP) - */ -void get_bound_ports(struct ctx *c, int ns, uint8_t proto) -{ - uint8_t *udp_map, *udp_excl, *tcp_map, *tcp_excl; - - if (ns) { - udp_map = c->udp.fwd_in.f.map; - udp_excl = c->udp.fwd_out.f.map; - tcp_map = c->tcp.fwd_in.map; - tcp_excl = c->tcp.fwd_out.map; - } else { - udp_map = c->udp.fwd_out.f.map; - udp_excl = c->udp.fwd_in.f.map; - tcp_map = c->tcp.fwd_out.map; - tcp_excl = c->tcp.fwd_in.map; - } - - if (proto == IPPROTO_UDP) { - memset(udp_map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(c, IPPROTO_UDP, V4, ns, udp_map, udp_excl); - procfs_scan_listen(c, IPPROTO_UDP, V6, ns, udp_map, udp_excl); - - procfs_scan_listen(c, IPPROTO_TCP, V4, ns, udp_map, udp_excl); - procfs_scan_listen(c, IPPROTO_TCP, V6, ns, udp_map, udp_excl); - } else if (proto == IPPROTO_TCP) { - memset(tcp_map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(c, IPPROTO_TCP, V4, ns, tcp_map, tcp_excl); - procfs_scan_listen(c, IPPROTO_TCP, V6, ns, tcp_map, tcp_excl); - } -} - -/** - * struct get_bound_ports_ns_arg - Arguments for get_bound_ports_ns() - * @c: Execution context - * @proto: Protocol number (IPPROTO_TCP or IPPROTO_UDP) - */ -struct get_bound_ports_ns_arg { - struct ctx *c; - uint8_t proto; -}; - -/** - * get_bound_ports_ns() - Get maps of ports in namespace with bound sockets - * @arg: See struct get_bound_ports_ns_arg - * - * Return: 0 - */ -static int get_bound_ports_ns(void *arg) -{ - struct get_bound_ports_ns_arg *a = (struct get_bound_ports_ns_arg *)arg; - struct ctx *c = a->c; - - if (!c->pasta_netns_fd) - return 0; - - ns_enter(c); - get_bound_ports(c, 1, a->proto); - - return 0; -} - /** * next_chunk - Return the next piece of a string delimited by a character * @s: String to search @@ -1235,7 +1169,6 @@ void conf(struct ctx *c, int argc, char **argv) {"no-copy-addrs", no_argument, NULL, 19 }, { 0 }, }; - struct get_bound_ports_ns_arg ns_ports_arg = { .c = c }; char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 }; bool copy_addrs_opt = false, copy_routes_opt = false; enum port_fwd_mode fwd_default = FWD_NONE; @@ -1814,23 +1747,7 @@ void conf(struct ctx *c, int argc, char **argv) if (!c->udp.fwd_out.f.mode) c->udp.fwd_out.f.mode = fwd_default; - c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1; - c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1; - c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1; - c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1; - - if (c->tcp.fwd_in.mode == FWD_AUTO) { - ns_ports_arg.proto = IPPROTO_TCP; - NS_CALL(get_bound_ports_ns, &ns_ports_arg); - } - if (c->udp.fwd_in.f.mode == FWD_AUTO) { - ns_ports_arg.proto = IPPROTO_UDP; - NS_CALL(get_bound_ports_ns, &ns_ports_arg); - } - if (c->tcp.fwd_out.mode == FWD_AUTO) - get_bound_ports(c, 0, IPPROTO_TCP); - if (c->udp.fwd_out.f.mode == FWD_AUTO) - get_bound_ports(c, 0, IPPROTO_UDP); + port_fwd_init(c); if (!c->quiet) conf_print(c); diff --git a/conf.h b/conf.h index ce7b8c5..9d2143d 100644 --- a/conf.h +++ b/conf.h @@ -7,6 +7,5 @@ #define CONF_H void conf(struct ctx *c, int argc, char **argv); -void get_bound_ports(struct ctx *c, int ns, uint8_t proto); #endif /* CONF_H */ diff --git a/port_fwd.c b/port_fwd.c new file mode 100644 index 0000000..136a450 --- /dev/null +++ b/port_fwd.c @@ -0,0 +1,179 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +/* PASST - Plug A Simple Socket Transport + * for qemu/UNIX domain socket mode + * + * PASTA - Pack A Subtle Tap Abstraction + * for network namespace/tap device mode + * + * port_fwd.c - Port forwarding helpers + * + * Copyright Red Hat + * Author: Stefano Brivio + * Author: David Gibson + */ + +#include +#include +#include +#include + +#include "util.h" +#include "port_fwd.h" +#include "passt.h" +#include "lineread.h" + +/** + * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs + * @proto: IPPROTO_TCP or IPPROTO_UDP + * @ip_version: IP version, V4 or V6 + * @ns: Use saved file descriptors for namespace if set + * @map: Bitmap where numbers of ports in listening state will be set + * @exclude: Bitmap of ports to exclude from setting (and clear) + * + * #syscalls:pasta lseek + * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek + */ +static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, + int ns, uint8_t *map, const uint8_t *exclude) +{ + char *path, *line; + struct lineread lr; + unsigned long port; + unsigned int state; + int *fd; + + if (proto == IPPROTO_TCP) { + fd = &c->proc_net_tcp[ip_version][ns]; + if (ip_version == V4) + path = "/proc/net/tcp"; + else + path = "/proc/net/tcp6"; + } else { + fd = &c->proc_net_udp[ip_version][ns]; + if (ip_version == V4) + path = "/proc/net/udp"; + else + path = "/proc/net/udp6"; + } + + if (*fd != -1) { + if (lseek(*fd, 0, SEEK_SET)) { + warn("lseek() failed on %s: %s", path, strerror(errno)); + return; + } + } else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0) { + return; + } + + lineread_init(&lr, *fd); + lineread_get(&lr, &line); /* throw away header */ + while (lineread_get(&lr, &line) > 0) { + /* NOLINTNEXTLINE(cert-err34-c): != 2 if conversion fails */ + if (sscanf(line, "%*u: %*x:%lx %*x:%*x %x", &port, &state) != 2) + continue; + + /* See enum in kernel's include/net/tcp_states.h */ + if ((proto == IPPROTO_TCP && state != 0x0a) || + (proto == IPPROTO_UDP && state != 0x07)) + continue; + + if (bitmap_isset(exclude, port)) + bitmap_clear(map, port); + else + bitmap_set(map, port); + } +} + +/** + * get_bound_ports() - Get maps of ports with bound sockets + * @c: Execution context + * @ns: If set, set bitmaps for ports to tap/ns -- to init otherwise + * @proto: Protocol number (IPPROTO_TCP or IPPROTO_UDP) + */ +void get_bound_ports(struct ctx *c, int ns, uint8_t proto) +{ + uint8_t *udp_map, *udp_excl, *tcp_map, *tcp_excl; + + if (ns) { + udp_map = c->udp.fwd_in.f.map; + udp_excl = c->udp.fwd_out.f.map; + tcp_map = c->tcp.fwd_in.map; + tcp_excl = c->tcp.fwd_out.map; + } else { + udp_map = c->udp.fwd_out.f.map; + udp_excl = c->udp.fwd_in.f.map; + tcp_map = c->tcp.fwd_out.map; + tcp_excl = c->tcp.fwd_in.map; + } + + if (proto == IPPROTO_UDP) { + memset(udp_map, 0, PORT_BITMAP_SIZE); + procfs_scan_listen(c, IPPROTO_UDP, V4, ns, udp_map, udp_excl); + procfs_scan_listen(c, IPPROTO_UDP, V6, ns, udp_map, udp_excl); + + procfs_scan_listen(c, IPPROTO_TCP, V4, ns, udp_map, udp_excl); + procfs_scan_listen(c, IPPROTO_TCP, V6, ns, udp_map, udp_excl); + } else if (proto == IPPROTO_TCP) { + memset(tcp_map, 0, PORT_BITMAP_SIZE); + procfs_scan_listen(c, IPPROTO_TCP, V4, ns, tcp_map, tcp_excl); + procfs_scan_listen(c, IPPROTO_TCP, V6, ns, tcp_map, tcp_excl); + } +} + +/** + * struct get_bound_ports_ns_arg - Arguments for get_bound_ports_ns() + * @c: Execution context + * @proto: Protocol number (IPPROTO_TCP or IPPROTO_UDP) + */ +struct get_bound_ports_ns_arg { + struct ctx *c; + uint8_t proto; +}; + +/** + * get_bound_ports_ns() - Get maps of ports in namespace with bound sockets + * @arg: See struct get_bound_ports_ns_arg + * + * Return: 0 + */ +static int get_bound_ports_ns(void *arg) +{ + struct get_bound_ports_ns_arg *a = (struct get_bound_ports_ns_arg *)arg; + struct ctx *c = a->c; + + if (!c->pasta_netns_fd) + return 0; + + ns_enter(c); + get_bound_ports(c, 1, a->proto); + + return 0; +} + +/** + * port_fwd_init() - Initial setup for port forwarding + * @c: Execution context + */ +void port_fwd_init(struct ctx *c) +{ + struct get_bound_ports_ns_arg ns_ports_arg = { .c = c }; + + c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1; + c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1; + c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1; + c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1; + + if (c->tcp.fwd_in.mode == FWD_AUTO) { + ns_ports_arg.proto = IPPROTO_TCP; + NS_CALL(get_bound_ports_ns, &ns_ports_arg); + } + if (c->udp.fwd_in.f.mode == FWD_AUTO) { + ns_ports_arg.proto = IPPROTO_UDP; + NS_CALL(get_bound_ports_ns, &ns_ports_arg); + } + if (c->tcp.fwd_out.mode == FWD_AUTO) + get_bound_ports(c, 0, IPPROTO_TCP); + if (c->udp.fwd_out.f.mode == FWD_AUTO) + get_bound_ports(c, 0, IPPROTO_UDP); +} diff --git a/port_fwd.h b/port_fwd.h index 6ed3f14..ad8ed1f 100644 --- a/port_fwd.h +++ b/port_fwd.h @@ -31,4 +31,7 @@ struct port_fwd { in_port_t delta[NUM_PORTS]; }; +void get_bound_ports(struct ctx *c, int ns, uint8_t proto); +void port_fwd_init(struct ctx *c); + #endif /* PORT_FWD_H */ diff --git a/tcp.c b/tcp.c index a9a6f2a..a2418ae 100644 --- a/tcp.c +++ b/tcp.c @@ -298,7 +298,6 @@ #include "tap.h" #include "siphash.h" #include "pcap.h" -#include "conf.h" #include "tcp_splice.h" #include "log.h" #include "inany.h" diff --git a/util.c b/util.c index 1f35382..a8f3b35 100644 --- a/util.c +++ b/util.c @@ -28,7 +28,6 @@ #include "util.h" #include "passt.h" #include "packet.h" -#include "lineread.h" #include "log.h" #define IPV6_NH_OPT(nh) \ @@ -326,69 +325,7 @@ int bitmap_isset(const uint8_t *map, int bit) return !!(*word & BITMAP_BIT(bit)); } -/** - * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs - * @proto: IPPROTO_TCP or IPPROTO_UDP - * @ip_version: IP version, V4 or V6 - * @ns: Use saved file descriptors for namespace if set - * @map: Bitmap where numbers of ports in listening state will be set - * @exclude: Bitmap of ports to exclude from setting (and clear) - * - * #syscalls:pasta lseek - * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek - */ -void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns, - uint8_t *map, const uint8_t *exclude) -{ - char *path, *line; - struct lineread lr; - unsigned long port; - unsigned int state; - int *fd; - - if (proto == IPPROTO_TCP) { - fd = &c->proc_net_tcp[ip_version][ns]; - if (ip_version == V4) - path = "/proc/net/tcp"; - else - path = "/proc/net/tcp6"; - } else { - fd = &c->proc_net_udp[ip_version][ns]; - if (ip_version == V4) - path = "/proc/net/udp"; - else - path = "/proc/net/udp6"; - } - - if (*fd != -1) { - if (lseek(*fd, 0, SEEK_SET)) { - warn("lseek() failed on %s: %s", path, strerror(errno)); - return; - } - } else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0) { - return; - } - - lineread_init(&lr, *fd); - lineread_get(&lr, &line); /* throw away header */ - while (lineread_get(&lr, &line) > 0) { - /* NOLINTNEXTLINE(cert-err34-c): != 2 if conversion fails */ - if (sscanf(line, "%*u: %*x:%lx %*x:%*x %x", &port, &state) != 2) - continue; - - /* See enum in kernel's include/net/tcp_states.h */ - if ((proto == IPPROTO_TCP && state != 0x0a) || - (proto == IPPROTO_UDP && state != 0x07)) - continue; - - if (bitmap_isset(exclude, port)) - bitmap_clear(map, port); - else - bitmap_set(map, port); - } -} - -/** +/* * ns_enter() - Enter configured user (unless already joined) and network ns * @c: Execution context * diff --git a/util.h b/util.h index 60d6872..9effc54 100644 --- a/util.h +++ b/util.h @@ -217,8 +217,6 @@ void bitmap_set(uint8_t *map, int bit); void bitmap_clear(uint8_t *map, int bit); int bitmap_isset(const uint8_t *map, int bit); char *line_read(char *buf, size_t len, int fd); -void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns, - uint8_t *map, const uint8_t *exclude); void ns_enter(const struct ctx *c); bool ns_is_init(void); void write_pidfile(int fd, pid_t pid); -- 2.41.0

David Gibson

5:44 a.m.

New subject: [PATCH 3/9] port_fwd: Better parameterise procfs_scan_listen()

procfs_scan_listen() does some slightly clunky logic to deduce the fd it wants to use, the path it wants to open and the state it's looking for based on parameters for protocol, IP version and whether we're in the namespace. However, the caller already has to make choices based on similar parameters so it can just pass in the things that procfs_scan_listen() needs directly. Signed-off-by: David Gibson --- port_fwd.c | 53 +++++++++++++++++++++++------------------------------ 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/port_fwd.c b/port_fwd.c index 136a450..4b54877 100644 --- a/port_fwd.c +++ b/port_fwd.c @@ -23,39 +23,27 @@ #include "passt.h" #include "lineread.h" +#define UDP_LISTEN 0x07 +#define TCP_LISTEN 0x0a + /** * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs - * @proto: IPPROTO_TCP or IPPROTO_UDP - * @ip_version: IP version, V4 or V6 - * @ns: Use saved file descriptors for namespace if set + * @fd: Pointer to fd for relevant /proc/net file + * @path: Path to /proc/net file to open (if fd is -1) + * @lstate: Code for listening state to scan for * @map: Bitmap where numbers of ports in listening state will be set * @exclude: Bitmap of ports to exclude from setting (and clear) * * #syscalls:pasta lseek * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek */ -static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, - int ns, uint8_t *map, const uint8_t *exclude) +static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate, + uint8_t *map, const uint8_t *exclude) { - char *path, *line; struct lineread lr; unsigned long port; unsigned int state; - int *fd; - - if (proto == IPPROTO_TCP) { - fd = &c->proc_net_tcp[ip_version][ns]; - if (ip_version == V4) - path = "/proc/net/tcp"; - else - path = "/proc/net/tcp6"; - } else { - fd = &c->proc_net_udp[ip_version][ns]; - if (ip_version == V4) - path = "/proc/net/udp"; - else - path = "/proc/net/udp6"; - } + char *line; if (*fd != -1) { if (lseek(*fd, 0, SEEK_SET)) { @@ -74,8 +62,7 @@ static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, continue; /* See enum in kernel's include/net/tcp_states.h */ - if ((proto == IPPROTO_TCP && state != 0x0a) || - (proto == IPPROTO_UDP && state != 0x07)) + if (state != lstate) continue; if (bitmap_isset(exclude, port)) @@ -109,15 +96,21 @@ void get_bound_ports(struct ctx *c, int ns, uint8_t proto) if (proto == IPPROTO_UDP) { memset(udp_map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(c, IPPROTO_UDP, V4, ns, udp_map, udp_excl); - procfs_scan_listen(c, IPPROTO_UDP, V6, ns, udp_map, udp_excl); - - procfs_scan_listen(c, IPPROTO_TCP, V4, ns, udp_map, udp_excl); - procfs_scan_listen(c, IPPROTO_TCP, V6, ns, udp_map, udp_excl); + procfs_scan_listen(&c->proc_net_udp[V4][ns], "/proc/net/udp", + UDP_LISTEN, udp_map, udp_excl); + procfs_scan_listen(&c->proc_net_udp[V6][ns], "/proc/net/udp6", + UDP_LISTEN, udp_map, udp_excl); + + procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp", + TCP_LISTEN, udp_map, udp_excl); + procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6", + TCP_LISTEN, udp_map, udp_excl); } else if (proto == IPPROTO_TCP) { memset(tcp_map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(c, IPPROTO_TCP, V4, ns, tcp_map, tcp_excl); - procfs_scan_listen(c, IPPROTO_TCP, V6, ns, tcp_map, tcp_excl); + procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp", + TCP_LISTEN, tcp_map, tcp_excl); + procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6", + TCP_LISTEN, tcp_map, tcp_excl); } } -- 2.41.0

Stefano Brivio

2 Nov 2 Nov

7:07 p.m.

New subject: [PATCH 3/9] port_fwd: Better parameterise procfs_scan_listen()

On Thu, 5 Oct 2023 14:44:39 +1100 David Gibson wrote:

...

procfs_scan_listen() does some slightly clunky logic to deduce the fd it wants to use, the path it wants to open and the state it's looking for based on parameters for protocol, IP version and whether we're in the namespace.

However, the caller already has to make choices based on similar parameters so it can just pass in the things that procfs_scan_listen() needs directly.

Signed-off-by: David Gibson --- port_fwd.c | 53 +++++++++++++++++++++++------------------------------ 1 file changed, 23 insertions(+), 30 deletions(-)

diff --git a/port_fwd.c b/port_fwd.c index 136a450..4b54877 100644 --- a/port_fwd.c +++ b/port_fwd.c @@ -23,39 +23,27 @@ #include "passt.h" #include "lineread.h"

+#define UDP_LISTEN 0x07 +#define TCP_LISTEN 0x0a + /** * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs - * @proto: IPPROTO_TCP or IPPROTO_UDP - * @ip_version: IP version, V4 or V6 - * @ns: Use saved file descriptors for namespace if set + * @fd: Pointer to fd for relevant /proc/net file + * @path: Path to /proc/net file to open (if fd is -1) + * @lstate: Code for listening state to scan for * @map: Bitmap where numbers of ports in listening state will be set * @exclude: Bitmap of ports to exclude from setting (and clear) * * #syscalls:pasta lseek * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek */ -static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, - int ns, uint8_t *map, const uint8_t *exclude) +static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate, + uint8_t *map, const uint8_t *exclude) { - char *path, *line; struct lineread lr; unsigned long port; unsigned int state; - int *fd; - - if (proto == IPPROTO_TCP) { - fd = &c->proc_net_tcp[ip_version][ns]; - if (ip_version == V4) - path = "/proc/net/tcp"; - else - path = "/proc/net/tcp6"; - } else { - fd = &c->proc_net_udp[ip_version][ns]; - if (ip_version == V4) - path = "/proc/net/udp"; - else - path = "/proc/net/udp6"; - } + char *line;

if (*fd != -1) { if (lseek(*fd, 0, SEEK_SET)) { @@ -74,8 +62,7 @@ static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, continue;

/* See enum in kernel's include/net/tcp_states.h */

This comment should now be moved before #define UDP_LISTEN and TCP_LISTEN, as it's not otherwise clear where they're coming from. Given how late I'm reviewing all this, and that presumably you have a bunch of series/patches based on it, I'm also fine to apply this and patch it in a separate commit, or also fix this up on merge myself -- let me know. Same for the comments to the next patches/series. -- Stefano

David Gibson

3 Nov 3 Nov

1:16 a.m.

New subject: [PATCH 3/9] port_fwd: Better parameterise procfs_scan_listen()

On Thu, Nov 02, 2023 at 07:07:19PM +0100, Stefano Brivio wrote:

...

On Thu, 5 Oct 2023 14:44:39 +1100 David Gibson wrote:

...
procfs_scan_listen() does some slightly clunky logic to deduce the fd it wants to use, the path it wants to open and the state it's looking for based on parameters for protocol, IP version and whether we're in the namespace.

However, the caller already has to make choices based on similar parameters so it can just pass in the things that procfs_scan_listen() needs directly.

Signed-off-by: David Gibson --- port_fwd.c | 53 +++++++++++++++++++++++------------------------------ 1 file changed, 23 insertions(+), 30 deletions(-)

diff --git a/port_fwd.c b/port_fwd.c index 136a450..4b54877 100644 --- a/port_fwd.c +++ b/port_fwd.c @@ -23,39 +23,27 @@ #include "passt.h" #include "lineread.h"

+#define UDP_LISTEN 0x07 +#define TCP_LISTEN 0x0a + /** * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs - * @proto: IPPROTO_TCP or IPPROTO_UDP - * @ip_version: IP version, V4 or V6 - * @ns: Use saved file descriptors for namespace if set + * @fd: Pointer to fd for relevant /proc/net file + * @path: Path to /proc/net file to open (if fd is -1) + * @lstate: Code for listening state to scan for * @map: Bitmap where numbers of ports in listening state will be set * @exclude: Bitmap of ports to exclude from setting (and clear) * * #syscalls:pasta lseek * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek */ -static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, - int ns, uint8_t *map, const uint8_t *exclude) +static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate, + uint8_t *map, const uint8_t *exclude) { - char *path, *line; struct lineread lr; unsigned long port; unsigned int state; - int *fd; - - if (proto == IPPROTO_TCP) { - fd = &c->proc_net_tcp[ip_version][ns]; - if (ip_version == V4) - path = "/proc/net/tcp"; - else - path = "/proc/net/tcp6"; - } else { - fd = &c->proc_net_udp[ip_version][ns]; - if (ip_version == V4) - path = "/proc/net/udp"; - else - path = "/proc/net/udp6"; - } + char *line;

if (*fd != -1) { if (lseek(*fd, 0, SEEK_SET)) { @@ -74,8 +62,7 @@ static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, continue;

/* See enum in kernel's include/net/tcp_states.h */

This comment should now be moved before #define UDP_LISTEN and TCP_LISTEN, as it's not otherwise clear where they're coming from.

Good point, fixed.

...

Given how late I'm reviewing all this, and that presumably you have a bunch of series/patches based on it, I'm also fine to apply this and patch it in a separate commit, or also fix this up on merge myself -- let me know.

Same for the comments to the next patches/series.

Eh, I don't think the rebasing should be too hard, so I'm happy to sort that out. -- 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

David Gibson

5 Oct 5 Oct

5:44 a.m.

New subject: [PATCH 4/9] util: Add open_in_ns() helper

Most of our helpers which need to enter the pasta network namespace are quite specialised. Add one which is rather general - it just open()s a given file in the namespace context and returns the fd back to the main namespace. This will have some future uses. Signed-off-by: David Gibson --- util.c | 39 +++++++++++++++++++++++++++++++++++++++ util.h | 1 + 2 files changed, 40 insertions(+) diff --git a/util.c b/util.c index a8f3b35..92ad375 100644 --- a/util.c +++ b/util.c @@ -364,6 +364,45 @@ bool ns_is_init(void) return ret; } +struct open_in_ns_args { + const struct ctx *c; + int fd; + int err; + const char *path; + int flags; +}; + +static int do_open_in_ns(void *arg) +{ + struct open_in_ns_args *a = (struct open_in_ns_args *)arg; + + ns_enter(a->c); + + a->fd = open(a->path, a->flags); + a->err = errno; + + return 0; +} + +/** + * open_in_ns() - open() within the pasta namespace + * @c: Execution context + * @path: Path to open + * @flags: open() flags + * + * Return: fd of open()ed file or -1 on error, errno is set to indicate error + */ +int open_in_ns(const struct ctx *c, const char *path, int flags) +{ + struct open_in_ns_args arg = { + .c = c, .path = path, .flags = flags, + }; + + NS_CALL(do_open_in_ns, &arg); + errno = arg.err; + return arg.fd; +} + /** * pid_file() - Write PID to file, if requested to do so, and close it * @fd: Open PID file descriptor, closed on exit, -1 to skip writing it diff --git a/util.h b/util.h index 9effc54..78a8fb2 100644 --- a/util.h +++ b/util.h @@ -219,6 +219,7 @@ int bitmap_isset(const uint8_t *map, int bit); char *line_read(char *buf, size_t len, int fd); void ns_enter(const struct ctx *c); bool ns_is_init(void); +int open_in_ns(const struct ctx *c, const char *path, int flags); void write_pidfile(int fd, pid_t pid); int __daemon(int pidfile_fd, int devnull_fd); int fls(unsigned long x); -- 2.41.0

Stefano Brivio

2 Nov 2 Nov

7:07 p.m.

New subject: [PATCH 4/9] util: Add open_in_ns() helper

On Thu, 5 Oct 2023 14:44:40 +1100 David Gibson wrote:

...

Most of our helpers which need to enter the pasta network namespace are quite specialised. Add one which is rather general - it just open()s a given file in the namespace context and returns the fd back to the main namespace. This will have some future uses.

Signed-off-by: David Gibson --- util.c | 39 +++++++++++++++++++++++++++++++++++++++ util.h | 1 + 2 files changed, 40 insertions(+)

diff --git a/util.c b/util.c index a8f3b35..92ad375 100644 --- a/util.c +++ b/util.c @@ -364,6 +364,45 @@ bool ns_is_init(void) return ret; }

+struct open_in_ns_args {

It would be nice to have the usual kerneldoc-style comment to this (at a first reading: are "flags" the flags for open(2) or something specialised for internal use?).

...

+ const struct ctx *c; + int fd; + int err; + const char *path; + int flags; +}; + +static int do_open_in_ns(void *arg)

Same for this one. The rest of the series makes sense and looks good to me. -- Stefano

David Gibson

3 Nov 3 Nov

1:20 a.m.

New subject: [PATCH 4/9] util: Add open_in_ns() helper

On Thu, Nov 02, 2023 at 07:07:25PM +0100, Stefano Brivio wrote:

...

On Thu, 5 Oct 2023 14:44:40 +1100 David Gibson wrote:

...
Most of our helpers which need to enter the pasta network namespace are quite specialised. Add one which is rather general - it just open()s a given file in the namespace context and returns the fd back to the main namespace. This will have some future uses.

Signed-off-by: David Gibson --- util.c | 39 +++++++++++++++++++++++++++++++++++++++ util.h | 1 + 2 files changed, 40 insertions(+)

diff --git a/util.c b/util.c index a8f3b35..92ad375 100644 --- a/util.c +++ b/util.c @@ -364,6 +364,45 @@ bool ns_is_init(void) return ret; }

+struct open_in_ns_args {

It would be nice to have the usual kerneldoc-style comment to this (at a first reading: are "flags" the flags for open(2) or something specialised for internal use?).

...
+ const struct ctx *c; + int fd; + int err; + const char *path; + int flags; +}; + +static int do_open_in_ns(void *arg)

Same for this one.

Eh, ok. I left it out originally, because they both seemed like they were essentially internals of open_in_ns() itself, but I've added them now.

...

The rest of the series makes sense and looks good to me.

-- 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

David Gibson

5 Oct 5 Oct

5:44 a.m.

New subject: [PATCH 5/9] port_fwd: Pre-open /proc/net/* files rather than on-demand

procfs_scan_listen() can either use an already opened fd for a /proc/net file, or it will open it. So, effectively it will open the file on the first call, then re-use the fd in subsequent calls. However, it's not possible to open the /proc/net files after we isolate our filesystem in isolate_prefork(). That means that for each /proc/net file we must call procfs_scan_listen() at least once before isolate_prefork(), or it won't work afterwards. That happens to be the case, but it's a pretty fragile requirement. To make this more robust, instead always pre-open the /proc files we will need in get_bounds_port_init() and have procfs_scan_listen() just use those. Signed-off-by: David Gibson --- port_fwd.c | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/port_fwd.c b/port_fwd.c index 4b54877..a3f69dd 100644 --- a/port_fwd.c +++ b/port_fwd.c @@ -28,8 +28,7 @@ /** * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs - * @fd: Pointer to fd for relevant /proc/net file - * @path: Path to /proc/net file to open (if fd is -1) + * @fd: fd for relevant /proc/net file * @lstate: Code for listening state to scan for * @map: Bitmap where numbers of ports in listening state will be set * @exclude: Bitmap of ports to exclude from setting (and clear) @@ -37,7 +36,7 @@ * #syscalls:pasta lseek * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek */ -static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate, +static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map, const uint8_t *exclude) { struct lineread lr; @@ -45,16 +44,12 @@ static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate, unsigned int state; char *line; - if (*fd != -1) { - if (lseek(*fd, 0, SEEK_SET)) { - warn("lseek() failed on %s: %s", path, strerror(errno)); - return; - } - } else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0) { + if (lseek(fd, 0, SEEK_SET)) { + warn("lseek() failed on /proc/net file: %s", strerror(errno)); return; } - lineread_init(&lr, *fd); + lineread_init(&lr, fd); lineread_get(&lr, &line); /* throw away header */ while (lineread_get(&lr, &line) > 0) { /* NOLINTNEXTLINE(cert-err34-c): != 2 if conversion fails */ @@ -96,20 +91,20 @@ void get_bound_ports(struct ctx *c, int ns, uint8_t proto) if (proto == IPPROTO_UDP) { memset(udp_map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(&c->proc_net_udp[V4][ns], "/proc/net/udp", + procfs_scan_listen(c->proc_net_udp[V4][ns], UDP_LISTEN, udp_map, udp_excl); - procfs_scan_listen(&c->proc_net_udp[V6][ns], "/proc/net/udp6", + procfs_scan_listen(c->proc_net_udp[V6][ns], UDP_LISTEN, udp_map, udp_excl); - procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp", + procfs_scan_listen(c->proc_net_tcp[V4][ns], TCP_LISTEN, udp_map, udp_excl); - procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6", + procfs_scan_listen(c->proc_net_tcp[V6][ns], TCP_LISTEN, udp_map, udp_excl); } else if (proto == IPPROTO_TCP) { memset(tcp_map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp", + procfs_scan_listen(c->proc_net_tcp[V4][ns], TCP_LISTEN, tcp_map, tcp_excl); - procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6", + procfs_scan_listen(c->proc_net_tcp[V6][ns], TCP_LISTEN, tcp_map, tcp_excl); } } @@ -151,6 +146,7 @@ static int get_bound_ports_ns(void *arg) void port_fwd_init(struct ctx *c) { struct get_bound_ports_ns_arg ns_ports_arg = { .c = c }; + const int flags = O_RDONLY | O_CLOEXEC; c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1; c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1; @@ -158,15 +154,25 @@ void port_fwd_init(struct ctx *c) c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1; if (c->tcp.fwd_in.mode == FWD_AUTO) { + c->proc_net_tcp[V4][1] = open_in_ns(c, "/proc/net/tcp", flags); + c->proc_net_tcp[V6][1] = open_in_ns(c, "/proc/net/tcp6", flags); ns_ports_arg.proto = IPPROTO_TCP; NS_CALL(get_bound_ports_ns, &ns_ports_arg); } if (c->udp.fwd_in.f.mode == FWD_AUTO) { + c->proc_net_udp[V4][1] = open_in_ns(c, "/proc/net/udp", flags); + c->proc_net_udp[V6][1] = open_in_ns(c, "/proc/net/udp6", flags); ns_ports_arg.proto = IPPROTO_UDP; NS_CALL(get_bound_ports_ns, &ns_ports_arg); } - if (c->tcp.fwd_out.mode == FWD_AUTO) + if (c->tcp.fwd_out.mode == FWD_AUTO) { + c->proc_net_tcp[V4][0] = open("/proc/net/tcp", flags); + c->proc_net_tcp[V6][0] = open("/proc/net/tcp6", flags); get_bound_ports(c, 0, IPPROTO_TCP); - if (c->udp.fwd_out.f.mode == FWD_AUTO) + } + if (c->udp.fwd_out.f.mode == FWD_AUTO) { + c->proc_net_udp[V4][0] = open("/proc/net/udp", flags); + c->proc_net_udp[V6][0] = open("/proc/net/udp6", flags); get_bound_ports(c, 0, IPPROTO_UDP); + } } -- 2.41.0

David Gibson

5:44 a.m.

New subject: [PATCH 6/9] port_fwd: Don't NS_CALL get_bound_ports()

When we want to scan for bound ports in the namespace we use NS_CALL() to run get_bound_ports() in the namespace. However, the only thing it actually needed to be in the namespace for was to open the /proc/net file it was scanning. Since we now always pre-open those, we no longer need to switch to the namespace for the actual get_bound_ports() calls. That in turn means that tcp_port_detect() doesn't need to run in the ns either, and we can just replace it with inline calls to get_bound_ports(). Signed-off-by: David Gibson --- port_fwd.c | 37 ++----------------------------------- tcp.c | 38 ++------------------------------------ 2 files changed, 4 insertions(+), 71 deletions(-) diff --git a/port_fwd.c b/port_fwd.c index a3f69dd..b91eafe 100644 --- a/port_fwd.c +++ b/port_fwd.c @@ -109,43 +109,12 @@ void get_bound_ports(struct ctx *c, int ns, uint8_t proto) } } -/** - * struct get_bound_ports_ns_arg - Arguments for get_bound_ports_ns() - * @c: Execution context - * @proto: Protocol number (IPPROTO_TCP or IPPROTO_UDP) - */ -struct get_bound_ports_ns_arg { - struct ctx *c; - uint8_t proto; -}; - -/** - * get_bound_ports_ns() - Get maps of ports in namespace with bound sockets - * @arg: See struct get_bound_ports_ns_arg - * - * Return: 0 - */ -static int get_bound_ports_ns(void *arg) -{ - struct get_bound_ports_ns_arg *a = (struct get_bound_ports_ns_arg *)arg; - struct ctx *c = a->c; - - if (!c->pasta_netns_fd) - return 0; - - ns_enter(c); - get_bound_ports(c, 1, a->proto); - - return 0; -} - /** * port_fwd_init() - Initial setup for port forwarding * @c: Execution context */ void port_fwd_init(struct ctx *c) { - struct get_bound_ports_ns_arg ns_ports_arg = { .c = c }; const int flags = O_RDONLY | O_CLOEXEC; c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1; @@ -156,14 +125,12 @@ void port_fwd_init(struct ctx *c) if (c->tcp.fwd_in.mode == FWD_AUTO) { c->proc_net_tcp[V4][1] = open_in_ns(c, "/proc/net/tcp", flags); c->proc_net_tcp[V6][1] = open_in_ns(c, "/proc/net/tcp6", flags); - ns_ports_arg.proto = IPPROTO_TCP; - NS_CALL(get_bound_ports_ns, &ns_ports_arg); + get_bound_ports(c, 1, IPPROTO_TCP); } if (c->udp.fwd_in.f.mode == FWD_AUTO) { c->proc_net_udp[V4][1] = open_in_ns(c, "/proc/net/udp", flags); c->proc_net_udp[V6][1] = open_in_ns(c, "/proc/net/udp6", flags); - ns_ports_arg.proto = IPPROTO_UDP; - NS_CALL(get_bound_ports_ns, &ns_ports_arg); + get_bound_ports(c, 1, IPPROTO_UDP); } if (c->tcp.fwd_out.mode == FWD_AUTO) { c->proc_net_tcp[V4][0] = open("/proc/net/tcp", flags); diff --git a/tcp.c b/tcp.c index a2418ae..63a3c64 100644 --- a/tcp.c +++ b/tcp.c @@ -3149,37 +3149,6 @@ int tcp_init(struct ctx *c) return 0; } -/** - * struct tcp_port_detect_arg - Arguments for tcp_port_detect() - * @c: Execution context - * @detect_in_ns: Detect ports bound in namespace, not in init - */ -struct tcp_port_detect_arg { - struct ctx *c; - int detect_in_ns; -}; - -/** - * tcp_port_detect() - Detect ports bound in namespace or init - * @arg: See struct tcp_port_detect_arg - * - * Return: 0 - */ -static int tcp_port_detect(void *arg) -{ - struct tcp_port_detect_arg *a = (struct tcp_port_detect_arg *)arg; - - if (a->detect_in_ns) { - ns_enter(a->c); - - get_bound_ports(a->c, 1, IPPROTO_TCP); - } else { - get_bound_ports(a->c, 0, IPPROTO_TCP); - } - - return 0; -} - /** * struct tcp_port_rebind_arg - Arguments for tcp_port_rebind() * @c: Execution context @@ -3268,19 +3237,16 @@ void tcp_timer(struct ctx *c, const struct timespec *ts) (void)ts; if (c->mode == MODE_PASTA) { - struct tcp_port_detect_arg detect_arg = { c, 0 }; struct tcp_port_rebind_arg rebind_arg = { c, 0 }; if (c->tcp.fwd_out.mode == FWD_AUTO) { - detect_arg.detect_in_ns = 0; - tcp_port_detect(&detect_arg); + get_bound_ports(c, 0, IPPROTO_TCP); rebind_arg.bind_in_ns = 1; NS_CALL(tcp_port_rebind, &rebind_arg); } if (c->tcp.fwd_in.mode == FWD_AUTO) { - detect_arg.detect_in_ns = 1; - NS_CALL(tcp_port_detect, &detect_arg); + get_bound_ports(c, 1, IPPROTO_TCP); rebind_arg.bind_in_ns = 0; tcp_port_rebind(&rebind_arg); } -- 2.41.0

David Gibson

5:44 a.m.

New subject: [PATCH 7/9] port_fwd: Split TCP and UDP cases for get_bound_ports()

Currently get_bound_ports() takes a parameter to determine if it scans for UDP or TCP bound ports, but in fact there's almost nothing in common between those two paths. The parameter appears primarily to have been a convenience for when we needed to invoke this function via NS_CALL(). Now that we don't need that, split it into separate TCP and UDP versions. Signed-off-by: David Gibson --- port_fwd.c | 76 ++++++++++++++++++++++++++++++------------------------ port_fwd.h | 3 ++- tcp.c | 4 +-- 3 files changed, 47 insertions(+), 36 deletions(-) diff --git a/port_fwd.c b/port_fwd.c index b91eafe..e6b3b14 100644 --- a/port_fwd.c +++ b/port_fwd.c @@ -68,45 +68,55 @@ static void procfs_scan_listen(int fd, unsigned int lstate, } /** - * get_bound_ports() - Get maps of ports with bound sockets + * get_bound_ports_tcp() - Get maps of TCP ports with bound sockets * @c: Execution context * @ns: If set, set bitmaps for ports to tap/ns -- to init otherwise - * @proto: Protocol number (IPPROTO_TCP or IPPROTO_UDP) */ -void get_bound_ports(struct ctx *c, int ns, uint8_t proto) +void get_bound_ports_tcp(struct ctx *c, int ns) { - uint8_t *udp_map, *udp_excl, *tcp_map, *tcp_excl; + uint8_t *map, *excl; if (ns) { - udp_map = c->udp.fwd_in.f.map; - udp_excl = c->udp.fwd_out.f.map; - tcp_map = c->tcp.fwd_in.map; - tcp_excl = c->tcp.fwd_out.map; + map = c->tcp.fwd_in.map; + excl = c->tcp.fwd_out.map; } else { - udp_map = c->udp.fwd_out.f.map; - udp_excl = c->udp.fwd_in.f.map; - tcp_map = c->tcp.fwd_out.map; - tcp_excl = c->tcp.fwd_in.map; + map = c->tcp.fwd_out.map; + excl = c->tcp.fwd_in.map; } - if (proto == IPPROTO_UDP) { - memset(udp_map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(c->proc_net_udp[V4][ns], - UDP_LISTEN, udp_map, udp_excl); - procfs_scan_listen(c->proc_net_udp[V6][ns], - UDP_LISTEN, udp_map, udp_excl); - - procfs_scan_listen(c->proc_net_tcp[V4][ns], - TCP_LISTEN, udp_map, udp_excl); - procfs_scan_listen(c->proc_net_tcp[V6][ns], - TCP_LISTEN, udp_map, udp_excl); - } else if (proto == IPPROTO_TCP) { - memset(tcp_map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(c->proc_net_tcp[V4][ns], - TCP_LISTEN, tcp_map, tcp_excl); - procfs_scan_listen(c->proc_net_tcp[V6][ns], - TCP_LISTEN, tcp_map, tcp_excl); + memset(map, 0, PORT_BITMAP_SIZE); + procfs_scan_listen(c->proc_net_tcp[V4][ns], TCP_LISTEN, map, excl); + procfs_scan_listen(c->proc_net_tcp[V6][ns], TCP_LISTEN, map, excl); +} + +/** + * get_bound_ports_udp() - Get maps of UDP ports with bound sockets + * @c: Execution context + * @ns: If set, set bitmaps for ports to tap/ns -- to init otherwise + */ +void get_bound_ports_udp(struct ctx *c, int ns) +{ + uint8_t *map, *excl; + + if (ns) { + map = c->udp.fwd_in.f.map; + excl = c->udp.fwd_out.f.map; + } else { + map = c->udp.fwd_out.f.map; + excl = c->udp.fwd_in.f.map; } + + memset(map, 0, PORT_BITMAP_SIZE); + procfs_scan_listen(c->proc_net_udp[V4][ns], UDP_LISTEN, map, excl); + procfs_scan_listen(c->proc_net_udp[V6][ns], UDP_LISTEN, map, excl); + + /* Also forward UDP ports with the same numbers as bound TCP ports. + * This is useful for a handful of protocols (e.g. iperf3) where a TCP + * control port is used to set up transfers on a corresponding UDP + * port. + */ + procfs_scan_listen(c->proc_net_tcp[V4][ns], TCP_LISTEN, map, excl); + procfs_scan_listen(c->proc_net_tcp[V6][ns], TCP_LISTEN, map, excl); } /** @@ -125,21 +135,21 @@ void port_fwd_init(struct ctx *c) if (c->tcp.fwd_in.mode == FWD_AUTO) { c->proc_net_tcp[V4][1] = open_in_ns(c, "/proc/net/tcp", flags); c->proc_net_tcp[V6][1] = open_in_ns(c, "/proc/net/tcp6", flags); - get_bound_ports(c, 1, IPPROTO_TCP); + get_bound_ports_tcp(c, 1); } if (c->udp.fwd_in.f.mode == FWD_AUTO) { c->proc_net_udp[V4][1] = open_in_ns(c, "/proc/net/udp", flags); c->proc_net_udp[V6][1] = open_in_ns(c, "/proc/net/udp6", flags); - get_bound_ports(c, 1, IPPROTO_UDP); + get_bound_ports_udp(c, 1); } if (c->tcp.fwd_out.mode == FWD_AUTO) { c->proc_net_tcp[V4][0] = open("/proc/net/tcp", flags); c->proc_net_tcp[V6][0] = open("/proc/net/tcp6", flags); - get_bound_ports(c, 0, IPPROTO_TCP); + get_bound_ports_tcp(c, 0); } if (c->udp.fwd_out.f.mode == FWD_AUTO) { c->proc_net_udp[V4][0] = open("/proc/net/udp", flags); c->proc_net_udp[V6][0] = open("/proc/net/udp6", flags); - get_bound_ports(c, 0, IPPROTO_UDP); + get_bound_ports_udp(c, 0); } } diff --git a/port_fwd.h b/port_fwd.h index ad8ed1f..2f8f526 100644 --- a/port_fwd.h +++ b/port_fwd.h @@ -31,7 +31,8 @@ struct port_fwd { in_port_t delta[NUM_PORTS]; }; -void get_bound_ports(struct ctx *c, int ns, uint8_t proto); +void get_bound_ports_tcp(struct ctx *c, int ns); +void get_bound_ports_udp(struct ctx *c, int ns); void port_fwd_init(struct ctx *c); #endif /* PORT_FWD_H */ diff --git a/tcp.c b/tcp.c index 63a3c64..921bd2a 100644 --- a/tcp.c +++ b/tcp.c @@ -3240,13 +3240,13 @@ void tcp_timer(struct ctx *c, const struct timespec *ts) struct tcp_port_rebind_arg rebind_arg = { c, 0 }; if (c->tcp.fwd_out.mode == FWD_AUTO) { - get_bound_ports(c, 0, IPPROTO_TCP); + get_bound_ports_tcp(c, 0); rebind_arg.bind_in_ns = 1; NS_CALL(tcp_port_rebind, &rebind_arg); } if (c->tcp.fwd_in.mode == FWD_AUTO) { - get_bound_ports(c, 1, IPPROTO_TCP); + get_bound_ports_tcp(c, 1); rebind_arg.bind_in_ns = 0; tcp_port_rebind(&rebind_arg); } -- 2.41.0

David Gibson

5:44 a.m.

New subject: [PATCH 8/9] port_fwd: Move port scanning /proc fds into struct port_fwd

Currently we store /proc/net fds used to implement automatic port forwarding in the proc_net_{tcp,udp} fields of the main context structure. However, in fact each of those is associated with a particular direction of forwarding, and we already have struct port_fwd which collects all other information related to a particular direction of port forwarding. We can simplify things a bit by moving the /proc fds into struct port_fwd. Signed-off-by: David Gibson --- passt.h | 5 ----- port_fwd.c | 62 ++++++++++++++++++++++++++++-------------------------- port_fwd.h | 4 ++++ 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/passt.h b/passt.h index 282bd1a..53defa4 100644 --- a/passt.h +++ b/passt.h @@ -203,8 +203,6 @@ struct ip6_ctx { * @no_netns_quit: In pasta mode, don't exit if fs-bound namespace is gone * @netns_base: Base name for fs-bound namespace, if any, in pasta mode * @netns_dir: Directory of fs-bound namespace, if any, in pasta mode - * @proc_net_tcp: Stored handles for /proc/net/tcp{,6} in init and ns - * @proc_net_udp: Stored handles for /proc/net/udp{,6} in init and ns * @epollfd: File descriptor for epoll instance * @fd_tap_listen: File descriptor for listening AF_UNIX socket, if any * @fd_tap: AF_UNIX socket, tuntap device, or pre-opened socket @@ -258,9 +256,6 @@ struct ctx { char netns_base[PATH_MAX]; char netns_dir[PATH_MAX]; - int proc_net_tcp[IP_VERSIONS][2]; - int proc_net_udp[IP_VERSIONS][2]; - int epollfd; int fd_tap_listen; int fd_tap; diff --git a/port_fwd.c b/port_fwd.c index e6b3b14..5308620 100644 --- a/port_fwd.c +++ b/port_fwd.c @@ -74,19 +74,19 @@ static void procfs_scan_listen(int fd, unsigned int lstate, */ void get_bound_ports_tcp(struct ctx *c, int ns) { - uint8_t *map, *excl; + struct port_fwd *fwd, *rev; if (ns) { - map = c->tcp.fwd_in.map; - excl = c->tcp.fwd_out.map; + fwd = &c->tcp.fwd_in; + rev = &c->tcp.fwd_out; } else { - map = c->tcp.fwd_out.map; - excl = c->tcp.fwd_in.map; + fwd = &c->tcp.fwd_out; + rev = &c->tcp.fwd_in; } - memset(map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(c->proc_net_tcp[V4][ns], TCP_LISTEN, map, excl); - procfs_scan_listen(c->proc_net_tcp[V6][ns], TCP_LISTEN, map, excl); + memset(fwd->map, 0, PORT_BITMAP_SIZE); + procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map, rev->map); + procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map, rev->map); } /** @@ -96,27 +96,29 @@ void get_bound_ports_tcp(struct ctx *c, int ns) */ void get_bound_ports_udp(struct ctx *c, int ns) { - uint8_t *map, *excl; + struct port_fwd *fwd, *rev, *tcp; if (ns) { - map = c->udp.fwd_in.f.map; - excl = c->udp.fwd_out.f.map; + fwd = &c->udp.fwd_in.f; + rev = &c->udp.fwd_out.f; + tcp = &c->tcp.fwd_in; } else { - map = c->udp.fwd_out.f.map; - excl = c->udp.fwd_in.f.map; + fwd = &c->udp.fwd_out.f; + rev = &c->udp.fwd_in.f; + tcp = &c->tcp.fwd_out; } - memset(map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(c->proc_net_udp[V4][ns], UDP_LISTEN, map, excl); - procfs_scan_listen(c->proc_net_udp[V6][ns], UDP_LISTEN, map, excl); + memset(fwd->map, 0, PORT_BITMAP_SIZE); + procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map, rev->map); + procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map, rev->map); /* Also forward UDP ports with the same numbers as bound TCP ports. * This is useful for a handful of protocols (e.g. iperf3) where a TCP * control port is used to set up transfers on a corresponding UDP * port. */ - procfs_scan_listen(c->proc_net_tcp[V4][ns], TCP_LISTEN, map, excl); - procfs_scan_listen(c->proc_net_tcp[V6][ns], TCP_LISTEN, map, excl); + procfs_scan_listen(tcp->scan4, TCP_LISTEN, fwd->map, rev->map); + procfs_scan_listen(tcp->scan6, TCP_LISTEN, fwd->map, rev->map); } /** @@ -127,29 +129,29 @@ void port_fwd_init(struct ctx *c) { const int flags = O_RDONLY | O_CLOEXEC; - c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1; - c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1; - c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1; - c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1; + c->tcp.fwd_in.scan4 = c->tcp.fwd_in.scan6 = -1; + c->tcp.fwd_out.scan4 = c->tcp.fwd_out.scan6 = -1; + c->udp.fwd_in.f.scan4 = c->udp.fwd_in.f.scan6 = -1; + c->udp.fwd_out.f.scan4 = c->udp.fwd_out.f.scan6 = -1; if (c->tcp.fwd_in.mode == FWD_AUTO) { - c->proc_net_tcp[V4][1] = open_in_ns(c, "/proc/net/tcp", flags); - c->proc_net_tcp[V6][1] = open_in_ns(c, "/proc/net/tcp6", flags); + c->tcp.fwd_in.scan4 = open_in_ns(c, "/proc/net/tcp", flags); + c->tcp.fwd_in.scan6 = open_in_ns(c, "/proc/net/tcp6", flags); get_bound_ports_tcp(c, 1); } if (c->udp.fwd_in.f.mode == FWD_AUTO) { - c->proc_net_udp[V4][1] = open_in_ns(c, "/proc/net/udp", flags); - c->proc_net_udp[V6][1] = open_in_ns(c, "/proc/net/udp6", flags); + c->udp.fwd_in.f.scan4 = open_in_ns(c, "/proc/net/udp", flags); + c->udp.fwd_in.f.scan6 = open_in_ns(c, "/proc/net/udp6", flags); get_bound_ports_udp(c, 1); } if (c->tcp.fwd_out.mode == FWD_AUTO) { - c->proc_net_tcp[V4][0] = open("/proc/net/tcp", flags); - c->proc_net_tcp[V6][0] = open("/proc/net/tcp6", flags); + c->tcp.fwd_out.scan4 = open("/proc/net/tcp", flags); + c->tcp.fwd_out.scan6 = open("/proc/net/tcp6", flags); get_bound_ports_tcp(c, 0); } if (c->udp.fwd_out.f.mode == FWD_AUTO) { - c->proc_net_udp[V4][0] = open("/proc/net/udp", flags); - c->proc_net_udp[V6][0] = open("/proc/net/udp6", flags); + c->udp.fwd_out.f.scan4 = open("/proc/net/udp", flags); + c->udp.fwd_out.f.scan6 = open("/proc/net/udp6", flags); get_bound_ports_udp(c, 0); } } diff --git a/port_fwd.h b/port_fwd.h index 2f8f526..8ab6b48 100644 --- a/port_fwd.h +++ b/port_fwd.h @@ -22,11 +22,15 @@ enum port_fwd_mode { /** * port_fwd - Describes port forwarding for one protocol and direction * @mode: Overall forwarding mode (all, none, auto, specific ports) + * @scan4: /proc/net fd to scan for IPv4 ports when in AUTO mode + * @scan6: /proc/net fd to scan for IPv6 ports when in AUTO mode * @map: Bitmap describing which ports are forwarded * @delta: Offset between the original destination and mapped port number */ struct port_fwd { enum port_fwd_mode mode; + int scan4; + int scan6; uint8_t map[PORT_BITMAP_SIZE]; in_port_t delta[NUM_PORTS]; }; -- 2.41.0

David Gibson

5:44 a.m.

New subject: [PATCH 9/9] port_fwd: Simplify get_bound_ports_*() to port_fwd_scan_*()

get_bound_ports_*() now only use their context and ns parameters to determine which forwarding maps they're operating on. Each function needs the map they're actually updating, as well as the map for the other direction, to avoid creating forwarding loops. The UDP function also requires the corresponding TCP map, to implement the behaviour where we forward UDP ports of the same number as bound TCP ports for tools like iperf3. Passing those maps directly as parameters simplifies the code without making the callers life harder, because those already know the relevant maps. IMO, invoking these functions in terms of where they're looking for updated forwarding also makes more logical sense than in terms of where they're looking for bound ports. Given that new way of looking at the functions, also rename them to port_fwd_scan_*(). Signed-off-by: David Gibson --- port_fwd.c | 50 ++++++++++++++++---------------------------------- port_fwd.h | 5 +++-- tcp.c | 4 ++-- 3 files changed, 21 insertions(+), 38 deletions(-) diff --git a/port_fwd.c b/port_fwd.c index 5308620..b6e256a 100644 --- a/port_fwd.c +++ b/port_fwd.c @@ -68,46 +68,26 @@ static void procfs_scan_listen(int fd, unsigned int lstate, } /** - * get_bound_ports_tcp() - Get maps of TCP ports with bound sockets - * @c: Execution context - * @ns: If set, set bitmaps for ports to tap/ns -- to init otherwise + * port_fwd_scan_tcp() - Scan /proc to update TCP forwarding map + * @fwd: Forwarding information to update + * @rev: Forwarding information for the reverse direction */ -void get_bound_ports_tcp(struct ctx *c, int ns) +void port_fwd_scan_tcp(struct port_fwd *fwd, const struct port_fwd *rev) { - struct port_fwd *fwd, *rev; - - if (ns) { - fwd = &c->tcp.fwd_in; - rev = &c->tcp.fwd_out; - } else { - fwd = &c->tcp.fwd_out; - rev = &c->tcp.fwd_in; - } - memset(fwd->map, 0, PORT_BITMAP_SIZE); procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map, rev->map); procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map, rev->map); } /** - * get_bound_ports_udp() - Get maps of UDP ports with bound sockets - * @c: Execution context - * @ns: If set, set bitmaps for ports to tap/ns -- to init otherwise + * port_fwd_scan_tcp() - Scan /proc to update TCP forwarding map + * @fwd: Forwarding information to update + * @rev: Forwarding information for the reverse direction + * @tcp: Corresponding TCP forwarding information */ -void get_bound_ports_udp(struct ctx *c, int ns) +void port_fwd_scan_udp(struct port_fwd *fwd, const struct port_fwd *rev, + const struct port_fwd *tcp) { - struct port_fwd *fwd, *rev, *tcp; - - if (ns) { - fwd = &c->udp.fwd_in.f; - rev = &c->udp.fwd_out.f; - tcp = &c->tcp.fwd_in; - } else { - fwd = &c->udp.fwd_out.f; - rev = &c->udp.fwd_in.f; - tcp = &c->tcp.fwd_out; - } - memset(fwd->map, 0, PORT_BITMAP_SIZE); procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map, rev->map); procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map, rev->map); @@ -137,21 +117,23 @@ void port_fwd_init(struct ctx *c) if (c->tcp.fwd_in.mode == FWD_AUTO) { c->tcp.fwd_in.scan4 = open_in_ns(c, "/proc/net/tcp", flags); c->tcp.fwd_in.scan6 = open_in_ns(c, "/proc/net/tcp6", flags); - get_bound_ports_tcp(c, 1); + port_fwd_scan_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out); } if (c->udp.fwd_in.f.mode == FWD_AUTO) { c->udp.fwd_in.f.scan4 = open_in_ns(c, "/proc/net/udp", flags); c->udp.fwd_in.f.scan6 = open_in_ns(c, "/proc/net/udp6", flags); - get_bound_ports_udp(c, 1); + port_fwd_scan_udp(&c->udp.fwd_in.f, &c->udp.fwd_out.f, + &c->tcp.fwd_in); } if (c->tcp.fwd_out.mode == FWD_AUTO) { c->tcp.fwd_out.scan4 = open("/proc/net/tcp", flags); c->tcp.fwd_out.scan6 = open("/proc/net/tcp6", flags); - get_bound_ports_tcp(c, 0); + port_fwd_scan_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in); } if (c->udp.fwd_out.f.mode == FWD_AUTO) { c->udp.fwd_out.f.scan4 = open("/proc/net/udp", flags); c->udp.fwd_out.f.scan6 = open("/proc/net/udp6", flags); - get_bound_ports_udp(c, 0); + port_fwd_scan_udp(&c->udp.fwd_out.f, &c->udp.fwd_in.f, + &c->tcp.fwd_out); } } diff --git a/port_fwd.h b/port_fwd.h index 8ab6b48..8a823d8 100644 --- a/port_fwd.h +++ b/port_fwd.h @@ -35,8 +35,9 @@ struct port_fwd { in_port_t delta[NUM_PORTS]; }; -void get_bound_ports_tcp(struct ctx *c, int ns); -void get_bound_ports_udp(struct ctx *c, int ns); +void port_fwd_scan_tcp(struct port_fwd *fwd, const struct port_fwd *rev); +void port_fwd_scan_udp(struct port_fwd *fwd, const struct port_fwd *rev, + const struct port_fwd *tcp); void port_fwd_init(struct ctx *c); #endif /* PORT_FWD_H */ diff --git a/tcp.c b/tcp.c index 921bd2a..1c3af76 100644 --- a/tcp.c +++ b/tcp.c @@ -3240,13 +3240,13 @@ void tcp_timer(struct ctx *c, const struct timespec *ts) struct tcp_port_rebind_arg rebind_arg = { c, 0 }; if (c->tcp.fwd_out.mode == FWD_AUTO) { - get_bound_ports_tcp(c, 0); + port_fwd_scan_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in); rebind_arg.bind_in_ns = 1; NS_CALL(tcp_port_rebind, &rebind_arg); } if (c->tcp.fwd_in.mode == FWD_AUTO) { - get_bound_ports_tcp(c, 1); + port_fwd_scan_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out); rebind_arg.bind_in_ns = 0; tcp_port_rebind(&rebind_arg); } -- 2.41.0

561

Age (days ago)

590

Last active (days ago)

List overview

Download

13 comments

2 participants

participants (2)

David Gibson
Stefano Brivio

[PATCH 0/9] Clean ups to automatic port forwarding

tags

participants (2)