public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>, passt-dev@passt.top
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 4/5] udp: Clarify connection tracking flags
Date: Wed, 17 May 2023 15:05:28 +1000	[thread overview]
Message-ID: <20230517050529.3505590-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20230517050529.3505590-1-david@gibson.dropbear.id.au>

The PORT_LOCAL, PORT_LOOPBACK and PORT_GUA flags in udp_tap_port are a
rudimentary form of "connection" tracking for UDP.  We need this because
there are several remote source addresses that we have to translate to the
gateway address when we present packets to the guest.  We need these flags
to work out which address to translate that gateway address back to for
reply packets.

The flags are confusing, though, it's not really clear what each one means
individually, and LOCAL and LOOPBACK are only ever tested in combination.
Really, it all boils down to just 3 options:
  - Packets from addr_seen got tanslated
  - Packets from (host side) loopback got translated
  - Packets from addr got translated
Replace the flags with an enum explicitly giving those options.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c | 67 +++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/udp.c b/udp.c
index 950d5a9..3c78fca 100644
--- a/udp.c
+++ b/udp.c
@@ -120,6 +120,18 @@
 #define UDP_CONN_TIMEOUT	180 /* s, timeout for ephemeral or local bind */
 #define UDP_MAX_FRAMES		32  /* max # of frames to receive at once */
 
+/**
+ * enum udp_port_remote - Original remote address of UDP "connection" to a port
+ * @PORT_LOOPBACK  - Original remote address was (host side) loopback
+ * @PORT_ADDR_SEEN - Original remote address was the same as the guest is using
+ * @PORT_ADDR      - Original remote address was guest assigned address
+ */
+enum udp_port_remote {
+	PORT_LOOPBACK = 0,
+	PORT_ADDR_SEEN = 1,
+	PORT_ADDR = 2,
+};
+
 /**
  * struct udp_tap_port - Port tracking based on tap-facing source port
  * @sock:	Socket bound to source port used as index
@@ -128,10 +140,7 @@
  */
 struct udp_tap_port {
 	int sock;
-	uint8_t flags;
-#define PORT_LOCAL	BIT(0)
-#define PORT_LOOPBACK	BIT(1)
-#define PORT_GUA	BIT(2)
+	enum udp_port_remote remote;
 
 	time_t ts;
 };
@@ -600,17 +609,13 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
 		   IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr)) {
 		b->iph.saddr = c->ip4.gw.s_addr;
 		udp_tap_map[V4][src_port].ts = now->tv_sec;
-		udp_tap_map[V4][src_port].flags |= PORT_LOCAL;
 
 		if (IN4_IS_ADDR_LOOPBACK(src))
-			udp_tap_map[V4][src_port].flags |= PORT_LOOPBACK;
-		else
-			udp_tap_map[V4][src_port].flags &= ~PORT_LOOPBACK;
-
-		if (IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr))
-			udp_tap_map[V4][src_port].flags |= PORT_GUA;
+			udp_tap_map[V4][src_port].remote = PORT_LOOPBACK;
+		else if (IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr))
+			udp_tap_map[V4][src_port].remote = PORT_ADDR;
 		else
-			udp_tap_map[V4][src_port].flags &= ~PORT_GUA;
+			udp_tap_map[V4][src_port].remote = PORT_ADDR_SEEN;
 
 		bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port);
 	} else {
@@ -668,17 +673,13 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport,
 			b->ip6h.saddr = c->ip6.addr_ll;
 
 		udp_tap_map[V6][src_port].ts = now->tv_sec;
-		udp_tap_map[V6][src_port].flags |= PORT_LOCAL;
 
 		if (IN6_IS_ADDR_LOOPBACK(src))
-			udp_tap_map[V6][src_port].flags |= PORT_LOOPBACK;
-		else
-			udp_tap_map[V6][src_port].flags &= ~PORT_LOOPBACK;
-
-		if (IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr))
-			udp_tap_map[V6][src_port].flags |= PORT_GUA;
+			udp_tap_map[V6][src_port].remote = PORT_LOOPBACK;
+		else if (IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr))
+			udp_tap_map[V6][src_port].remote = PORT_ADDR;
 		else
-			udp_tap_map[V6][src_port].flags &= ~PORT_GUA;
+			udp_tap_map[V6][src_port].remote = PORT_ADDR_SEEN;
 
 		bitmap_set(udp_act[V6][UDP_ACT_TAP], src_port);
 	} else {
@@ -855,13 +856,17 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 			s_in.sin_addr = c->ip4.dns_host;
 		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
 			   !c->no_map_gw) {
-			if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) ||
-			    (udp_tap_map[V4][dst].flags & PORT_LOOPBACK))
+			switch (udp_tap_map[V4][dst].remote) {
+			case PORT_LOOPBACK:
 				s_in.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
-			else if (udp_tap_map[V4][dst].flags & PORT_GUA)
+				break;
+			case PORT_ADDR:
 				s_in.sin_addr = c->ip4.addr;
-			else
+				break;
+			case PORT_ADDR_SEEN:
 				s_in.sin_addr = c->ip4.addr_seen;
+				break;
+			}
 		}
 
 		if (!(s = udp_tap_map[V4][src].sock)) {
@@ -903,13 +908,17 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 			s_in6.sin6_addr = c->ip6.dns_host;
 		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw) &&
 			   !c->no_map_gw) {
-			if (!(udp_tap_map[V6][dst].flags & PORT_LOCAL) ||
-			    (udp_tap_map[V6][dst].flags & PORT_LOOPBACK))
+			switch (udp_tap_map[V6][dst].remote) {
+			case PORT_LOOPBACK:
 				s_in6.sin6_addr = in6addr_loopback;
-			else if (udp_tap_map[V6][dst].flags & PORT_GUA)
+				break;
+			case PORT_ADDR:
 				s_in6.sin6_addr = c->ip6.addr;
-			else
+				break;
+			case PORT_ADDR_SEEN:
 				s_in6.sin6_addr = c->ip6.addr_seen;
+				break;
+			}
 		} else if (IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) {
 			bind_addr = &c->ip6.addr_ll;
 		}
@@ -1162,7 +1171,7 @@ static void udp_timer_one(struct ctx *c, int v6, enum udp_act_type type,
 
 		if (ts->tv_sec - tp->ts > UDP_CONN_TIMEOUT) {
 			s = tp->sock;
-			tp->flags = 0;
+			tp->remote = PORT_LOOPBACK;
 		}
 
 		break;
-- 
@@ -120,6 +120,18 @@
 #define UDP_CONN_TIMEOUT	180 /* s, timeout for ephemeral or local bind */
 #define UDP_MAX_FRAMES		32  /* max # of frames to receive at once */
 
+/**
+ * enum udp_port_remote - Original remote address of UDP "connection" to a port
+ * @PORT_LOOPBACK  - Original remote address was (host side) loopback
+ * @PORT_ADDR_SEEN - Original remote address was the same as the guest is using
+ * @PORT_ADDR      - Original remote address was guest assigned address
+ */
+enum udp_port_remote {
+	PORT_LOOPBACK = 0,
+	PORT_ADDR_SEEN = 1,
+	PORT_ADDR = 2,
+};
+
 /**
  * struct udp_tap_port - Port tracking based on tap-facing source port
  * @sock:	Socket bound to source port used as index
@@ -128,10 +140,7 @@
  */
 struct udp_tap_port {
 	int sock;
-	uint8_t flags;
-#define PORT_LOCAL	BIT(0)
-#define PORT_LOOPBACK	BIT(1)
-#define PORT_GUA	BIT(2)
+	enum udp_port_remote remote;
 
 	time_t ts;
 };
@@ -600,17 +609,13 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
 		   IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr)) {
 		b->iph.saddr = c->ip4.gw.s_addr;
 		udp_tap_map[V4][src_port].ts = now->tv_sec;
-		udp_tap_map[V4][src_port].flags |= PORT_LOCAL;
 
 		if (IN4_IS_ADDR_LOOPBACK(src))
-			udp_tap_map[V4][src_port].flags |= PORT_LOOPBACK;
-		else
-			udp_tap_map[V4][src_port].flags &= ~PORT_LOOPBACK;
-
-		if (IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr))
-			udp_tap_map[V4][src_port].flags |= PORT_GUA;
+			udp_tap_map[V4][src_port].remote = PORT_LOOPBACK;
+		else if (IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr))
+			udp_tap_map[V4][src_port].remote = PORT_ADDR;
 		else
-			udp_tap_map[V4][src_port].flags &= ~PORT_GUA;
+			udp_tap_map[V4][src_port].remote = PORT_ADDR_SEEN;
 
 		bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port);
 	} else {
@@ -668,17 +673,13 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport,
 			b->ip6h.saddr = c->ip6.addr_ll;
 
 		udp_tap_map[V6][src_port].ts = now->tv_sec;
-		udp_tap_map[V6][src_port].flags |= PORT_LOCAL;
 
 		if (IN6_IS_ADDR_LOOPBACK(src))
-			udp_tap_map[V6][src_port].flags |= PORT_LOOPBACK;
-		else
-			udp_tap_map[V6][src_port].flags &= ~PORT_LOOPBACK;
-
-		if (IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr))
-			udp_tap_map[V6][src_port].flags |= PORT_GUA;
+			udp_tap_map[V6][src_port].remote = PORT_LOOPBACK;
+		else if (IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr))
+			udp_tap_map[V6][src_port].remote = PORT_ADDR;
 		else
-			udp_tap_map[V6][src_port].flags &= ~PORT_GUA;
+			udp_tap_map[V6][src_port].remote = PORT_ADDR_SEEN;
 
 		bitmap_set(udp_act[V6][UDP_ACT_TAP], src_port);
 	} else {
@@ -855,13 +856,17 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 			s_in.sin_addr = c->ip4.dns_host;
 		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
 			   !c->no_map_gw) {
-			if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) ||
-			    (udp_tap_map[V4][dst].flags & PORT_LOOPBACK))
+			switch (udp_tap_map[V4][dst].remote) {
+			case PORT_LOOPBACK:
 				s_in.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
-			else if (udp_tap_map[V4][dst].flags & PORT_GUA)
+				break;
+			case PORT_ADDR:
 				s_in.sin_addr = c->ip4.addr;
-			else
+				break;
+			case PORT_ADDR_SEEN:
 				s_in.sin_addr = c->ip4.addr_seen;
+				break;
+			}
 		}
 
 		if (!(s = udp_tap_map[V4][src].sock)) {
@@ -903,13 +908,17 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 			s_in6.sin6_addr = c->ip6.dns_host;
 		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw) &&
 			   !c->no_map_gw) {
-			if (!(udp_tap_map[V6][dst].flags & PORT_LOCAL) ||
-			    (udp_tap_map[V6][dst].flags & PORT_LOOPBACK))
+			switch (udp_tap_map[V6][dst].remote) {
+			case PORT_LOOPBACK:
 				s_in6.sin6_addr = in6addr_loopback;
-			else if (udp_tap_map[V6][dst].flags & PORT_GUA)
+				break;
+			case PORT_ADDR:
 				s_in6.sin6_addr = c->ip6.addr;
-			else
+				break;
+			case PORT_ADDR_SEEN:
 				s_in6.sin6_addr = c->ip6.addr_seen;
+				break;
+			}
 		} else if (IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) {
 			bind_addr = &c->ip6.addr_ll;
 		}
@@ -1162,7 +1171,7 @@ static void udp_timer_one(struct ctx *c, int v6, enum udp_act_type type,
 
 		if (ts->tv_sec - tp->ts > UDP_CONN_TIMEOUT) {
 			s = tp->sock;
-			tp->flags = 0;
+			tp->remote = PORT_LOOPBACK;
 		}
 
 		break;
-- 
2.40.1


  parent reply	other threads:[~2023-05-17  5:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-17  5:05 [PATCH 0/5] Improvements to "connection" tracking for UDP David Gibson
2023-05-17  5:05 ` [PATCH 1/5] udp: Don't attempt to translate a 0.0.0.0 source address David Gibson
2023-05-17  5:05 ` [PATCH 2/5] udp: Small streamline to udp_update_hdr4() David Gibson
2023-05-17  5:05 ` [PATCH 3/5] udp: Implement IPv6 PORT_GUA logic for IPv4 as well David Gibson
2023-05-17  5:05 ` David Gibson [this message]
2023-05-17  5:05 ` [PATCH 5/5] udp: Remove PORT_ADDR_SEEN "connection" tracking mode David Gibson
2023-05-18  5:48 ` [PATCH 0/5] Improvements to "connection" tracking for UDP David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230517050529.3505590-5-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).