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=L27F4f7w; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id C1FE85A0265 for ; Fri, 05 Jun 2026 13:57:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1780660666; 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=epGaM/Ev5JhfBKYIxaHDWmi4Ei1vbup4VTK+Sqy8Pjw=; b=L27F4f7wVkr1jzXCC0+BEwmuouvwWWMZgdWzBFclO1JEIdhHXg+RBANBysfHbgG+C+m9Vh JH64S+iBrkYwXqQuFgckGx9CdMQPgpO9B7hIM9Bkcl6wV7CPYKsPHJKCDrB9OsQr7M8dwF T7npyE9Mtla0Ez1tgTTeYaAeldDGw5k= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-643-B2mzUNR1NLyu8d1JssU_Hw-1; Fri, 05 Jun 2026 07:57:45 -0400 X-MC-Unique: B2mzUNR1NLyu8d1JssU_Hw-1 X-Mimecast-MFC-AGG-ID: B2mzUNR1NLyu8d1JssU_Hw_1780660664 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-45ef66cc579so865782f8f.3 for ; Fri, 05 Jun 2026 04:57:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780660664; x=1781265464; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=epGaM/Ev5JhfBKYIxaHDWmi4Ei1vbup4VTK+Sqy8Pjw=; b=O0EM+/jXWeU0QlvXMKuPENfhMgawxnyTscVd+FG4VGpH0DVQKIalEmOXIUxGUYXGcP L54tUx5WaS5xAGo8QVXARKDLdibCtPcWKM/aiQuXAOKdgRn4uKZkaGzLR9v3rWBk9Wmr H/G+ALymNOKz0ASWv9DIEZ2zCmRpwItUDwb4dbF0EXkXK/OHkEXP8Xx6+cZM59XLoZzG c44AK9Xv1N6x3h2jbzIHcglATs0Q/FIAhlC1nJGhpWtMYgJfYygKm4S2Vzd5mcx6cbgj SAGuteCQGYt0aKqlHNORV6VwoezpAi8dO70v2ReQnTQSRI3Dc09pxnlm3vM249tfMwVS hTDg== X-Forwarded-Encrypted: i=1; AFNElJ/jInpLIf4QaMwLI7IL9uTDEGuFg+OLzqKAc4/xXvV+yXRzJxB//u6G0B6ofYIX/TWauw6CiseXmI8=@passt.top X-Gm-Message-State: AOJu0YwlaPa76DT/KPHIVyj1yTiDhRrsTbxBeNKcbcMQDVpdWEk3atDN ENp71Z3lvYv7ASOQVs/xtjJ4S71/4mI7BXVzBlcjes4YmipYkrsnD8aGNGl9NB9XTSI3gEtPgED 932G+U1M97z2fJCj3y9Ry9rPptjvb+5vpx+o+/iEQDzb/HvoWyWIv4w== X-Gm-Gg: Acq92OH/u//o7hMAAU1kCBcWAWlfmn7jzJm/EBX+6fZaCxFKwuPdIhfemqIuaZ7PSB6 y9uUtSdIOCX3o4+yE2OlVUeHTRwQOkknF/Po7ayV1+mUbh67RZYw3hC26Jxy7k9ki11q/FDhilD fXdkNfbeSt/1MZe/I+a1FaI1s9RI6TnokukE+wC7HILsuYQHFNhZ9K4ULqk2C4F9qZl2cRHhYMu 8MKiW75C4twNm1uoDf29w8OGNjzk+S5Njfxpyw6qYfb/N5vyRRjXb4ZQS/SwPKfdExWWO1aD/pg EeOd8+9kMr5RJ5s823E1/fe8vFtFbAer3MGqoUfBe5i1YOoKFixplaHBlmBlQkLlqrqi+ZE8EYX q5LKzgqXoohbXRmKpUjBxp2HBr1ccCMvdi35yoqAuzTxqXAV3uibkcNJWnbAY X-Received: by 2002:adf:f40b:0:b0:45e:f28e:a00d with SMTP id ffacd0b85a97d-460302ec07fmr4155225f8f.16.1780660663607; Fri, 05 Jun 2026 04:57:43 -0700 (PDT) X-Received: by 2002:adf:f40b:0:b0:45e:f28e:a00d with SMTP id ffacd0b85a97d-460302ec07fmr4155182f8f.16.1780660662999; Fri, 05 Jun 2026 04:57:42 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4601f2f2710sm19597587f8f.14.2026.06.05.04.57.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Jun 2026 04:57:42 -0700 (PDT) From: Stefano Brivio To: EJ Campbell Subject: Re: [PATCH] tap: don't let overheard traffic move addr_seen when serving a fixed address Message-ID: <20260605135741.60d87874@elisabeth> In-Reply-To: References: Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Fri, 05 Jun 2026 13:57:41 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: ckdHDORaAF9M-nH2wMG2sJf28CpfVezTNbeUMt4ltwU_1780660664 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: HQ74SC7DZQ6TXIF2AMP6654UICNLO3S7 X-Message-ID-Hash: HQ74SC7DZQ6TXIF2AMP6654UICNLO3S7 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: ej@campbell.name, passt-dev@passt.top, Jon Maloy , David Gibson 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: EJ, thanks for the detailed patch and commit message, and apologies for the delayed response. Some remarks: On Tue, 2 Jun 2026 18:15:24 -0700 EJ Campbell wrote: > The latest source address seen on the tap interface (ip4.addr_seen) is > taken to be the guest's and is used as the target for inbound port > forwarding. The update accepts any source address. > > On a point-to-point tap that is always correct. But when the tap sits on > a shared L2 segment -- for example when it is bridged together with other > hosts -- pasta also sees frames from those other hosts. That's a interesting use case, it's the first time I see pasta used like this. Thanks for sharing. > With a static > configuration (-a / address set, DHCP disabled), an overheard frame from > another host silently moves addr_seen to that host, and every subsequent > inbound forwarded connection is sent there instead of to the guest, so > connections are reset or hang. > > I hit this running pasta with its tap enslaved to a Linux bridge that also > carries other traffic: inbound port forwarding to the guest broke as soon > as another address was seen on the segment. Oops. We should definitely fix this, and maybe your patch is good enough for the moment (or better than the alternative), but I have a concern here: > Restrict the update so that, when DHCP is disabled (no_dhcp), only frames > sourced from the configured guest address may move addr_seen. With DHCP > enabled -- the default -- behaviour is unchanged, since the guest's > address is whatever pasta leases and tracking it is still correct. ...inferring possibly but not necessarily related information from whether DHCP is disabled or enabled looks a bit risky to me: - somebody might have DHCP enabled (just because it's the default option), but one single connected container / guest not using DHCP at all, and picking an address that we didn't expect. Your patch would break this (perhaps unlikely?) usage - somebody might have DHCP enabled but just the intended container / guest using it, and your same problem. Your patch wouldn't fix that (definitely less likely) usage So I've been thinking: shouldn't we rather use as a condition the fact that -a / --address was explicitly given? We don't track that from conf() at the moment but it would be a small change. If -a / --address is given, I'd say the user is very much expecting the container / guest to be using that address, and in that case I think it's safer to ignore other addresses. It would be even better if we could track our designated container / guest among a number of them, only if there are more than one. For context, Jon is working on a series adding support for multiple addresses (assigned, used, or observed): https://archives.passt.top/passt-dev/20260413005319.3295910-1-jmaloy@redhat.com/ and patch 8/13: https://archives.passt.top/passt-dev/20260413005319.3295910-9-jmaloy@redhat.com/ refactors the same exact part your patch is touching, here: https://archives.passt.top/passt-dev/7f04e12/s/?b=tap.c#n764 with no functional change. By the way, if you want to try things out on top of that series, you can apply it with: b4 shazam https://archives.passt.top/passt-dev/20260413005319.3295910-1-jmaloy@redhat.com/ ...you might want to check out a slightly older revision to apply cleanly without conflicts. The main purpose of this work is actually to implement a netlink monitor to track changes in host addresses: https://bugs.passt.top/show_bug.cgi?id=141 https://pad.passt.top/p/netlinkMonitor ...but it might also be convenient for this kind of tracking. For guests, we could establish some preference rules, for example: - if any guest made a DHCP / DHCPv6 request, select its address (I guess we should eventually implement guest-side MAC address tracking for a full solution but just preferring addresses we assigned might be quite good). I'm not sure how we would handle SLAAC - if we haven't seen any DHCP / DHCPv6 request, but we have seen a single address, default to that one as "guest" address and so on. For containers, we actually know which namespace we're associated to, so we could, on any new observed address, check the source MAC address of the frame and see if it corresponds to our namespace (querying via netlink). Thoughts? Jon? If this is all too complicated for the moment, would a condition based on explicit -a / --address option work for you? > The same reasoning applies to ip6.addr_seen on a shared segment; I left > IPv6 alone for now since I only exercise IPv4 in this setup, and would be > glad to extend it the same way if you prefer. I think we should definitely take care of IPv6 as well, I'm just not sure how. A couple of comments about the patch itself: > Signed-off-by: EJ Campbell > --- > tap.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/tap.c b/tap.c > index 4cba4c7..514379e 100644 > --- a/tap.c > +++ b/tap.c > @@ -770,7 +770,19 @@ resume: > continue; It looks like the patch itself was copied and pasted from a terminal, because all tabs became spaces, and the patch doesn't apply. It's not a problem for small patches like this one as I tend to fix things up manually, but it would be good to have it directly from git send-email instead. > } > > - if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr) > + /* The latest source address seen on the tap is taken to be the > + * guest's, and becomes the target for inbound forwarding. On a > + * point-to-point tap that's always correct. But if the tap sits > + * on a shared L2 segment (for example bridged together with > + * other hosts) and we serve a fixed address (DHCP disabled), a > + * frame overheard from another host on that segment must not > + * move addr_seen and silently retarget forwarded connections > + * away from the guest. When DHCP is disabled, only the > + * configured guest address may update addr_seen; with DHCP > + * enabled (the default) behaviour is unchanged. The fact that the behaviour is _unchanged_ (by your patch) belongs in the commit message rather than the code, because if one reads the code they aren't explicitly reading your change and it's not clear to what previous behaviour you would be referring to. Other than that it's pretty well described, thanks. Let's see if we can find a better solution though. > + */ > + if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr && > + (!c->no_dhcp || iph->saddr == c->ip4.addr.s_addr)) > c->ip4.addr_seen.s_addr = iph->saddr; > > if (!iov_drop_header(&data, hlen)) > > base-commit: 4b2823784aab04a70dfc295b16fd6f0592955790 > -- > 2.53.0 > -- Stefano