[PATCH 0/8] RFC: Cleanups to auto port scanning

older
[PATCH v2 0/8] Cleanups to auto...

David Gibson

11 Oct 2025 11 Oct '25

6:48 a.m.

One of the trickiest parts of implementing the improved forwarding table is integrating it properly with automatic forwarding (-t auto etc.). Here are some preliminary cleanups to the automatic port scanning that will make the job a bit easier. David Gibson (8): icmp: Remove vestiges of ICMP timer tcp, udp, fwd: Run all port scanning from a single timer fwd: Consolidate scans (not rebinds) in fwd.c fwd: Move port exclusion handling from procfs_scan_listen() to callers fwd: Share port scanning logic between init and timer cases fwd: Check forwarding mode in fwd_scan_ports_*() rather than caller fwd: Update all port maps before applying exclusions tcp, udp: Don't exclude ports in {tcp,udp}_port_rebind() fwd.c | 107 ++++++++++++++++++++++++++++++++++++++------------------ fwd.h | 7 ++-- icmp.h | 2 -- passt.c | 8 ++--- tcp.c | 34 +++++++++--------- tcp.h | 3 +- udp.c | 31 ++++------------ udp.h | 4 +-- util.c | 23 ++++++++++++ util.h | 1 + 10 files changed, 129 insertions(+), 91 deletions(-) -- 2.51.0

Show replies by date

David Gibson

11 Oct 11 Oct

6:48 a.m.

New subject: [PATCH 4/8] fwd: Move port exclusion handling from procfs_scan_listen() to callers

To avoid forwarding loops, we need to exclude certain ports from being auto-forwarded. To accomplish this, procfs_scan_listen() takes a bitmap of exclusions. As it detects each port, it checks against that bitmap. This is a complicated way of accomplishing what we need. We can instead mask out the excluded ports in the callers using a new bitmap_andc() helper. Signed-off-by: David Gibson --- fwd.c | 32 ++++++++++++++------------------ util.c | 23 +++++++++++++++++++++++ util.h | 1 + 3 files changed, 38 insertions(+), 18 deletions(-) diff --git a/fwd.c b/fwd.c index 19309f14..c7b768d5 100644 --- a/fwd.c +++ b/fwd.c @@ -110,13 +110,11 @@ bool fwd_port_is_ephemeral(in_port_t port) * @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) * * #syscalls:pasta lseek * #syscalls:pasta ppc64le:_llseek ppc64:_llseek arm:_llseek */ -static void procfs_scan_listen(int fd, unsigned int lstate, - uint8_t *map, const uint8_t *exclude) +static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map) { struct lineread lr; unsigned long port; @@ -141,10 +139,7 @@ static void procfs_scan_listen(int fd, unsigned int lstate, if (state != lstate) continue; - if (bitmap_isset(exclude, port)) - bitmap_clear(map, port); - else - bitmap_set(map, port); + bitmap_set(map, port); } } @@ -157,8 +152,9 @@ static void fwd_scan_ports_tcp(struct fwd_ports *fwd, const struct fwd_ports *rev) { 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); + procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map); + procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map); } /** @@ -173,26 +169,26 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, const struct fwd_ports *tcp_fwd, const struct fwd_ports *tcp_rev) { - uint8_t exclude[PORT_BITMAP_SIZE]; - - bitmap_or(exclude, PORT_BITMAP_SIZE, rev->map, tcp_rev->map); - memset(fwd->map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map, exclude); - procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map, exclude); + procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map); + procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->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(tcp_fwd->scan4, TCP_LISTEN, fwd->map); + procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map); + + /* * This means we need to skip numbers of TCP ports bound on the other * side, too. Otherwise, we would detect corresponding UDP ports as * bound and try to forward them from the opposite side, but it's * already us handling them. */ - procfs_scan_listen(tcp_fwd->scan4, TCP_LISTEN, fwd->map, exclude); - procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map, exclude); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map); } /** diff --git a/util.c b/util.c index c492f904..eabadf7d 100644 --- a/util.c +++ b/util.c @@ -322,6 +322,7 @@ void bitmap_set(uint8_t *map, unsigned bit) * @map: Pointer to bitmap * @bit: Bit number to clear */ +/* cppcheck-suppress unusedFunction */ void bitmap_clear(uint8_t *map, unsigned bit) { unsigned long *word = (unsigned long *)map + BITMAP_WORD(bit); @@ -351,6 +352,7 @@ bool bitmap_isset(const uint8_t *map, unsigned bit) * @a: First operand * @b: Second operand */ +/* cppcheck-suppress unusedFunction */ void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) { unsigned long *dw = (unsigned long *)dst; @@ -365,6 +367,27 @@ void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) dst[i] = a[i] | b[i]; } +/** + * bitmap_andc() - Logical conjunction with complement (AND NOT) of bitmap + * @dst: Pointer to result bitmap + * @size: Size of bitmaps, in bytes + * @a: First operand + * @b: Second operand + */ +void bitmap_andc(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) +{ + unsigned long *dw = (unsigned long *)dst; + unsigned long *aw = (unsigned long *)a; + unsigned long *bw = (unsigned long *)b; + size_t i; + + for (i = 0; i < size / sizeof(long); i++, dw++, aw++, bw++) + *dw = *aw & ~*bw; + + for (i = size / sizeof(long) * sizeof(long); i < size; i++) + dst[i] = a[i] & ~b[i]; +} + /** * ns_enter() - Enter configured user (unless already joined) and network ns * @c: Execution context diff --git a/util.h b/util.h index 22eaac56..ac8339a3 100644 --- a/util.h +++ b/util.h @@ -213,6 +213,7 @@ void bitmap_set(uint8_t *map, unsigned bit); void bitmap_clear(uint8_t *map, unsigned bit); bool bitmap_isset(const uint8_t *map, unsigned bit); void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b); +void bitmap_andc(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b); char *line_read(char *buf, size_t len, int fd); void ns_enter(const struct ctx *c); bool ns_is_init(void); -- 2.51.0

David Gibson

6:48 a.m.

New subject: [PATCH 1/8] icmp: Remove vestiges of ICMP timer

We no longer have a global ICMP timer (timers for individual flows are handled through the flow timer). We still have an ICMP_TIMER_INTERVAL define, though. Remove it. Signed-off-by: David Gibson --- icmp.h | 2 -- passt.c | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/icmp.h b/icmp.h index d1cecb20..1a0e6205 100644 --- a/icmp.h +++ b/icmp.h @@ -6,8 +6,6 @@ #ifndef ICMP_H #define ICMP_H -#define ICMP_TIMER_INTERVAL 10000 /* ms */ - struct ctx; struct icmp_ping_flow; diff --git a/passt.c b/passt.c index 31fbb75b..b877659e 100644 --- a/passt.c +++ b/passt.c @@ -56,8 +56,7 @@ #define EPOLL_EVENTS 8 -#define TIMER_INTERVAL__ MIN(TCP_TIMER_INTERVAL, UDP_TIMER_INTERVAL) -#define TIMER_INTERVAL_ MIN(TIMER_INTERVAL__, ICMP_TIMER_INTERVAL) +#define TIMER_INTERVAL_ MIN(TCP_TIMER_INTERVAL, UDP_TIMER_INTERVAL) #define TIMER_INTERVAL MIN(TIMER_INTERVAL_, FLOW_TIMER_INTERVAL) char pkt_buf[PKT_BUF_BYTES] __attribute__ ((aligned(PAGE_SIZE))); -- 2.51.0

David Gibson

6:48 a.m.

New subject: [PATCH 6/8] fwd: Check forwarding mode in fwd_scan_ports_*() rather than caller

fwd_scan_ports() needs to check for FWD_AUTO mode before calling each scan function - otherwise it would clobber the forwarding bitmap which should retain the user's fixed configuration. Make this slightly cleaner and safer by moving the mode check into fwd_scan_ports_{tcp,udp}(). Signed-off-by: David Gibson --- fwd.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/fwd.c b/fwd.c index 6f89acf5..57941759 100644 --- a/fwd.c +++ b/fwd.c @@ -151,6 +151,9 @@ static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map) static void fwd_scan_ports_tcp(struct fwd_ports *fwd, const struct fwd_ports *rev) { + if (fwd->mode != FWD_AUTO) + return; + memset(fwd->map, 0, PORT_BITMAP_SIZE); procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map); procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map); @@ -169,6 +172,9 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, const struct fwd_ports *tcp_fwd, const struct fwd_ports *tcp_rev) { + if (fwd->mode != FWD_AUTO) + return; + memset(fwd->map, 0, PORT_BITMAP_SIZE); procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map); procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map); @@ -197,18 +203,12 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, */ static void fwd_scan_ports(struct ctx *c) { - if (c->tcp.fwd_out.mode == FWD_AUTO) - fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in); - if (c->tcp.fwd_in.mode == FWD_AUTO) - fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out); - if (c->udp.fwd_out.mode == FWD_AUTO) { - fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in, - &c->tcp.fwd_out, &c->tcp.fwd_in); - } - if (c->udp.fwd_in.mode == FWD_AUTO) { - fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out, - &c->tcp.fwd_in, &c->tcp.fwd_out); - } + fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in); + fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out); + fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in, + &c->tcp.fwd_out, &c->tcp.fwd_in); + fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out, + &c->tcp.fwd_in, &c->tcp.fwd_out); } /** -- 2.51.0

David Gibson

6:48 a.m.

New subject: [PATCH 8/8] tcp, udp: Don't exclude ports in {tcp,udp}_port_rebind()

To avoid circular forwarding, {tcp,udp}_port_rebind() refuse to listen on ports that we're already listening on in the reverse direction. This is redundant, because we already remove such ports from the forward map when we scan. I believe this was needed, because previously our reverse maps might be one cycle out of date, so could be missing a newly appeared port. We've now rearrange the port scanning code to avoid that, so we don't need the check in tcp_port_rebind() any more. Signed-off-by: David Gibson --- tcp.c | 5 ----- udp.c | 6 ------ 2 files changed, 11 deletions(-) diff --git a/tcp.c b/tcp.c index a330538d..99a8a4c0 100644 --- a/tcp.c +++ b/tcp.c @@ -2817,7 +2817,6 @@ int tcp_init(struct ctx *c) static void tcp_port_rebind(struct ctx *c, bool outbound) { const uint8_t *fmap = outbound ? c->tcp.fwd_out.map : c->tcp.fwd_in.map; - const uint8_t *rmap = outbound ? c->tcp.fwd_in.map : c->tcp.fwd_out.map; int (*socks)[IP_VERSIONS] = outbound ? tcp_sock_ns : tcp_sock_init_ext; unsigned port; @@ -2836,10 +2835,6 @@ static void tcp_port_rebind(struct ctx *c, bool outbound) continue; } - /* Don't loop back our own ports */ - if (bitmap_isset(rmap, port)) - continue; - if ((c->ifi4 && socks[port][V4] == -1) || (c->ifi6 && socks[port][V6] == -1)) { if (outbound) diff --git a/udp.c b/udp.c index e27b2219..4af98503 100644 --- a/udp.c +++ b/udp.c @@ -1191,8 +1191,6 @@ static void udp_port_rebind(struct ctx *c, bool outbound) int (*socks)[NUM_PORTS] = outbound ? udp_splice_ns : udp_splice_init; const uint8_t *fmap = outbound ? c->udp.fwd_out.map : c->udp.fwd_in.map; - const uint8_t *rmap - = outbound ? c->udp.fwd_in.map : c->udp.fwd_out.map; unsigned port; for (port = 0; port < NUM_PORTS; port++) { @@ -1210,10 +1208,6 @@ static void udp_port_rebind(struct ctx *c, bool outbound) continue; } - /* Don't loop back our own ports */ - if (bitmap_isset(rmap, port)) - continue; - if ((c->ifi4 && socks[V4][port] == -1) || (c->ifi6 && socks[V6][port] == -1)) udp_sock_init(c, outbound, NULL, NULL, port); -- 2.51.0

David Gibson

6:48 a.m.

New subject: [PATCH 7/8] fwd: Update all port maps before applying exclusions

In fwd_scan_ports() we go through each of the automatic forwarding cases (tcp, udp, inbound and outbound) in turn, scanning and calculating the new forwarding map. However, to avoid avoid circular forwarding, some of these maps affect each other. This has the odd effect that the ones handled earlier are based on the previous scan of other maps, whereas the later ones are based on the latest scan. That's not generally harmful, but it is counter-intuitive and results in a few odd edge cases. Avoid this by performing all the scans first, without regard to other maps, then applying the exclusions afterwards. One case has an extra wrinkle: for UDP we forwarded not just ports that were listening on UDP but ones listening on TCP as well, for the benefit of protocols like iperf3. We therefore also excluded listening ports from both UDP and TCP from the other direction to avoid circular forwarding. This doesn't really make sense, though. To avoid circular forwarding, we don't care *why* the other side is listening on UDP, just that it *is* listening. I believe the explicit handling of the reverse TCP map was only needed because the reverse map might have been one cycle out of date and therefore not included a port opened because of the corresponding TCP port. Now that we avoid that out of date map possibility, it's sufficient to just mask out UDP listening ports in the other direction. Signed-off-by: David Gibson --- fwd.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/fwd.c b/fwd.c index 57941759..395b0def 100644 --- a/fwd.c +++ b/fwd.c @@ -146,10 +146,8 @@ static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map) /** * fwd_scan_ports_tcp() - Scan /proc to update TCP forwarding map * @fwd: Forwarding information to update - * @rev: Forwarding information for the reverse direction */ -static void fwd_scan_ports_tcp(struct fwd_ports *fwd, - const struct fwd_ports *rev) +static void fwd_scan_ports_tcp(struct fwd_ports *fwd) { if (fwd->mode != FWD_AUTO) return; @@ -157,20 +155,15 @@ static void fwd_scan_ports_tcp(struct fwd_ports *fwd, memset(fwd->map, 0, PORT_BITMAP_SIZE); procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map); procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map); - bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map); } /** * fwd_scan_ports_udp() - Scan /proc to update UDP forwarding map * @fwd: Forwarding information to update - * @rev: Forwarding information for the reverse direction * @tcp_fwd: Corresponding TCP forwarding information - * @tcp_rev: TCP forwarding information for the reverse direction */ static void fwd_scan_ports_udp(struct fwd_ports *fwd, - const struct fwd_ports *rev, - const struct fwd_ports *tcp_fwd, - const struct fwd_ports *tcp_rev) + const struct fwd_ports *tcp_fwd) { if (fwd->mode != FWD_AUTO) return; @@ -186,15 +179,6 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, */ procfs_scan_listen(tcp_fwd->scan4, TCP_LISTEN, fwd->map); procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map); - - /* - * This means we need to skip numbers of TCP ports bound on the other - * side, too. Otherwise, we would detect corresponding UDP ports as - * bound and try to forward them from the opposite side, but it's - * already us handling them. - */ - bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map); - bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map); } /** @@ -203,12 +187,28 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, */ static void fwd_scan_ports(struct ctx *c) { - fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in); - fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out); - fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in, - &c->tcp.fwd_out, &c->tcp.fwd_in); - fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out, - &c->tcp.fwd_in, &c->tcp.fwd_out); + fwd_scan_ports_tcp(&c->tcp.fwd_out); + fwd_scan_ports_tcp(&c->tcp.fwd_in); + fwd_scan_ports_udp(&c->udp.fwd_out, &c->tcp.fwd_out); + fwd_scan_ports_udp(&c->udp.fwd_in, &c->tcp.fwd_in); + + if (c->tcp.fwd_out.mode == FWD_AUTO) { + bitmap_andc(c->tcp.fwd_out.map, PORT_BITMAP_SIZE, + c->tcp.fwd_out.map, c->tcp.fwd_in.map); + } + if (c->tcp.fwd_in.mode == FWD_AUTO) { + bitmap_andc(c->tcp.fwd_in.map, PORT_BITMAP_SIZE, + c->tcp.fwd_in.map, c->tcp.fwd_out.map); + } + + if (c->udp.fwd_out.mode == FWD_AUTO) { + bitmap_andc(c->udp.fwd_out.map, PORT_BITMAP_SIZE, + c->udp.fwd_out.map, c->udp.fwd_in.map); + } + if (c->udp.fwd_in.mode == FWD_AUTO) { + bitmap_andc(c->udp.fwd_in.map, PORT_BITMAP_SIZE, + c->udp.fwd_in.map, c->udp.fwd_out.map); + } } /** -- 2.51.0

David Gibson

6:48 a.m.

New subject: [PATCH 5/8] fwd: Share port scanning logic between init and timer cases

When using -t auto and the like, we scan for listening ports once at startup, then repeatedly on a timer. With previous rearrangements the logic for each of these cases is very nearly repeated. Factor it out into a fwd_scan_ports() function. Signed-off-by: David Gibson --- fwd.c | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/fwd.c b/fwd.c index c7b768d5..6f89acf5 100644 --- a/fwd.c +++ b/fwd.c @@ -191,6 +191,26 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map); } +/** + * fwd_scan_ports() - Scan automatic port forwarding information + * @c: Execution context + */ +static void fwd_scan_ports(struct ctx *c) +{ + if (c->tcp.fwd_out.mode == FWD_AUTO) + fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in); + if (c->tcp.fwd_in.mode == FWD_AUTO) + fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out); + if (c->udp.fwd_out.mode == FWD_AUTO) { + fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in, + &c->tcp.fwd_out, &c->tcp.fwd_in); + } + if (c->udp.fwd_in.mode == FWD_AUTO) { + fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out, + &c->tcp.fwd_in, &c->tcp.fwd_out); + } +} + /** * fwd_scan_ports_init() - Initial setup for automatic port forwarding * @c: Execution context @@ -207,25 +227,20 @@ void fwd_scan_ports_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); - fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out); } if (c->udp.fwd_in.mode == FWD_AUTO) { c->udp.fwd_in.scan4 = open_in_ns(c, "/proc/net/udp", flags); c->udp.fwd_in.scan6 = open_in_ns(c, "/proc/net/udp6", flags); - fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out, - &c->tcp.fwd_in, &c->tcp.fwd_out); } 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); - fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in); } if (c->udp.fwd_out.mode == FWD_AUTO) { c->udp.fwd_out.scan4 = open("/proc/net/udp", flags); c->udp.fwd_out.scan6 = open("/proc/net/udp6", flags); - fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in, - &c->tcp.fwd_out, &c->tcp.fwd_in); } + fwd_scan_ports(c); } /* Last time we scanned for open ports */ @@ -246,18 +261,7 @@ void fwd_scan_ports_timer(struct ctx *c, const struct timespec *now) scan_ports_run = *now; - if (c->tcp.fwd_out.mode == FWD_AUTO) - fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in); - if (c->tcp.fwd_in.mode == FWD_AUTO) - fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out); - if (c->udp.fwd_out.mode == FWD_AUTO) { - fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in, - &c->tcp.fwd_out, &c->tcp.fwd_in); - } - if (c->udp.fwd_in.mode == FWD_AUTO) { - fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out, - &c->tcp.fwd_in, &c->tcp.fwd_out); - } + fwd_scan_ports(c); if (!c->no_tcp) tcp_port_rebind_all(c); -- 2.51.0

Stefano Brivio

30 Oct 30 Oct

9:24 p.m.

New subject: [PATCH 4/8] fwd: Move port exclusion handling from procfs_scan_listen() to callers

On Sat, 11 Oct 2025 15:48:23 +1100 David Gibson wrote:

...

To avoid forwarding loops, we need to exclude certain ports from being auto-forwarded. To accomplish this, procfs_scan_listen() takes a bitmap of exclusions. As it detects each port, it checks against that bitmap. This is a complicated way of accomplishing what we need. We can instead mask out the excluded ports in the callers using a new bitmap_andc() helper.

Signed-off-by: David Gibson --- fwd.c | 32 ++++++++++++++------------------ util.c | 23 +++++++++++++++++++++++ util.h | 1 + 3 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/fwd.c b/fwd.c index 19309f14..c7b768d5 100644 --- a/fwd.c +++ b/fwd.c @@ -110,13 +110,11 @@ bool fwd_port_is_ephemeral(in_port_t port) * @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) * * #syscalls:pasta lseek * #syscalls:pasta ppc64le:_llseek ppc64:_llseek arm:_llseek */ -static void procfs_scan_listen(int fd, unsigned int lstate, - uint8_t *map, const uint8_t *exclude) +static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map) { struct lineread lr; unsigned long port; @@ -141,10 +139,7 @@ static void procfs_scan_listen(int fd, unsigned int lstate, if (state != lstate) continue;

- if (bitmap_isset(exclude, port)) - bitmap_clear(map, port); - else - bitmap_set(map, port); + bitmap_set(map, port); } }

@@ -157,8 +152,9 @@ static void fwd_scan_ports_tcp(struct fwd_ports *fwd, const struct fwd_ports *rev) { 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); + procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map); + procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);

I'm not entirely sure why I implemented it this way in 9657b6ed05cc ("conf, tcp: Periodic detection of bound ports for pasta port forwarding"). I guess the original idea was that only procfs_scan_listen() would manipulate bitmaps, for whatever reason. In any case, I'm fairly sure that this is equivalent (it obviously look like it, but I'm having a hard time to convince myself because of the weird way I implemented it originally).

...

}

/** @@ -173,26 +169,26 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, const struct fwd_ports *tcp_fwd, const struct fwd_ports *tcp_rev) { - uint8_t exclude[PORT_BITMAP_SIZE]; - - bitmap_or(exclude, PORT_BITMAP_SIZE, rev->map, tcp_rev->map); - memset(fwd->map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map, exclude); - procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map, exclude); + procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map); + procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->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(tcp_fwd->scan4, TCP_LISTEN, fwd->map); + procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map); + + /* * This means we need to skip numbers of TCP ports bound on the other

Nit: /* This ...

...

* side, too. Otherwise, we would detect corresponding UDP ports as * bound and try to forward them from the opposite side, but it's * already us handling them. */ - procfs_scan_listen(tcp_fwd->scan4, TCP_LISTEN, fwd->map, exclude); - procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map, exclude); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map); }

/** diff --git a/util.c b/util.c index c492f904..eabadf7d 100644 --- a/util.c +++ b/util.c @@ -322,6 +322,7 @@ void bitmap_set(uint8_t *map, unsigned bit) * @map: Pointer to bitmap * @bit: Bit number to clear */ +/* cppcheck-suppress unusedFunction */ void bitmap_clear(uint8_t *map, unsigned bit) { unsigned long *word = (unsigned long *)map + BITMAP_WORD(bit); @@ -351,6 +352,7 @@ bool bitmap_isset(const uint8_t *map, unsigned bit) * @a: First operand * @b: Second operand */ +/* cppcheck-suppress unusedFunction */

Should we just drop those? git stores them for us.

...

void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) { unsigned long *dw = (unsigned long *)dst; @@ -365,6 +367,27 @@ void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) dst[i] = a[i] | b[i]; }

+/** + * bitmap_andc() - Logical conjunction with complement (AND NOT) of bitmap

Nit: this function name mixes classic logic terminology (conjunction, complement) and operator names (and, not), which makes it hard to guess, I think. The Linux kernel has __bitmap_andnot(), which I find rather cacophonic, so what about bitmap_and_not() instead? It kind of fits with how we call these things in passt.

...

+ * @dst: Pointer to result bitmap + * @size: Size of bitmaps, in bytes + * @a: First operand + * @b: Second operand + */ +void bitmap_andc(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) +{ + unsigned long *dw = (unsigned long *)dst; + unsigned long *aw = (unsigned long *)a; + unsigned long *bw = (unsigned long *)b; + size_t i; + + for (i = 0; i < size / sizeof(long); i++, dw++, aw++, bw++) + *dw = *aw & ~*bw; + + for (i = size / sizeof(long) * sizeof(long); i < size; i++) + dst[i] = a[i] & ~b[i]; +} + /** * ns_enter() - Enter configured user (unless already joined) and network ns * @c: Execution context diff --git a/util.h b/util.h index 22eaac56..ac8339a3 100644 --- a/util.h +++ b/util.h @@ -213,6 +213,7 @@ void bitmap_set(uint8_t *map, unsigned bit); void bitmap_clear(uint8_t *map, unsigned bit); bool bitmap_isset(const uint8_t *map, unsigned bit); void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b); +void bitmap_andc(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b); char *line_read(char *buf, size_t len, int fd); void ns_enter(const struct ctx *c); bool ns_is_init(void);

-- Stefano

Stefano Brivio

9:24 p.m.

New subject: [PATCH 7/8] fwd: Update all port maps before applying exclusions

On Sat, 11 Oct 2025 15:48:26 +1100 David Gibson wrote:

...

In fwd_scan_ports() we go through each of the automatic forwarding cases (tcp, udp, inbound and outbound) in turn, scanning and calculating the new forwarding map. However, to avoid avoid circular forwarding, some of these maps affect each other. This has the odd effect that the ones handled earlier are based on the previous scan of other maps, whereas the later ones are based on the latest scan.

That's not generally harmful, but it is counter-intuitive and results in a few odd edge cases. Avoid this by performing all the scans first, without regard to other maps, then applying the exclusions afterwards.

One case has an extra wrinkle: for UDP we forwarded not just ports that were listening on UDP but ones listening on TCP as well, for the benefit of protocols like iperf3. We therefore also excluded listening ports from both UDP and TCP from the other direction to avoid circular forwarding.

This doesn't really make sense, though. To avoid circular forwarding, we don't care *why* the other side is listening on UDP, just that it *is* listening. I believe the explicit handling of the reverse TCP map was only needed because the reverse map might have been one cycle out of date and therefore not included a port opened because of the corresponding TCP port.

Right, yes, that was the reason. I guess it makes sense to make this less hypothetical in the commit message if you re-spin. Same in 8/8. The rest of the series looks good to me.

...

Now that we avoid that out of date map possibility, it's sufficient to just mask out UDP listening ports in the other direction.

Signed-off-by: David Gibson --- fwd.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/fwd.c b/fwd.c index 57941759..395b0def 100644 --- a/fwd.c +++ b/fwd.c @@ -146,10 +146,8 @@ static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map) /** * fwd_scan_ports_tcp() - Scan /proc to update TCP forwarding map * @fwd: Forwarding information to update - * @rev: Forwarding information for the reverse direction */ -static void fwd_scan_ports_tcp(struct fwd_ports *fwd, - const struct fwd_ports *rev) +static void fwd_scan_ports_tcp(struct fwd_ports *fwd) { if (fwd->mode != FWD_AUTO) return; @@ -157,20 +155,15 @@ static void fwd_scan_ports_tcp(struct fwd_ports *fwd, memset(fwd->map, 0, PORT_BITMAP_SIZE); procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map); procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map); - bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map); }

/** * fwd_scan_ports_udp() - Scan /proc to update UDP forwarding map * @fwd: Forwarding information to update - * @rev: Forwarding information for the reverse direction * @tcp_fwd: Corresponding TCP forwarding information - * @tcp_rev: TCP forwarding information for the reverse direction */ static void fwd_scan_ports_udp(struct fwd_ports *fwd, - const struct fwd_ports *rev, - const struct fwd_ports *tcp_fwd, - const struct fwd_ports *tcp_rev) + const struct fwd_ports *tcp_fwd) { if (fwd->mode != FWD_AUTO) return; @@ -186,15 +179,6 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, */ procfs_scan_listen(tcp_fwd->scan4, TCP_LISTEN, fwd->map); procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map); - - /* - * This means we need to skip numbers of TCP ports bound on the other - * side, too. Otherwise, we would detect corresponding UDP ports as - * bound and try to forward them from the opposite side, but it's - * already us handling them. - */ - bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map); - bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map); }

/** @@ -203,12 +187,28 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, */ static void fwd_scan_ports(struct ctx *c) { - fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in); - fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out); - fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in, - &c->tcp.fwd_out, &c->tcp.fwd_in); - fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out, - &c->tcp.fwd_in, &c->tcp.fwd_out); + fwd_scan_ports_tcp(&c->tcp.fwd_out); + fwd_scan_ports_tcp(&c->tcp.fwd_in); + fwd_scan_ports_udp(&c->udp.fwd_out, &c->tcp.fwd_out); + fwd_scan_ports_udp(&c->udp.fwd_in, &c->tcp.fwd_in); + + if (c->tcp.fwd_out.mode == FWD_AUTO) { + bitmap_andc(c->tcp.fwd_out.map, PORT_BITMAP_SIZE, + c->tcp.fwd_out.map, c->tcp.fwd_in.map); + } + if (c->tcp.fwd_in.mode == FWD_AUTO) { + bitmap_andc(c->tcp.fwd_in.map, PORT_BITMAP_SIZE, + c->tcp.fwd_in.map, c->tcp.fwd_out.map); + } + + if (c->udp.fwd_out.mode == FWD_AUTO) { + bitmap_andc(c->udp.fwd_out.map, PORT_BITMAP_SIZE, + c->udp.fwd_out.map, c->udp.fwd_in.map); + } + if (c->udp.fwd_in.mode == FWD_AUTO) { + bitmap_andc(c->udp.fwd_in.map, PORT_BITMAP_SIZE, + c->udp.fwd_in.map, c->udp.fwd_out.map); + } }

/**

-- Stefano

David Gibson

31 Oct 31 Oct

3:51 a.m.

New subject: [PATCH 7/8] fwd: Update all port maps before applying exclusions

On Thu, Oct 30, 2025 at 09:24:07PM +0100, Stefano Brivio wrote:

...

On Sat, 11 Oct 2025 15:48:26 +1100 David Gibson wrote:

...
In fwd_scan_ports() we go through each of the automatic forwarding cases (tcp, udp, inbound and outbound) in turn, scanning and calculating the new forwarding map. However, to avoid avoid circular forwarding, some of these maps affect each other. This has the odd effect that the ones handled earlier are based on the previous scan of other maps, whereas the later ones are based on the latest scan.

That's not generally harmful, but it is counter-intuitive and results in a few odd edge cases. Avoid this by performing all the scans first, without regard to other maps, then applying the exclusions afterwards.

One case has an extra wrinkle: for UDP we forwarded not just ports that were listening on UDP but ones listening on TCP as well, for the benefit of protocols like iperf3. We therefore also excluded listening ports from both UDP and TCP from the other direction to avoid circular forwarding.

This doesn't really make sense, though. To avoid circular forwarding, we don't care *why* the other side is listening on UDP, just that it *is* listening. I believe the explicit handling of the reverse TCP map was only needed because the reverse map might have been one cycle out of date and therefore not included a port opened because of the corresponding TCP port.

Right, yes, that was the reason. I guess it makes sense to make this less hypothetical in the commit message if you re-spin. Same in 8/8.

There are some (trivial) conflicts with other things you've merged, so I will respin. I've fixed this up. -- David Gibson (he or they) | 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

3:47 a.m.

New subject: [PATCH 4/8] fwd: Move port exclusion handling from procfs_scan_listen() to callers

On Thu, Oct 30, 2025 at 09:24:04PM +0100, Stefano Brivio wrote:

...

On Sat, 11 Oct 2025 15:48:23 +1100 David Gibson wrote:

...
To avoid forwarding loops, we need to exclude certain ports from being auto-forwarded. To accomplish this, procfs_scan_listen() takes a bitmap of exclusions. As it detects each port, it checks against that bitmap. This is a complicated way of accomplishing what we need. We can instead mask out the excluded ports in the callers using a new bitmap_andc() helper.

Signed-off-by: David Gibson --- fwd.c | 32 ++++++++++++++------------------ util.c | 23 +++++++++++++++++++++++ util.h | 1 + 3 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/fwd.c b/fwd.c index 19309f14..c7b768d5 100644 --- a/fwd.c +++ b/fwd.c @@ -110,13 +110,11 @@ bool fwd_port_is_ephemeral(in_port_t port) * @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) * * #syscalls:pasta lseek * #syscalls:pasta ppc64le:_llseek ppc64:_llseek arm:_llseek */ -static void procfs_scan_listen(int fd, unsigned int lstate, - uint8_t *map, const uint8_t *exclude) +static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map) { struct lineread lr; unsigned long port; @@ -141,10 +139,7 @@ static void procfs_scan_listen(int fd, unsigned int lstate, if (state != lstate) continue;

- if (bitmap_isset(exclude, port)) - bitmap_clear(map, port); - else - bitmap_set(map, port); + bitmap_set(map, port); } }

@@ -157,8 +152,9 @@ static void fwd_scan_ports_tcp(struct fwd_ports *fwd, const struct fwd_ports *rev) { 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); + procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map); + procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);

I'm not entirely sure why I implemented it this way in 9657b6ed05cc ("conf, tcp: Periodic detection of bound ports for pasta port forwarding").

I guess the original idea was that only procfs_scan_listen() would manipulate bitmaps, for whatever reason. In any case, I'm fairly sure that this is equivalent (it obviously look like it, but I'm having a hard time to convince myself because of the weird way I implemented it originally).

...
}

/** @@ -173,26 +169,26 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, const struct fwd_ports *tcp_fwd, const struct fwd_ports *tcp_rev) { - uint8_t exclude[PORT_BITMAP_SIZE]; - - bitmap_or(exclude, PORT_BITMAP_SIZE, rev->map, tcp_rev->map); - memset(fwd->map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map, exclude); - procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map, exclude); + procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map); + procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->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(tcp_fwd->scan4, TCP_LISTEN, fwd->map); + procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map); + + /* * This means we need to skip numbers of TCP ports bound on the other

Nit: /* This ...

Fixed.

...

...
* side, too. Otherwise, we would detect corresponding UDP ports as * bound and try to forward them from the opposite side, but it's * already us handling them. */ - procfs_scan_listen(tcp_fwd->scan4, TCP_LISTEN, fwd->map, exclude); - procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map, exclude); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map); }

/** diff --git a/util.c b/util.c index c492f904..eabadf7d 100644 --- a/util.c +++ b/util.c @@ -322,6 +322,7 @@ void bitmap_set(uint8_t *map, unsigned bit) * @map: Pointer to bitmap * @bit: Bit number to clear */ +/* cppcheck-suppress unusedFunction */ void bitmap_clear(uint8_t *map, unsigned bit) { unsigned long *word = (unsigned long *)map + BITMAP_WORD(bit); @@ -351,6 +352,7 @@ bool bitmap_isset(const uint8_t *map, unsigned bit) * @a: First operand * @b: Second operand */ +/* cppcheck-suppress unusedFunction */

Should we just drop those? git stores them for us.

Eh, maybe? It felt weird to remove a function that seemed like it belonged in the bitmap "library" of functions. Plus at the time I thought I might use it again later in the series. I didn't but I still might use it relatively soon in follow up series.

...

...
void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) { unsigned long *dw = (unsigned long *)dst; @@ -365,6 +367,27 @@ void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) dst[i] = a[i] | b[i]; }

+/** + * bitmap_andc() - Logical conjunction with complement (AND NOT) of bitmap

Nit: this function name mixes classic logic terminology (conjunction, complement) and operator names (and, not), which makes it hard to guess, I think.

That's my POWER background showing: https://www.ibm.com/docs/en/aix/7.3.0?topic=set-andc-complement-instruction

...

The Linux kernel has __bitmap_andnot(), which I find rather cacophonic, so what about bitmap_and_not() instead? It kind of fits with how we call these things in passt.

Makes sense, done.

...

...
+ * @dst: Pointer to result bitmap + * @size: Size of bitmaps, in bytes + * @a: First operand + * @b: Second operand + */ +void bitmap_andc(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) +{ + unsigned long *dw = (unsigned long *)dst; + unsigned long *aw = (unsigned long *)a; + unsigned long *bw = (unsigned long *)b; + size_t i; + + for (i = 0; i < size / sizeof(long); i++, dw++, aw++, bw++) + *dw = *aw & ~*bw; + + for (i = size / sizeof(long) * sizeof(long); i < size; i++) + dst[i] = a[i] & ~b[i]; +} + /** * ns_enter() - Enter configured user (unless already joined) and network ns * @c: Execution context diff --git a/util.h b/util.h index 22eaac56..ac8339a3 100644 --- a/util.h +++ b/util.h @@ -213,6 +213,7 @@ void bitmap_set(uint8_t *map, unsigned bit); void bitmap_clear(uint8_t *map, unsigned bit); bool bitmap_isset(const uint8_t *map, unsigned bit); void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b); +void bitmap_andc(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b); char *line_read(char *buf, size_t len, int fd); void ns_enter(const struct ctx *c); bool ns_is_init(void);

-- Stefano

-- David Gibson (he or they) | 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

Stefano Brivio

9:21 a.m.

New subject: [PATCH 4/8] fwd: Move port exclusion handling from procfs_scan_listen() to callers

On Fri, 31 Oct 2025 13:47:10 +1100 David Gibson wrote:

...

On Thu, Oct 30, 2025 at 09:24:04PM +0100, Stefano Brivio wrote:

...
On Sat, 11 Oct 2025 15:48:23 +1100 David Gibson wrote:

...
To avoid forwarding loops, we need to exclude certain ports from being auto-forwarded. To accomplish this, procfs_scan_listen() takes a bitmap of exclusions. As it detects each port, it checks against that bitmap. This is a complicated way of accomplishing what we need. We can instead mask out the excluded ports in the callers using a new bitmap_andc() helper.

Signed-off-by: David Gibson --- fwd.c | 32 ++++++++++++++------------------ util.c | 23 +++++++++++++++++++++++ util.h | 1 + 3 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/fwd.c b/fwd.c index 19309f14..c7b768d5 100644 --- a/fwd.c +++ b/fwd.c @@ -110,13 +110,11 @@ bool fwd_port_is_ephemeral(in_port_t port) * @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) * * #syscalls:pasta lseek * #syscalls:pasta ppc64le:_llseek ppc64:_llseek arm:_llseek */ -static void procfs_scan_listen(int fd, unsigned int lstate, - uint8_t *map, const uint8_t *exclude) +static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map) { struct lineread lr; unsigned long port; @@ -141,10 +139,7 @@ static void procfs_scan_listen(int fd, unsigned int lstate, if (state != lstate) continue;

- if (bitmap_isset(exclude, port)) - bitmap_clear(map, port); - else - bitmap_set(map, port); + bitmap_set(map, port); } }

@@ -157,8 +152,9 @@ static void fwd_scan_ports_tcp(struct fwd_ports *fwd, const struct fwd_ports *rev) { 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); + procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map); + procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);

I'm not entirely sure why I implemented it this way in 9657b6ed05cc ("conf, tcp: Periodic detection of bound ports for pasta port forwarding").

I guess the original idea was that only procfs_scan_listen() would manipulate bitmaps, for whatever reason. In any case, I'm fairly sure that this is equivalent (it obviously look like it, but I'm having a hard time to convince myself because of the weird way I implemented it originally).

...
}

/** @@ -173,26 +169,26 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd, const struct fwd_ports *tcp_fwd, const struct fwd_ports *tcp_rev) { - uint8_t exclude[PORT_BITMAP_SIZE]; - - bitmap_or(exclude, PORT_BITMAP_SIZE, rev->map, tcp_rev->map); - memset(fwd->map, 0, PORT_BITMAP_SIZE); - procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map, exclude); - procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map, exclude); + procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map); + procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->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(tcp_fwd->scan4, TCP_LISTEN, fwd->map); + procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map); + + /* * This means we need to skip numbers of TCP ports bound on the other

Nit: /* This ...

Fixed.

...
...
* side, too. Otherwise, we would detect corresponding UDP ports as * bound and try to forward them from the opposite side, but it's * already us handling them. */ - procfs_scan_listen(tcp_fwd->scan4, TCP_LISTEN, fwd->map, exclude); - procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map, exclude); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map); + bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map); }

/** diff --git a/util.c b/util.c index c492f904..eabadf7d 100644 --- a/util.c +++ b/util.c @@ -322,6 +322,7 @@ void bitmap_set(uint8_t *map, unsigned bit) * @map: Pointer to bitmap * @bit: Bit number to clear */ +/* cppcheck-suppress unusedFunction */ void bitmap_clear(uint8_t *map, unsigned bit) { unsigned long *word = (unsigned long *)map + BITMAP_WORD(bit); @@ -351,6 +352,7 @@ bool bitmap_isset(const uint8_t *map, unsigned bit) * @a: First operand * @b: Second operand */ +/* cppcheck-suppress unusedFunction */

Should we just drop those? git stores them for us.

Eh, maybe? It felt weird to remove a function that seemed like it belonged in the bitmap "library" of functions. Plus at the time I thought I might use it again later in the series. I didn't but I still might use it relatively soon in follow up series.

Sure.

...

...
...
void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) { unsigned long *dw = (unsigned long *)dst; @@ -365,6 +367,27 @@ void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) dst[i] = a[i] | b[i]; }

+/** + * bitmap_andc() - Logical conjunction with complement (AND NOT) of bitmap

Nit: this function name mixes classic logic terminology (conjunction, complement) and operator names (and, not), which makes it hard to guess, I think.

That's my POWER background showing: https://www.ibm.com/docs/en/aix/7.3.0?topic=set-andc-complement-instruction

I obviously don't have a POWER background but I do remember a couple of instructions simply because I found their names hilariously dissonant (other than finding them in whatever disassembly I was looking at). My favourite mouthful is RLWIMI: https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.3?topic=rf-rldimi-buil... but ANDC is also remarkable (cf. ANDN in the successful CISC set, which obviously sounds like the only right choice to me). -- Stefano

David Gibson

3 Nov 3 Nov

2:43 a.m.

New subject: [PATCH 4/8] fwd: Move port exclusion handling from procfs_scan_listen() to callers

On Fri, Oct 31, 2025 at 09:21:20AM +0100, Stefano Brivio wrote:

...

On Fri, 31 Oct 2025 13:47:10 +1100 David Gibson wrote:

...
On Thu, Oct 30, 2025 at 09:24:04PM +0100, Stefano Brivio wrote:

...
On Sat, 11 Oct 2025 15:48:23 +1100 David Gibson wrote: [snip]

...
void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) { unsigned long *dw = (unsigned long *)dst; @@ -365,6 +367,27 @@ void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b) dst[i] = a[i] | b[i]; }

+/** + * bitmap_andc() - Logical conjunction with complement (AND NOT) of bitmap

Nit: this function name mixes classic logic terminology (conjunction, complement) and operator names (and, not), which makes it hard to guess, I think.

That's my POWER background showing: https://www.ibm.com/docs/en/aix/7.3.0?topic=set-andc-complement-instruction

I obviously don't have a POWER background but I do remember a couple of instructions simply because I found their names hilariously dissonant (other than finding them in whatever disassembly I was looking at). My favourite mouthful is RLWIMI:

Yes, rlwimi is fun. Along with rlwimn, and once you go 64-bit rldimi, rldicr and rldicl. They're utterly inscrutable when you first encounter them, but are super useful and natural once you get used to them. Well, rlwimi and rlwimn at least. The 64-bit bit versions are upsettingly less general, simply because there aren't enough bits in an instruction to encode true equivalents of the 32-bit versions.

...

https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.3?topic=rf-rldimi-buil...

but ANDC is also remarkable (cf. ANDN in the successful CISC set, which obviously sounds like the only right choice to me).

And, of course, there's "eieio". -- David Gibson (he or they) | 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

Age (days ago)

Last active (days ago)

List overview

Download

12 comments

2 participants

participants (2)

David Gibson
Stefano Brivio

[PATCH 0/8] RFC: Cleanups to auto port scanning

tags

participants (2)