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=atYogtr0; 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 AED975A061C for ; Thu, 30 Oct 2025 21:24:14 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1761855853; 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=6522K3DlE7XRjSCOPI2QVNi6OL3IKGfAU65NSDnB8YI=; b=atYogtr07UTwQc2ga7DUVsGBl+FRmOejI0WTSQFYX8mdEp1aZTOsnoAz7/skTX3lYCcToM nw1vkAZPXK1rHU5nAK6NA/qbDWSl27gn4RpOcn5Tx//ScdV1NU9Nj5l3bsCkZqTnRgPgc0 IqN2VGS35nwd1nIz8o1cwxy8kNZXOgU= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-130-pHm6mAkNPkmwisPss9Kq9Q-1; Thu, 30 Oct 2025 16:24:11 -0400 X-MC-Unique: pHm6mAkNPkmwisPss9Kq9Q-1 X-Mimecast-MFC-AGG-ID: pHm6mAkNPkmwisPss9Kq9Q_1761855850 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-3f384f10762so1634642f8f.3 for ; Thu, 30 Oct 2025 13:24:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761855850; x=1762460650; 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=6522K3DlE7XRjSCOPI2QVNi6OL3IKGfAU65NSDnB8YI=; b=ebwrRM3SdPLTec6MjSPiisONBf7rr/x6gcR5QAS8cyTnM48urlsXMD0elEOjN562rX S3Be40IotCFMmfzbuhJh5kOIqw02sHd5hbOYL6k0GxX3XtRXMcJcwlJWy2JY+b4PWUaW 6n4LafBAGggtrlgPS4gE0HZpneqPn9IY90/MxNTEnDn6csC9STdBJBwhJu5wAm4fFE3f cZdbIYJahxz1ZQOA6OSwxcDd5NKdCSYplkRyI2xanTPOQvVL2ldGyuzH/oAetzyNzjFH Q/tl2Jzs/iD/rMaQ+o1zKBKRWuGF91BNVCrRQUMu2rqbWoYzKRvWE057g+c6zzBH6Kpj 2gow== X-Gm-Message-State: AOJu0YxKfUWIMpON340HswLSDJhYT1J5VzoON8KDZjickzxwq704DzRL qZwwtsPC4GEz7XeAru+HKRbkrCLUNc8RxBKIHqH66bSVqeW0aWd3m3pymYM6DeamihNX0jKtchP +IiXPQkNjUjMpuKWu6OFptIhKr2D5RifbSe8bXl2bh0lwQsZVMn4bQkxnFyirZQ== X-Gm-Gg: ASbGncsni7v/4qn8UfKz+R8pJlUAhKySsuJbFSjC74tcFb2KCVfCY4te1TBdKlAS6Pg 40t8MzTZxQAFwcpo0uXVri5wMXmvuRvF8Z0RuHBl5VFWA/ZMQjpskJnzP5fwVGECpLSXlc90BhU WLCDI2S3LTxvgl/dzuc0WxddFXEZI3ODkbgJMMKdaJCSC4FYZ7uRuPiw0ec0B8NehZrRhGRkpO4 a5tTSNTwk9B3ekaXadqmAHwYSyuIomUL4ZCsgKd37iR7CuRmptnKBkQBIjkUAGr6Ism/jhf8k9M KLAhBPnVel93BQGs3JkNMCl1Dr3PLbMjwokPXK0ut2wXDC21z8DZIy4AxgBaygPapTqM2i61q0y jWE0ILGWbFNfThM/3xaPRp9dHUeU= X-Received: by 2002:a05:600c:4e05:b0:475:de12:d3b5 with SMTP id 5b1f17b1804b1-477308a8f1amr10680345e9.34.1761855849963; Thu, 30 Oct 2025 13:24:09 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEnU13dC+YmmmVPBOLf+pjI9n6Nlagid6578v1rADzVwbPksVlsrBozeWy9qxWqxkPCo0LkzQ== X-Received: by 2002:a05:600c:4e05:b0:475:de12:d3b5 with SMTP id 5b1f17b1804b1-477308a8f1amr10680205e9.34.1761855849471; Thu, 30 Oct 2025 13:24:09 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-477289adc18sm62131805e9.6.2025.10.30.13.24.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Oct 2025 13:24:08 -0700 (PDT) Date: Thu, 30 Oct 2025 21:24:07 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 7/8] fwd: Update all port maps before applying exclusions Message-ID: <20251030212407.66e07446@elisabeth> In-Reply-To: <20251011044827.862757-8-david@gibson.dropbear.id.au> References: <20251011044827.862757-1-david@gibson.dropbear.id.au> <20251011044827.862757-8-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: Cccf4mHMXc5i3mWIAbgdVq7Lb8GCuk-R7r7uXjT77a8_1761855850 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: LI2ET6QYCD4FBUJBHG7YBASHTSCF3452 X-Message-ID-Hash: LI2ET6QYCD4FBUJBHG7YBASHTSCF3452 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: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