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 7/7] tcp_vu: Remove unnecessary tcp_vu_update_check() function
Date: Wed, 27 Nov 2024 14:54:10 +1100	[thread overview]
Message-ID: <20241127035410.1167773-8-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20241127035410.1167773-1-david@gibson.dropbear.id.au>

Because the vhost-user <-> virtio-net path ignores checksums, we usually
don't calculate them when sending packets to the guest.  So, we always
pass no_tcp_csum=true to tcp_fill_headers().  We do want accurate
checksums when capturing packets though, so the captures don't show bogus
values.

Currently we handle this by updating the checksum field immediately before
writing the packet to the capture file, using tcp_vu_update_check().  This
is unnecessary, though: in each case tcp_fill_headers() is called not very
long before, so we can alter its no_tcp_csum parameter pased on whether
we're generating captures or not.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp_vu.c | 59 +++++++++++---------------------------------------------
 1 file changed, 11 insertions(+), 48 deletions(-)

diff --git a/tcp_vu.c b/tcp_vu.c
index a611a5b..4c60897 100644
--- a/tcp_vu.c
+++ b/tcp_vu.c
@@ -60,40 +60,6 @@ static size_t tcp_vu_hdrlen(bool v6)
 	return hdrlen;
 }
 
-/**
- * tcp_vu_update_check() - Calculate TCP checksum
- * @tapside:	Address information for one side of the flow
- * @iov:	Pointer to the array of IO vectors
- * @iov_cnt:	Length of the array
- */
-static void tcp_vu_update_check(const struct flowside *tapside,
-			        struct iovec *iov, int iov_cnt)
-{
-	char *base = iov[0].iov_base;
-	struct iov_tail payload;
-	struct tcphdr *th;
-	uint32_t psum;
-
-	if (inany_v4(&tapside->oaddr)) {
-		const struct in_addr *src4 = inany_v4(&tapside->oaddr);
-		const struct in_addr *dst4 = inany_v4(&tapside->eaddr);
-		const struct iphdr *iph = vu_ip(base);
-		size_t l4len = ntohs(iph->tot_len) - sizeof(*iph);
-
-		th = vu_payloadv4(base);
-		psum = proto_ipv4_header_psum(l4len, IPPROTO_TCP, *src4, *dst4);
-	} else {
-		const struct ipv6hdr *ip6h = vu_ip(base);
-		size_t l4len = ntohs(ip6h->payload_len);
-
-		th = vu_payloadv6(base);
-		psum = proto_ipv6_header_psum(l4len, IPPROTO_TCP,
-					      &ip6h->saddr, &ip6h->daddr);
-	}
-	payload = IOV_TAIL(iov, iov_cnt, (char *)(th + 1) - base);
-	tcp_update_csum(psum, th, &payload);
-}
-
 /**
  * tcp_vu_send_flag() - Send segment with flags to vhost-user (no payload)
  * @c:		Execution context
@@ -106,7 +72,6 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 {
 	struct vu_dev *vdev = c->vdev;
 	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
-	const struct flowside *tapside = TAPFLOW(conn);
 	size_t optlen, hdrlen;
 	struct vu_virtq_element flags_elem[2];
 	struct ipv6hdr *ip6h = NULL;
@@ -171,10 +136,9 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 	payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
 
 	tcp_fill_headers(conn, NULL, ip4h, ip6h, th, &payload,
-			 NULL, seq, true);
+			 NULL, seq, !*c->pcap);
 
 	if (*c->pcap) {
-		tcp_vu_update_check(tapside, &flags_elem[0].in_sg[0], 1);
 		pcap_iov(&flags_elem[0].in_sg[0], 1,
 			 sizeof(struct virtio_net_hdr_mrg_rxbuf));
 	}
@@ -318,15 +282,16 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c,
 
 /**
  * tcp_vu_prepare() - Prepare the frame header
- * @c:		Execution context
- * @conn:	Connection pointer
- * @iov:	Pointer to the array of IO vectors
- * @iov_cnt:	Number of entries in @iov
- * @check:	Checksum, if already known
+ * @c:			Execution context
+ * @conn:		Connection pointer
+ * @iov:		Pointer to the array of IO vectors
+ * @iov_cnt:		Number of entries in @iov
+ * @check:		Checksum, if already known
+ * @no_tcp_csum:	Do not set TCP checksum
  */
 static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
 			   struct iovec *iov, size_t iov_cnt,
-			   const uint16_t **check)
+			   const uint16_t **check, bool no_tcp_csum)
 {
 	const struct flowside *toside = TAPFLOW(conn);
 	bool v6 = !(inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr));
@@ -370,7 +335,7 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
 	th->ack = 1;
 
 	tcp_fill_headers(conn, NULL, ip4h, ip6h, th, &payload,
-			 *check, conn->seq_to_tap, true);
+			 *check, conn->seq_to_tap, no_tcp_csum);
 	if (ip4h)
 		*check = &ip4h->check;
 }
@@ -388,8 +353,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
 	uint32_t wnd_scaled = conn->wnd_from_tap << conn->ws_from_tap;
 	struct vu_dev *vdev = c->vdev;
 	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
-	const struct flowside *tapside = TAPFLOW(conn);
-	size_t fillsize, hdrlen;
+	size_t hdrlen, fillsize;
 	int v6 = CONN_V6(conn);
 	uint32_t already_sent;
 	const uint16_t *check;
@@ -482,10 +446,9 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
 		if (i + 1 == head_cnt)
 			check = NULL;
 
-		tcp_vu_prepare(c, conn, iov, buf_cnt, &check);
+		tcp_vu_prepare(c, conn, iov, buf_cnt, &check, !*c->pcap);
 
 		if (*c->pcap) {
-			tcp_vu_update_check(tapside, iov, buf_cnt);
 			pcap_iov(iov, buf_cnt,
 				 sizeof(struct virtio_net_hdr_mrg_rxbuf));
 		}
-- 
@@ -60,40 +60,6 @@ static size_t tcp_vu_hdrlen(bool v6)
 	return hdrlen;
 }
 
-/**
- * tcp_vu_update_check() - Calculate TCP checksum
- * @tapside:	Address information for one side of the flow
- * @iov:	Pointer to the array of IO vectors
- * @iov_cnt:	Length of the array
- */
-static void tcp_vu_update_check(const struct flowside *tapside,
-			        struct iovec *iov, int iov_cnt)
-{
-	char *base = iov[0].iov_base;
-	struct iov_tail payload;
-	struct tcphdr *th;
-	uint32_t psum;
-
-	if (inany_v4(&tapside->oaddr)) {
-		const struct in_addr *src4 = inany_v4(&tapside->oaddr);
-		const struct in_addr *dst4 = inany_v4(&tapside->eaddr);
-		const struct iphdr *iph = vu_ip(base);
-		size_t l4len = ntohs(iph->tot_len) - sizeof(*iph);
-
-		th = vu_payloadv4(base);
-		psum = proto_ipv4_header_psum(l4len, IPPROTO_TCP, *src4, *dst4);
-	} else {
-		const struct ipv6hdr *ip6h = vu_ip(base);
-		size_t l4len = ntohs(ip6h->payload_len);
-
-		th = vu_payloadv6(base);
-		psum = proto_ipv6_header_psum(l4len, IPPROTO_TCP,
-					      &ip6h->saddr, &ip6h->daddr);
-	}
-	payload = IOV_TAIL(iov, iov_cnt, (char *)(th + 1) - base);
-	tcp_update_csum(psum, th, &payload);
-}
-
 /**
  * tcp_vu_send_flag() - Send segment with flags to vhost-user (no payload)
  * @c:		Execution context
@@ -106,7 +72,6 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 {
 	struct vu_dev *vdev = c->vdev;
 	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
-	const struct flowside *tapside = TAPFLOW(conn);
 	size_t optlen, hdrlen;
 	struct vu_virtq_element flags_elem[2];
 	struct ipv6hdr *ip6h = NULL;
@@ -171,10 +136,9 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 	payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
 
 	tcp_fill_headers(conn, NULL, ip4h, ip6h, th, &payload,
-			 NULL, seq, true);
+			 NULL, seq, !*c->pcap);
 
 	if (*c->pcap) {
-		tcp_vu_update_check(tapside, &flags_elem[0].in_sg[0], 1);
 		pcap_iov(&flags_elem[0].in_sg[0], 1,
 			 sizeof(struct virtio_net_hdr_mrg_rxbuf));
 	}
@@ -318,15 +282,16 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c,
 
 /**
  * tcp_vu_prepare() - Prepare the frame header
- * @c:		Execution context
- * @conn:	Connection pointer
- * @iov:	Pointer to the array of IO vectors
- * @iov_cnt:	Number of entries in @iov
- * @check:	Checksum, if already known
+ * @c:			Execution context
+ * @conn:		Connection pointer
+ * @iov:		Pointer to the array of IO vectors
+ * @iov_cnt:		Number of entries in @iov
+ * @check:		Checksum, if already known
+ * @no_tcp_csum:	Do not set TCP checksum
  */
 static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
 			   struct iovec *iov, size_t iov_cnt,
-			   const uint16_t **check)
+			   const uint16_t **check, bool no_tcp_csum)
 {
 	const struct flowside *toside = TAPFLOW(conn);
 	bool v6 = !(inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr));
@@ -370,7 +335,7 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
 	th->ack = 1;
 
 	tcp_fill_headers(conn, NULL, ip4h, ip6h, th, &payload,
-			 *check, conn->seq_to_tap, true);
+			 *check, conn->seq_to_tap, no_tcp_csum);
 	if (ip4h)
 		*check = &ip4h->check;
 }
@@ -388,8 +353,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
 	uint32_t wnd_scaled = conn->wnd_from_tap << conn->ws_from_tap;
 	struct vu_dev *vdev = c->vdev;
 	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
-	const struct flowside *tapside = TAPFLOW(conn);
-	size_t fillsize, hdrlen;
+	size_t hdrlen, fillsize;
 	int v6 = CONN_V6(conn);
 	uint32_t already_sent;
 	const uint16_t *check;
@@ -482,10 +446,9 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
 		if (i + 1 == head_cnt)
 			check = NULL;
 
-		tcp_vu_prepare(c, conn, iov, buf_cnt, &check);
+		tcp_vu_prepare(c, conn, iov, buf_cnt, &check, !*c->pcap);
 
 		if (*c->pcap) {
-			tcp_vu_update_check(tapside, iov, buf_cnt);
 			pcap_iov(iov, buf_cnt,
 				 sizeof(struct virtio_net_hdr_mrg_rxbuf));
 		}
-- 
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 ` [PATCH 6/7] tcp: Merge tcp_fill_headers[46]() with each other David Gibson
2024-11-27  3:54 ` David Gibson [this message]

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-8-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).