From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=TiOEXGZ6; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id A1C715A0619 for ; Thu, 30 Oct 2025 21:24:10 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1761855849; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EEF7Jf11pmIvrDBfswEpT4UxRv3NR3mNiw22w4CfZT0=; b=TiOEXGZ6KMmyNQ7+0zAOqie/GYkkndfmwNyNM0FyWV2dWtrhRS0g6WWiYn0/ZbtUw8F4Sn HR1F8AZVnjvebYUjYQms+TuAmXrNqbKEdkGN7+uwoLa5O5oy12J2PkFmXOA5CTQqBdR6Pa 6U2rikuUO/A4AQp8pVhLmZX/Wl4rcWQ= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-460-idQu_100PceLm44XRmpG6w-1; Thu, 30 Oct 2025 16:24:08 -0400 X-MC-Unique: idQu_100PceLm44XRmpG6w-1 X-Mimecast-MFC-AGG-ID: idQu_100PceLm44XRmpG6w_1761855847 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-475db4369b1so8054215e9.3 for ; Thu, 30 Oct 2025 13:24:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761855846; x=1762460646; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=EEF7Jf11pmIvrDBfswEpT4UxRv3NR3mNiw22w4CfZT0=; b=MN8zzpaIEl2+3wplRJCfbnUhfOUqxuJtP1arW14UZULef0cy0QcSHPuuKwOIWh0nsv nYHgHX0GkQ7vq6z7fgmUFLfPMgzOKYSZjXo0HtHrUWy3QU8fk5KHsvaFayITcBCLlTzJ sZarCxRSbZcGbz53em0+Jk2c2BXGSnykAo5U3vXTfDybFT7ziDvR8KL2OZ9VjQnEEUMe HA2+QJ85ucY+MdZtoKRklIBuixG34nqleOwca7cf7ZfarY8BYX0RjmDCT4HcFs14Sbop 8fTQE09wIvnB5Aj4ZpHsK+sX6CjzMTlJWMikCgoFtIJWP5UB6EAsNuEVfkd6gxCM5DKX I3aw== X-Gm-Message-State: AOJu0YyhViK7POL3OELIOJ0S87nGXyq7kKO/nWdzlh30tRmhK1sagXGI blurgdlKWbcWK6FyNUiLmxAF+RnWEvRpfqa6JeLkoBTVOIvHwOjkZ7AXDQXlwGAzco1fv1QhT4k 48oAeoklpKrHWm2UrYKAf5akeGOtLJoFj/s0g+ty7v7TrW45YHmxXbmZ4jGjLtw== X-Gm-Gg: ASbGnctLLlHpCFV8qtN2+GjWCtGZRXl24Gt1ler9Uj9HjtXb8waDeMAfBnz3inP2s0D eoyqIfHHbkUGh0iVbks5Ot4TL7nKqvCsK/ExcdWhtuRCa87URjm3EeWu99QIsFj63lTAvIZbEK1 WqtgNe1DCafUxH8fKIIvXMmTkOjdJnAdiaef5orqUpzrTU8XboTSo7/g6/f/hE7c84tYDgVctOp PBHTAOw02MMLzS7tzV1ODU6QJIIp0TgF91upJGgTOcJf+ho1QRa4pxZunbaSxZ17GVHK367giWW 6s5RfJLl3Ykg08GaxWJJbiLm6UQ5AEALzjoYMMuvb4nUxu/7+XH0WHEfmRMy2+aCCTqWViOVAah 81bb/Ny/0WQ== X-Received: by 2002:a05:600c:a11:b0:475:db8f:ae0e with SMTP id 5b1f17b1804b1-477307944damr11346345e9.2.1761855846549; Thu, 30 Oct 2025 13:24:06 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGSLjXAC8KiKdAqGzZN7P+j+ezp1PszQCNnGIImyWs8tYh5BNe/rO8sk536ZSQuTbzo/jgbhg== X-Received: by 2002:a05:600c:a11:b0:475:db8f:ae0e with SMTP id 5b1f17b1804b1-477307944damr11346105e9.2.1761855845958; Thu, 30 Oct 2025 13:24:05 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4772899fe36sm56447335e9.2.2025.10.30.13.24.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Oct 2025 13:24:05 -0700 (PDT) Date: Thu, 30 Oct 2025 21:24:04 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 4/8] fwd: Move port exclusion handling from procfs_scan_listen() to callers Message-ID: <20251030212404.3eda9c82@elisabeth> In-Reply-To: <20251011044827.862757-5-david@gibson.dropbear.id.au> References: <20251011044827.862757-1-david@gibson.dropbear.id.au> <20251011044827.862757-5-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: kHZocMBmr1I6T2sN3ASdP2-cjIQvq-5C2gdvV-y_WK8_1761855847 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 4MUYI5ENUL2JSESNUV74D7WWABOZNMSJ X-Message-ID-Hash: 4MUYI5ENUL2JSESNUV74D7WWABOZNMSJ X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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