public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Laurent Vivier <lvivier@redhat.com>
Cc: passt-dev@passt.top
Subject: Re: [PATCH v3 06/10] checksum: Pass explicit L4 length to checksum functions
Date: Mon, 11 May 2026 11:33:39 +1000	[thread overview]
Message-ID: <agEx80UEIqk_1_jL@zatzit> (raw)
In-Reply-To: <20260416155721.3807225-7-lvivier@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 12012 bytes --]

On Thu, Apr 16, 2026 at 05:57:17PM +0200, Laurent Vivier wrote:
> The iov_tail passed to csum_iov_tail() may contain padding or trailing
> data beyond the actual L4 payload.  Rather than relying on
> iov_tail_size() to determine how many bytes to checksum, pass the
> length explicitly so that only the relevant payload bytes are included
> in the checksum computation.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  checksum.c | 43 +++++++++++++++++++++++++------------------
>  checksum.h |  6 +++---
>  tap.c      |  4 ++--
>  tcp.c      | 12 +++++++-----
>  udp.c      |  5 +++--
>  udp_vu.c   | 21 +++++++++------------
>  6 files changed, 49 insertions(+), 42 deletions(-)
> 
> diff --git a/checksum.c b/checksum.c
> index 828f9ecc9c02..7c62e42d6d4c 100644
> --- a/checksum.c
> +++ b/checksum.c
> @@ -182,21 +182,22 @@ static uint16_t csum(const void *buf, size_t len, uint32_t init)
>   * @saddr:	IPv4 source address
>   * @daddr:	IPv4 destination address
>   * @data:	UDP payload (as IO vector tail)
> + * @dlen:	UDP payload length
>   */
>  void csum_udp4(struct udphdr *udp4hr,
>  	       struct in_addr saddr, struct in_addr daddr,
> -	       struct iov_tail *data)
> +	       struct iov_tail *data, size_t dlen)
>  {
>  	/* UDP checksums are optional, so don't bother */
>  	udp4hr->check = 0;
>  
>  	if (UDP4_REAL_CHECKSUMS) {
> -		uint16_t l4len = iov_tail_size(data) + sizeof(struct udphdr);
> -		uint32_t psum = proto_ipv4_header_psum(l4len, IPPROTO_UDP,
> -						       saddr, daddr);
> +		uint32_t psum = proto_ipv4_header_psum(sizeof(*udp4hr) + dlen,
> +						       IPPROTO_UDP, saddr,
> +						       daddr);
>  
> -		psum = csum_unfolded(udp4hr, sizeof(struct udphdr), psum);
> -		udp4hr->check = csum_iov_tail(data, psum);
> +		psum = csum_unfolded(udp4hr, sizeof(*udp4hr), psum);
> +		udp4hr->check = csum_iov_tail(data, psum, dlen);
>  	}
>  }
>  
> @@ -245,19 +246,19 @@ uint32_t proto_ipv6_header_psum(uint16_t payload_len, uint8_t protocol,
>   * @saddr:	Source address
>   * @daddr:	Destination address
>   * @data:	UDP payload (as IO vector tail)
> + * @dlen:	UDP payload length
>   */
>  void csum_udp6(struct udphdr *udp6hr,
>  	       const struct in6_addr *saddr, const struct in6_addr *daddr,
> -	       struct iov_tail *data)
> +	       struct iov_tail *data, size_t dlen)
>  {
> -	uint16_t l4len = iov_tail_size(data) + sizeof(struct udphdr);
> -	uint32_t psum = proto_ipv6_header_psum(l4len, IPPROTO_UDP,
> -					       saddr, daddr);
> +	uint32_t psum = proto_ipv6_header_psum(dlen + sizeof(*udp6hr),
> +					       IPPROTO_UDP, saddr, daddr);
>  
>  	udp6hr->check = 0;
>  
> -	psum = csum_unfolded(udp6hr, sizeof(struct udphdr), psum);
> -	udp6hr->check = csum_iov_tail(data, psum);
> +	psum = csum_unfolded(udp6hr, sizeof(*udp6hr), psum);
> +	udp6hr->check = csum_iov_tail(data, psum, dlen);
>  }
>  
>  /**
> @@ -604,20 +605,26 @@ uint32_t csum_unfolded(const void *buf, size_t len, uint32_t init)
>  /**
>   * csum_iov_tail() - Calculate unfolded checksum for the tail of an IO vector
>   * @tail:	IO vector tail to checksum
> - * @init	Initial 32-bit checksum, 0 for no pre-computed checksum
> + * @init:	Initial 32-bit checksum, 0 for no pre-computed checksum
> + * @len:	Number of bytes to checksum from @tail
>   *
>   * Return: 16-bit folded, complemented checksum
>   */
> -uint16_t csum_iov_tail(struct iov_tail *tail, uint32_t init)
> +uint16_t csum_iov_tail(struct iov_tail *tail, uint32_t init, size_t len)
>  {
>  	if (iov_tail_prune(tail)) {
> -		size_t i;
> +		size_t i, n;
>  
> +		n = MIN(len, tail->iov[0].iov_len - tail->off);
>  		init = csum_unfolded((char *)tail->iov[0].iov_base + tail->off,
> -				     tail->iov[0].iov_len - tail->off, init);
> -		for (i = 1; i < tail->cnt; i++) {
> +				     n, init);
> +		len -= n;
> +
> +		for (i = 1; len && i < tail->cnt; i++) {
>  			const struct iovec *iov = &tail->iov[i];
> -			init = csum_unfolded(iov->iov_base, iov->iov_len, init);
> +			n = MIN(len, iov->iov_len);
> +			init = csum_unfolded(iov->iov_base, n, init);
> +			len -= n;
>  		}
>  	}
>  	return (uint16_t)~csum_fold(init);
> diff --git a/checksum.h b/checksum.h
> index 4e3b098db072..6270f1457a73 100644
> --- a/checksum.h
> +++ b/checksum.h
> @@ -21,18 +21,18 @@ uint32_t proto_ipv4_header_psum(uint16_t l4len, uint8_t protocol,
>  				struct in_addr saddr, struct in_addr daddr);
>  void csum_udp4(struct udphdr *udp4hr,
>  	       struct in_addr saddr, struct in_addr daddr,
> -	       struct iov_tail *data);
> +	       struct iov_tail *data, size_t dlen);
>  void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t dlen);
>  uint32_t proto_ipv6_header_psum(uint16_t payload_len, uint8_t protocol,
>  				const struct in6_addr *saddr,
>  				const struct in6_addr *daddr);
>  void csum_udp6(struct udphdr *udp6hr,
>  	       const struct in6_addr *saddr, const struct in6_addr *daddr,
> -	       struct iov_tail *data);
> +	       struct iov_tail *data, size_t dlen);
>  void csum_icmp6(struct icmp6hdr *icmp6hr,
>  		const struct in6_addr *saddr, const struct in6_addr *daddr,
>  		const void *payload, size_t dlen);
>  uint32_t csum_unfolded(const void *buf, size_t len, uint32_t init);
> -uint16_t csum_iov_tail(struct iov_tail *tail, uint32_t init);
> +uint16_t csum_iov_tail(struct iov_tail *tail, uint32_t init, size_t len);
>  
>  #endif /* CHECKSUM_H */
> diff --git a/tap.c b/tap.c
> index 1049e023bcd2..41a61a36c279 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -252,7 +252,7 @@ void *tap_push_uh4(struct udphdr *uh, struct in_addr src, in_port_t sport,
>  	uh->source = htons(sport);
>  	uh->dest = htons(dport);
>  	uh->len = htons(l4len);
> -	csum_udp4(uh, src, dst, &payload);
> +	csum_udp4(uh, src, dst, &payload, dlen);
>  	return (char *)uh + sizeof(*uh);
>  }
>  
> @@ -357,7 +357,7 @@ void *tap_push_uh6(struct udphdr *uh,
>  	uh->source = htons(sport);
>  	uh->dest = htons(dport);
>  	uh->len = htons(l4len);
> -	csum_udp6(uh, src, dst, &payload);
> +	csum_udp6(uh, src, dst, &payload, dlen);
>  	return (char *)uh + sizeof(*uh);
>  }
>  
> diff --git a/tcp.c b/tcp.c
> index 8ea9be84a9f3..5b2a732cf26d 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -815,13 +815,14 @@ static void tcp_sock_set_nodelay(int s)
>   * @psum:	Unfolded partial checksum of the IPv4 or IPv6 pseudo-header
>   * @th:		TCP header (updated)
>   * @payload:	TCP payload
> + * @dlen:	TCP payload length
>   */
>  static void tcp_update_csum(uint32_t psum, struct tcphdr *th,
> -			    struct iov_tail *payload)
> +			    struct iov_tail *payload, size_t dlen)
>  {
>  	th->check = 0;
>  	psum = csum_unfolded(th, sizeof(*th), psum);
> -	th->check = csum_iov_tail(payload, psum);
> +	th->check = csum_iov_tail(payload, psum, dlen);
>  }
>  
>  /**
> @@ -958,7 +959,8 @@ size_t tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn,
>  			bool no_tcp_csum)
>  {
>  	const struct flowside *tapside = TAPFLOW(conn);
> -	size_t l4len = iov_tail_size(payload) + sizeof(*th);
> +	size_t dlen = iov_tail_size(payload);
> +	size_t l4len = dlen + sizeof(*th);
>  	uint8_t *omac = conn->f.tap_omac;
>  	size_t l3len = l4len;
>  	uint32_t psum = 0;
> @@ -1019,7 +1021,7 @@ size_t tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn,
>  	if (no_tcp_csum)
>  		th->check = 0;
>  	else
> -		tcp_update_csum(psum, th, payload);
> +		tcp_update_csum(psum, th, payload, dlen);
>  
>  	return MAX(l3len + sizeof(struct ethhdr), ETH_ZLEN);
>  }
> @@ -2196,7 +2198,7 @@ static void tcp_rst_no_conn(const struct ctx *c, int af,
>  		rsth->ack = 1;
>  	}
>  
> -	tcp_update_csum(psum, rsth, &payload);
> +	tcp_update_csum(psum, rsth, &payload, 0);
>  	rst_l2len = ((char *)rsth - buf) + sizeof(*rsth);
>  	tap_send_single(c, buf, rst_l2len);
>  }
> diff --git a/udp.c b/udp.c
> index 1fc5a42c5ca7..4eef10854d8a 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -289,7 +289,7 @@ size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp,
>  			.iov_len = dlen
>  		};
>  		struct iov_tail data = IOV_TAIL(&iov, 1, 0);
> -		csum_udp4(&bp->uh, *src, *dst, &data);
> +		csum_udp4(&bp->uh, *src, *dst, &data, dlen);
>  	}
>  
>  	return l4len;
> @@ -334,7 +334,8 @@ size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct udp_payload_t *bp,
>  			.iov_len = dlen
>  		};
>  		struct iov_tail data = IOV_TAIL(&iov, 1, 0);
> -		csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, &data);
> +		csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, &data,
> +			  dlen);
>  	}
>  
>  	return l4len;
> diff --git a/udp_vu.c b/udp_vu.c
> index 277bd5688907..1a73d997f683 100644
> --- a/udp_vu.c
> +++ b/udp_vu.c
> @@ -105,14 +105,11 @@ static ssize_t udp_vu_sock_recv(struct iovec *iov, size_t *cnt, int s, bool v6)
>   * @iov:	IO vector for the frame (including vnet header)
>   * @toside:	Address information for one side of the flow
>   * @dlen:	Packet data length
> - *
> - * Return: Layer-4 length
>   */
> -static size_t udp_vu_prepare(const struct ctx *c, const struct iovec *iov,
> +static void udp_vu_prepare(const struct ctx *c, const struct iovec *iov,
>  			     const struct flowside *toside, ssize_t dlen)
>  {
>  	struct ethhdr *eh;
> -	size_t l4len;
>  
>  	/* ethernet header */
>  	eh = vu_eth(iov[0].iov_base);
> @@ -129,7 +126,7 @@ static size_t udp_vu_prepare(const struct ctx *c, const struct iovec *iov,
>  
>  		*iph = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_UDP);
>  
> -		l4len = udp_update_hdr4(iph, bp, toside, dlen, true);
> +		udp_update_hdr4(iph, bp, toside, dlen, true);
>  	} else {
>  		struct ipv6hdr *ip6h = vu_ip(iov[0].iov_base);
>  		struct udp_payload_t *bp = vu_payloadv6(iov[0].iov_base);
> @@ -138,10 +135,8 @@ static size_t udp_vu_prepare(const struct ctx *c, const struct iovec *iov,
>  
>  		*ip6h = (struct ipv6hdr)L2_BUF_IP6_INIT(IPPROTO_UDP);
>  
> -		l4len = udp_update_hdr6(ip6h, bp, toside, dlen, true);
> +		udp_update_hdr6(ip6h, bp, toside, dlen, true);
>  	}
> -
> -	return l4len;
>  }
>  
>  /**
> @@ -149,9 +144,10 @@ static size_t udp_vu_prepare(const struct ctx *c, const struct iovec *iov,
>   * @toside:	Address information for one side of the flow
>   * @iov:	IO vector for the frame
>   * @cnt:	Number of IO vector entries
> + * @dlen:	Data length
>   */
>  static void udp_vu_csum(const struct flowside *toside, const struct iovec *iov,
> -			size_t cnt)
> +			size_t cnt, size_t dlen)
>  {
>  	const struct in_addr *src4 = inany_v4(&toside->oaddr);
>  	const struct in_addr *dst4 = inany_v4(&toside->eaddr);
> @@ -162,11 +158,12 @@ static void udp_vu_csum(const struct flowside *toside, const struct iovec *iov,
>  	if (src4 && dst4) {
>  		bp = vu_payloadv4(base);
>  		data = IOV_TAIL(iov, cnt, (char *)&bp->data - base);
> -		csum_udp4(&bp->uh, *src4, *dst4, &data);
> +		csum_udp4(&bp->uh, *src4, *dst4, &data, dlen);
>  	} else {
>  		bp = vu_payloadv6(base);
>  		data = IOV_TAIL(iov, cnt, (char *)&bp->data - base);
> -		csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, &data);
> +		csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, &data,
> +			  dlen);
>  	}
>  }
>  
> @@ -229,7 +226,7 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, int n, flow_sidx_t tosidx)
>  		if (iov_cnt > 0) {
>  			udp_vu_prepare(c, iov_vu, toside, dlen);
>  			if (*c->pcap) {
> -				udp_vu_csum(toside, iov_vu, iov_cnt);
> +				udp_vu_csum(toside, iov_vu, iov_cnt, dlen);
>  				pcap_iov(iov_vu, iov_cnt, VNET_HLEN);
>  			}
>  			vu_flush(vdev, vq, elem, elem_used);
> -- 
> 2.53.0
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2026-05-11  1:42 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-16 15:57 [PATCH v3 00/10] vhost-user: Preparatory series for multiple iovec entries per virtqueue element Laurent Vivier
2026-04-16 15:57 ` [PATCH v3 01/10] iov: Introduce iov_memset() Laurent Vivier
2026-04-16 15:57 ` [PATCH v3 02/10] iov: Add iov_memcpy() to copy data between iovec arrays Laurent Vivier
2026-05-11  9:07   ` Jon Maloy
2026-04-16 15:57 ` [PATCH v3 03/10] vu_common: Move vnethdr setup into vu_flush() Laurent Vivier
2026-04-16 15:57 ` [PATCH v3 04/10] udp_vu: Move virtqueue management from udp_vu_sock_recv() to its caller Laurent Vivier
2026-05-11  9:30   ` Jon Maloy
2026-05-11 10:44     ` David Gibson
2026-04-16 15:57 ` [PATCH v3 05/10] udp_vu: Pass iov explicitly to helpers instead of using file-scoped array Laurent Vivier
2026-05-11  9:37   ` Jon Maloy
2026-04-16 15:57 ` [PATCH v3 06/10] checksum: Pass explicit L4 length to checksum functions Laurent Vivier
2026-05-11  1:33   ` David Gibson [this message]
2026-04-16 15:57 ` [PATCH v3 07/10] pcap: Pass explicit L2 length to pcap_iov() Laurent Vivier
2026-05-11  1:42   ` David Gibson
2026-05-11  9:50   ` Jon Maloy
2026-04-16 15:57 ` [PATCH v3 08/10] vu_common: Pass explicit frame length to vu_flush() Laurent Vivier
2026-04-16 15:57 ` [PATCH v3 09/10] tcp: Pass explicit data length to tcp_fill_headers() Laurent Vivier
2026-04-16 15:57 ` [PATCH v3 10/10] vhost-user: Centralise Ethernet frame padding in vu_collect() and vu_pad() Laurent Vivier
2026-05-11  2:01   ` David Gibson
2026-05-11  9:53 ` [PATCH v3 00/10] vhost-user: Preparatory series for multiple iovec entries per virtqueue element Jon Maloy

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=agEx80UEIqk_1_jL@zatzit \
    --to=david@gibson.dropbear.id.au \
    --cc=lvivier@redhat.com \
    --cc=passt-dev@passt.top \
    /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).