From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202606 header.b=lq/OKEW/; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id B28D95A0269 for ; Mon, 22 Jun 2026 04:40:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1782096000; bh=3ZlxgyRQfjrOhLwANC+RE+17ej7ApfWiG/Y2AE9BDx8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=lq/OKEW/a879+dJuddWLUiUuZn48fP/Cz2gtmQIGM4Jw7I1n3FqSQMqbrCYIP+UOp b5QL1QNR3wIAipbcsay2V9bbYa+P7Qa/BXpc8v/QRKZQJaJTvFUh0706Aqjf1aq26b /s8Sms3JfOLw3oJT74Q6GgeI7ji/4kRLVVCia040Vo8P5RZweWghv+2iSR3bBu6XOJ o5kBrIb6iC+7YH6AldapUwd/22OWz0ksmWmJhmco+brFmr3BdI/gxAFX04JttmR19h bwgUwJAKITrW5X09WDXYMud5tk/wMOOMP/zGEc+0Q6S9UsmzDYx2mHO0y+8DVC2dXj u6F0xx1RAyqaQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gkC8w47PYz4wT1; Mon, 22 Jun 2026 12:40:00 +1000 (AEST) Date: Mon, 22 Jun 2026 11:46:45 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v7 08/13] conf, pasta: Track observed guest IPv4 addresses in unified address array Message-ID: References: <20260413005319.3295910-1-jmaloy@redhat.com> <20260413005319.3295910-9-jmaloy@redhat.com> <20260620001040.76c9d2b1@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="QwCMe5l+fWI/nolA" Content-Disposition: inline In-Reply-To: <20260620001040.76c9d2b1@elisabeth> Message-ID-Hash: OWMMK74CS6HP6EHV72KBWPKD3Z7QZN6X X-Message-ID-Hash: OWMMK74CS6HP6EHV72KBWPKD3Z7QZN6X X-MailFrom: dgibson@gandalf.ozlabs.org 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: Jon Maloy , 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: --QwCMe5l+fWI/nolA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Jun 20, 2026 at 12:10:41AM +0200, Stefano Brivio wrote: > On Sun, 12 Apr 2026 20:53:14 -0400 > Jon Maloy wrote: >=20 > > We remove the addr_seen field in struct ip4_ctx and replace it by > > setting a new CONF_ADDR_OBSERVED flag in the corresponding entry > > in the unified address array. > >=20 > > The observed IPv4 address is always added at or moved to position 0, > > increasing chances for a fast lookup. > >=20 > > Signed-off-by: Jon Maloy > >=20 > > --- > > v4: - Removed migration protocol update, to be added in later commit > > - Allow only one OBSERVED address at a time > > - Some other changes based on feedback from David G > > v5: - Allowing multiple observed IPv4 addresses > > v6: - Refactored fwd_set_addr(), notably: > > o Limited number of allowed observed addresses to four per protoc= ol > > o I kept the memmove() calls, since I find no more elegant way to > > do this. Performance cost should be minimal, since these parts > > of the code will execute only very exceptionally. Note that > > removing the 'oldest' entry implicitly means removing the least > > used one, since the latter will migrate to the highest position > > after a few iterations of remove/add. > > o Also kept the prefix_len update. Not sure about this, but I > > cannot see how the current approach can cause any harm. > > - Other changes suggested by David G, notably reversing some > > residues after an accidental merge/re-split with the next > > commit. > > v7: - Changed fwd_set_addr() to only accept keeping one observed-only > > address per protocol, as suggested by David. >=20 > Sorry, I just spotted this in David's review of v6. Actually, I > think that keeping track of a few multiple observed addresses > (especially with different scope) might be convenient and it would > already be useful here together with 4/13 to avoid resolving via ARP > any of a few addresses recently seen from the guest. So.. not resolving ARPs is the one thing where we could actualy use multiple guest observed addresses - mostly we use it for directing traffic to the guest, for which we need a single address. But.. I feel like switching the ARP resolution from "everything except" to "only these" would be a better solution. That also lets the guest move to a brand new unused address without getting bogus DAD failures. > While ARP probes for duplicate addresses are usually coming from DHCP > clients, there might other mechanisms to assign addresses using those. > Besides, I think David's suggestion was to keep a single observed > address per IP version _and_ scope, not just one per IP version. Ah, yes, that is a good point. That was absolutely my intention. There's no question we need separate observed addresses for IPv6 link-local and IPv6 global. (or at least no question as long as we accept the need for observed addresses at all, which is a different discussion). > If we > just keep one per version, regardless of the scope, we'll now cycle > between one link-local and one global unicast address (in most cases), > right? >=20 > > - Eliminated redundant tap_check_src_addr4() call level. > > - I keep fwd_select_addr() for the same pragmatic reason it was > > introduced: to avoid ugly, deeply indented code that tends > > to wrap across several lines. > > --- > > conf.c | 6 --- > > fwd.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++------- > > fwd.h | 4 ++ > > migrate.c | 17 +++++++- > > passt.h | 6 +-- > > tap.c | 8 +++- > > 6 files changed, 136 insertions(+), 29 deletions(-) > >=20 > > diff --git a/conf.c b/conf.c > > index 924ade2..f503d0f 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -767,13 +767,8 @@ static unsigned int conf_ip4(struct ctx *c, unsign= ed int ifi) > > } > > if (!rc || !fwd_get_addr(c, AF_INET, 0, 0)) > > return 0; > > - > > - a =3D fwd_get_addr(c, AF_INET, CONF_ADDR_HOST, 0); > > } > > =20 > > - if (a) > > - ip4->addr_seen =3D *inany_v4(&a->addr); > > - > > ip4->our_tap_addr =3D ip4->guest_gw; > > =20 > > return ifi; > > @@ -787,7 +782,6 @@ static void conf_ip4_local(struct ctx *c) > > { > > struct ip4_ctx *ip4 =3D &c->ip4; > > =20 > > - ip4->addr_seen =3D IP4_LL_GUEST_ADDR; > > ip4->our_tap_addr =3D ip4->guest_gw =3D IP4_LL_GUEST_GW; > > ip4->no_copy_addrs =3D ip4->no_copy_routes =3D true; > > fwd_set_addr(c, &inany_from_v4(IP4_LL_GUEST_ADDR), > > diff --git a/fwd.c b/fwd.c > > index d3f576a..8c7bf91 100644 > > --- a/fwd.c > > +++ b/fwd.c > > @@ -28,6 +28,7 @@ > > #include "inany.h" > > #include "fwd.h" > > #include "passt.h" > > +#include "conf.h" > > #include "lineread.h" > > #include "flow_table.h" > > #include "netlink.h" > > @@ -260,21 +261,68 @@ void fwd_neigh_table_init(const struct ctx *c) > > void fwd_set_addr(struct ctx *c, const union inany_addr *addr, > > uint8_t flags, int prefix_len) > > { > > - struct guest_addr *a; > > + struct guest_addr *a, *arr =3D &c->addrs[0], *rm =3D NULL; > > + int count =3D c->addr_count; > > + int af_cnt =3D 0; > > =20 > > - for_each_addr(a, c->addrs, c->addr_count, inany_af(addr)) { > > - goto found; > > + for_each_addr(a, c->addrs, c->addr_count, AF_UNSPEC) { > > + if (!inany_equals(&a->addr, addr)) > > + continue; > > + > > + /* Adjust and update prefix_len if provided and applicable */ > > + if (prefix_len && !(a->flags & CONF_ADDR_USER)) > > + a->prefix_len =3D inany_prefix_len(addr, prefix_len); > > + > > + /* Nothing more to change */ > > + if ((a->flags & flags) =3D=3D flags) > > + return; > > + > > + a->flags |=3D flags; > > + if (!(flags & CONF_ADDR_OBSERVED)) > > + return; > > + > > + /* Observed address moves to position 0: remove, re-add later */ > > + prefix_len =3D a->prefix_len; > > + memmove(a, a + 1, (&arr[count] - (a + 1)) * sizeof(*a)); > > + c->addr_count =3D --count; > > + break; > > } > > =20 > > - if (c->addr_count >=3D MAX_GUEST_ADDRS) > > + if (count >=3D MAX_GUEST_ADDRS) { > > + debug("Address table full, can't add address"); > > return; > > + } > > =20 > > - a =3D &c->addrs[c->addr_count++]; > > - > > -found: > > + /* Add to head or tail, depending on flag */ > > + if (flags & CONF_ADDR_OBSERVED) { > > + a =3D &arr[0]; > > + memmove(&arr[1], a, count * sizeof(*a)); > > + } else { > > + a =3D &arr[count]; > > + } > > + c->addr_count =3D ++count; > > a->addr =3D *addr; > > a->prefix_len =3D inany_prefix_len6(addr, prefix_len); > > a->flags =3D flags; > > + > > + if (!(flags & CONF_ADDR_OBSERVED)) > > + return; > > + > > + /* Remove excess observed-only address if more than one */ > > + for (int i =3D count - 1; i >=3D 0; i--) { > > + a =3D &arr[i]; > > + if (inany_af(&a->addr) !=3D inany_af(addr)) > > + continue; > > + if (a->flags !=3D CONF_ADDR_OBSERVED) > > + continue; > > + if (!rm) > > + rm =3D a; > > + af_cnt++; > > + } > > + if (af_cnt > 1) { > > + memmove(rm, rm + 1, (&arr[count] - (rm + 1)) * sizeof(*rm)); > > + c->addr_count--; > > + } > > } > > =20 > > /** > > @@ -985,6 +1033,38 @@ static bool is_dns_flow(uint8_t proto, const stru= ct flowside *ini) > > ((ini->oport =3D=3D 53) || (ini->oport =3D=3D 853)); > > } > > =20 > > +/** > > + * fwd_select_addr() - Select address with priority-based search > > + * @c: Execution context > > + * @af: Address family (AF_INET or AF_INET6) > > + * @primary: Primary flags to match (or 0 to skip) > > + * @secondary: Secondary flags to match (or 0 to skip) > > + * @skip: Flags to exclude from search > > + * > > + * Search for address entries in priority order. > > + * > > + * Return: pointer to selected address entry, or NULL if none found > > + */ > > +const struct guest_addr *fwd_select_addr(const struct ctx *c, int af, > > + int primary, int secondary, int skip) > > +{ > > + const struct guest_addr *a; > > + > > + if (primary) { > > + a =3D fwd_get_addr(c, af, primary, skip); > > + if (a) > > + return a; > > + } > > + > > + if (secondary) { > > + a =3D fwd_get_addr(c, af, secondary, skip); > > + if (a) > > + return a; > > + } > > + > > + return NULL; > > +} > > + > > /** > > * fwd_guest_accessible() - Is address guest-accessible > > * @c: Execution context > > @@ -1014,11 +1094,6 @@ static bool fwd_guest_accessible(const struct ct= x *c, > > if (inany_equals(addr, &a->addr)) > > return false; > > } > > - /* Also check addr_seen: it tracks the address the guest is actually > > - * using, which may differ from configured addresses. > > - */ > > - if (inany_equals4(addr, &c->ip4.addr_seen)) > > - return false; > > =20 > > /* For IPv6, addr_seen starts unspecified, because we don't know what= LL > > * address the guest will take until we see it. Only check against it > > @@ -1214,10 +1289,20 @@ uint8_t fwd_nat_from_host(const struct ctx *c, > > * match. > > */ > > if (inany_v4(&ini->eaddr)) { > > - if (c->host_lo_to_ns_lo) > > + if (c->host_lo_to_ns_lo) { > > tgt->eaddr =3D inany_loopback4; > > - else > > - tgt->eaddr =3D inany_from_v4(c->ip4.addr_seen); > > + } else { > > + const struct guest_addr *a; > > + > > + a =3D fwd_select_addr(c, AF_INET, > > + CONF_ADDR_OBSERVED, > > + CONF_ADDR_USER | > > + CONF_ADDR_HOST, 0); > > + if (!a) > > + return PIF_NONE; > > + > > + tgt->eaddr =3D a->addr; > > + } > > tgt->oaddr =3D inany_any4; > > } else { > > if (c->host_lo_to_ns_lo) > > @@ -1252,7 +1337,14 @@ uint8_t fwd_nat_from_host(const struct ctx *c, > > tgt->oport =3D ini->eport; > > =20 > > if (inany_v4(&tgt->oaddr)) { > > - tgt->eaddr =3D inany_from_v4(c->ip4.addr_seen); > > + const struct guest_addr *a; > > + > > + a =3D fwd_select_addr(c, AF_INET, CONF_ADDR_OBSERVED, > > + CONF_ADDR_USER | CONF_ADDR_HOST, 0); > > + if (!a) > > + return PIF_NONE; > > + > > + tgt->eaddr =3D a->addr; > > } else { > > if (inany_is_linklocal6(&tgt->oaddr)) > > tgt->eaddr.a6 =3D c->ip6.addr_ll_seen; > > diff --git a/fwd.h b/fwd.h > > index c5a1068..9893856 100644 > > --- a/fwd.h > > +++ b/fwd.h > > @@ -25,6 +25,10 @@ void fwd_probe_ephemeral(void); > > bool fwd_port_is_ephemeral(in_port_t port); > > const struct guest_addr *fwd_get_addr(const struct ctx *c, sa_family_t= af, > > uint8_t incl, uint8_t excl); > > +const struct guest_addr *fwd_select_addr(const struct ctx *c, int af, > > + int primary, int secondary, int skip); > > +void fwd_set_addr(struct ctx *c, const union inany_addr *addr, > > + uint8_t flags, int prefix_len); > > =20 > > /** > > * struct fwd_rule - Forwarding rule governing a range of ports > > diff --git a/migrate.c b/migrate.c > > index 1e8858a..1e02720 100644 > > --- a/migrate.c > > +++ b/migrate.c > > @@ -18,6 +18,8 @@ > > #include "util.h" > > #include "ip.h" > > #include "passt.h" > > +#include "conf.h" > > +#include "fwd.h" > > #include "inany.h" > > #include "flow.h" > > #include "flow_table.h" > > @@ -57,11 +59,18 @@ static int seen_addrs_source_v2(struct ctx *c, > > struct migrate_seen_addrs_v2 addrs =3D { > > .addr6 =3D c->ip6.addr_seen, > > .addr6_ll =3D c->ip6.addr_ll_seen, > > - .addr4 =3D c->ip4.addr_seen, > > }; > > + const struct guest_addr *a; > > =20 > > (void)stage; > > =20 > > + /* IPv4 observed address, with fallback to configured address */ > > + a =3D fwd_select_addr(c, AF_INET, CONF_ADDR_OBSERVED, > > + CONF_ADDR_USER | CONF_ADDR_HOST, > > + CONF_ADDR_LINKLOCAL); > > + if (a) > > + addrs.addr4 =3D *inany_v4(&a->addr); > > + > > memcpy(addrs.mac, c->guest_mac, sizeof(addrs.mac)); > > =20 > > if (write_all_buf(fd, &addrs, sizeof(addrs))) > > @@ -90,7 +99,11 @@ static int seen_addrs_target_v2(struct ctx *c, > > =20 > > c->ip6.addr_seen =3D addrs.addr6; > > c->ip6.addr_ll_seen =3D addrs.addr6_ll; > > - c->ip4.addr_seen =3D addrs.addr4; > > + > > + if (addrs.addr4.s_addr) > > + fwd_set_addr(c, &inany_from_v4(addrs.addr4), > > + CONF_ADDR_OBSERVED, 0); > > + > > memcpy(c->guest_mac, addrs.mac, sizeof(c->guest_mac)); > > =20 > > return 0; > > diff --git a/passt.h b/passt.h > > index f75656d..5da1d55 100644 > > --- a/passt.h > > +++ b/passt.h > > @@ -64,8 +64,9 @@ enum passt_modes { > > MODE_VU, > > }; > > =20 > > -/* Maximum number of addresses in context address array */ > > +/* Limits on number of addresses in context address array */ > > #define MAX_GUEST_ADDRS 32 > > +#define MAX_OBSERVED_ADDRS 4 > > =20 > > /** > > * struct guest_addr - Unified IPv4/IPv6 address entry > > @@ -81,11 +82,11 @@ struct guest_addr { > > #define CONF_ADDR_HOST BIT(1) /* From host interface */ > > #define CONF_ADDR_GENERATED BIT(2) /* Generated by PASST/PASTA */ > > #define CONF_ADDR_LINKLOCAL BIT(3) /* Link-local address */ > > +#define CONF_ADDR_OBSERVED BIT(4) /* Seen in guest traffic */ > > }; > > =20 > > /** > > * struct ip4_ctx - IPv4 execution context > > - * @addr_seen: Latest IPv4 address seen as source from tap > > * @guest_gw: IPv4 gateway as seen by the guest > > * @map_host_loopback: Outbound connections to this address are NATted= to the > > * host's 127.0.0.1 > > @@ -101,7 +102,6 @@ struct guest_addr { > > * @no_copy_addrs: Don't copy all addresses when configuring namespace > > */ > > struct ip4_ctx { > > - struct in_addr addr_seen; > > struct in_addr guest_gw; > > struct in_addr map_host_loopback; > > struct in_addr map_guest_addr; > > diff --git a/tap.c b/tap.c > > index eb93f74..7f04e12 100644 > > --- a/tap.c > > +++ b/tap.c > > @@ -47,6 +47,7 @@ > > #include "ip.h" > > #include "iov.h" > > #include "passt.h" > > +#include "fwd.h" > > #include "arp.h" > > #include "dhcp.h" > > #include "ndp.h" > > @@ -756,9 +757,12 @@ resume: > > continue; > > } > > =20 > > - if (iph->saddr && c->ip4.addr_seen.s_addr !=3D iph->saddr) > > - c->ip4.addr_seen.s_addr =3D iph->saddr; > > + if (iph->saddr) { > > + const union inany_addr *addr; > > =20 > > + addr =3D &inany_from_v4(*(struct in_addr *) &iph->saddr); > > + fwd_set_addr(c, addr, CONF_ADDR_OBSERVED, 0); > > + } > > if (!iov_drop_header(&data, hlen)) > > continue; > > if (iov_tail_size(&data) !=3D l4len) >=20 > --=20 > Stefano >=20 --=20 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 --QwCMe5l+fWI/nolA Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmo4k/MACgkQzQJF27ox 2GfW0g//frZPv72uMhws84DAnGfbQWi5ThmKVzpUdxr4aAKgQFvqg8u+1x8AOxp/ UIqtqrBeim1pczfPMrfxxeJd7YTS66Fh5BHb3RfP2x+gtvNwNMbGMCVctnPGUV+L cCAuXKRk1cezLbRWSFZHzRVuSDApJNU9+B94Ufq5wDsXDnItTLBhwsx9FGLXpN7e KQnPYSNy/+zA7R5smnBivO8BKlliQplSM5fgycDB5TrhhGMiZYAwHZrZxWR3/GSj n5qQSeFHVv5ANuBP0QF8CP46hn2kxKznyb4Zep/rQFrdmh/0Qef92MosZoSjLuSv r0Z1vHWGphk1rM5v9W1svkK5xDgeQwT4KcmRAf9bW8O0iWWPdx8yAZ9A0xVXBhfK nmjcpEhdLOPKrjYS0DH9pnbvs1X8mCzIiS3VKcBdAPdMLJd6ObgURveyjzbbfIX9 XK7TVjXSqvcxFxI567OXd/mCVBJ05AQTTsorm43OF8h4Fvg4LZj0zHTBL0wAZXDh mB4roduuf9Wk08JBfu+znIjSzj8cq6lA58A532Ca7PfM3QGWveAL5+gx7prTyt4c C91v9FEqgeKiYfFpZJtiTWdmS8hsKf22l9CRqsmpxweNcRLNoPl5VZgfosHbwFUC bH/FxvAki0IVZo0WVwCX/mWOodIh6PP2JRXIKdtLMVlypFv2RPQ= =8jQk -----END PGP SIGNATURE----- --QwCMe5l+fWI/nolA--