Handling of each protocol needs some degree of tracking of the addresses
and ports at the end of each connection or flow.  Sometimes that's explicit
(as in the guest visible addresses for TCP connections), sometimes implicit
(the bound and connected addresses of sockets).

To allow more general and robust handling, and more consistency across
protocols we want to uniformly track the address and port at each end of
the connection.  Furthermore, because we allow port remapping, and we
sometimes need to apply NAT, the addresses and ports can be different as
seen by the guest/namespace and as by the host.

Introduce 'struct flowside' to keep track of common information
related to one side of each flow.  For now that's the addresses, ports
and the pif id.  Store two of these in the common fields of a flow to
track that information for both sides.  For now we just introduce the
structure itself, helpers to populate it, and logging of the contents
when starting and ending flows.  Later patches will actually put
something useful there.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 flow.c     | 28 ++++++++++++++++++--
 flow.h     | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 passt.h    |  3 +++
 pif.h      |  1 -
 tcp_conn.h |  1 -
 5 files changed, 104 insertions(+), 4 deletions(-)

diff --git a/flow.c b/flow.c
index 80dd269..02d6008 100644
--- a/flow.c
+++ b/flow.c
@@ -51,10 +51,11 @@ static_assert(ARRAY_SIZE(flow_proto) == FLOW_NUM_TYPES,
  *
  *    ALLOC - A tentatively allocated entry
  *        Operations:
+ *            - Common flow fields other than type may be accessed
  *            - flow_alloc_cancel() returns the entry to FREE state
  *            - FLOW_START() set the entry's type and moves to START state
  *        Caveats:
- *            - It's not safe to write fields in the flow entry
+ *            - It's not safe to write flow type specific fields in the entry
  *            - It's not safe to allocate further entries with flow_alloc()
  *            - It's not safe to return to the main epoll loop (use FLOW_START()
  *              to move to START state before doing so)
@@ -62,6 +63,7 @@ static_assert(ARRAY_SIZE(flow_proto) == FLOW_NUM_TYPES,
  *
  *    START - An entry being prepared by flow type specific code
  *        Operations:
+ *            - Common flow fields other than type may be accessed
  *            - Flow type specific fields may be accessed
  *            - flow_*() logging functions
  *            - flow_alloc_cancel() returns the entry to FREE state
@@ -168,9 +170,21 @@ void flow_log_(const struct flow_common *f, int pri, const char *fmt,
...)
 union flow *flow_start(union flow *flow, enum flow_type type,
 		       unsigned iniside)
 {
-	(void)iniside;
+	char ebuf[INANY_ADDRSTRLEN], fbuf[INANY_ADDRSTRLEN];
+	const struct flowside *a = &flow->f.side[iniside];
+	const struct flowside *b = &flow->f.side[!iniside];
+
 	flow->f.type = type;
 	flow_dbg(flow, "START %s", flow_type_str[flow->f.type]);
+	flow_dbg(flow, "  from side %u (%s): [%s]:%hu -> [%s]:%hu",
+		 iniside, pif_name(a->pif),
+		 inany_ntop(&a->eaddr, ebuf, sizeof(ebuf)), a->eport,
+		 inany_ntop(&a->faddr, fbuf, sizeof(fbuf)), a->fport);
+	flow_dbg(flow, "    to side %u (%s): [%s]:%hu -> [%s]:%hu",
+		 !iniside, pif_name(b->pif),
+		 inany_ntop(&b->faddr, fbuf, sizeof(fbuf)), b->fport,
+		 inany_ntop(&b->eaddr, ebuf, sizeof(ebuf)), b->eport);
+
 	return flow;
 }
 
@@ -180,10 +194,20 @@ union flow *flow_start(union flow *flow, enum flow_type type,
  */
 static void flow_end(union flow *flow)
 {
+	char ebuf[INANY_ADDRSTRLEN], fbuf[INANY_ADDRSTRLEN];
+	const struct flowside *a = &flow->f.side[0];
+	const struct flowside *b = &flow->f.side[1];
+
 	if (flow->f.type == FLOW_TYPE_NONE)
 		return; /* Nothing to do */
 
 	flow_dbg(flow, "END %s", flow_type_str[flow->f.type]);
+	flow_dbg(flow, "  side 0 (%s): [%s]:%hu <-> [%s]:%hu",
pif_name(a->pif),
+		 inany_ntop(&a->faddr, fbuf, sizeof(fbuf)), a->fport,
+		 inany_ntop(&a->eaddr, ebuf, sizeof(ebuf)), a->eport);
+	flow_dbg(flow, "  side 1 (%s): [%s]:%hu <-> [%s]:%hu",
pif_name(b->pif),
+		 inany_ntop(&b->faddr, fbuf, sizeof(fbuf)), b->fport,
+		 inany_ntop(&b->eaddr, ebuf, sizeof(ebuf)), b->eport);
 	flow->f.type = FLOW_TYPE_NONE;
 }
 
diff --git a/flow.h b/flow.h
index c943c44..f7fb537 100644
--- a/flow.h
+++ b/flow.h
@@ -35,11 +35,86 @@ extern const uint8_t flow_proto[];
 #define FLOW_PROTO(f)				\
 	((f)->type < FLOW_NUM_TYPES ? flow_proto[(f)->type] : 0)
 
+/**
+ * struct flowside - Common information for one side of a flow
+ * @eaddr:	Endpoint address (remote address from passt's PoV)
+ * @faddr:	Forwarding address (local address from passt's PoV)
+ * @eport:	Endpoint port
+ * @fport:	Forwarding port
+ * @pif:	pif ID on which this side of the flow exists
+ */
+struct flowside {
+	union inany_addr	faddr;
+	union inany_addr	eaddr;
+	in_port_t		fport;
+	in_port_t		eport;
+	uint8_t			pif;
+};
+static_assert(_Alignof(struct flowside) == _Alignof(uint32_t),
+	      "Unexpected alignment for struct flowside");
+
+/** flowside_from_inany - Initialize flowside from inany addresses
+ * @fside:	flowside to initialize
+ * @pif:	pif id of this flowside
+ * @faddr:	Forwarding address (inany)
+ * @fport:	Forwarding port
+ * @eaddr:	Endpoint address (inany)
+ * @eport:	Endpoint port
+ */
+/* cppcheck-suppress unusedFunction */
+static inline void flowside_from_inany(struct flowside *fside, uint8_t pif,
+				const union inany_addr *faddr, in_port_t fport,
+				const union inany_addr *eaddr, in_port_t eport)
+{
+	fside->pif = pif;
+	fside->faddr = *faddr;
+	fside->eaddr = *eaddr;
+	fside->fport = fport;
+	fside->eport = eport;
+}
+
+/** flowside_from_af - Initialize flowside from addresses
+ * @fside:	flowside to initialize
+ * @pif:	pif id of this flowside
+ * @af:		Address family (AF_INET or AF_INET6)
+ * @faddr:	Forwarding address (pointer to in_addr or in6_addr, or NULL)
+ * @fport:	Forwarding port
+ * @eaddr:	Endpoint address (pointer to in_addr or in6_addr, or NULL)
+ * @eport:	Endpoint port
+ *
+ * If NULL is given for either address, the appropriate unspecified/any address
+ * for the address family is substituted.
+ */
+/* cppcheck-suppress unusedFunction */
+static inline void flowside_from_af(struct flowside *fside,
+				    uint8_t pif, sa_family_t af,
+				    const void *faddr, in_port_t fport,
+				    const void *eaddr, in_port_t eport)
+{
+	const union inany_addr *any = af == AF_INET ? &inany_any4 : &inany_any6;
+
+	fside->pif = pif;
+	if (faddr)
+		inany_from_af(&fside->faddr, af, faddr);
+	else
+		fside->faddr = *any;
+	if (eaddr)
+		inany_from_af(&fside->eaddr, af, eaddr);
+	else
+		fside->eaddr = *any;
+	fside->fport = fport;
+	fside->eport = eport;
+}
+
+#define SIDES			2
+
 /**
  * struct flow_common - Common fields for packet flows
+ * @side[]:	Information for each side of the flow
  * @type:	Type of packet flow
  */
 struct flow_common {
+	struct flowside	side[SIDES];
 	uint8_t		type;
 };
 
diff --git a/passt.h b/passt.h
index bc58d64..3db0b8e 100644
--- a/passt.h
+++ b/passt.h
@@ -17,6 +17,9 @@ union epoll_ref;
 
 #include "pif.h"
 #include "packet.h"
+#include "siphash.h"
+#include "ip.h"
+#include "inany.h"
 #include "flow.h"
 #include "icmp.h"
 #include "fwd.h"
diff --git a/pif.h b/pif.h
index bd52936..ca85b34 100644
--- a/pif.h
+++ b/pif.h
@@ -38,7 +38,6 @@ static inline const char *pif_type(enum pif_type pt)
 		return "?";
 }
 
-/* cppcheck-suppress unusedFunction */
 static inline const char *pif_name(uint8_t pif)
 {
 	return pif_type(pif);
diff --git a/tcp_conn.h b/tcp_conn.h
index d280b22..1a07dd5 100644
--- a/tcp_conn.h
+++ b/tcp_conn.h
@@ -106,7 +106,6 @@ struct tcp_tap_conn {
 	uint32_t	seq_init_from_tap;
 };
 
-#define SIDES			2
 /**
  * struct tcp_splice_conn - Descriptor for a spliced TCP connection
  * @f:			Generic flow information
-- 
2.44.0

Every flow in the flow table now has space for the the addresses as seen by
both the host and guest side.  We already fill that information in for
regular "tap" TCP connections, now do it for spliced connections too.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 tcp.c        | 20 ++++++++++----------
 tcp_splice.c | 31 +++++++++++++++++--------------
 tcp_splice.h |  6 ++----
 3 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/tcp.c b/tcp.c
index 1835b86..cd5bffe 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2731,22 +2731,16 @@ static void tcp_snat_inbound(const struct ctx *c, union inany_addr
*addr)
  * @dstport:	Destination port for connection (host side)
  * @flow:	flow to initialise
  * @s:		Accepted socket
- * @sa:		Peer socket address (from accept())
  * @now:	Current timestamp
  */
 static void tcp_tap_conn_from_sock(struct ctx *c, in_port_t dstport,
 				   union flow *flow, int s,
-				   const union sockaddr_inany *sa,
 				   const struct timespec *now)
 {
-	struct flowside *sockside = &flow->f.side[SOCKSIDE];
+	const struct flowside *sockside = &flow->f.side[SOCKSIDE];
 	struct flowside *tapside = &flow->f.side[TAPSIDE];
 	struct tcp_tap_conn *conn;
 
-	sockside->pif = PIF_HOST;
-	inany_from_sockaddr(&sockside->eaddr, &sockside->eport, sa);
-	sockside->fport = dstport;
-
 	tapside->pif = PIF_TAP;
 	tapside->faddr = sockside->eaddr;
 	tapside->fport = sockside->eport;
@@ -2792,16 +2786,23 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 {
 	union sockaddr_inany sa;
 	socklen_t sl = sizeof(sa);
+	struct flowside *side0;
 	union flow *flow;
 	int s;
 
 	if (c->no_tcp || !(flow = flow_alloc()))
 		return;
 
+	side0 = &flow->f.side[0];
+
 	s = accept4(ref.fd, &sa.sa, &sl, SOCK_NONBLOCK);
 	if (s < 0)
 		goto cancel;
 
+	side0->pif = ref.tcp_listen.pif;
+	inany_from_sockaddr(&side0->eaddr, &side0->eport, &sa);
+	side0->fport = ref.tcp_listen.port;
+
 	if (sa.sa_family == AF_INET) {
 		const struct in_addr *addr = &sa.sa4.sin_addr;
 		in_port_t port = sa.sa4.sin_port;
@@ -2829,11 +2830,10 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 		}
 	}
 
-	if (tcp_splice_conn_from_sock(c, ref.tcp_listen.pif,
-				      ref.tcp_listen.port, flow, s, &sa))
+	if (tcp_splice_conn_from_sock(c, ref.tcp_listen.port, flow, s))
 		return;
 
-	tcp_tap_conn_from_sock(c, ref.tcp_listen.port, flow, s, &sa, now);
+	tcp_tap_conn_from_sock(c, ref.tcp_listen.port, flow, s, now);
 	return;
 
 cancel:
diff --git a/tcp_splice.c b/tcp_splice.c
index 42b7be0..462ed0c 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -414,42 +414,38 @@ static int tcp_conn_sock_ns(const struct ctx *c, sa_family_t af)
 /**
  * tcp_splice_conn_from_sock() - Attempt to init state for a spliced connection
  * @c:		Execution context
- * @pif0:	pif id of side 0
  * @dstport:	Side 0 destination port of connection
  * @flow:	flow to initialise
  * @s0:		Accepted (side 0) socket
- * @sa:		Peer address of connection
  *
  * Return: true if able to create a spliced connection, false otherwise
  * #syscalls:pasta setsockopt
  */
-bool tcp_splice_conn_from_sock(const struct ctx *c,
-			       uint8_t pif0, in_port_t dstport,
-			       union flow *flow, int s0,
-			       const union sockaddr_inany *sa)
+bool tcp_splice_conn_from_sock(const struct ctx *c, in_port_t dstport,
+			       union flow *flow, int s0)
 {
+	const struct flowside *side0 = &flow->f.side[0];
+	const union inany_addr *src = &side0->eaddr;
+	struct flowside *side1 = &flow->f.side[1];
 	struct tcp_splice_conn *conn;
-	union inany_addr src;
-	in_port_t srcport;
 	sa_family_t af;
 	uint8_t pif1;
 
 	if (c->mode != MODE_PASTA)
 		return false;
 
-	inany_from_sockaddr(&src, &srcport, sa);
-	af = inany_v4(&src) ? AF_INET : AF_INET6;
+	af = inany_v4(src) ? AF_INET : AF_INET6;
 
-	switch (pif0) {
+	switch (side0->pif) {
 	case PIF_SPLICE:
-		if (!inany_is_loopback(&src)) {
+		if (!inany_is_loopback(src)) {
 			char str[INANY_ADDRSTRLEN];
 
 			/* We can't use flow_err() etc. because we haven't set
 			 * the flow type yet
 			 */
 			warn("Bad source address %s for splice, closing",
-			     inany_ntop(&src, str, sizeof(str)));
+			     inany_ntop(src, str, sizeof(str)));
 
 			/* We *don't* want to fall back to tap */
 			flow_alloc_cancel(flow);
@@ -461,7 +457,7 @@ bool tcp_splice_conn_from_sock(const struct ctx *c,
 		break;
 
 	case PIF_HOST:
-		if (!inany_is_loopback(&src))
+		if (!inany_is_loopback(src))
 			return false;
 
 		pif1 = PIF_SPLICE;
@@ -472,6 +468,13 @@ bool tcp_splice_conn_from_sock(const struct ctx *c,
 		return false;
 	}
 
+	if (af == AF_INET)
+		flowside_from_af(side1, pif1, AF_INET, NULL, 0,
+				 &in4addr_loopback, dstport);
+	else
+		flowside_from_af(side1, pif1, AF_INET6, NULL, 0,
+				 &in6addr_loopback, dstport);
+
 	conn = FLOW_START(flow, FLOW_TCP_SPLICE, tcp_splice, 0);
 
 	conn->flags = af == AF_INET ? 0 : SPLICE_V6;
diff --git a/tcp_splice.h b/tcp_splice.h
index ed8f0c5..e523c7e 100644
--- a/tcp_splice.h
+++ b/tcp_splice.h
@@ -11,10 +11,8 @@ union sockaddr_inany;
 
 void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref,
 			     uint32_t events);
-bool tcp_splice_conn_from_sock(const struct ctx *c,
-			       uint8_t pif0, in_port_t dstport,
-			       union flow *flow, int s0,
-			       const union sockaddr_inany *sa);
+bool tcp_splice_conn_from_sock(const struct ctx *c, in_port_t dstport,
+			       union flow *flow, int s0);
 void tcp_splice_init(struct ctx *c);
 
 #endif /* TCP_SPLICE_H */
-- 
2.44.0

Now that we store all our endpoints in the flowside structure, use some
inany helpers to make validation of those endpoints simpler.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 inany.h |  1 -
 tcp.c   | 69 +++++++++++++--------------------------------------------
 2 files changed, 15 insertions(+), 55 deletions(-)

diff --git a/inany.h b/inany.h
index 407690e..c0228a1 100644
--- a/inany.h
+++ b/inany.h
@@ -123,7 +123,6 @@ static inline bool inany_is_multicast(const union inany_addr *a)
  *
  * Return: true if @a is specified and a unicast address
  */
-/* cppcheck-suppress unusedFunction */
 static inline bool inany_is_unicast(const union inany_addr *a)
 {
 	return !inany_is_unspecified(a) && !inany_is_multicast(a);
diff --git a/tcp.c b/tcp.c
index 5ff7480..e669b18 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1954,36 +1954,16 @@ static void tcp_conn_from_tap(struct ctx *c, sa_family_t af,
 
 	flowside_from_af(tapside, PIF_TAP, af, daddr, dstport, saddr, srcport);
 
-	if (af == AF_INET) {
-		if (IN4_IS_ADDR_UNSPECIFIED(saddr) ||
-		    IN4_IS_ADDR_BROADCAST(saddr) ||
-		    IN4_IS_ADDR_MULTICAST(saddr) || srcport == 0 ||
-		    IN4_IS_ADDR_UNSPECIFIED(daddr) ||
-		    IN4_IS_ADDR_BROADCAST(daddr) ||
-		    IN4_IS_ADDR_MULTICAST(daddr) || dstport == 0) {
-			char sstr[INET_ADDRSTRLEN], dstr[INET_ADDRSTRLEN];
-
-			debug("Invalid endpoint in TCP SYN: %s:%hu -> %s:%hu",
-			      inet_ntop(AF_INET, saddr, sstr, sizeof(sstr)),
-			      srcport,
-			      inet_ntop(AF_INET, daddr, dstr, sizeof(dstr)),
-			      dstport);
-			goto cancel;
-		}
-	} else if (af == AF_INET6) {
-		if (IN6_IS_ADDR_UNSPECIFIED(saddr) ||
-		    IN6_IS_ADDR_MULTICAST(saddr) || srcport == 0 ||
-		    IN6_IS_ADDR_UNSPECIFIED(daddr) ||
-		    IN6_IS_ADDR_MULTICAST(daddr) || dstport == 0) {
-			char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN];
-
-			debug("Invalid endpoint in TCP SYN: %s:%hu -> %s:%hu",
-			      inet_ntop(AF_INET6, saddr, sstr, sizeof(sstr)),
-			      srcport,
-			      inet_ntop(AF_INET6, daddr, dstr, sizeof(dstr)),
-			      dstport);
-			goto cancel;
-		}
+	if (!inany_is_unicast(&tapside->eaddr) || tapside->eport == 0 ||
+	    !inany_is_unicast(&tapside->faddr) || tapside->fport == 0) {
+		char sstr[INANY_ADDRSTRLEN], dstr[INANY_ADDRSTRLEN];
+
+		debug("Invalid endpoint in TCP SYN: %s:%hu -> %s:%hu",
+		      inany_ntop(&tapside->eaddr, sstr, sizeof(sstr)),
+		      tapside->eport,
+		      inany_ntop(&tapside->faddr, dstr, sizeof(dstr)),
+		      tapside->fport);
+		goto cancel;
 	}
 
 	if ((s = tcp_conn_sock(c, af)) < 0)
@@ -2788,31 +2768,12 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 	inany_from_sockaddr(&side0->eaddr, &side0->eport, &sa);
 	side0->fport = ref.tcp_listen.port;
 
-	if (sa.sa_family == AF_INET) {
-		const struct in_addr *addr = &sa.sa4.sin_addr;
-		in_port_t port = sa.sa4.sin_port;
-
-		if (IN4_IS_ADDR_UNSPECIFIED(addr) ||
-		    IN4_IS_ADDR_BROADCAST(addr) ||
-		    IN4_IS_ADDR_MULTICAST(addr) || port == 0) {
-			char str[INET_ADDRSTRLEN];
+	if (!inany_is_unicast(&side0->eaddr) || side0->eport == 0) {
+		char str[INANY_ADDRSTRLEN];
 
-			err("Invalid endpoint from TCP accept(): %s:%hu",
-			    inet_ntop(AF_INET, addr, str, sizeof(str)), port);
-			goto cancel;
-		}
-	} else if (sa.sa_family == AF_INET6) {
-		const struct in6_addr *addr = &sa.sa6.sin6_addr;
-		in_port_t port = sa.sa6.sin6_port;
-
-		if (IN6_IS_ADDR_UNSPECIFIED(addr) ||
-		    IN6_IS_ADDR_MULTICAST(addr) || port == 0) {
-			char str[INET6_ADDRSTRLEN];
-
-			err("Invalid endpoint from TCP accept(): %s:%hu",
-			    inet_ntop(AF_INET6, addr, str, sizeof(str)), port);
-			goto cancel;
-		}
+		err("Invalid endpoint from TCP accept(): %s:%hu",
+		    inany_ntop(&side0->eaddr, str, sizeof(str)), side0->eport);
+		goto cancel;
 	}
 
 	if (tcp_splice_conn_from_sock(c, ref.tcp_listen.port, flow, s))
-- 
2.44.0

Since we're now constructing socket addresses based on information in the
flowside, we no longer need an explicit flag to tell if we're dealing with
an IPv4 or IPv6 connection.  Hence, drop the now unused SPLICE_V6 flag.

As well as just simplifying the code, this allows for possible future
extensions where we could splice an IPv4 connection to an IPv6 connection
or vice versa.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 tcp_conn.h   | 11 +++++------
 tcp_splice.c |  3 ---
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/tcp_conn.h b/tcp_conn.h
index f55f144..dd31d10 100644
--- a/tcp_conn.h
+++ b/tcp_conn.h
@@ -129,12 +129,11 @@ struct tcp_splice_conn {
 #define FIN_SENT_1			BIT(7)
 
 	uint8_t flags;
-#define SPLICE_V6			BIT(0)
-#define RCVLOWAT_SET_0			BIT(1)
-#define RCVLOWAT_SET_1			BIT(2)
-#define RCVLOWAT_ACT_0			BIT(3)
-#define RCVLOWAT_ACT_1			BIT(4)
-#define CLOSING				BIT(5)
+#define RCVLOWAT_SET_0			BIT(0)
+#define RCVLOWAT_SET_1			BIT(1)
+#define RCVLOWAT_ACT_0			BIT(2)
+#define RCVLOWAT_ACT_1			BIT(3)
+#define CLOSING				BIT(4)
 
 	uint32_t read[SIDES];
 	uint32_t written[SIDES];
diff --git a/tcp_splice.c b/tcp_splice.c
index aa04a9b..00b88c5 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -73,8 +73,6 @@ static int ns_sock_pool6	[TCP_SOCK_POOL_SIZE];
 /* Pool of pre-opened pipes */
 static int splice_pipe_pool		[TCP_SPLICE_PIPE_POOL_SIZE][2];
 
-#define CONN_V6(x)			(x->flags & SPLICE_V6)
-#define CONN_V4(x)			(!CONN_V6(x))
 #define CONN_HAS(conn, set)		((conn->events & (set)) == (set))
 #define CONN(idx)			(&FLOW(idx)->tcp_splice)
 
@@ -459,7 +457,6 @@ bool tcp_splice_conn_from_sock(const struct ctx *c, in_port_t
dstport,
 
 	conn = FLOW_START(flow, FLOW_TCP_SPLICE, tcp_splice, 0);
 
-	conn->flags = af == AF_INET ? 0 : SPLICE_V6;
 	conn->s[0] = s0;
 	conn->s[1] = -1;
 	conn->pipe[0][0] = conn->pipe[0][1] = -1;
-- 
2.44.0

Move the data structures and helper functions for the TCP hash table to
flow.c, making it a general hash table indexing sides of flows.  This is
largely code motion and straightforward renames.  There are two semantic
changes:

 * flow_hash_lookup() now needs to verify that the entry has a matching
   protocol as well as matching addresses, ports and interface

 * We double the size of the hash table, because it's now at least
   theoretically possible for both sides of each flow to be hashed.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 flow.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++-
 flow.h |  10 ++--
 tcp.c  | 146 +++++----------------------------------------------------
 3 files changed, 147 insertions(+), 138 deletions(-)

diff --git a/flow.c b/flow.c
index 370d422..8cefbed 100644
--- a/flow.c
+++ b/flow.c
@@ -138,6 +138,16 @@ union flow flowtab[FLOW_MAX];
 /* Last time the flow timers ran */
 static struct timespec flow_timer_run;
 
+/* Hash table to index it */
+#define FLOW_HASH_LOAD		70		/* % */
+#define FLOW_HASH_SIZE		((2 * FLOW_MAX * 100 / FLOW_HASH_LOAD))
+
+/* Table for lookup from flowside information */
+static flow_sidx_t flow_hashtab[FLOW_HASH_SIZE];
+
+static_assert(ARRAY_SIZE(flow_hashtab) >= 2 * FLOW_MAX,
+"Safe linear probing requires hash table with more entries than the number of sides
in the flow table");
+
 /** flow_log_ - Log flow-related message
  * @f:		flow the message is related to
  * @pri:	Log priority
@@ -276,8 +286,8 @@ void flow_alloc_cancel(union flow *flow)
  *
  * Return: hash value
  */
-uint64_t flow_hash(const struct ctx *c, uint8_t proto,
-		   const struct flowside *fside)
+static uint64_t flow_hash(const struct ctx *c, uint8_t proto,
+			  const struct flowside *fside)
 {
 	struct siphash_state state = SIPHASH_INIT(c->hash_secret);
 
@@ -296,6 +306,116 @@ uint64_t flow_hash(const struct ctx *c, uint8_t proto,
 			     fside->fport << 16 | fside->eport);
 }
 
+/**
+ * flow_sidx_hash() - Calculate hash value for given side of a given flow
+ * @c:		Execution context
+ * @sidx:	Flow & side index to get hash for
+ *
+ * Return: hash value, of the flow & side represented by @sidx
+ */
+static uint64_t flow_sidx_hash(const struct ctx *c, flow_sidx_t sidx)
+{
+	const struct flow_common *f = &flow_at_sidx(sidx)->f;
+	return flow_hash(c, FLOW_PROTO(f), &f->side[sidx.side]);
+}
+
+/**
+ * flow_hash_probe() - Find hash bucket for a flow
+ * @c:		Execution context
+ * @sidx:	Flow and side to find bucket for
+ *
+ * Return: If @sidx is in the hash table, its current bucket, otherwise a
+ *         suitable free bucket for it.
+ */
+static inline unsigned flow_hash_probe(const struct ctx *c, flow_sidx_t sidx)
+{
+	unsigned b = flow_sidx_hash(c, sidx) % FLOW_HASH_SIZE;
+
+	/* Linear probing */
+	while (!flow_sidx_eq(flow_hashtab[b], FLOW_SIDX_NONE) &&
+	       !flow_sidx_eq(flow_hashtab[b], sidx))
+		b = mod_sub(b, 1, FLOW_HASH_SIZE);
+
+	return b;
+}
+
+/**
+ * flow_hash_insert() - Insert side of a flow into into hash table
+ * @c:		Execution context
+ * @sidx:	Flow & side index
+ */
+void flow_hash_insert(const struct ctx *c, flow_sidx_t sidx)
+{
+	unsigned b = flow_hash_probe(c, sidx);
+
+	flow_hashtab[b] = sidx;
+	flow_dbg(flow_at_sidx(sidx), "hash table insert: bucket: %u", b);
+}
+
+/**
+ * flow_hash_remove() - Drop side of a flow from the hash table
+ * @c:		Execution context
+ * @sidx:	Side of flow to remove
+ */
+void flow_hash_remove(const struct ctx *c, flow_sidx_t sidx)
+{
+	unsigned b = flow_hash_probe(c, sidx), s;
+
+	if (flow_sidx_eq(flow_hashtab[b], FLOW_SIDX_NONE))
+		return; /* Redundant remove */
+
+	flow_dbg(flow_at_sidx(sidx), "hash table remove: bucket: %u", b);
+
+	/* Scan the remainder of the cluster */
+	for (s = mod_sub(b, 1, FLOW_HASH_SIZE);
+	     !flow_sidx_eq(flow_hashtab[s], FLOW_SIDX_NONE);
+	     s = mod_sub(s, 1, FLOW_HASH_SIZE)) {
+		unsigned h = flow_sidx_hash(c, flow_hashtab[s]) % FLOW_HASH_SIZE;
+
+		if (!mod_between(h, s, b, FLOW_HASH_SIZE)) {
+			/* flow_hashtab[s] can live in flow_hashtab[b]'s slot */
+			debug("hash table remove: shuffle %u -> %u", s, b);
+			flow_hashtab[b] = flow_hashtab[s];
+			b = s;
+		}
+	}
+
+	flow_hashtab[b] = FLOW_SIDX_NONE;
+}
+
+/**
+ * flow_hash_lookup() - Look up a flow given addressing information
+ * @c:		Execution context
+ * @proto:	Protocol of the flow (IP L4 protocol number)
+ * @pif:	Interface of the flow
+ * @af:		Address family, AF_INET or AF_INET6
+ * @eaddr:	Guest side endpoint address (guest local address)
+ * @faddr:	Guest side forwarding address (guest remote address)
+ * @eport:	Guest side endpoint port (guest local port)
+ * @fport:	Guest side forwarding port (guest remote port)
+ *
+ * Return: sidx of the matching flow & side, FLOW_SIDX_NONE if not found
+ */
+flow_sidx_t flow_hash_lookup(const struct ctx *c,
+			     uint8_t proto, uint8_t pif, sa_family_t af,
+			     const void *eaddr, const void *faddr,
+			     in_port_t eport, in_port_t fport)
+{
+	struct flowside fside;
+	union flow *flow;
+	int b;
+
+	flowside_from_af(&fside, pif, af, faddr, fport, eaddr, eport);
+
+	b = flow_hash(c, proto, &fside) % FLOW_HASH_SIZE;
+	while ((flow = flow_at_sidx(flow_hashtab[b])) &&
+	       FLOW_PROTO(&flow->f) == proto &&
+	       !flowside_eq(&flow->f.side[flow_hashtab[b].side], &fside))
+		b = (b + 1) % FLOW_HASH_SIZE;
+
+	return flow_hashtab[b];
+}
+
 /**
  * flow_defer_handler() - Handler for per-flow deferred and timed tasks
  * @c:		Execution context
@@ -389,7 +509,12 @@ void flow_defer_handler(const struct ctx *c, const struct timespec
*now)
  */
 void flow_init(void)
 {
+	unsigned b;
+
 	/* Initial state is a single free cluster containing the whole table */
 	flowtab[0].free.n = FLOW_MAX;
 	flowtab[0].free.next = FLOW_MAX;
+
+	for (b = 0; b < FLOW_HASH_SIZE; b++)
+		flow_hashtab[b] = FLOW_SIDX_NONE;
 }
diff --git a/flow.h b/flow.h
index 218edc6..c1181cc 100644
--- a/flow.h
+++ b/flow.h
@@ -144,9 +144,6 @@ union flow *flow_start(union flow *flow, enum flow_type type,
 #define FLOW_START(flow_, t_, var_, i_)		\
 	(&flow_start((flow_), (t_), (i_))->var_)
 
-uint64_t flow_hash(const struct ctx *c, uint8_t proto,
-		   const struct flowside *fside);
-
 /**
  * struct flow_sidx - ID for one side of a specific flow
  * @side:	Side referenced (0 or 1)
@@ -172,6 +169,13 @@ static inline bool flow_sidx_eq(flow_sidx_t a, flow_sidx_t b)
 	return (a.flow == b.flow) && (a.side == b.side);
 }
 
+void flow_hash_insert(const struct ctx *c, flow_sidx_t sidx);
+void flow_hash_remove(const struct ctx *c, flow_sidx_t sidx);
+flow_sidx_t flow_hash_lookup(const struct ctx *c,
+			     uint8_t proto, uint8_t pif, sa_family_t af,
+			     const void *eaddr, const void *faddr,
+			     in_port_t eport, in_port_t fport);
+
 union flow;
 
 void flow_init(void);
diff --git a/tcp.c b/tcp.c
index 2ad47c5..7f83014 100644
--- a/tcp.c
+++ b/tcp.c
@@ -311,9 +311,6 @@
 #define TCP_FRAMES							\
 	(c->mode == MODE_PASST ? TCP_FRAMES_MEM : 1)
 
-#define TCP_HASH_TABLE_LOAD		70		/* % */
-#define TCP_HASH_TABLE_SIZE		(FLOW_MAX * 100 / TCP_HASH_TABLE_LOAD)
-
 #define MAX_WS				8
 #define MAX_WINDOW			(1 << (16 + (MAX_WS)))
 
@@ -524,12 +521,6 @@ static struct iovec	tcp_iov			[UIO_MAXIOV];
 
 #define CONN(idx)		(&(FLOW(idx)->tcp))
 
-/* Table for lookup from flowside information */
-static flow_sidx_t tc_hash[TCP_HASH_TABLE_SIZE];
-
-static_assert(ARRAY_SIZE(tc_hash) >= FLOW_MAX,
-	"Safe linear probing requires hash table larger than connection table");
-
 /* Pools for pre-opened sockets (in init) */
 int init_sock_pool4		[TCP_SOCK_POOL_SIZE];
 int init_sock_pool6		[TCP_SOCK_POOL_SIZE];
@@ -723,9 +714,6 @@ static void conn_flag_do(const struct ctx *c, struct tcp_tap_conn
*conn,
 		tcp_timer_ctl(c, conn);
 }
 
-static void tcp_hash_remove(const struct ctx *c,
-			    const struct tcp_tap_conn *conn);
-
 /**
  * conn_event_do() - Set and log connection events, update epoll state
  * @c:		Execution context
@@ -771,7 +759,7 @@ static void conn_event_do(const struct ctx *c, struct tcp_tap_conn
*conn,
 			 num == -1 	       ? "CLOSED" : tcp_event_str[num]);
 
 	if (event == CLOSED)
-		tcp_hash_remove(c, conn);
+		flow_hash_remove(c, FLOW_SIDX(conn, TAPSIDE));
 	else if ((event == TAP_FIN_RCVD) && !(conn->events & SOCK_FIN_RCVD))
 		conn_flag(c, conn, ACTIVE_CLOSE);
 	else
@@ -1074,116 +1062,6 @@ static int tcp_opt_get(const char *opts, size_t len, uint8_t
type_find,
 	return -1;
 }
 
-/**
- * tcp_conn_hash() - Calculate hash bucket of an existing connection
- * @c:		Execution context
- * @conn:	Connection
- *
- * Return: hash value, needs to be adjusted for table size
- */
-static uint64_t tcp_conn_hash(const struct ctx *c,
-			      const struct tcp_tap_conn *conn)
-{
-	return flow_hash(c, IPPROTO_TCP, &conn->f.side[TAPSIDE]);
-}
-
-/**
- * tcp_hash_probe() - Find hash bucket for a connection
- * @c:		Execution context
- * @conn:	Connection to find bucket for
- *
- * Return: If @conn is in the table, its current bucket, otherwise a suitable
- *         free bucket for it.
- */
-static inline unsigned tcp_hash_probe(const struct ctx *c,
-				      const struct tcp_tap_conn *conn)
-{
-	flow_sidx_t sidx = FLOW_SIDX(conn, TAPSIDE);
-	unsigned b = tcp_conn_hash(c, conn) % TCP_HASH_TABLE_SIZE;
-
-	/* Linear probing */
-	while (!flow_sidx_eq(tc_hash[b], FLOW_SIDX_NONE) &&
-	       !flow_sidx_eq(tc_hash[b], sidx))
-		b = mod_sub(b, 1, TCP_HASH_TABLE_SIZE);
-
-	return b;
-}
-
-/**
- * tcp_hash_insert() - Insert connection into hash table, chain link
- * @c:		Execution context
- * @conn:	Connection pointer
- */
-static void tcp_hash_insert(const struct ctx *c, struct tcp_tap_conn *conn)
-{
-	unsigned b = tcp_hash_probe(c, conn);
-
-	tc_hash[b] = FLOW_SIDX(conn, TAPSIDE);
-	flow_dbg(conn, "hash table insert: sock %i, bucket: %u", conn->sock, b);
-}
-
-/**
- * tcp_hash_remove() - Drop connection from hash table, chain unlink
- * @c:		Execution context
- * @conn:	Connection pointer
- */
-static void tcp_hash_remove(const struct ctx *c,
-			    const struct tcp_tap_conn *conn)
-{
-	unsigned b = tcp_hash_probe(c, conn), s;
-	union flow *flow = flow_at_sidx(tc_hash[b]);
-
-	if (!flow)
-		return; /* Redundant remove */
-
-	flow_dbg(conn, "hash table remove: sock %i, bucket: %u", conn->sock, b);
-
-	/* Scan the remainder of the cluster */
-	for (s = mod_sub(b, 1, TCP_HASH_TABLE_SIZE);
-	     (flow = flow_at_sidx(tc_hash[s]));
-	     s = mod_sub(s, 1, TCP_HASH_TABLE_SIZE)) {
-		unsigned h = tcp_conn_hash(c, &flow->tcp) % TCP_HASH_TABLE_SIZE;
-
-		if (!mod_between(h, s, b, TCP_HASH_TABLE_SIZE)) {
-			/* tc_hash[s] can live in tc_hash[b]'s slot */
-			debug("hash table remove: shuffle %u -> %u", s, b);
-			tc_hash[b] = tc_hash[s];
-			b = s;
-		}
-	}
-
-	tc_hash[b] = FLOW_SIDX_NONE;
-}
-
-/**
- * tcp_hash_lookup() - Look up connection given remote address and ports
- * @c:		Execution context
- * @af:		Address family, AF_INET or AF_INET6
- * @eaddr:	Guest side endpoint address (guest local address)
- * @faddr:	Guest side forwarding address (guest remote address)
- * @eport:	Guest side endpoint port (guest local port)
- * @fport:	Guest side forwarding port (guest remote port)
- *
- * Return: connection pointer, if found, -ENOENT otherwise
- */
-static struct tcp_tap_conn *tcp_hash_lookup(const struct ctx *c, sa_family_t af,
-					    const void *eaddr, const void *faddr,
-					    in_port_t eport, in_port_t fport)
-{
-	struct flowside fside;
-	union flow *flow;
-	unsigned b;
-
-	flowside_from_af(&fside, PIF_TAP, af, faddr, fport, eaddr, eport);
-
-	b = flow_hash(c, IPPROTO_TCP, &fside) % TCP_HASH_TABLE_SIZE;
-	while ((flow = flow_at_sidx(tc_hash[b])) &&
-	       !flowside_eq(&flow->f.side[TAPSIDE], &fside))
-		b = mod_sub(b, 1, TCP_HASH_TABLE_SIZE);
-
-	return &flow->tcp;
-}
-
 /**
  * tcp_flow_defer() - Deferred per-flow handling (clean up closed connections)
  * @flow:	Flow table entry for this connection
@@ -1970,7 +1848,7 @@ static void tcp_conn_from_tap(struct ctx *c, sa_family_t af,
 	tcp_seq_init(c, conn, now);
 	conn->seq_ack_from_tap = conn->seq_to_tap;
 
-	tcp_hash_insert(c, conn);
+	flow_hash_insert(c, FLOW_SIDX(conn, TAPSIDE));
 
 	sockaddr_from_inany(&sa, &sl, &sockside->eaddr, sockside->eport,
 			    c->ifi6);
@@ -2466,6 +2344,8 @@ int tcp_tap_handler(struct ctx *c, uint8_t pif, sa_family_t af,
 	const struct tcphdr *th;
 	size_t optlen, len;
 	const char *opts;
+	union flow *flow;
+	flow_sidx_t sidx;
 	int ack_due = 0;
 	int count;
 
@@ -2481,17 +2361,22 @@ int tcp_tap_handler(struct ctx *c, uint8_t pif, sa_family_t af,
 	optlen = MIN(optlen, ((1UL << 4) /* from doff width */ - 6) * 4UL);
 	opts = packet_get(p, idx, sizeof(*th), optlen, NULL);
 
-	conn = tcp_hash_lookup(c, af, saddr, daddr,
-			       ntohs(th->source), ntohs(th->dest));
+	sidx = flow_hash_lookup(c, IPPROTO_TCP, PIF_TAP, af, saddr, daddr,
+				ntohs(th->source), ntohs(th->dest));
+	flow = flow_at_sidx(sidx);
 
 	/* New connection from tap */
-	if (!conn) {
+	if (!flow) {
 		if (opts && th->syn && !th->ack)
 			tcp_conn_from_tap(c, af, saddr, daddr, th,
 					  opts, optlen, now);
 		return 1;
 	}
 
+	ASSERT(flow->f.type == FLOW_TCP);
+	ASSERT(sidx.side == TAPSIDE);
+	conn = &flow->tcp;
+
 	flow_trace(conn, "packet length %zu from tap", len);
 
 	if (th->rst) {
@@ -2671,7 +2556,7 @@ static void tcp_tap_conn_from_sock(struct ctx *c, in_port_t
dstport,
 	conn_event(c, conn, SOCK_ACCEPTED);
 
 	tcp_seq_init(c, conn, now);
-	tcp_hash_insert(c, conn);
+	flow_hash_insert(c, FLOW_SIDX(conn, TAPSIDE));
 
 	conn->seq_ack_from_tap = conn->seq_to_tap;
 
@@ -3060,11 +2945,6 @@ static void tcp_sock_refill_init(const struct ctx *c)
  */
 int tcp_init(struct ctx *c)
 {
-	unsigned b;
-
-	for (b = 0; b < TCP_HASH_TABLE_SIZE; b++)
-		tc_hash[b] = FLOW_SIDX_NONE;
-
 	if (c->ifi4)
 		tcp_sock4_iov_init(c);
 
-- 
2.44.0

For ICMP (ping) flows we currently don't populate the common flowside
fields.   Fill out those parts of the common information that we can easily
obtain, using the ICMP id as the "port" on both ends.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 icmp.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/icmp.c b/icmp.c
index 1c5cf84..c000175 100644
--- a/icmp.c
+++ b/icmp.c
@@ -150,16 +150,23 @@ static void icmp_ping_close(const struct ctx *c,
  * @id_sock:	Pointer to ping flow entry slot in icmp_id_map[] to update
  * @af:		Address family, AF_INET or AF_INET6
  * @id:		ICMP id for the new socket
+ * @pif:	pif originating the ping request
+ * @saddr:	Source address
+ * @daddr:	Destination address
  *
  * Return: Newly opened ping flow, or NULL on failure
  */
 static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 					    struct icmp_ping_flow **id_sock,
-					    sa_family_t af, uint16_t id)
+					    sa_family_t af, uint16_t id,
+					    uint8_t pif, const void *saddr,
+					    const void *daddr)
 {
 	uint8_t flowtype = af == AF_INET ? FLOW_PING4 : FLOW_PING6;
 	union epoll_ref ref = { .type = EPOLL_TYPE_PING };
 	union flow *flow = flow_alloc();
+	struct flowside *sockside = &flow->f.side[SOCKSIDE];
+	struct flowside *tapside = &flow->f.side[TAPSIDE];
 	struct icmp_ping_flow *pingf;
 	const void *bind_addr;
 	const char *bind_if;
@@ -167,6 +174,9 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 	if (!flow)
 		return NULL;
 
+	flowside_from_af(tapside, pif, af, daddr, id, saddr, id);
+	flowside_from_af(sockside, PIF_HOST, af, NULL, 0, daddr, 0);
+
 	pingf = FLOW_START(flow, flowtype, ping, TAPSIDE);
 
 	pingf->seq = -1;
@@ -228,7 +238,6 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t
af,
 	uint16_t id, seq;
 	void *pkt;
 
-	(void)saddr;
 	ASSERT(pif == PIF_TAP);
 
 	if (af == AF_INET) {
@@ -269,7 +278,8 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t
af,
 	}
 
 	if (!(pingf = *id_sock))
-		if (!(pingf = icmp_ping_new(c, id_sock, af, id)))
+		if (!(pingf = icmp_ping_new(c, id_sock, af, id,
+					    pif, saddr, daddr)))
 			return 1;
 
 	pingf->ts = now->tv_sec;
-- 
2.44.0

When we receive a ping packet from the tap interface, we currently locate
the correct flow entry (if present) using an anciliary data structure, the
icmp_id_map[] tables.  However, we can look this up using the flow hash
table - that's what it's for.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 icmp.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/icmp.c b/icmp.c
index e29416f..3b8f282 100644
--- a/icmp.c
+++ b/icmp.c
@@ -145,6 +145,7 @@ static void icmp_ping_close(const struct ctx *c,
 
 	epoll_ctl(c->epollfd, EPOLL_CTL_DEL, pingf->sock, NULL);
 	close(pingf->sock);
+	flow_hash_remove(c, FLOW_SIDX(pingf, TAPSIDE));
 
 	if (pingf->f.type == FLOW_PING4)
 		icmp_id_map[V4][id] = NULL;
@@ -213,6 +214,7 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 
 	flow_dbg(pingf, "new socket %i for echo ID %"PRIu16, pingf->sock, id);
 
+	flow_hash_insert(c, FLOW_SIDX(pingf, TAPSIDE));
 	*id_sock = pingf;
 
 	return pingf;
@@ -243,6 +245,8 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t
af,
 	union sockaddr_inany sa;
 	size_t dlen, l4len;
 	uint16_t id, seq;
+	union flow *flow;
+	uint8_t proto;
 	socklen_t sl;
 	void *pkt;
 
@@ -260,6 +264,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t
af,
 		if (ih->type != ICMP_ECHO)
 			return 1;
 
+		proto = IPPROTO_ICMP;
 		id = ntohs(ih->un.echo.id);
 		id_sock = &icmp_id_map[V4][id];
 		seq = ntohs(ih->un.echo.sequence);
@@ -275,6 +280,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t
af,
 		if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)
 			return 1;
 
+		proto = IPPROTO_ICMPV6;
 		id = ntohs(ih->icmp6_identifier);
 		id_sock = &icmp_id_map[V6][id];
 		seq = ntohs(ih->icmp6_sequence);
@@ -282,12 +288,17 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t
af,
 		ASSERT(0);
 	}
 
-	if (!(pingf = *id_sock))
-		if (!(pingf = icmp_ping_new(c, id_sock, af, id,
-					    pif, saddr, daddr)))
-			return 1;
+	flow = flow_at_sidx(flow_hash_lookup(c, proto, PIF_TAP,
+					     af, saddr, daddr, id, id));
+
+	if (flow)
+		pingf = &flow->ping;
+	else if (!(pingf = icmp_ping_new(c, id_sock, af, id, pif, saddr, daddr)))
+		return 1;
 
 	sockside = &pingf->f.side[SOCKSIDE];
+
+	ASSERT(flow_proto[pingf->f.type] == proto);
 	pingf->ts = now->tv_sec;
 
 	sockaddr_from_inany(&sa, &sl, &sockside->eaddr, 0, c->ifi6);
-- 
2.44.0

Currently the code to translate host side addresses and ports to guest side
addresses and ports, and vice versa, is scattered across the TCP code.
This includes both port redirection as controlled by the -t and -T options,
and our special case NAT controlled by the --no-map-gw option.

Gather all this logic into a new fwd_nat_flow() function in fwd.c which
takes protocol and flowside as input and generates the protocol and
flowside to which to forward the flow, including applying any NAT or port
translation.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 fwd.c        | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fwd.h        |   5 ++
 tcp.c        |  92 +++++++++++-----------------------
 tcp_splice.c |  57 +--------------------
 tcp_splice.h |   3 +-
 5 files changed, 175 insertions(+), 121 deletions(-)

diff --git a/fwd.c b/fwd.c
index b3d5a37..d0ca492 100644
--- a/fwd.c
+++ b/fwd.c
@@ -25,6 +25,7 @@
 #include "fwd.h"
 #include "passt.h"
 #include "lineread.h"
+#include "flow_table.h"
 
 /* See enum in kernel's include/net/tcp_states.h */
 #define UDP_LISTEN	0x07
@@ -154,3 +155,141 @@ void fwd_scan_ports_init(struct ctx *c)
 				   &c->tcp.fwd_out, &c->tcp.fwd_in);
 	}
 }
+
+static bool fwd_from_tap(const struct ctx *c, uint8_t proto,
+			 const struct flowside *a, struct flowside *b)
+{
+	(void)proto;
+
+	b->pif = PIF_HOST;
+	b->eaddr = a->faddr;
+	b->eport = a->fport;
+
+	if (!c->no_map_gw) {
+		struct in_addr *v4 = inany_v4(&b->eaddr);
+
+		if (v4 && IN4_ARE_ADDR_EQUAL(v4, &c->ip4.gw))
+			*v4 = in4addr_loopback;
+		if (IN6_ARE_ADDR_EQUAL(&b->eaddr, &c->ip6.gw))
+			b->eaddr.a6 = in6addr_loopback;
+	}
+
+	return true;
+}
+
+static bool fwd_from_splice(const struct ctx *c, uint8_t proto,
+			 const struct flowside *a, struct flowside *b)
+{
+	const struct in_addr *ae4 = inany_v4(&a->eaddr);
+
+	if (!inany_is_loopback(&a->eaddr) ||
+	    (!inany_is_loopback(&a->faddr) &&
!inany_is_unspecified(&a->faddr))) {
+		char estr[INANY_ADDRSTRLEN], fstr[INANY_ADDRSTRLEN];
+
+		debug("Non loopback address on %s: [%s]:%hu -> [%s]:%hu",
+		      pif_name(a->pif),
+		      inany_ntop(&a->eaddr, estr, sizeof(estr)), a->eport,
+		      inany_ntop(&a->faddr, fstr, sizeof(fstr)), a->fport);
+		return false;
+	}
+
+	b->pif = PIF_HOST;
+
+	if (ae4)
+		inany_from_af(&b->eaddr, AF_INET, &in4addr_loopback);
+	else
+		inany_from_af(&b->eaddr, AF_INET6, &in6addr_loopback);
+
+	b->eport = a->fport;
+
+	if (proto == IPPROTO_TCP)
+		b->eport += c->tcp.fwd_out.delta[b->eport];
+
+	return true;
+}
+
+static bool fwd_from_host(const struct ctx *c, uint8_t proto,
+			  const struct flowside *a, struct flowside *b)
+{
+	struct in_addr *bf4;
+
+	if (c->mode == MODE_PASTA && inany_is_loopback(&a->eaddr) &&
+	    proto == IPPROTO_TCP) {
+		/* spliceable */
+		b->pif = PIF_SPLICE;
+		b->faddr = a->eaddr;
+
+		if (inany_v4(&a->eaddr))
+			inany_from_af(&b->eaddr, AF_INET, &in4addr_loopback);
+		else
+			inany_from_af(&b->eaddr, AF_INET6, &in6addr_loopback);
+		b->eport = a->fport;
+		if (proto == IPPROTO_TCP)
+			b->eport += c->tcp.fwd_in.delta[b->eport];
+
+		return true;
+	}
+
+	b->pif = PIF_TAP;
+	b->faddr = a->eaddr;
+	b->fport = a->eport;
+
+	bf4 = inany_v4(&b->faddr);
+
+	if (bf4) {
+		if (IN4_IS_ADDR_LOOPBACK(bf4) ||
+		    IN4_IS_ADDR_UNSPECIFIED(bf4) ||
+		    IN4_ARE_ADDR_EQUAL(bf4, &c->ip4.addr_seen))
+			*bf4 = c->ip4.gw;
+	} else {
+		struct in6_addr *bf6 = &b->faddr.a6;
+
+		if (IN6_IS_ADDR_LOOPBACK(bf6) ||
+		    IN6_ARE_ADDR_EQUAL(bf6, &c->ip6.addr_seen) ||
+		    IN6_ARE_ADDR_EQUAL(bf6, &c->ip6.addr)) {
+			if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
+				*bf6 = c->ip6.gw;
+			else
+				*bf6 = c->ip6.addr_ll;
+		}
+	}
+
+	if (bf4) {
+		inany_from_af(&b->eaddr, AF_INET, &c->ip4.addr_seen);
+	} else {
+		if (IN6_IS_ADDR_LINKLOCAL(&b->faddr.a6))
+			b->eaddr.a6 = c->ip6.addr_ll_seen;
+		else
+			b->eaddr.a6 = c->ip6.addr_seen;
+	}
+
+	b->eport = a->fport;
+	if (proto == IPPROTO_TCP)
+		b->eport += c->tcp.fwd_in.delta[b->eport];
+
+	return true;
+}
+
+bool fwd_nat_flow(const struct ctx *c, uint8_t proto,
+		  const struct flowside *a, struct flowside *b)
+{
+	char estr[INANY_ADDRSTRLEN], fstr[INANY_ADDRSTRLEN];
+
+	switch (a->pif) {
+	case PIF_TAP:
+		return fwd_from_tap(c, proto, a, b);
+
+	case PIF_SPLICE:
+		return fwd_from_splice(c, proto, a, b);
+
+	case PIF_HOST:
+		return fwd_from_host(c, proto, a, b);
+
+	default:
+		debug("No rules to forward from %s: [%s]:%hu -> [%s]:%hu",
+		      pif_name(a->pif),
+		      inany_ntop(&a->eaddr, estr, sizeof(estr)), a->eport,
+		      inany_ntop(&a->faddr, fstr, sizeof(fstr)), a->fport);
+		return false;
+	}
+}
diff --git a/fwd.h b/fwd.h
index 23281d9..e884f4e 100644
--- a/fwd.h
+++ b/fwd.h
@@ -7,6 +7,8 @@
 #ifndef FWD_H
 #define FWD_H
 
+struct flowside;
+
 /* Number of ports for both TCP and UDP */
 #define	NUM_PORTS	(1U << 16)
 
@@ -41,4 +43,7 @@ void fwd_scan_ports_udp(struct fwd_ports *fwd, const struct fwd_ports
*rev,
 			const struct fwd_ports *tcp_rev);
 void fwd_scan_ports_init(struct ctx *c);
 
+bool fwd_nat_flow(const struct ctx *c, uint8_t proto,
+		  const struct flowside *a, struct flowside *b);
+
 #endif /* FWD_H */
diff --git a/tcp.c b/tcp.c
index 8b4d792..e131280 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1782,17 +1782,13 @@ static void tcp_conn_from_tap(struct ctx *c, sa_family_t af,
 		goto cancel;
 	}
 
-	sockside->pif = PIF_HOST;
-	sockside->eaddr = tapside->faddr;
-	sockside->eport = tapside->fport;
-
-	if (!c->no_map_gw) {
-		struct in_addr *v4 = inany_v4(&sockside->eaddr);
+	if (!fwd_nat_flow(c, IPPROTO_TCP, tapside, sockside))
+		goto cancel;
 
-		if (v4 && IN4_ARE_ADDR_EQUAL(v4, &c->ip4.gw))
-			*v4 = in4addr_loopback;
-		if (IN6_ARE_ADDR_EQUAL(&sockside->eaddr, &c->ip6.gw))
-			sockside->eaddr.a6 = in6addr_loopback;
+	if (sockside->pif != PIF_HOST) {
+		err("No support for forwarding TCP from %s to %s",
+		    pif_name(tapside->pif), pif_name(sockside->pif));
+		goto cancel;
 	}
 
 	if ((s = tcp_conn_sock(c, af)) < 0)
@@ -2479,67 +2475,19 @@ static void tcp_connect_finish(struct ctx *c, struct tcp_tap_conn
*conn)
 	conn_flag(c, conn, ACK_FROM_TAP_DUE);
 }
 
-/**
- * tcp_snat_inbound() - Translate source address for inbound data if needed
- * @c:		Execution context
- * @addr:	Source address of inbound packet/connection
- */
-static void tcp_snat_inbound(const struct ctx *c, union inany_addr *addr)
-{
-	struct in_addr *addr4 = inany_v4(addr);
-
-	if (addr4) {
-		if (IN4_IS_ADDR_LOOPBACK(addr4) ||
-		    IN4_IS_ADDR_UNSPECIFIED(addr4) ||
-		    IN4_ARE_ADDR_EQUAL(addr4, &c->ip4.addr_seen))
-			*addr4 = c->ip4.gw;
-	} else {
-		struct in6_addr *addr6 = &addr->a6;
-
-		if (IN6_IS_ADDR_LOOPBACK(addr6) ||
-		    IN6_ARE_ADDR_EQUAL(addr6, &c->ip6.addr_seen) ||
-		    IN6_ARE_ADDR_EQUAL(addr6, &c->ip6.addr)) {
-			if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
-				*addr6 = c->ip6.gw;
-			else
-				*addr6 = c->ip6.addr_ll;
-		}
-	}
-}
-
 /**
  * tcp_tap_conn_from_sock() - Initialize state for non-spliced connection
  * @c:		Execution context
- * @dstport:	Destination port for connection (host side)
  * @flow:	flow to initialise
  * @s:		Accepted socket
  * @now:	Current timestamp
  */
-static void tcp_tap_conn_from_sock(struct ctx *c, in_port_t dstport,
-				   union flow *flow, int s,
+static void tcp_tap_conn_from_sock(struct ctx *c, union flow *flow, int s,
 				   const struct timespec *now)
 {
-	const struct flowside *sockside = &flow->f.side[SOCKSIDE];
-	struct flowside *tapside = &flow->f.side[TAPSIDE];
-	struct tcp_tap_conn *conn;
+	struct tcp_tap_conn *conn = FLOW_START(flow, FLOW_TCP, tcp, SOCKSIDE);
 	uint64_t hash;
 
-	tapside->pif = PIF_TAP;
-	tapside->faddr = sockside->eaddr;
-	tapside->fport = sockside->eport;
-	tcp_snat_inbound(c, &tapside->faddr);
-	if (CONN_V4(flow)) {
-		inany_from_af(&tapside->eaddr, AF_INET, &c->ip4.addr_seen);
-	} else {
-		if (IN6_IS_ADDR_LINKLOCAL(&tapside->faddr.a6))
-			tapside->eaddr.a6 = c->ip6.addr_ll_seen;
-		else
-			tapside->eaddr.a6 = c->ip6.addr_seen;
-	}
-	tapside->eport = dstport + c->tcp.fwd_in.delta[dstport];
-
-	conn = FLOW_START(flow, FLOW_TCP, tcp, SOCKSIDE);
-
 	conn->sock = s;
 	conn->timer = -1;
 	conn->ws_to_tap = conn->ws_from_tap = 0;
@@ -2567,9 +2515,9 @@ static void tcp_tap_conn_from_sock(struct ctx *c, in_port_t
dstport,
 void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 			const struct timespec *now)
 {
+	struct flowside *side0, *side1;
 	union sockaddr_inany sa;
 	socklen_t sl = sizeof(sa);
-	struct flowside *side0;
 	union flow *flow;
 	int s;
 
@@ -2577,6 +2525,7 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 		return;
 
 	side0 = &flow->f.side[0];
+	side1 = &flow->f.side[1];
 
 	s = accept4(ref.fd, &sa.sa, &sl, SOCK_NONBLOCK);
 	if (s < 0)
@@ -2594,10 +2543,25 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 		goto cancel;
 	}
 
-	if (tcp_splice_conn_from_sock(c, ref.tcp_listen.port, flow, s))
-		return;
+	if (!fwd_nat_flow(c, IPPROTO_TCP, side0, side1))
+		goto cancel;
+
+	switch (side1->pif) {
+	case PIF_SPLICE:
+	case PIF_HOST:
+		tcp_splice_conn_from_sock(c, flow, s);
+		break;
+
+	case PIF_TAP:
+		tcp_tap_conn_from_sock(c, flow, s, now);
+		break;
+
+	default:
+		err("No support for forwarding TCP from %s to %s",
+		    pif_name(side0->pif), pif_name(side1->pif));
+		goto cancel;
+	}
 
-	tcp_tap_conn_from_sock(c, ref.tcp_listen.port, flow, s, now);
 	return;
 
 cancel:
diff --git a/tcp_splice.c b/tcp_splice.c
index 00b88c5..c4fbbdd 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -394,67 +394,16 @@ static int tcp_conn_sock_ns(const struct ctx *c, sa_family_t af)
 /**
  * tcp_splice_conn_from_sock() - Attempt to init state for a spliced connection
  * @c:		Execution context
- * @dstport:	Side 0 destination port of connection
  * @flow:	flow to initialise
  * @s0:		Accepted (side 0) socket
  *
- * Return: true if able to create a spliced connection, false otherwise
  * #syscalls:pasta setsockopt
  */
-bool tcp_splice_conn_from_sock(const struct ctx *c, in_port_t dstport,
-			       union flow *flow, int s0)
+void tcp_splice_conn_from_sock(const struct ctx *c, union flow *flow, int s0)
 {
-	const struct flowside *side0 = &flow->f.side[0];
-	const union inany_addr *src = &side0->eaddr;
-	struct flowside *side1 = &flow->f.side[1];
 	struct tcp_splice_conn *conn;
-	sa_family_t af;
-	uint8_t pif1;
-
-	if (c->mode != MODE_PASTA)
-		return false;
-
-	af = inany_v4(src) ? AF_INET : AF_INET6;
-
-	switch (side0->pif) {
-	case PIF_SPLICE:
-		if (!inany_is_loopback(src)) {
-			char str[INANY_ADDRSTRLEN];
-
-			/* We can't use flow_err() etc. because we haven't set
-			 * the flow type yet
-			 */
-			warn("Bad source address %s for splice, closing",
-			     inany_ntop(src, str, sizeof(str)));
-
-			/* We *don't* want to fall back to tap */
-			flow_alloc_cancel(flow);
-			return true;
-		}
-
-		pif1 = PIF_HOST;
-		dstport += c->tcp.fwd_out.delta[dstport];
-		break;
-
-	case PIF_HOST:
-		if (!inany_is_loopback(src))
-			return false;
-
-		pif1 = PIF_SPLICE;
-		dstport += c->tcp.fwd_in.delta[dstport];
-		break;
-
-	default:
-		return false;
-	}
-
-	if (af == AF_INET)
-		flowside_from_af(side1, pif1, AF_INET, NULL, 0,
-				 &in4addr_loopback, dstport);
-	else
-		flowside_from_af(side1, pif1, AF_INET6, NULL, 0,
-				 &in6addr_loopback, dstport);
 
+	ASSERT(c->mode == MODE_PASTA);
 	conn = FLOW_START(flow, FLOW_TCP_SPLICE, tcp_splice, 0);
 
 	conn->s[0] = s0;
@@ -467,8 +416,6 @@ bool tcp_splice_conn_from_sock(const struct ctx *c, in_port_t
dstport,
 
 	if (tcp_splice_connect(c, conn))
 		conn_flag(c, conn, CLOSING);
-
-	return true;
 }
 
 /**
diff --git a/tcp_splice.h b/tcp_splice.h
index e523c7e..a20f3e2 100644
--- a/tcp_splice.h
+++ b/tcp_splice.h
@@ -11,8 +11,7 @@ union sockaddr_inany;
 
 void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref,
 			     uint32_t events);
-bool tcp_splice_conn_from_sock(const struct ctx *c, in_port_t dstport,
-			       union flow *flow, int s0);
+void tcp_splice_conn_from_sock(const struct ctx *c, union flow *flow, int s0);
 void tcp_splice_init(struct ctx *c);
 
 #endif /* TCP_SPLICE_H */
-- 
2.44.0