[PATCH v7 08/13] conf, pasta: Track observed guest IPv4 addresses in unified address array

[Jon Maloy <jmaloy@redhat.com>]

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 <jmaloy@redhat.com>

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