On Sun, Jan 07, 2024 at 01:38:23PM +0100, Jan Klemkow wrote:
> Hi,
> 
> we get packets with pseudo checksums from vio(4), since we use checksum
> offloading.  Thus, programs which are dealing with raw packets like
> dhcpleased(8) also need to check for this kind of checksums or return
> errors.
> 
> ok?

I think kernel should fix this.  If we have devices like vio(4)
that report checksum OK, but we know that the ckecksum field is
wrong, and we pass the packet header to user land, kernel should
calculate correct checksum.  At least divert_packet() does a trick
like this.  For raw sockets I would prefer something like this.

Another possibility would be to pass a flag in bpf header that
checksum is OK.  Then dhcpleased could skip the checksum check.
That would also help tcpdump complaining about bad checksums in bpf
packets.

As dhcpleased uses bpf, I think the second option is the way to go.

bluhm

> Index: checksum.c
> ===================================================================
> RCS file: /cvs/src/sbin/dhcpleased/checksum.c,v
> diff -u -p -r1.1 checksum.c
> --- checksum.c	26 Feb 2021 16:16:37 -0000	1.1
> +++ checksum.c	7 Jan 2024 12:17:23 -0000
> @@ -78,3 +78,26 @@ wrapsum(uint32_t sum)
>  	sum = ~sum & 0xFFFF;
>  	return htons(sum);
>  }
> +
> +/*
> + *	Compute significant parts of the IPv4 checksum pseudo-header
> + *	for use in a delayed TCP/UDP checksum calculation.
> + */
> +uint16_t
> +checksum_phdr(uint32_t src, uint32_t dst, uint32_t lenproto)
> +{
> +	uint32_t sum;
> +
> +	sum = lenproto +
> +	    (uint16_t)(src >> 16) +
> +	    (uint16_t)(src /*& 0xffff*/) +
> +	    (uint16_t)(dst >> 16) +
> +	    (uint16_t)(dst /*& 0xffff*/);
> +
> +	sum = (uint16_t)(sum >> 16) + (uint16_t)(sum /*& 0xffff*/);
> +
> +	if (sum > 0xffff)
> +		sum -= 0xffff;
> +
> +	return (sum);
> +}
> Index: checksum.h
> ===================================================================
> RCS file: /cvs/src/sbin/dhcpleased/checksum.h,v
> diff -u -p -r1.1 checksum.h
> --- checksum.h	26 Feb 2021 16:16:37 -0000	1.1
> +++ checksum.h	7 Jan 2024 12:17:23 -0000
> @@ -42,3 +42,4 @@
>  
>  uint32_t	 checksum(uint8_t *, uint32_t, uint32_t);
>  uint32_t	 wrapsum(uint32_t);
> +uint16_t	 checksum_phdr(uint32_t, uint32_t, uint32_t);
> Index: engine.c
> ===================================================================
> RCS file: /cvs/src/sbin/dhcpleased/engine.c,v
> diff -u -p -r1.41 engine.c
> --- engine.c	14 Dec 2023 09:58:37 -0000	1.41
> +++ engine.c	7 Jan 2024 12:17:23 -0000
> @@ -729,7 +729,7 @@ parse_dhcp(struct dhcpleased_iface *ifac
>  	struct in_addr		 nameservers[MAX_RDNS_COUNT];
>  	struct dhcp_route	 routes[MAX_DHCP_ROUTES];
>  	size_t			 rem, i;
> -	uint32_t		 sum, usum, lease_time = 0, renewal_time = 0;
> +	uint32_t		 sum, usum, psum, lease_time = 0, renewal_time = 0;
>  	uint32_t		 rebinding_time = 0;
>  	uint32_t		 ipv6_only_time = 0;
>  	uint8_t			*p, dho = DHO_PAD, dho_len, slen;
> @@ -841,7 +841,10 @@ parse_dhcp(struct dhcpleased_iface *ifac
>  	    checksum((uint8_t *)&ip->ip_src, 2 * sizeof(ip->ip_src),
>  	    IPPROTO_UDP + ntohs(udp->uh_ulen)))));
>  
> -	if (usum != 0 && usum != sum) {
> +	psum = checksum_phdr(ip->ip_src.s_addr, ip->ip_dst.s_addr,
> +	    htonl(ntohs(ip->ip_len) - (ip->ip_hl << 2) + ip->ip_p));
> +
> +	if (usum != 0 && usum != sum && usum != psum) {
>  		log_warnx("%s: bad UDP checksum", __func__);
>  		return;
>  	}