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 6/7] tcp: Merge tcp_fill_headers[46]() with each other
Date: Wed, 27 Nov 2024 14:54:09 +1100	[thread overview]
Message-ID: <20241127035410.1167773-7-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20241127035410.1167773-1-david@gibson.dropbear.id.au>

We have different versions of this function for IPv4 and IPv6, but the
caller already requires some IP version specific code to get the right
header pointers.  Instead, have a common function that fills either an
IPv4 or an IPv6 header based on which header pointer it is passed.  This
allows us to remove a small amount of code duplication and make a few
slightly ugly conditionals.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp.c          | 108 ++++++++++++++++++++++---------------------------
 tcp_buf.c      |  19 ++++-----
 tcp_internal.h |  13 +++---
 tcp_vu.c       |  32 ++++++---------
 4 files changed, 75 insertions(+), 97 deletions(-)

diff --git a/tcp.c b/tcp.c
index 482e460..1872ccb 100644
--- a/tcp.c
+++ b/tcp.c
@@ -877,94 +877,82 @@ static void tcp_fill_header(struct tcphdr *th,
 }
 
 /**
- * tcp_fill_headers4() - Fill 802.3, IPv4, TCP headers in pre-cooked buffers
+ * tcp_fill_headers() - Fill 802.3, IP, TCP headers
  * @conn:		Connection pointer
  * @taph:		tap backend specific header
- * @iph:		Pointer to IPv4 header
+ * @ip4h:		Pointer to IPv4 header, or NULL
+ * @ip6h:		Pointer to IPv6 header, or NULL
  * @th:			Pointer to TCP header
  * @payload:		TCP payload
- * @check:		Checksum, if already known
+ * @ip4_check:		IPv4 checksum, if already known
  * @seq:		Sequence number for this segment
  * @no_tcp_csum:	Do not set TCP checksum
  */
-void tcp_fill_headers4(const struct tcp_tap_conn *conn,
-		       struct tap_hdr *taph, struct iphdr *iph,
-		       struct tcphdr *th, struct iov_tail *payload,
-		       const uint16_t *check, uint32_t seq, bool no_tcp_csum)
+void tcp_fill_headers(const struct tcp_tap_conn *conn,
+		      struct tap_hdr *taph,
+		      struct iphdr *ip4h, struct ipv6hdr *ip6h,
+		      struct tcphdr *th, struct iov_tail *payload,
+		      const uint16_t *ip4_check, uint32_t seq, bool no_tcp_csum)
 {
 	const struct flowside *tapside = TAPFLOW(conn);
-	const struct in_addr *src4 = inany_v4(&tapside->oaddr);
-	const struct in_addr *dst4 = inany_v4(&tapside->eaddr);
 	size_t l4len = iov_tail_size(payload) + sizeof(*th);
-	size_t l3len = l4len + sizeof(*iph);
+	size_t l3len = l4len;
+	uint32_t psum = 0;
 
-	ASSERT(src4 && dst4);
+	if (ip4h) {
+		const struct in_addr *src4 = inany_v4(&tapside->oaddr);
+		const struct in_addr *dst4 = inany_v4(&tapside->eaddr);
 
-	iph->tot_len = htons(l3len);
-	iph->saddr = src4->s_addr;
-	iph->daddr = dst4->s_addr;
+		ASSERT(src4 && dst4);
 
-	iph->check = check ? *check :
-			     csum_ip4_header(l3len, IPPROTO_TCP, *src4, *dst4);
+		l3len += + sizeof(*ip4h);
 
-	tcp_fill_header(th, conn, seq);
+		ip4h->tot_len = htons(l3len);
+		ip4h->saddr = src4->s_addr;
+		ip4h->daddr = dst4->s_addr;
 
-	if (no_tcp_csum) {
-		th->check = 0;
-	} else {
-		uint32_t psum = proto_ipv4_header_psum(l4len, IPPROTO_TCP,
-						       *src4, *dst4);
+		if (ip4_check)
+			ip4h->check = *ip4_check;
+		else
+			ip4h->check = csum_ip4_header(l3len, IPPROTO_TCP,
+						      *src4, *dst4);
 
-		tcp_update_csum(psum, th, payload);
+		if (!no_tcp_csum) {
+			psum = proto_ipv4_header_psum(l4len, IPPROTO_TCP,
+						      *src4, *dst4);
+		}
 	}
 
-	tap_hdr_update(taph, l3len + sizeof(struct ethhdr));
-}
+	if (ip6h) {
+		l3len += sizeof(*ip6h);
 
-/**
- * tcp_fill_headers6() - Fill 802.3, IPv6, TCP headers in pre-cooked buffers
- * @conn:		Connection pointer
- * @taph:		tap backend specific header
- * @ip6h:		Pointer to IPv6 header
- * @th:			Pointer to TCP header
- * @payload:		TCP payload
- * @check:		Checksum, if already known
- * @seq:		Sequence number for this segment
- * @no_tcp_csum:	Do not set TCP checksum
- */
-void tcp_fill_headers6(const struct tcp_tap_conn *conn,
-		       struct tap_hdr *taph, struct ipv6hdr *ip6h,
-		       struct tcphdr *th, struct iov_tail *payload,
-		       uint32_t seq, bool no_tcp_csum)
-{
-	size_t l4len = iov_tail_size(payload) + sizeof(*th);
-	const struct flowside *tapside = TAPFLOW(conn);
+		ip6h->payload_len = htons(l4len);
+		ip6h->saddr = tapside->oaddr.a6;
+		ip6h->daddr = tapside->eaddr.a6;
 
-	ip6h->payload_len = htons(l4len);
-	ip6h->saddr = tapside->oaddr.a6;
-	ip6h->daddr = tapside->eaddr.a6;
+		ip6h->hop_limit = 255;
+		ip6h->version = 6;
+		ip6h->nexthdr = IPPROTO_TCP;
 
-	ip6h->hop_limit = 255;
-	ip6h->version = 6;
-	ip6h->nexthdr = IPPROTO_TCP;
+		ip6h->flow_lbl[0] = (conn->sock >> 16) & 0xf;
+		ip6h->flow_lbl[1] = (conn->sock >> 8) & 0xff;
+		ip6h->flow_lbl[2] = (conn->sock >> 0) & 0xff;
 
-	ip6h->flow_lbl[0] = (conn->sock >> 16) & 0xf;
-	ip6h->flow_lbl[1] = (conn->sock >> 8) & 0xff;
-	ip6h->flow_lbl[2] = (conn->sock >> 0) & 0xff;
+		if (!no_tcp_csum) {
+			psum = proto_ipv6_header_psum(l4len, IPPROTO_TCP,
+						      &ip6h->saddr,
+						      &ip6h->daddr);
+		}
+	}
 
 	tcp_fill_header(th, conn, seq);
 
-	if (no_tcp_csum) {
+	if (no_tcp_csum)
 		th->check = 0;
-	} else {
-		uint32_t psum = proto_ipv6_header_psum(l4len, IPPROTO_TCP,
-						       &ip6h->saddr,
-						       &ip6h->daddr);
-
+	else
 		tcp_update_csum(psum, th, payload);
-	}
 
-	tap_hdr_update(taph, l4len + sizeof(*ip6h) + sizeof(struct ethhdr));
+	tap_hdr_update(taph, l3len + sizeof(struct ethhdr));
 }
 
 /**
diff --git a/tcp_buf.c b/tcp_buf.c
index 830c23d..a975a55 100644
--- a/tcp_buf.c
+++ b/tcp_buf.c
@@ -161,18 +161,19 @@ static void tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
 {
 	struct iov_tail tail = IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0);
 	struct tcphdr *th = IOV_REMOVE_HEADER(&tail, struct tcphdr);
+	struct tap_hdr *taph = iov[TCP_IOV_TAP].iov_base;
 	const struct flowside *tapside = TAPFLOW(conn);
 	const struct in_addr *a4 = inany_v4(&tapside->oaddr);
+	struct ipv6hdr *ip6h = NULL;
+	struct iphdr *ip4h = NULL;
 
-	if (a4) {
-		tcp_fill_headers4(conn, iov[TCP_IOV_TAP].iov_base,
-				  iov[TCP_IOV_IP].iov_base, th,
-				  &tail,  check, seq, no_tcp_csum);
-	} else {
-		tcp_fill_headers6(conn, iov[TCP_IOV_TAP].iov_base,
-				  iov[TCP_IOV_IP].iov_base, th,
-				  &tail, seq, no_tcp_csum);
-	}
+	if (a4)
+		ip4h = iov[TCP_IOV_IP].iov_base;
+	else
+		ip6h = iov[TCP_IOV_IP].iov_base;
+
+	tcp_fill_headers(conn, taph, ip4h, ip6h, th, &tail,
+			 check, seq, no_tcp_csum);
 }
 
 /**
diff --git a/tcp_internal.h b/tcp_internal.h
index cff06e0..94e5780 100644
--- a/tcp_internal.h
+++ b/tcp_internal.h
@@ -164,14 +164,11 @@ struct tcp_info_linux;
 
 void tcp_update_csum(uint32_t psum, struct tcphdr *th,
 		     struct iov_tail *payload);
-void tcp_fill_headers4(const struct tcp_tap_conn *conn,
-		       struct tap_hdr *taph, struct iphdr *iph,
-		       struct tcphdr *th, struct iov_tail *payload,
-		       const uint16_t *check, uint32_t seq, bool no_tcp_csum);
-void tcp_fill_headers6(const struct tcp_tap_conn *conn,
-		       struct tap_hdr *taph, struct ipv6hdr *ip6h,
-		       struct tcphdr *th, struct iov_tail *payload,
-		       uint32_t seq, bool no_tcp_csum);
+void tcp_fill_headers(const struct tcp_tap_conn *conn,
+		      struct tap_hdr *taph,
+		      struct iphdr *ip4h, struct ipv6hdr *ip6h,
+		      struct tcphdr *th, struct iov_tail *payload,
+		      const uint16_t *ip4_check, uint32_t seq, bool no_tcp_csum);
 
 int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn,
 			  bool force_seq, struct tcp_info_linux *tinfo);
diff --git a/tcp_vu.c b/tcp_vu.c
index 081c752..a611a5b 100644
--- a/tcp_vu.c
+++ b/tcp_vu.c
@@ -110,9 +110,9 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 	size_t optlen, hdrlen;
 	struct vu_virtq_element flags_elem[2];
 	struct ipv6hdr *ip6h = NULL;
+	struct iphdr *ip4h = NULL;
 	struct iovec flags_iov[2];
 	struct tcp_syn_opts *opts;
-	struct iphdr *iph = NULL;
 	struct iov_tail payload;
 	struct tcphdr *th;
 	struct ethhdr *eh;
@@ -143,8 +143,8 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 	if (CONN_V4(conn)) {
 		eh->h_proto = htons(ETH_P_IP);
 
-		iph = vu_ip(flags_elem[0].in_sg[0].iov_base);
-		*iph = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
+		ip4h = vu_ip(flags_elem[0].in_sg[0].iov_base);
+		*ip4h = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
 
 		th = vu_payloadv4(flags_elem[0].in_sg[0].iov_base);
 	} else {
@@ -170,12 +170,8 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 	flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
 	payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
 
-	if (CONN_V4(conn)) {
-		tcp_fill_headers4(conn, NULL, iph, th, &payload,
-				  NULL, seq, true);
-	} else {
-		tcp_fill_headers6(conn, NULL, ip6h, th, &payload, seq, true);
-	}
+	tcp_fill_headers(conn, NULL, ip4h, ip6h, th, &payload,
+			 NULL, seq, true);
 
 	if (*c->pcap) {
 		tcp_vu_update_check(tapside, &flags_elem[0].in_sg[0], 1);
@@ -338,7 +334,7 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
 	struct iov_tail payload = IOV_TAIL(iov, iov_cnt, hdrlen);
 	char *base = iov[0].iov_base;
 	struct ipv6hdr *ip6h = NULL;
-	struct iphdr *iph = NULL;
+	struct iphdr *ip4h = NULL;
 	struct tcphdr *th;
 	struct ethhdr *eh;
 
@@ -357,8 +353,8 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
 	if (!v6) {
 		eh->h_proto = htons(ETH_P_IP);
 
-		iph = vu_ip(base);
-		*iph = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
+		ip4h = vu_ip(base);
+		*ip4h = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
 		th = vu_payloadv4(base);
 	} else {
 		eh->h_proto = htons(ETH_P_IPV6);
@@ -373,14 +369,10 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
 	th->doff = sizeof(*th) / 4;
 	th->ack = 1;
 
-	if (!v6) {
-		tcp_fill_headers4(conn, NULL, iph, th, &payload,
-				  *check, conn->seq_to_tap, true);
-		*check = &iph->check;
-	} else {
-		tcp_fill_headers6(conn, NULL, ip6h, th, &payload,
-				  conn->seq_to_tap, true);
-	}
+	tcp_fill_headers(conn, NULL, ip4h, ip6h, th, &payload,
+			 *check, conn->seq_to_tap, true);
+	if (ip4h)
+		*check = &ip4h->check;
 }
 
 /**
-- 
@@ -110,9 +110,9 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 	size_t optlen, hdrlen;
 	struct vu_virtq_element flags_elem[2];
 	struct ipv6hdr *ip6h = NULL;
+	struct iphdr *ip4h = NULL;
 	struct iovec flags_iov[2];
 	struct tcp_syn_opts *opts;
-	struct iphdr *iph = NULL;
 	struct iov_tail payload;
 	struct tcphdr *th;
 	struct ethhdr *eh;
@@ -143,8 +143,8 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 	if (CONN_V4(conn)) {
 		eh->h_proto = htons(ETH_P_IP);
 
-		iph = vu_ip(flags_elem[0].in_sg[0].iov_base);
-		*iph = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
+		ip4h = vu_ip(flags_elem[0].in_sg[0].iov_base);
+		*ip4h = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
 
 		th = vu_payloadv4(flags_elem[0].in_sg[0].iov_base);
 	} else {
@@ -170,12 +170,8 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 	flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
 	payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
 
-	if (CONN_V4(conn)) {
-		tcp_fill_headers4(conn, NULL, iph, th, &payload,
-				  NULL, seq, true);
-	} else {
-		tcp_fill_headers6(conn, NULL, ip6h, th, &payload, seq, true);
-	}
+	tcp_fill_headers(conn, NULL, ip4h, ip6h, th, &payload,
+			 NULL, seq, true);
 
 	if (*c->pcap) {
 		tcp_vu_update_check(tapside, &flags_elem[0].in_sg[0], 1);
@@ -338,7 +334,7 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
 	struct iov_tail payload = IOV_TAIL(iov, iov_cnt, hdrlen);
 	char *base = iov[0].iov_base;
 	struct ipv6hdr *ip6h = NULL;
-	struct iphdr *iph = NULL;
+	struct iphdr *ip4h = NULL;
 	struct tcphdr *th;
 	struct ethhdr *eh;
 
@@ -357,8 +353,8 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
 	if (!v6) {
 		eh->h_proto = htons(ETH_P_IP);
 
-		iph = vu_ip(base);
-		*iph = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
+		ip4h = vu_ip(base);
+		*ip4h = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
 		th = vu_payloadv4(base);
 	} else {
 		eh->h_proto = htons(ETH_P_IPV6);
@@ -373,14 +369,10 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
 	th->doff = sizeof(*th) / 4;
 	th->ack = 1;
 
-	if (!v6) {
-		tcp_fill_headers4(conn, NULL, iph, th, &payload,
-				  *check, conn->seq_to_tap, true);
-		*check = &iph->check;
-	} else {
-		tcp_fill_headers6(conn, NULL, ip6h, th, &payload,
-				  conn->seq_to_tap, true);
-	}
+	tcp_fill_headers(conn, NULL, ip4h, ip6h, th, &payload,
+			 *check, conn->seq_to_tap, true);
+	if (ip4h)
+		*check = &ip4h->check;
 }
 
 /**
-- 
2.47.0


  parent reply	other threads:[~2024-11-27  3:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-27  3:54 [PATCH 0/7] TCP buffer handling cleanups (including vhost) David Gibson
2024-11-27  3:54 ` [PATCH 1/7] iov: iov tail helpers David Gibson
2024-11-27  3:54 ` [PATCH 2/7] iov, checksum: Replace csum_iov() with csum_iov_tail() David Gibson
2024-11-27  3:54 ` [PATCH 3/7] tcp: Pass TCP header and payload separately to tcp_update_check_tcp[46]() David Gibson
2024-11-27  3:54 ` [PATCH 4/7] tcp: Pass TCP header and payload separately to tcp_fill_headers[46]() David Gibson
2024-11-27  3:54 ` [PATCH 5/7] tcp: Merge tcp_update_check_tcp[46]() David Gibson
2024-11-27  3:54 ` David Gibson [this message]
2024-11-27  3:54 ` [PATCH 7/7] tcp_vu: Remove unnecessary tcp_vu_update_check() function 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=20241127035410.1167773-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).