Re: [PATCH] tap: don't let overheard traffic move addr_seen when serving a fixed address

On Fri, Jun 05, 2026 at 01:57:41PM +0200, Stefano Brivio wrote:

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.

Likewise, it's the first I've heard of someone using it specifically like this, but it is a use model I've had in mind for a while. So this is a known limitation I've been thinking about for a while. It hasn't reached the top of my priority list because while your very specific case almost works already, the more general case of a bridge on the guest side requires a bunch more infrastructure. That's in progress or at least planned, but it's going to be a while. Fwiw, I really dislike the addr_seen mechanism even in cases where it "works": it allows poorly configured guests to work at the expense of substantial complexity which impacts all cases. However, it's established behaviour, so Stefano is understandably against simply removing it.

...

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

Right. I agree with Stefano that basing this of --no-dhcp is not a good idea. It happens to work for your case, but it doesn't generalise well. In addition to the cases Stefano mentions, for the more general case of a guest-side bridge, we'd eventually like to have our DHCP implementation serve multiple address, in which case we wouldn't want addr_seen, even though DHCP is enabled.

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.

I think having an explcit -a is a sufficient condition to disable addr_seen, but maybe not a necessary one. I think requiring the guest to use the assigned address would be a reasonable request, even if that address is taken from the host. Stefano and others talked me out of an explicit --no-address-snooping option, on the grounds of option proliferation at our last call. However, thinking about it again, I'm still wondering if it might be useful: we can think of addr_seen as a (not very good) address assignment mechanism, along with DHCP, NDP, DHCPv6 and --config-net. All of those (better) mechanisms can be explicitly disabled, so why should we not be able to disable the worst one.

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

and patch 8/13:

https://archives.passt.top/passt-dev/20260413005319.3295910-9-jmaloy@redhat....

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

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

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