[PATCH v8 09/14] 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.
v8: - Refactoring of and fix to fwd_set_addr, after feedback from David.
    - Removed test for 'primary' in fwd_select_addr. (David)
    - Kept the use of fwd_select_addr() in fwd_nat_from_host(), because
      we aren't guaranteed there is (yet) an OBSERVED address at all
      in the array.
    - Removed exclusion of CONF_ADDR_LINKLOCAL in seen_addrs_source_v2().
    - In the current code tap.c::tap4/6_handler() does:
      if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr)
          c->ip4.addr_seen.s_addr = iph->saddr;
      In the new code fwd_set_addr() will almost always return after
      a hit in slot 0 or 1, anything else will be extremely rare.
      Although there will be a few more instructions to execute,
      I don't share David's concern about performance here.
---
 conf.c    |   2 -
 fwd.c     | 123 +++++++++++++++++++++++++++++++++++++++++++++++-------
 fwd.h     |   3 ++
 migrate.c |  16 ++++++-
 passt.h   |   2 +-
 tap.c     |   9 ++--
 6 files changed, 131 insertions(+), 24 deletions(-)

diff --git a/conf.c b/conf.c
index 353f871c..32734188 100644
--- a/conf.c
+++ b/conf.c
@@ -415,7 +415,6 @@ static unsigned int conf_ip4(struct ctx *c, unsigned int ifi)
 			return 0;
 	}
 
-	ip4->addr_seen = *inany_v4(&a->addr);
 	ip4->our_tap_addr = ip4->guest_gw;
 
 	return ifi;
@@ -429,7 +428,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 9a88fa19..10ed6034 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"
@@ -256,21 +257,72 @@ 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 *tail = &c->addrs[c->addr_count - 1];
+	struct guest_addr *head = &c->addrs[0];
+	struct guest_addr *a, *rm = NULL;
+	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 = prefix_len;
+
+		/* Nothing more to change? */
+		if ((a->flags & flags) == flags)
+			return;
+
+		a->flags |= flags;
+		if (!(flags & CONF_ADDR_OBSERVED))
+			return;
+
+		/* Remove found observed address, re-add it later */
+		flags = a->flags;
+		prefix_len = a->prefix_len;
+		memmove(a, a + 1, (tail - a) * sizeof(*a));
+		c->addr_count--;
+		tail--;
+		break;
 	}
 
-	if (c->addr_count >= MAX_GUEST_ADDRS)
+	if (c->addr_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) {
+		memmove(&head[1], &head[0], c->addr_count * sizeof(*a));
+		a = &head[0];
+	} else {
+		a = &tail[1];
+	}
+	c->addr_count++;
+	tail++;
 	a->addr = *addr;
 	a->prefix_len = prefix_len;
 	a->flags = flags;
+
+	if (!(flags & CONF_ADDR_OBSERVED))
+		return;
+
+	/* Remove excess observed-only address if more than one */
+	for (int i = c->addr_count - 1; i >= 0; i--) {
+		a = &head[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, (tail - rm) * sizeof(*rm));
+		c->addr_count--;
+	}
 }
 
 /**
@@ -840,6 +892,33 @@ 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
+ * @secondary:	Secondary flags to match
+ * @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,
+					 uint8_t primary, uint8_t secondary,
+					 uint8_t skip)
+{
+	const struct guest_addr *a;
+
+	a = fwd_get_addr(c, af, primary, skip);
+	if (a)
+		return a;
+
+	a = fwd_get_addr(c, af, secondary, skip);
+
+	return a;
+}
+
 /**
  * fwd_guest_accessible() - Is address guest-accessible
  * @c:		Execution context
@@ -869,11 +948,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
@@ -1071,10 +1145,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)
@@ -1109,7 +1193,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 d677440f..6adf6b7a 100644
--- a/fwd.h
+++ b/fwd.h
@@ -83,5 +83,8 @@ void fwd_set_addr(struct ctx *c, const union inany_addr *addr,
 		  uint8_t flags, int prefix_len);
 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,
+					 uint8_t primary, uint8_t secondary,
+					 uint8_t skip);
 
 #endif /* FWD_H */
diff --git a/migrate.c b/migrate.c
index 8937b85f..795f2818 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"
@@ -58,11 +60,17 @@ 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 any other non-LL address */
+	a = fwd_select_addr(c, AF_INET, CONF_ADDR_OBSERVED,
+			    CONF_ADDR_USER | CONF_ADDR_HOST, 0);
+	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)))
@@ -91,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 (!IN4_IS_ADDR_UNSPECIFIED(&addrs.addr4)) {
+		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 11ccff05..20ad7022 100644
--- a/passt.h
+++ b/passt.h
@@ -81,12 +81,12 @@ 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 */
 #define CONF_ADDR_ANY		0xff		/* Match any flag */
 };
 
 /**
  * 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
diff --git a/tap.c b/tap.c
index 573a3f10..93db2f86 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,10 +757,12 @@ resume:
 			continue;
 		}
 
-		if (!fwd_get_addr(c, AF_INET, CONF_ADDR_USER, 0) &&
-		    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;
 
-- 
2.52.0