Re: [PATCH 03/14] Add csum_udp6() helper for calculating UDP over IPv6 checksums

[Stefano Brivio <sbrivio@redhat.com>]

On Mon, 17 Oct 2022 19:57:56 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Add a helper for calculating UDP checksums when used over IPv6
> For future flexibility, the new helper takes parameters for the fields in
> the IPv6 pseudo-header, so an IPv6 header or pseudo-header doesn't need to
> be explicitly constructed.  It also allows the UDP header and payload to
> be in separate buffers, although we don't use this yet.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  checksum.c | 23 +++++++++++++++++++++++
>  checksum.h |  5 +++++
>  tap.c      |  5 ++---
>  3 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/checksum.c b/checksum.c
> index c8b6b42..0849fb1 100644
> --- a/checksum.c
> +++ b/checksum.c
> @@ -52,6 +52,7 @@
>  #include <stddef.h>
>  #include <stdint.h>
>  
> +#include <linux/udp.h>
>  #include <linux/icmp.h>
>  #include <linux/icmpv6.h>
>  
> @@ -122,6 +123,28 @@ void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len)
>  	icmp4hr->checksum = csum_unaligned(payload, len, hrsum);
>  }
>  
> +/**
> + * csum_udp6() - Calculate checksum for a UDP over IPv6 packet

Calculate and set.

> + * @udp6hr:	UDP header, initialized apart from checksum

-ised.

> + * @payload:	UDP packet payload
> + * @len:	Length of @payload (not including UDP header)
> + */
> +void csum_udp6(struct udphdr *udp6hr,
> +	       const struct in6_addr *saddr,
> +	       const struct in6_addr *daddr,

You could use some horizontal space.

> +	       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);

Alignment:

	uint32_t psum = sum_16b(saddr, sizeof(*saddr)) +
			sum_16b(daddr, sizeof(*daddr)) +
			htons(len + sizeof(*udp6hr))   + htons(IPPROTO_UDP);

> +	udp6hr->check = 0;
> +	/* Add in partial checksum for the UDP header alone */
> +	psum += sum_16b(udp6hr, sizeof(*udp6hr));
> +	udp6hr->check = csum_unaligned(payload, len, psum);
> +}
> +
>  /**
>   * csum_icmp6() - Calculate checksum for an ICMPv6 packet
>   * @icmp6hr:	ICMPv6 header, initialized apart from checksum
> diff --git a/checksum.h b/checksum.h
> index ff95cf9..1b9f48e 100644
> --- a/checksum.h
> +++ b/checksum.h
> @@ -6,6 +6,7 @@
>  #ifndef CHECKSUM_H
>  #define CHECKSUM_H
>  
> +struct udphdr;
>  struct icmphdr;
>  struct icmp6hdr;
>  
> @@ -13,6 +14,10 @@ uint32_t sum_16b(const void *buf, size_t len);
>  uint16_t csum_fold(uint32_t sum);
>  uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init);
>  void csum_icmp4(struct icmphdr *ih, const void *payload, size_t len);
> +void csum_udp6(struct udphdr *udp6hr,
> +	       const struct in6_addr *saddr,
> +	       const struct in6_addr *daddr,
> +	       const void *payload, size_t len);

Use some horizontal space.

>  void csum_icmp6(struct icmp6hdr *ih,
>  		const struct in6_addr *saddr,
>  		const struct in6_addr *daddr,
> diff --git a/tap.c b/tap.c
> index f082901..9c197cb 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -183,9 +183,8 @@ void tap_ip_send(const struct ctx *c, const struct in6_addr *src, uint8_t proto,
>  		} else if (proto == IPPROTO_UDP) {
>  			struct udphdr *uh = (struct udphdr *)(ip6h + 1);
>  
> -			uh->check = 0;
> -			uh->check = csum_unaligned(ip6h, len + sizeof(*ip6h),
> -						   0);
> +			csum_udp6(uh, &ip6h->saddr, &ip6h->daddr,
> +				  uh + 1, len - sizeof(*uh));
>  		} else if (proto == IPPROTO_ICMPV6) {
>  			struct icmp6hdr *ih = (struct icmp6hdr *)(ip6h + 1);
>  

-- 
Stefano