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 3/3] icmp: Use 'flowside' epoll references for ping sockets
Date: Thu, 29 Feb 2024 15:15:34 +1100	[thread overview]
Message-ID: <20240229041534.2573559-4-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240229041534.2573559-1-david@gibson.dropbear.id.au>

Currently ping sockets use a custom epoll reference type which includes
the ICMP id.  However, now that we have entries in the flow table for
ping flows, finding that is sufficient to get everything else we want,
including the id.  Therefore remove the icmp_epoll_ref type and use the
general 'flowside' field for ping sockets.

Having done this we no longer need separate EPOLL_TYPE_ICMP and
EPOLL_TYPE_ICMPV6 reference types, because we can easily determine
which case we have from the flow type. Merge both types into
EPOLL_TYPE_PING.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 icmp.c  | 34 +++++++++++++++++-----------------
 icmp.h  | 13 +------------
 passt.c | 10 +++-------
 passt.h |  7 ++-----
 util.c  |  4 +---
 5 files changed, 24 insertions(+), 44 deletions(-)

diff --git a/icmp.c b/icmp.c
index 63adafcf..ec28e419 100644
--- a/icmp.c
+++ b/icmp.c
@@ -48,19 +48,19 @@
 #define	SOCKSIDE	0
 #define	TAPSIDE		1
 
+#define PINGF(idx)		(&(FLOW(idx)->ping))
+
 /* Indexed by ICMP echo identifier */
 static struct icmp_ping_flow *icmp_id_map[IP_VERSIONS][ICMP_NUM_IDS];
 
 /**
  * icmp_sock_handler() - Handle new data from ICMP or ICMPv6 socket
  * @c:		Execution context
- * @af:		Address family (AF_INET or AF_INET6)
  * @ref:	epoll reference
  */
-void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
+void icmp_sock_handler(const struct ctx *c, union epoll_ref ref)
 {
-	struct icmp_ping_flow *pingf = af == AF_INET
-		? icmp_id_map[V4][ref.icmp.id] : icmp_id_map[V6][ref.icmp.id];
+	struct icmp_ping_flow *pingf = PINGF(ref.flowside.flow);
 	union sockaddr_inany sr;
 	socklen_t sl = sizeof(sr);
 	char buf[USHRT_MAX];
@@ -77,27 +77,26 @@ void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
 		flow_err(pingf, "recvfrom() error: %s", strerror(errno));
 		return;
 	}
-	if (sr.sa_family != af)
-		goto unexpected;
 
-	if (af == AF_INET) {
+	if (pingf->f.type == FLOW_PING4) {
 		struct icmphdr *ih4 = (struct icmphdr *)buf;
 
-		if ((size_t)n < sizeof(*ih4) || ih4->type != ICMP_ECHOREPLY)
+		if (sr.sa_family != AF_INET || (size_t)n < sizeof(*ih4) ||
+		    ih4->type != ICMP_ECHOREPLY)
 			goto unexpected;
 
 		/* Adjust packet back to guest-side ID */
-		ih4->un.echo.id = htons(ref.icmp.id);
+		ih4->un.echo.id = htons(pingf->id);
 		seq = ntohs(ih4->un.echo.sequence);
-	} else if (af == AF_INET6) {
+	} else if (pingf->f.type == FLOW_PING6) {
 		struct icmp6hdr *ih6 = (struct icmp6hdr *)buf;
 
-		if ((size_t)n < sizeof(*ih6) ||
+		if (sr.sa_family != AF_INET6 || (size_t)n < sizeof(*ih6) ||
 		    ih6->icmp6_type != ICMPV6_ECHO_REPLY)
 			goto unexpected;
 
 		/* Adjust packet back to guest-side ID */
-		ih6->icmp6_identifier = htons(ref.icmp.id);
+		ih6->icmp6_identifier = htons(pingf->id);
 		seq = ntohs(ih6->icmp6_sequence);
 	} else {
 		ASSERT(0);
@@ -112,11 +111,11 @@ void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
 	}
 
 	flow_dbg(pingf, "echo reply to tap, ID: %"PRIu16", seq: %"PRIu16,
-		 ref.icmp.id, seq);
+		 pingf->id, seq);
 
-	if (af == AF_INET)
+	if (pingf->f.type == FLOW_PING4)
 		tap_icmp4_send(c, sr.sa4.sin_addr, tap_ip4_daddr(c), buf, n);
-	else if (af == AF_INET6)
+	else if (pingf->f.type == FLOW_PING6)
 		tap_icmp6_send(c, &sr.sa6.sin6_addr,
 			       tap_ip6_daddr(c, &sr.sa6.sin6_addr), buf, n);
 	return;
@@ -158,7 +157,7 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 					    sa_family_t af, uint16_t id)
 {
 	uint8_t flowtype = af == AF_INET ? FLOW_PING4 : FLOW_PING6;
-	union icmp_epoll_ref iref = { .id = id };
+	union epoll_ref ref = { .type = EPOLL_TYPE_PING };
 	union flow *flow = flow_alloc();
 	struct icmp_ping_flow *pingf;
 	const void *bind_addr;
@@ -180,8 +179,9 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 		bind_if = c->ip6.ifname_out;
 	}
 
+	ref.flowside = FLOW_SIDX(flow, SOCKSIDE);
 	pingf->sock = sock_l4(c, af, flow_proto[flowtype], bind_addr, bind_if,
-			      0, iref.u32);
+			      0, ref.data);
 
 	if (pingf->sock < 0) {
 		warn("Cannot open \"ping\" socket. You might need to:");
diff --git a/icmp.h b/icmp.h
index 4f695da1..5ce22b5e 100644
--- a/icmp.h
+++ b/icmp.h
@@ -11,23 +11,12 @@
 struct ctx;
 struct icmp_ping_flow;
 
-void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref);
+void icmp_sock_handler(const struct ctx *c, union epoll_ref ref);
 int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 		     const void *saddr, const void *daddr,
 		     const struct pool *p, const struct timespec *now);
 void icmp_init(void);
 
-/**
- * union icmp_epoll_ref - epoll reference portion for ICMP tracking
- * @v6:			Set for IPv6 sockets or connections
- * @u32:		Opaque u32 value of reference
- * @id:			Associated echo identifier, needed if bind() fails
- */
-union icmp_epoll_ref {
-	uint16_t id;
-	uint32_t u32;
-};
-
 /**
  * struct icmp_ctx - Execution context for ICMP routines
  * @timer_run:		Timestamp of most recent timer run
diff --git a/passt.c b/passt.c
index 0f794927..f4306483 100644
--- a/passt.c
+++ b/passt.c
@@ -66,8 +66,7 @@ char *epoll_type_str[] = {
 	[EPOLL_TYPE_TCP_LISTEN]		= "listening TCP socket",
 	[EPOLL_TYPE_TCP_TIMER]		= "TCP timer",
 	[EPOLL_TYPE_UDP]		= "UDP socket",
-	[EPOLL_TYPE_ICMP]		= "ICMP socket",
-	[EPOLL_TYPE_ICMPV6]		= "ICMPv6 socket",
+	[EPOLL_TYPE_PING]	= "ICMP/ICMPv6 ping socket",
 	[EPOLL_TYPE_NSQUIT_INOTIFY]	= "namespace inotify watch",
 	[EPOLL_TYPE_NSQUIT_TIMER]	= "namespace timer watch",
 	[EPOLL_TYPE_TAP_PASTA]		= "/dev/net/tun device",
@@ -377,11 +376,8 @@ loop:
 		case EPOLL_TYPE_UDP:
 			udp_sock_handler(&c, ref, eventmask, &now);
 			break;
-		case EPOLL_TYPE_ICMP:
-			icmp_sock_handler(&c, AF_INET, ref);
-			break;
-		case EPOLL_TYPE_ICMPV6:
-			icmp_sock_handler(&c, AF_INET6, ref);
+		case EPOLL_TYPE_PING:
+			icmp_sock_handler(&c, ref);
 			break;
 		default:
 			/* Can't happen */
diff --git a/passt.h b/passt.h
index e6d43585..76026f09 100644
--- a/passt.h
+++ b/passt.h
@@ -59,10 +59,8 @@ enum epoll_type {
 	EPOLL_TYPE_TCP_TIMER,
 	/* UDP sockets */
 	EPOLL_TYPE_UDP,
-	/* IPv4 ICMP sockets */
-	EPOLL_TYPE_ICMP,
-	/* ICMPv6 sockets */
-	EPOLL_TYPE_ICMPV6,
+	/* ICMP/ICMPv6 ping sockets */
+	EPOLL_TYPE_PING,
 	/* inotify fd watching for end of netns (pasta) */
 	EPOLL_TYPE_NSQUIT_INOTIFY,
 	/* timer fd watching for end of netns, fallback for inotify (pasta) */
@@ -100,7 +98,6 @@ union epoll_ref {
 			flow_sidx_t flowside;
 			union tcp_listen_epoll_ref tcp_listen;
 			union udp_epoll_ref udp;
-			union icmp_epoll_ref icmp;
 			uint32_t data;
 			int nsdir_fd;
 		};
diff --git a/util.c b/util.c
index 03e393f6..5f18bd61 100644
--- a/util.c
+++ b/util.c
@@ -127,10 +127,8 @@ int sock_l4(const struct ctx *c, sa_family_t af, uint8_t proto,
 		ref.type = EPOLL_TYPE_UDP;
 		break;
 	case IPPROTO_ICMP:
-		ref.type = EPOLL_TYPE_ICMP;
-		break;
 	case IPPROTO_ICMPV6:
-		ref.type = EPOLL_TYPE_ICMPV6;
+		ref.type = EPOLL_TYPE_PING;
 		break;
 	default:
 		return -EPFNOSUPPORT;	/* Not implemented. */
-- 
@@ -127,10 +127,8 @@ int sock_l4(const struct ctx *c, sa_family_t af, uint8_t proto,
 		ref.type = EPOLL_TYPE_UDP;
 		break;
 	case IPPROTO_ICMP:
-		ref.type = EPOLL_TYPE_ICMP;
-		break;
 	case IPPROTO_ICMPV6:
-		ref.type = EPOLL_TYPE_ICMPV6;
+		ref.type = EPOLL_TYPE_PING;
 		break;
 	default:
 		return -EPFNOSUPPORT;	/* Not implemented. */
-- 
2.44.0


  parent reply	other threads:[~2024-02-29  4:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-29  4:15 [PATCH 0/3] Basic integration of ICMP with flow table David Gibson
2024-02-29  4:15 ` [PATCH 1/3] icmp: Store ping socket information in " David Gibson
2024-03-07  8:53   ` Stefano Brivio
2024-03-07 10:24     ` David Gibson
2024-03-07 23:25       ` Stefano Brivio
2024-03-08  0:48         ` David Gibson
2024-02-29  4:15 ` [PATCH 2/3] icmp: Flow based error reporting David Gibson
2024-03-07 23:26   ` Stefano Brivio
2024-03-08  0:49     ` David Gibson
2024-03-08  6:06       ` Stefano Brivio
2024-02-29  4:15 ` David Gibson [this message]
2024-03-13 10:08 ` [PATCH 0/3] Basic integration of ICMP with flow table Stefano Brivio

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=20240229041534.2573559-4-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).