public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v4 6/8] udp: Unify udp_sock_handler_splice() with udp_sock_handler()
Date: Thu,  5 Jan 2023 15:26:23 +1100	[thread overview]
Message-ID: <20230105042625.1981812-7-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20230105042625.1981812-1-david@gibson.dropbear.id.au>

These two functions now have a very similar structure, and their first
part (calling recvmmsg()) is functionally identical.  So, merge the two
functions into one.

This does have the side effect of meaning we no longer receive multiple
packets at once for splice (we already didn't for tap).  This does hurt
throughput for small spliced packets, but improves it for large spliced
packets and tap packets.

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

diff --git a/udp.c b/udp.c
index f6c07a5..f160ecc 100644
--- a/udp.c
+++ b/udp.c
@@ -590,52 +590,6 @@ static void udp_splice_sendfrom(const struct ctx *c, unsigned start, unsigned n,
 	sendmmsg(s, mmh_send + start, n, MSG_NOSIGNAL);
 }
 
-/**
- * udp_sock_handler_splice() - Handler for socket mapped to "spliced" connection
- * @c:		Execution context
- * @ref:	epoll reference
- * @events:	epoll events bitmap
- * @now:	Current timestamp
- */
-static void udp_sock_handler_splice(const struct ctx *c, union epoll_ref ref,
-				    uint32_t events, const struct timespec *now)
-{
-	in_port_t dst = ref.r.p.udp.udp.port;
-	int v6 = ref.r.p.udp.udp.v6, n, i, m;
-	struct mmsghdr *mmh_recv;
-
-	if (!(events & EPOLLIN))
-		return;
-
-	if (v6)
-		mmh_recv = udp6_l2_mh_sock;
-	else
-		mmh_recv = udp4_l2_mh_sock;
-
-	n = recvmmsg(ref.r.s, mmh_recv, UDP_MAX_FRAMES, 0, NULL);
-
-	if (n <= 0)
-		return;
-
-	if (v6)
-		udp6_localname.sin6_port = htons(dst);
-	else
-		udp4_localname.sin_port = htons(dst);
-
-	for (i = 0; i < n; i += m) {
-		in_port_t src = sa_port(v6, mmh_recv[i].msg_hdr.msg_name);
-
-		for (m = 1; i + m < n; m++) {
-			void *mname = mmh_recv[i + m].msg_hdr.msg_name;
-			if (sa_port(v6, mname) != src)
-				break;
-		}
-
-		udp_splice_sendfrom(c, i, m, src, dst, v6, ref.r.p.udp.udp.ns,
-				    ref.r.p.udp.udp.orig, now);
-	}
-}
-
 /**
  * udp_update_hdr4() - Update headers for one IPv4 datagram
  * @c:		Execution context
@@ -944,32 +898,52 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events,
 		      const struct timespec *now)
 {
 	/* For not entirely clear reasons (data locality?) pasta gets
-	 * better throughput if we receive the datagrams one at a
-	 * time.
+	 * better throughput if we receive tap datagrams one at a
+	 * atime.  For small splice datagrams throughput is slightly
+	 * better if we do batch, but it's slightly worse for large
+	 * splice datagrams.  Since we don't know before we receive
+	 * whether we'll use tap or splice, always go one at a time
+	 * for pasta mode.
 	 */
 	ssize_t n = (c->mode == MODE_PASST ? UDP_MAX_FRAMES : 1);
 	in_port_t dstport = ref.r.p.udp.udp.port;
 	bool v6 = ref.r.p.udp.udp.v6;
-	struct mmsghdr *sock_mmh;
+	struct mmsghdr *mmh_recv;
+	unsigned int i, m;
 
-	if (events == EPOLLERR)
+	if (!(events & EPOLLIN))
 		return;
 
-	if (ref.r.p.udp.udp.splice) {
-		udp_sock_handler_splice(c, ref, events, now);
-		return;
+	if (v6) {
+		mmh_recv = udp6_l2_mh_sock;
+		udp6_localname.sin6_port = htons(dstport);
+	} else {
+		mmh_recv = udp4_l2_mh_sock;
+		udp4_localname.sin_port = htons(dstport);
 	}
 
-	if (ref.r.p.udp.udp.v6)
-		sock_mmh = udp6_l2_mh_sock;
-	else
-		sock_mmh = udp4_l2_mh_sock;
-
-	n = recvmmsg(ref.r.s, sock_mmh, n, 0, NULL);
+	n = recvmmsg(ref.r.s, mmh_recv, n, 0, NULL);
 	if (n <= 0)
 		return;
 
-	udp_tap_send(c, 0, n, dstport, v6, now);
+	if (!ref.r.p.udp.udp.splice) {
+		udp_tap_send(c, 0, n, dstport, v6, now);
+		return;
+	}
+
+	for (i = 0; i < n; i += m) {
+		in_port_t src = sa_port(v6, mmh_recv[i].msg_hdr.msg_name);
+
+		for (m = 1; i + m < n; m++) {
+			void *mname = mmh_recv[i + m].msg_hdr.msg_name;
+			if (sa_port(v6, mname) != src)
+				break;
+		}
+
+		udp_splice_sendfrom(c, i, m, src, dstport, v6,
+				    ref.r.p.udp.udp.ns, ref.r.p.udp.udp.orig,
+				    now);
+	}
 }
 
 /**
-- 
@@ -590,52 +590,6 @@ static void udp_splice_sendfrom(const struct ctx *c, unsigned start, unsigned n,
 	sendmmsg(s, mmh_send + start, n, MSG_NOSIGNAL);
 }
 
-/**
- * udp_sock_handler_splice() - Handler for socket mapped to "spliced" connection
- * @c:		Execution context
- * @ref:	epoll reference
- * @events:	epoll events bitmap
- * @now:	Current timestamp
- */
-static void udp_sock_handler_splice(const struct ctx *c, union epoll_ref ref,
-				    uint32_t events, const struct timespec *now)
-{
-	in_port_t dst = ref.r.p.udp.udp.port;
-	int v6 = ref.r.p.udp.udp.v6, n, i, m;
-	struct mmsghdr *mmh_recv;
-
-	if (!(events & EPOLLIN))
-		return;
-
-	if (v6)
-		mmh_recv = udp6_l2_mh_sock;
-	else
-		mmh_recv = udp4_l2_mh_sock;
-
-	n = recvmmsg(ref.r.s, mmh_recv, UDP_MAX_FRAMES, 0, NULL);
-
-	if (n <= 0)
-		return;
-
-	if (v6)
-		udp6_localname.sin6_port = htons(dst);
-	else
-		udp4_localname.sin_port = htons(dst);
-
-	for (i = 0; i < n; i += m) {
-		in_port_t src = sa_port(v6, mmh_recv[i].msg_hdr.msg_name);
-
-		for (m = 1; i + m < n; m++) {
-			void *mname = mmh_recv[i + m].msg_hdr.msg_name;
-			if (sa_port(v6, mname) != src)
-				break;
-		}
-
-		udp_splice_sendfrom(c, i, m, src, dst, v6, ref.r.p.udp.udp.ns,
-				    ref.r.p.udp.udp.orig, now);
-	}
-}
-
 /**
  * udp_update_hdr4() - Update headers for one IPv4 datagram
  * @c:		Execution context
@@ -944,32 +898,52 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events,
 		      const struct timespec *now)
 {
 	/* For not entirely clear reasons (data locality?) pasta gets
-	 * better throughput if we receive the datagrams one at a
-	 * time.
+	 * better throughput if we receive tap datagrams one at a
+	 * atime.  For small splice datagrams throughput is slightly
+	 * better if we do batch, but it's slightly worse for large
+	 * splice datagrams.  Since we don't know before we receive
+	 * whether we'll use tap or splice, always go one at a time
+	 * for pasta mode.
 	 */
 	ssize_t n = (c->mode == MODE_PASST ? UDP_MAX_FRAMES : 1);
 	in_port_t dstport = ref.r.p.udp.udp.port;
 	bool v6 = ref.r.p.udp.udp.v6;
-	struct mmsghdr *sock_mmh;
+	struct mmsghdr *mmh_recv;
+	unsigned int i, m;
 
-	if (events == EPOLLERR)
+	if (!(events & EPOLLIN))
 		return;
 
-	if (ref.r.p.udp.udp.splice) {
-		udp_sock_handler_splice(c, ref, events, now);
-		return;
+	if (v6) {
+		mmh_recv = udp6_l2_mh_sock;
+		udp6_localname.sin6_port = htons(dstport);
+	} else {
+		mmh_recv = udp4_l2_mh_sock;
+		udp4_localname.sin_port = htons(dstport);
 	}
 
-	if (ref.r.p.udp.udp.v6)
-		sock_mmh = udp6_l2_mh_sock;
-	else
-		sock_mmh = udp4_l2_mh_sock;
-
-	n = recvmmsg(ref.r.s, sock_mmh, n, 0, NULL);
+	n = recvmmsg(ref.r.s, mmh_recv, n, 0, NULL);
 	if (n <= 0)
 		return;
 
-	udp_tap_send(c, 0, n, dstport, v6, now);
+	if (!ref.r.p.udp.udp.splice) {
+		udp_tap_send(c, 0, n, dstport, v6, now);
+		return;
+	}
+
+	for (i = 0; i < n; i += m) {
+		in_port_t src = sa_port(v6, mmh_recv[i].msg_hdr.msg_name);
+
+		for (m = 1; i + m < n; m++) {
+			void *mname = mmh_recv[i + m].msg_hdr.msg_name;
+			if (sa_port(v6, mname) != src)
+				break;
+		}
+
+		udp_splice_sendfrom(c, i, m, src, dstport, v6,
+				    ref.r.p.udp.udp.ns, ref.r.p.udp.udp.orig,
+				    now);
+	}
 }
 
 /**
-- 
2.39.0


  parent reply	other threads:[~2023-01-05  4:26 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-05  4:26 [PATCH v4 0/8] Don't use additional sockets for receiving "spliced" UDP communications David Gibson
2023-01-05  4:26 ` [PATCH v4 1/8] udp: Move sending pasta tap frames to the end of udp_sock_handler() David Gibson
2023-01-05  4:26 ` [PATCH v4 2/8] udp: Split sending to passt tap interface into separate function David Gibson
2023-01-05  4:26 ` [PATCH v4 3/8] udp: Split receive from preparation and send in udp_sock_handler() David Gibson
2023-01-05  4:26 ` [PATCH v4 4/8] udp: Don't handle tap receive batch size calculation within a #define David Gibson
2023-01-05  4:26 ` [PATCH v4 5/8] udp: Pre-populate msg_names with local address David Gibson
2023-01-05  4:26 ` David Gibson [this message]
2023-01-05  4:26 ` [PATCH v4 7/8] udp: Decide whether to "splice" per datagram rather than per socket David Gibson
2023-01-05  4:26 ` [PATCH v4 8/8] udp: Don't use separate sockets to listen for spliced packets David Gibson
2023-01-05 21:50 ` [PATCH v4 0/8] Don't use additional sockets for receiving "spliced" UDP communications Stefano Brivio
2023-01-06  0:59   ` David Gibson
2023-01-13  0:07   ` 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=20230105042625.1981812-7-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).