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 v5 7/9] checksum: introduce functions to compute the header part checksum for TCP/UDP
Date: Mon, 4 Mar 2024 11:52:31 +1100	[thread overview]
Message-ID: <ZeUbTxiwBZFVhNVZ@zatzit> (raw)
In-Reply-To: <20240303135114.1023026-8-lvivier@redhat.com>

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

On Sun, Mar 03, 2024 at 02:51:12PM +0100, Laurent Vivier wrote:
> The TCP and UDP checksums are computed using the data in the TCP/UDP
> payload but also some informations in the IP header (protocol,
> length, source and destination addresses).
> 
> We add two functions, proto_ipv4_header_psum() and
> proto_ipv6_header_psum(), to compute the checksum of the IP
> header part.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

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

> ---
> 
> Notes:
>     v5:
>       - use struct in_addr
>       - update tcp_update_check_tcp4()/tcp_update_check_tcp6()
>         function comment and add tcphdr as a parameter
>     
>     v4:
>       - fix payload length endianness
>     
>     v3:
>       - function parameters provide tot_len, saddr, daddr and protocol
>         rather than an iphdr/ipv6hdr
>     
>     v2:
>       - move new function to checksum.c
>       - use _psum rather than _checksum in the name
>       - replace csum_udp4() and csum_udp6() by the new function
>     
>     v5:
>       - use struct in_addr
>     
>     v4:
>       - fix payload length endianness
>     
>     v3:
>       - function parameters provide tot_len, saddr, daddr and protocol
>         rather than an iphdr/ipv6hdr
>     
>     v2:
>       - move new function to checksum.c
>       - use _psum rather than _checksum in the name
>       - replace csum_udp4() and csum_udp6() by the new function
> 
>  checksum.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
>  checksum.h |  5 ++++
>  tcp.c      | 50 +++++++++++++++++++---------------------
>  udp.c      | 18 ++++++++-------
>  4 files changed, 90 insertions(+), 50 deletions(-)
> 
> diff --git a/checksum.c b/checksum.c
> index ad81a7104a07..7625d7a41303 100644
> --- a/checksum.c
> +++ b/checksum.c
> @@ -139,6 +139,29 @@ uint16_t csum_ip4_header(uint16_t tot_len, uint8_t protocol,
>  	return ~csum_fold(sum);
>  }
>  
> +/**
> + * proto_ipv4_header_psum() - Calculates the partial checksum of an
> + * 			      IPv4 header for UDP or TCP
> + * @tot_len:	IPv4 Payload length (host order)
> + * @proto:	Protocol number (host order)

"host order" is meaningless here, because it's a single byte value.
That can be fixed in followup though.

> + * @saddr:	Source address  (network order)
> + * @daddr:	Destination address (network order)
> + * Returns:	Partial checksum of the IPv4 header
> + */
> +uint32_t proto_ipv4_header_psum(uint16_t tot_len, uint8_t protocol,
> +				struct in_addr saddr, struct in_addr daddr)
> +{
> +	uint32_t psum = htons(protocol);
> +
> +	psum += (saddr.s_addr >> 16) & 0xffff;
> +	psum += saddr.s_addr & 0xffff;
> +	psum += (daddr.s_addr >> 16) & 0xffff;
> +	psum += daddr.s_addr & 0xffff;
> +	psum += htons(tot_len);
> +
> +	return psum;
> +}
> +
>  /**
>   * csum_udp4() - Calculate and set checksum for a UDP over IPv4 packet
>   * @udp4hr:	UDP header, initialised apart from checksum
> @@ -155,14 +178,10 @@ void csum_udp4(struct udphdr *udp4hr,
>  	udp4hr->check = 0;
>  
>  	if (UDP4_REAL_CHECKSUMS) {
> -		/* UNTESTED: if we did want real UDPv4 checksums, this
> -		 * is roughly what we'd need */
> -		uint32_t psum = csum_fold(saddr.s_addr)
> -			+ csum_fold(daddr.s_addr)
> -			+ htons(len + sizeof(*udp4hr))
> -			+ htons(IPPROTO_UDP);
> -		/* Add in partial checksum for the UDP header alone */
> -		psum += sum_16b(udp4hr, sizeof(*udp4hr));
> +		uint16_t tot_len = len + sizeof(struct udphdr);
> +		uint32_t psum = proto_ipv4_header_psum(tot_len, IPPROTO_UDP,
> +						       saddr, daddr);
> +		psum = csum_unfolded(udp4hr, sizeof(struct udphdr), psum);
>  		udp4hr->check = csum(payload, len, psum);
>  	}
>  }
> @@ -185,6 +204,27 @@ void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len)
>  	icmp4hr->checksum = csum(payload, len, psum);
>  }
>  
> +/**
> + * proto_ipv6_header_psum() - Calculates the partial checksum of an
> + * 			      IPv6 header for UDP or TCP
> + * @payload_len:	IPv6 payload length (host order)
> + * @proto:		Protocol number (host order)
> + * @saddr:		Source address (network order)
> + * @daddr:		Destination address (network order)
> + * Returns:	Partial checksum of the IPv6 header
> + */
> +uint32_t proto_ipv6_header_psum(uint16_t payload_len, uint8_t protocol,
> +				const struct in6_addr *saddr,
> +				const struct in6_addr *daddr)
> +{
> +	uint32_t sum = htons(protocol) + htons(payload_len);
> +
> +	sum += sum_16b(saddr, sizeof(*saddr));
> +	sum += sum_16b(daddr, sizeof(*daddr));
> +
> +	return sum;
> +}
> +
>  /**
>   * csum_udp6() - Calculate and set checksum for a UDP over IPv6 packet
>   * @udp6hr:	UDP header, initialised apart from checksum
> @@ -195,14 +235,11 @@ void csum_udp6(struct udphdr *udp6hr,
>  	       const struct in6_addr *saddr, const struct in6_addr *daddr,
>  	       const void *payload, size_t len)
>  {
> -	/* Partial checksum for the pseudo-IPv6 header */
> -	uint32_t psum = sum_16b(saddr, sizeof(*saddr)) +
> -		        sum_16b(daddr, sizeof(*daddr)) +
> -		        htons(len + sizeof(*udp6hr)) + htons(IPPROTO_UDP);
> -
> +	uint32_t psum = proto_ipv6_header_psum(len + sizeof(struct udphdr),
> +					       IPPROTO_UDP, saddr, daddr);
>  	udp6hr->check = 0;
> -	/* Add in partial checksum for the UDP header alone */
> -	psum += sum_16b(udp6hr, sizeof(*udp6hr));
> +
> +	psum = csum_unfolded(udp6hr, sizeof(struct udphdr), psum);
>  	udp6hr->check = csum(payload, len, psum);
>  }
>  
> diff --git a/checksum.h b/checksum.h
> index e990fefa7f39..d4e563c85f4f 100644
> --- a/checksum.h
> +++ b/checksum.h
> @@ -15,10 +15,15 @@ uint16_t csum_fold(uint32_t sum);
>  uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init);
>  uint16_t csum_ip4_header(uint16_t tot_len, uint8_t protocol,
>  			 struct in_addr saddr, struct in_addr daddr);
> +uint32_t proto_ipv4_header_psum(uint16_t tot_len, 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,
>  	       const void *payload, size_t len);
>  void csum_icmp4(struct icmphdr *ih, const void *payload, size_t len);
> +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,
>  	       const void *payload, size_t len);
> diff --git a/tcp.c b/tcp.c
> index 7fb9dba95a6a..53da8b84780f 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -937,41 +937,33 @@ static void tcp_sock_set_bufsize(const struct ctx *c, int s)
>  
>  /**
>   * tcp_update_check_tcp4() - Update TCP checksum from stored one
> - * @buf:	L2 packet buffer with final IPv4 header
> + * @iph:	IPv4 header
> + * @th:		TCP header followed by TCP payload
>   */
> -static void tcp_update_check_tcp4(struct tcp4_l2_buf_t *buf)
> +static void tcp_update_check_tcp4(struct iphdr *iph, struct tcphdr *th)
>  {
> -	uint16_t tlen = ntohs(buf->iph.tot_len) - 20;
> -	uint32_t sum = htons(IPPROTO_TCP);
> +	uint16_t tlen = ntohs(iph->tot_len) - sizeof(struct iphdr);
> +	uint32_t sum = proto_ipv4_header_psum(tlen, IPPROTO_TCP,
> +				(struct in_addr){ .s_addr = iph->saddr },
> +				(struct in_addr){ .s_addr = iph->daddr });
>  
> -	sum += (buf->iph.saddr >> 16) & 0xffff;
> -	sum += buf->iph.saddr & 0xffff;
> -	sum += (buf->iph.daddr >> 16) & 0xffff;
> -	sum += buf->iph.daddr & 0xffff;
> -	sum += htons(ntohs(buf->iph.tot_len) - 20);
> -
> -	buf->th.check = 0;
> -	buf->th.check = csum(&buf->th, tlen, sum);
> +	th->check = 0;
> +	th->check = csum(th, tlen, sum);
>  }
>  
>  /**
>   * tcp_update_check_tcp6() - Calculate TCP checksum for IPv6
> - * @buf:	L2 packet buffer with final IPv6 header
> + * @ip6h:	IPv6 header
> + * @th:		TCP header followed by TCP payload
>   */
> -static void tcp_update_check_tcp6(struct tcp6_l2_buf_t *buf)
> +static void tcp_update_check_tcp6(struct ipv6hdr *ip6h, struct tcphdr *th)
>  {
> -	int len = ntohs(buf->ip6h.payload_len) + sizeof(struct ipv6hdr);
> -
> -	buf->ip6h.hop_limit = IPPROTO_TCP;
> -	buf->ip6h.version = 0;
> -	buf->ip6h.nexthdr = 0;
> +	uint16_t payload_len = ntohs(ip6h->payload_len);
> +	uint32_t sum = proto_ipv6_header_psum(payload_len, IPPROTO_TCP,
> +					      &ip6h->saddr, &ip6h->daddr);
>  
> -	buf->th.check = 0;
> -	buf->th.check = csum(&buf->ip6h, len, 0);
> -
> -	buf->ip6h.hop_limit = 255;
> -	buf->ip6h.version = 6;
> -	buf->ip6h.nexthdr = IPPROTO_TCP;
> +	th->check = 0;
> +	th->check = csum(th, payload_len, sum);
>  }
>  
>  /**
> @@ -1383,7 +1375,7 @@ do {									\
>  
>  		SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq);
>  
> -		tcp_update_check_tcp4(b);
> +		tcp_update_check_tcp4(&b->iph, &b->th);
>  
>  		tlen = tap_iov_len(c, &b->taph, ip_len);
>  	} else {
> @@ -1402,7 +1394,11 @@ do {									\
>  
>  		SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq);
>  
> -		tcp_update_check_tcp6(b);
> +		tcp_update_check_tcp6(&b->ip6h, &b->th);
> +
> +		b->ip6h.hop_limit = 255;
> +		b->ip6h.version = 6;
> +		b->ip6h.nexthdr = IPPROTO_TCP;
>  
>  		b->ip6h.flow_lbl[0] = (conn->sock >> 16) & 0xf;
>  		b->ip6h.flow_lbl[1] = (conn->sock >> 8) & 0xff;
> diff --git a/udp.c b/udp.c
> index fb8373beba40..2fd67925f368 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -625,6 +625,7 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport,
>  {
>  	struct udp6_l2_buf_t *b = &udp6_l2_buf[n];
>  	const struct in6_addr *src, *dst;
> +	uint16_t payload_len;
>  	in_port_t src_port;
>  	size_t ip_len;
>  
> @@ -634,7 +635,8 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport,
>  
>  	ip_len = udp6_l2_mh_sock[n].msg_len + sizeof(b->ip6h) + sizeof(b->uh);
>  
> -	b->ip6h.payload_len = htons(udp6_l2_mh_sock[n].msg_len + sizeof(b->uh));
> +	payload_len = udp6_l2_mh_sock[n].msg_len + sizeof(b->uh);
> +	b->ip6h.payload_len = htons(payload_len);
>  
>  	if (IN6_IS_ADDR_LINKLOCAL(src)) {
>  		dst = &c->ip6.addr_ll_seen;
> @@ -670,17 +672,17 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport,
>  	}
>  	b->ip6h.daddr = *dst;
>  	b->ip6h.saddr = *src;
> +	b->ip6h.version = 6;
> +	b->ip6h.nexthdr = IPPROTO_UDP;
> +	b->ip6h.hop_limit = 255;
>  
>  	b->uh.source = b->s_in6.sin6_port;
>  	b->uh.dest = htons(dstport);
>  	b->uh.len = b->ip6h.payload_len;
> -
> -	b->ip6h.hop_limit = IPPROTO_UDP;
> -	b->ip6h.version = b->ip6h.nexthdr = b->uh.check = 0;
> -	b->uh.check = csum(&b->ip6h, ip_len, 0);
> -	b->ip6h.version = 6;
> -	b->ip6h.nexthdr = IPPROTO_UDP;
> -	b->ip6h.hop_limit = 255;
> +	b->uh.check = 0;
> +	b->uh.check = csum(&b->uh, payload_len,
> +			   proto_ipv6_header_psum(payload_len, IPPROTO_UDP,
> +						  src, dst));
>  
>  	return tap_iov_len(c, &b->taph, ip_len);
>  }

-- 
David Gibson			| 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:[~2024-03-04  0:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-03 13:51 [PATCH v5 0/9] Add vhost-user support to passt (part 1) Laurent Vivier
2024-03-03 13:51 ` [PATCH v5 1/9] pcap: add pcap_iov() Laurent Vivier
2024-03-03 13:51 ` [PATCH v5 2/9] checksum: align buffers Laurent Vivier
2024-03-03 13:51 ` [PATCH v5 3/9] checksum: add csum_iov() Laurent Vivier
2024-03-03 13:51 ` [PATCH v5 4/9] util: move IP stuff from util.[ch] to ip.[ch] Laurent Vivier
2024-03-03 13:51 ` [PATCH v5 5/9] udp: little cleanup in udp_update_hdrX() to prepare future changes Laurent Vivier
2024-03-04  0:46   ` David Gibson
2024-03-03 13:51 ` [PATCH v5 6/9] checksum: use csum_ip4_header() in udp.c and tcp.c Laurent Vivier
2024-03-03 13:51 ` [PATCH v5 7/9] checksum: introduce functions to compute the header part checksum for TCP/UDP Laurent Vivier
2024-03-04  0:52   ` David Gibson [this message]
2024-03-03 13:51 ` [PATCH v5 8/9] tap: make tap_update_mac() generic Laurent Vivier
2024-03-03 13:51 ` [PATCH v5 9/9] tcp: Introduce ipv4_fill_headers()/ipv6_fill_headers() Laurent Vivier
2024-03-04  0:54   ` David Gibson
2024-03-05 22:12 ` [PATCH v5 0/9] Add vhost-user support to passt (part 1) Stefano Brivio
2024-03-06  3:22   ` 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=ZeUbTxiwBZFVhNVZ@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).