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/4] udp: Factor out control structure management from udp_sock_fill_data_v[46]
Date: Thu, 24 Nov 2022 19:54:21 +1100	[thread overview]
Message-ID: <20221124085421.3027886-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20221124085421.3027886-1-david@gibson.dropbear.id.au>

The main purpose of udp_sock_fill_data_v[46]() is to construct the IP, UDP
and other headers we'll need to forward data onto the tap interface.  In
addition they update the control structures (iovec and mmsghdr) we'll need
to send the messages, and in the case of pasta actually sends it.

This leads the control structure management and the send itself awkwardly
split between udp_sock_fill_data_v[46]() and their caller
udp_sock_handler().  In addition, this tail part of udp_sock_fill_datav[46]
is essentially common between the IPv4 and IPv6 versions, apart from which
control array we're working on.

Clean this up by reducing these functions to just construct the headers
and renaming them to udp_update_hdr[46]() accordingly.  The control
structure updates are now all in the caller, and common for IPv4 and IPv6.
---
 udp.c | 118 +++++++++++++++++++++++++---------------------------------
 1 file changed, 50 insertions(+), 68 deletions(-)

diff --git a/udp.c b/udp.c
index e2eb504..431d268 100644
--- a/udp.c
+++ b/udp.c
@@ -640,20 +640,17 @@ static void udp_sock_handler_splice(const struct ctx *c, union epoll_ref ref,
 }
 
 /**
- * udp_sock_fill_data_v4() - Fill and queue one buffer. In pasta mode, write it
+ * udp_update_hdr4() - Update headers for one IPv4 datagram
  * @c:		Execution context
  * @n:		Index of buffer in udp4_l2_buf pool
- * @ref:	epoll reference from socket
- * @msg_idx:	Index within message being prepared (spans multiple buffers)
- * @msg_len:	Length of current message being prepared for sending
+ * @dstport:	Destination port number
  * @now:	Current timestamp
+ *
+ * Return: size of tap frame with headers
  */
-static void udp_sock_fill_data_v4(const struct ctx *c, int n,
-				  union epoll_ref ref,
-				  int *msg_idx, int *msg_bufs, ssize_t *msg_len,
-				  const struct timespec *now)
+static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
+			      const struct timespec *now)
 {
-	struct msghdr *mh = &udp4_l2_mh_tap[*msg_idx].msg_hdr;
 	struct udp4_l2_buf_t *b = &udp4_l2_buf[n];
 	size_t ip_len, buf_len;
 	in_port_t src_port;
@@ -687,51 +684,31 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
 
 	udp_update_check4(b);
 	b->uh.source = b->s_in.sin_port;
-	b->uh.dest = htons(ref.r.p.udp.udp.port);
+	b->uh.dest = htons(dstport);
 	b->uh.len = htons(udp4_l2_mh_sock[n].msg_len + sizeof(b->uh));
 
-	if (c->mode == MODE_PASTA) {
-		void *frame = udp4_l2_iov_tap[n].iov_base;
-
-		if (write(c->fd_tap, frame, sizeof(b->eh) + ip_len) < 0)
-			debug("tap write: %s", strerror(errno));
-		pcap(frame, sizeof(b->eh) + ip_len);
-
-		return;
-	}
-
-	b->vnet_len = htonl(ip_len + sizeof(struct ethhdr));
-	buf_len = sizeof(uint32_t) + sizeof(struct ethhdr) + ip_len;
-	udp4_l2_iov_tap[n].iov_len = buf_len;
-
-	/* With bigger messages, qemu closes the connection. */
-	if (*msg_bufs && *msg_len + buf_len > SHRT_MAX) {
-		mh->msg_iovlen = *msg_bufs;
+	buf_len = ip_len + sizeof(b->eh);
 
-		(*msg_idx)++;
-		udp4_l2_mh_tap[*msg_idx].msg_hdr.msg_iov = &udp4_l2_iov_tap[n];
-		*msg_len = *msg_bufs = 0;
+	if (c->mode == MODE_PASST) {
+		b->vnet_len = htonl(buf_len);
+		buf_len += sizeof(b->vnet_len);
 	}
 
-	*msg_len += buf_len;
-	(*msg_bufs)++;
+	return buf_len;
 }
 
 /**
- * udp_sock_fill_data_v6() - Fill and queue one buffer. In pasta mode, write it
+ * udp_update_hdr6() - Update headers for one IPv6 datagram
  * @c:		Execution context
  * @n:		Index of buffer in udp6_l2_buf pool
- * @ref:	epoll reference from socket
- * @msg_idx:	Index within message being prepared (spans multiple buffers)
- * @msg_len:	Length of current message being prepared for sending
+ * @dstport:	Destination port number
  * @now:	Current timestamp
+ *
+ * Return: size of tap frame with headers
  */
-static void udp_sock_fill_data_v6(const struct ctx *c, int n,
-				  union epoll_ref ref,
-				  int *msg_idx, int *msg_bufs, ssize_t *msg_len,
-				  const struct timespec *now)
+static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport,
+			      const struct timespec *now)
 {
-	struct msghdr *mh = &udp6_l2_mh_tap[*msg_idx].msg_hdr;
 	struct udp6_l2_buf_t *b = &udp6_l2_buf[n];
 	size_t ip_len, buf_len;
 	struct in6_addr *src;
@@ -782,7 +759,7 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n,
 	}
 
 	b->uh.source = b->s_in6.sin6_port;
-	b->uh.dest = htons(ref.r.p.udp.udp.port);
+	b->uh.dest = htons(dstport);
 	b->uh.len = b->ip6h.payload_len;
 
 	b->ip6h.hop_limit = IPPROTO_UDP;
@@ -792,31 +769,14 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n,
 	b->ip6h.nexthdr = IPPROTO_UDP;
 	b->ip6h.hop_limit = 255;
 
-	if (c->mode == MODE_PASTA) {
-		void *frame = udp6_l2_iov_tap[n].iov_base;
-
-		if (write(c->fd_tap, frame, sizeof(b->eh) + ip_len) < 0)
-			debug("tap write: %s", strerror(errno));
-		pcap(frame, sizeof(b->eh) + ip_len);
+	buf_len = ip_len + sizeof(b->eh);
 
-		return;
+	if (c->mode == MODE_PASST) {
+		b->vnet_len = htonl(buf_len);
+		buf_len += sizeof(b->vnet_len);
 	}
 
-	b->vnet_len = htonl(ip_len + sizeof(struct ethhdr));
-	buf_len = sizeof(uint32_t) + sizeof(struct ethhdr) + ip_len;
-	udp6_l2_iov_tap[n].iov_len = buf_len;
-
-	/* With bigger messages, qemu closes the connection. */
-	if (*msg_bufs && *msg_len + buf_len > SHRT_MAX) {
-		mh->msg_iovlen = *msg_bufs;
-
-		(*msg_idx)++;
-		udp6_l2_mh_tap[*msg_idx].msg_hdr.msg_iov = &udp6_l2_iov_tap[n];
-		*msg_len = *msg_bufs = 0;
-	}
-
-	*msg_len += buf_len;
-	(*msg_bufs)++;
+	return buf_len;
 }
 
 /**
@@ -832,6 +792,7 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n,
 void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events,
 		      const struct timespec *now)
 {
+	in_port_t dstport = ref.r.p.udp.udp.port;
 	ssize_t n, msg_len = 0, missing = 0;
 	struct mmsghdr *tap_mmh, *sock_mmh;
 	int msg_bufs = 0, msg_i = 0, ret;
@@ -863,12 +824,33 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events,
 
 	tap_mmh[0].msg_hdr.msg_iov = &tap_iov[0];
 	for (i = 0; i < (unsigned)n; i++) {
+		size_t buf_len;
+
 		if (ref.r.p.udp.udp.v6)
-			udp_sock_fill_data_v6(c, i, ref,
-					      &msg_i, &msg_bufs, &msg_len, now);
+			buf_len = udp_update_hdr6(c, i, dstport, now);
 		else
-			udp_sock_fill_data_v4(c, i, ref,
-					      &msg_i, &msg_bufs, &msg_len, now);
+			buf_len = udp_update_hdr4(c, i, dstport, now);
+
+		if (c->mode == MODE_PASTA) {
+			void *frame = tap_iov[i].iov_base;
+
+			if (write(c->fd_tap, frame, buf_len) < 0)
+				debug("tap write: %s", strerror(errno));
+			pcap(frame, buf_len);
+		} else {
+			tap_iov[i].iov_len = buf_len;
+
+			/* With bigger messages, qemu closes the connection. */
+			if (msg_bufs && msg_len + buf_len > SHRT_MAX) {
+				tap_mmh[msg_i].msg_hdr.msg_iovlen = msg_bufs;
+				msg_i++;
+				tap_mmh[msg_i].msg_hdr.msg_iov = &tap_iov[i];
+				msg_len = msg_bufs = 0;
+			}
+
+			msg_len += buf_len;
+			msg_bufs++;
+		}
 	}
 	tap_mmh[msg_i].msg_hdr.msg_iovlen = msg_bufs;
 
-- 
@@ -640,20 +640,17 @@ static void udp_sock_handler_splice(const struct ctx *c, union epoll_ref ref,
 }
 
 /**
- * udp_sock_fill_data_v4() - Fill and queue one buffer. In pasta mode, write it
+ * udp_update_hdr4() - Update headers for one IPv4 datagram
  * @c:		Execution context
  * @n:		Index of buffer in udp4_l2_buf pool
- * @ref:	epoll reference from socket
- * @msg_idx:	Index within message being prepared (spans multiple buffers)
- * @msg_len:	Length of current message being prepared for sending
+ * @dstport:	Destination port number
  * @now:	Current timestamp
+ *
+ * Return: size of tap frame with headers
  */
-static void udp_sock_fill_data_v4(const struct ctx *c, int n,
-				  union epoll_ref ref,
-				  int *msg_idx, int *msg_bufs, ssize_t *msg_len,
-				  const struct timespec *now)
+static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
+			      const struct timespec *now)
 {
-	struct msghdr *mh = &udp4_l2_mh_tap[*msg_idx].msg_hdr;
 	struct udp4_l2_buf_t *b = &udp4_l2_buf[n];
 	size_t ip_len, buf_len;
 	in_port_t src_port;
@@ -687,51 +684,31 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
 
 	udp_update_check4(b);
 	b->uh.source = b->s_in.sin_port;
-	b->uh.dest = htons(ref.r.p.udp.udp.port);
+	b->uh.dest = htons(dstport);
 	b->uh.len = htons(udp4_l2_mh_sock[n].msg_len + sizeof(b->uh));
 
-	if (c->mode == MODE_PASTA) {
-		void *frame = udp4_l2_iov_tap[n].iov_base;
-
-		if (write(c->fd_tap, frame, sizeof(b->eh) + ip_len) < 0)
-			debug("tap write: %s", strerror(errno));
-		pcap(frame, sizeof(b->eh) + ip_len);
-
-		return;
-	}
-
-	b->vnet_len = htonl(ip_len + sizeof(struct ethhdr));
-	buf_len = sizeof(uint32_t) + sizeof(struct ethhdr) + ip_len;
-	udp4_l2_iov_tap[n].iov_len = buf_len;
-
-	/* With bigger messages, qemu closes the connection. */
-	if (*msg_bufs && *msg_len + buf_len > SHRT_MAX) {
-		mh->msg_iovlen = *msg_bufs;
+	buf_len = ip_len + sizeof(b->eh);
 
-		(*msg_idx)++;
-		udp4_l2_mh_tap[*msg_idx].msg_hdr.msg_iov = &udp4_l2_iov_tap[n];
-		*msg_len = *msg_bufs = 0;
+	if (c->mode == MODE_PASST) {
+		b->vnet_len = htonl(buf_len);
+		buf_len += sizeof(b->vnet_len);
 	}
 
-	*msg_len += buf_len;
-	(*msg_bufs)++;
+	return buf_len;
 }
 
 /**
- * udp_sock_fill_data_v6() - Fill and queue one buffer. In pasta mode, write it
+ * udp_update_hdr6() - Update headers for one IPv6 datagram
  * @c:		Execution context
  * @n:		Index of buffer in udp6_l2_buf pool
- * @ref:	epoll reference from socket
- * @msg_idx:	Index within message being prepared (spans multiple buffers)
- * @msg_len:	Length of current message being prepared for sending
+ * @dstport:	Destination port number
  * @now:	Current timestamp
+ *
+ * Return: size of tap frame with headers
  */
-static void udp_sock_fill_data_v6(const struct ctx *c, int n,
-				  union epoll_ref ref,
-				  int *msg_idx, int *msg_bufs, ssize_t *msg_len,
-				  const struct timespec *now)
+static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport,
+			      const struct timespec *now)
 {
-	struct msghdr *mh = &udp6_l2_mh_tap[*msg_idx].msg_hdr;
 	struct udp6_l2_buf_t *b = &udp6_l2_buf[n];
 	size_t ip_len, buf_len;
 	struct in6_addr *src;
@@ -782,7 +759,7 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n,
 	}
 
 	b->uh.source = b->s_in6.sin6_port;
-	b->uh.dest = htons(ref.r.p.udp.udp.port);
+	b->uh.dest = htons(dstport);
 	b->uh.len = b->ip6h.payload_len;
 
 	b->ip6h.hop_limit = IPPROTO_UDP;
@@ -792,31 +769,14 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n,
 	b->ip6h.nexthdr = IPPROTO_UDP;
 	b->ip6h.hop_limit = 255;
 
-	if (c->mode == MODE_PASTA) {
-		void *frame = udp6_l2_iov_tap[n].iov_base;
-
-		if (write(c->fd_tap, frame, sizeof(b->eh) + ip_len) < 0)
-			debug("tap write: %s", strerror(errno));
-		pcap(frame, sizeof(b->eh) + ip_len);
+	buf_len = ip_len + sizeof(b->eh);
 
-		return;
+	if (c->mode == MODE_PASST) {
+		b->vnet_len = htonl(buf_len);
+		buf_len += sizeof(b->vnet_len);
 	}
 
-	b->vnet_len = htonl(ip_len + sizeof(struct ethhdr));
-	buf_len = sizeof(uint32_t) + sizeof(struct ethhdr) + ip_len;
-	udp6_l2_iov_tap[n].iov_len = buf_len;
-
-	/* With bigger messages, qemu closes the connection. */
-	if (*msg_bufs && *msg_len + buf_len > SHRT_MAX) {
-		mh->msg_iovlen = *msg_bufs;
-
-		(*msg_idx)++;
-		udp6_l2_mh_tap[*msg_idx].msg_hdr.msg_iov = &udp6_l2_iov_tap[n];
-		*msg_len = *msg_bufs = 0;
-	}
-
-	*msg_len += buf_len;
-	(*msg_bufs)++;
+	return buf_len;
 }
 
 /**
@@ -832,6 +792,7 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n,
 void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events,
 		      const struct timespec *now)
 {
+	in_port_t dstport = ref.r.p.udp.udp.port;
 	ssize_t n, msg_len = 0, missing = 0;
 	struct mmsghdr *tap_mmh, *sock_mmh;
 	int msg_bufs = 0, msg_i = 0, ret;
@@ -863,12 +824,33 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events,
 
 	tap_mmh[0].msg_hdr.msg_iov = &tap_iov[0];
 	for (i = 0; i < (unsigned)n; i++) {
+		size_t buf_len;
+
 		if (ref.r.p.udp.udp.v6)
-			udp_sock_fill_data_v6(c, i, ref,
-					      &msg_i, &msg_bufs, &msg_len, now);
+			buf_len = udp_update_hdr6(c, i, dstport, now);
 		else
-			udp_sock_fill_data_v4(c, i, ref,
-					      &msg_i, &msg_bufs, &msg_len, now);
+			buf_len = udp_update_hdr4(c, i, dstport, now);
+
+		if (c->mode == MODE_PASTA) {
+			void *frame = tap_iov[i].iov_base;
+
+			if (write(c->fd_tap, frame, buf_len) < 0)
+				debug("tap write: %s", strerror(errno));
+			pcap(frame, buf_len);
+		} else {
+			tap_iov[i].iov_len = buf_len;
+
+			/* With bigger messages, qemu closes the connection. */
+			if (msg_bufs && msg_len + buf_len > SHRT_MAX) {
+				tap_mmh[msg_i].msg_hdr.msg_iovlen = msg_bufs;
+				msg_i++;
+				tap_mmh[msg_i].msg_hdr.msg_iov = &tap_iov[i];
+				msg_len = msg_bufs = 0;
+			}
+
+			msg_len += buf_len;
+			msg_bufs++;
+		}
 	}
 	tap_mmh[msg_i].msg_hdr.msg_iovlen = msg_bufs;
 
-- 
2.38.1


  parent reply	other threads:[~2022-11-24  8:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-24  8:54 [PATCH 0/4] udp: Fix some confusion of IPv4 and IPv6 control structures David Gibson
2022-11-24  8:54 ` [PATCH 1/4] udp: Fix inorrect use of IPv6 mh buffers in IPv4 path David Gibson
2022-11-24  8:54 ` [PATCH 2/4] udp: Better factor IPv4 and IPv6 paths in udp_sock_handler() David Gibson
2022-11-24  8:54 ` [PATCH 3/4] udp: Preadjust udp[46]_l2_iov_tap[].iov_base for pasta mode David Gibson
2022-11-24  8:54 ` David Gibson [this message]
2022-11-25  2:01 ` [PATCH 0/4] udp: Fix some confusion of IPv4 and IPv6 control structures Stefano Brivio
2022-11-25  7:11   ` David Gibson
2022-12-06  6:47 ` 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=20221124085421.3027886-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).