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=TZfUUZ6o; 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 DB6AC5A0262 for ; Sat, 20 Jun 2026 00:10:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1781907045; 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=sLrAKrYvOfmXr2OqdXNrNm7lmqbbhbMHRkX7meoArS0=; b=TZfUUZ6oCJ7iIOhSlK93xC2zI9e9hym0ncXyWeU1RKVaYexw3H0cwtTG90D6vZWE5foGqO lIyGlm3fFSa/+PGsHx+VfPNL4oYwycXmKytGqy+dEMh24XD1+vsMvS3DNrZL2G8BK0Coae z4mfs7T8jBBdywCPC5Ybe/A1cGlN61E= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-641-wyjI5tI5N4OTLZki6wVLZw-1; Fri, 19 Jun 2026 18:10:44 -0400 X-MC-Unique: wyjI5tI5N4OTLZki6wVLZw-1 X-Mimecast-MFC-AGG-ID: wyjI5tI5N4OTLZki6wVLZw_1781907043 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-490b9cd54f3so15618425e9.2 for ; Fri, 19 Jun 2026 15:10:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781907043; x=1782511843; 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=sLrAKrYvOfmXr2OqdXNrNm7lmqbbhbMHRkX7meoArS0=; b=GVf+R1+tQOUa7g7xZfA++EtrzK40QJX24HQkBbM4di7apZvc6QgTkPHLvNvAkRPNCH Sb5SWLE3Ygq3O5HA5En0G6K6o9MJfeu2ubUKXgi08xUFDCZS8tWNlMhgIPv/6qtm40Hg a29HrAJUqE8+w5rdzn8yv2joXU0v9cXLJ+QAVufMnrSaiuKdRPzs4KWZicLPh1gFdVrt GaR+u5K/T2eqiustPvfMf5UHY10WYFQ3qDm2OzW+gfFMEvvl30+peWbdUJ05BmLMqbNx t9nwTqE9SaH3vnHQ7QgmX9biuvbOzns4I1/G25EHjEMG42MyTN4u45wWn3NuyxhZhV4+ GNIQ== X-Forwarded-Encrypted: i=1; AFNElJ/hmdEdr5UMozk/pC7Pv4/DsO9Aog/BdAO7cLbL/TgTpqPcKOZT1s8qEsh5RJ5qVlfkHL+uW4t9auY=@passt.top X-Gm-Message-State: AOJu0YwMr27IkH07Gn/nbr9feF3gMY2lFvp0zglZT8LvrhIlhdt487lI 2qKIgKXIiii2Up4KJbcgOX499I+HGJgKbJTuP3RV5/0FQIyvfbDJOL0MTUYRcYxtG1a8nyckxvd 3eJPs2lu+hKitCKtP2v0vr9HfpDl6WDC+9oO2OvLJ0usd0Hr4hsxw5A== X-Gm-Gg: AfdE7cngjTePYdbD/AYA3heDSKUUE1+w/EnlmZocJsdpcYZKv5jpYiWesPZlyRXSslD g2ZmwdX5aqSZoUQhy1z6WrELfb3u18hb0g5RgaxsnjlV95YCe6n4LYyQ0rcM1JM8lLMPy2tWY4Z JgvvLfivoQcLAutAFQBgBRjZhyQrek6Sd3YJE0IYvK9fj3vBqQQB8Irw9ncBOr/XsLNM05iuS2W GNe+ay/Kll8oHHaPS4jb0Tx7JfiE6Iv+Ca3JDfmjlqf1hvpvWAj/P8wttxxCTutSqriaEbaKiar wlliGhpxFUwndUYnmYS60vPBBNjdnZ8vllPh4IGVrRBmsYU0Bri8M0eVaQZx4MjFU0Zu5qiLJkW XeDBC9E32qi1wgpUk/Q6FcH3xwAsFwYkwJBEJbB0= X-Received: by 2002:a05:600c:5494:b0:485:9a50:3370 with SMTP id 5b1f17b1804b1-4923ef53be4mr90303045e9.8.1781907042813; Fri, 19 Jun 2026 15:10:42 -0700 (PDT) X-Received: by 2002:a05:600c:5494:b0:485:9a50:3370 with SMTP id 5b1f17b1804b1-4923ef53be4mr90302625e9.8.1781907042229; Fri, 19 Jun 2026 15:10:42 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-49240ee9bc2sm59873075e9.1.2026.06.19.15.10.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 Jun 2026 15:10:41 -0700 (PDT) From: Stefano Brivio To: Jon Maloy Subject: Re: [PATCH v7 08/13] conf, pasta: Track observed guest IPv4 addresses in unified address array Message-ID: <20260620001040.76c9d2b1@elisabeth> In-Reply-To: <20260413005319.3295910-9-jmaloy@redhat.com> References: <20260413005319.3295910-1-jmaloy@redhat.com> <20260413005319.3295910-9-jmaloy@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Sat, 20 Jun 2026 00:10:41 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: AqH3R0dVxCf2vqNmBNOgJTUbDOQfCws7-KlDbgMkUlY_1781907043 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: XWBKCCRDB4EDFUZEQQC7QXB7TE5MG7EY X-Message-ID-Hash: XWBKCCRDB4EDFUZEQQC7QXB7TE5MG7EY 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: david@gibson.dropbear.id.au, 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 Sun, 12 Apr 2026 20:53:14 -0400 Jon Maloy wrote: > 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. > > The observed IPv4 address is always added at or moved to position 0, > increasing chances for a fast lookup. > > Signed-off-by: Jon Maloy > > --- > 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 protocol > 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. 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. 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. 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? > - 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(-) > > 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, unsigned int ifi) > } > if (!rc || !fwd_get_addr(c, AF_INET, 0, 0)) > return 0; > - > - a = fwd_get_addr(c, AF_INET, CONF_ADDR_HOST, 0); > } > > - if (a) > - ip4->addr_seen = *inany_v4(&a->addr); > - > ip4->our_tap_addr = ip4->guest_gw; > > return ifi; > @@ -787,7 +782,6 @@ static void conf_ip4_local(struct ctx *c) > { > struct ip4_ctx *ip4 = &c->ip4; > > - ip4->addr_seen = IP4_LL_GUEST_ADDR; > ip4->our_tap_addr = ip4->guest_gw = IP4_LL_GUEST_GW; > ip4->no_copy_addrs = ip4->no_copy_routes = 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 = &c->addrs[0], *rm = NULL; > + int count = c->addr_count; > + int af_cnt = 0; > > - 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 = inany_prefix_len(addr, prefix_len); > + > + /* Nothing more to change */ > + if ((a->flags & flags) == flags) > + return; > + > + a->flags |= flags; > + if (!(flags & CONF_ADDR_OBSERVED)) > + return; > + > + /* Observed address moves to position 0: remove, re-add later */ > + prefix_len = a->prefix_len; > + memmove(a, a + 1, (&arr[count] - (a + 1)) * sizeof(*a)); > + c->addr_count = --count; > + break; > } > > - if (c->addr_count >= MAX_GUEST_ADDRS) > + if (count >= MAX_GUEST_ADDRS) { > + debug("Address table full, can't add address"); > return; > + } > > - a = &c->addrs[c->addr_count++]; > - > -found: > + /* Add to head or tail, depending on flag */ > + if (flags & CONF_ADDR_OBSERVED) { > + a = &arr[0]; > + memmove(&arr[1], a, count * sizeof(*a)); > + } else { > + a = &arr[count]; > + } > + c->addr_count = ++count; > a->addr = *addr; > a->prefix_len = inany_prefix_len6(addr, prefix_len); > a->flags = flags; > + > + if (!(flags & CONF_ADDR_OBSERVED)) > + return; > + > + /* Remove excess observed-only address if more than one */ > + for (int i = count - 1; i >= 0; i--) { > + a = &arr[i]; > + if (inany_af(&a->addr) != inany_af(addr)) > + continue; > + if (a->flags != CONF_ADDR_OBSERVED) > + continue; > + if (!rm) > + rm = a; > + af_cnt++; > + } > + if (af_cnt > 1) { > + memmove(rm, rm + 1, (&arr[count] - (rm + 1)) * sizeof(*rm)); > + c->addr_count--; > + } > } > > /** > @@ -985,6 +1033,38 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini) > ((ini->oport == 53) || (ini->oport == 853)); > } > > +/** > + * 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 = fwd_get_addr(c, af, primary, skip); > + if (a) > + return a; > + } > + > + if (secondary) { > + a = 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 ctx *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; > > /* 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 = inany_loopback4; > - else > - tgt->eaddr = inany_from_v4(c->ip4.addr_seen); > + } else { > + const struct guest_addr *a; > + > + a = fwd_select_addr(c, AF_INET, > + CONF_ADDR_OBSERVED, > + CONF_ADDR_USER | > + CONF_ADDR_HOST, 0); > + if (!a) > + return PIF_NONE; > + > + tgt->eaddr = a->addr; > + } > tgt->oaddr = 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 = ini->eport; > > if (inany_v4(&tgt->oaddr)) { > - tgt->eaddr = inany_from_v4(c->ip4.addr_seen); > + const struct guest_addr *a; > + > + a = fwd_select_addr(c, AF_INET, CONF_ADDR_OBSERVED, > + CONF_ADDR_USER | CONF_ADDR_HOST, 0); > + if (!a) > + return PIF_NONE; > + > + tgt->eaddr = a->addr; > } else { > if (inany_is_linklocal6(&tgt->oaddr)) > tgt->eaddr.a6 = 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); > > /** > * 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 = { > .addr6 = c->ip6.addr_seen, > .addr6_ll = c->ip6.addr_ll_seen, > - .addr4 = c->ip4.addr_seen, > }; > + const struct guest_addr *a; > > (void)stage; > > + /* IPv4 observed address, with fallback to configured address */ > + a = fwd_select_addr(c, AF_INET, CONF_ADDR_OBSERVED, > + CONF_ADDR_USER | CONF_ADDR_HOST, > + CONF_ADDR_LINKLOCAL); > + if (a) > + addrs.addr4 = *inany_v4(&a->addr); > + > memcpy(addrs.mac, c->guest_mac, sizeof(addrs.mac)); > > if (write_all_buf(fd, &addrs, sizeof(addrs))) > @@ -90,7 +99,11 @@ static int seen_addrs_target_v2(struct ctx *c, > > c->ip6.addr_seen = addrs.addr6; > c->ip6.addr_ll_seen = addrs.addr6_ll; > - c->ip4.addr_seen = 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)); > > 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, > }; > > -/* 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 > > /** > * 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 */ > }; > > /** > * 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; > } > > - if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr) > - c->ip4.addr_seen.s_addr = iph->saddr; > + if (iph->saddr) { > + const union inany_addr *addr; > > + addr = &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) != l4len) -- Stefano