Re: [PATCH 2/2] conf, fwd: Prefer same-scope address as inbound source address from host

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6620 bytes --]

On Thu, Jul 23, 2026 at 11:42:36AM +0200, Stefano Brivio wrote:
> On Thu, 23 Jul 2026 13:36:58 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Thu, Jul 23, 2026 at 01:26:39AM +0200, Stefano Brivio wrote:
> > > We might have situations, such as the one described in
> > > https://bugs.passt.top/show_bug.cgi?id=217, where using a link-local
> > > address as source in a given namespace doesn't guarantee that we can
> > > reach the intended destination, because, for instance, the inbound
> > > traffic we forward is in turn forwarded to a different interface, such
> > > as a bridge.
> > > 
> > > In that case, the assumption from 9618d247006a ("ndp, dhcpv6, tcp,
> > > udp: Always use link-local as source if gateway isn't") isn't a safe
> > > one: the user might have specified a valid gateway address, matching
> > > the scope of the destination address, but we won't use it as address
> > > of last resort, and prefer a link-local address with a mismatch in
> > > scope instead.
> > > 
> > > This should only be an issue in IPv6 local mode, because, otherwise,
> > > we source address and default gateway address (presumably compatible)
> > > from the host.
> > > 
> > > So, in local mode, if the user specifies a given default gateway
> > > address for IPv6, note that as 'our_tap_addr', like we would do with
> > > with IPv4, and stick to Rule 2 of RFC 6724, Section 5, when selecting
> > > a source address, by preferring an address with the same scope, if
> > > available.
> > > 
> > > Reported-by: Paul Holzinger <pholzing@redhat.com>
> > > Link: https://bugs.passt.top/show_bug.cgi?id=217
> > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> > 
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > 
> > [snip]
> > > diff --git a/fwd.c b/fwd.c
> > > index 7152169..4ba0af3 100644
> > > --- a/fwd.c
> > > +++ b/fwd.c
> > > @@ -1090,7 +1090,11 @@ uint8_t fwd_nat_from_host(const struct ctx *c,
> > >  				return PIF_NONE;
> > >  			tgt->oaddr = inany_from_v4(c->ip4.our_tap_addr);
> > >  		} else {
> > > -			tgt->oaddr.a6 = c->ip6.our_tap_ll;
> > > +			if (inany_is_linklocal6(&tgt->eaddr) ||  
> > 
> > Just to make sure you're aware: this will only trigger if tgt->eaddr
> > has been set at this point, which is not always the case.  In fact it
> > will usually only be the case when the rule specifies a target
> > address.  In other cases we pick tgt->oaddr first then pick
> > tgt->eaddr's scope to try to match it.
> 
> Right, yes, I had half a mind to try and change this slightly (see
> below) but then I realised that luckily it wasn't needed for this
> minimal fix, as Podman will specify an explicit destination address, so
> I preferred to avoid the topic altogether for the moment (including
> avoiding comments that risk ignoring some corner cases) because:
> 
> > I think the fact that the order in which we pick eaddr and oaddr
> > varies is pretty confusing, but I haven't so far seen a way to avoid
> > it without breaking something worse.
> 
> ...I think that, at least in the !nat_inbound() case, we should avoid
> a strict ordering in the selection of source and destination address
> (we should look into both at the same time) because in general we know
> upfront if we can match the scope between source and destination,

I'm not entirely sure what you mean by that.

> and we
> should always try to do that (same here, RFC 6724 Section 5 Rule 2).

AFAICT the RFC rules are assuming you already know the destination address.

> And, if there are multiple ways to match the scopes, we should prefer
> the most specific / smaller common scope (same rule as above, Section
> 3.1 helps in the interpretation).

Ok, makes sense, although again I'm not sure it really follows from
the RFC which seems to assume a known destination.

> But this would be much simpler to implement once we have Jon's changes
> generalising address storage, because at that point we could have a
> lookup function for the smallest scope of usable address, or even a
> joint lookup function altogether. In detail, I think we should do this
> (again, under the !nat_inbound() condition):

Right.

> 
> 1. if there's a possible link-local source address and a possible
>    destination source address, pick both

Did you mean specifically a link-local possible destination address
here?

> 2. if there's a possible unicast source address and a possible unicast
>    destination address, pick both
> 
> 3. if there's a possible link-local source address, pick it, and then
>    pick any (mismatching) destination address
> 
> 4. otherwise, pick any available source address, and any available
>    destination address
> 
> ...and once we have that series, we can probably avoid implementing
> these as distinct steps. I would defer all this to that point, and
> meanwhile just fix whatever critical case might come up (like the
> current one with Podman).

Ok.

> > I still think this change is correct: not previously having
> > ip6.our_tap_addr was only possible because we did this odd dance to
> > pick eaddr based on oaddr's scope rather than the other way around.
> > 
> > > +			    IN6_IS_ADDR_UNSPECIFIED(&c->ip6.our_tap_addr))
> > > +				tgt->oaddr.a6 = c->ip6.our_tap_ll;
> > > +			else
> > > +				tgt->oaddr.a6 = c->ip6.our_tap_addr;
> > >  		}
> > >  	}
> > >  	tgt->oport = ini->eport;
> > > diff --git a/passt.h b/passt.h
> > > index a61baca..51ccd4f 100644
> > > --- a/passt.h
> > > +++ b/passt.h
> > > @@ -121,6 +121,7 @@ struct ip4_ctx {
> > >   * @dns:		DNS addresses for DHCPv6 and NDP
> > >   * @dns_match:		Forward DNS query if sent to this address
> > >   * @our_tap_ll:		Link-local IPv6 address for passt's use on tap
> > > + * @our_tap_addr:	Non-LL IPv6 address for passt's use on tap (if any)
> > >   * @dns_host:		Use this DNS on the host for forwarding
> > >   * @addr_out:		Optional source address for outbound traffic
> > >   * @ifname_out:		Optional interface name to bind outbound sockets to
> > > @@ -140,6 +141,7 @@ struct ip6_ctx {
> > >  	struct in6_addr dns[MAXNS];
> > >  	struct in6_addr dns_match;
> > >  	struct in6_addr our_tap_ll;
> > > +	struct in6_addr our_tap_addr;
> > >  
> > >  	/* PIF_HOST addresses */
> > >  	struct in6_addr dns_host;
> > > -- 
> > > 2.43.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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]