On Thu, Jan 25, 2024 at 02:49:30PM +0100, Jan Klemkow wrote:
> On Sun, Jan 07, 2024 at 09:46:22PM +0100, Alexander Bluhm wrote:
> > On Sun, Jan 07, 2024 at 01:38:23PM +0100, Jan Klemkow wrote:
> > > 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.
> > 
> > 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.
> 
> Sounds like a better approach.
> 
> ok?

OK bluhm@

> Index: sys/net/bpf.c
> ===================================================================
> RCS file: /cvs/src/sys/net/bpf.c,v
> diff -u -p -r1.221 bpf.c
> --- sys/net/bpf.c	9 Mar 2023 05:56:58 -0000	1.221
> +++ sys/net/bpf.c	25 Jan 2024 13:34:17 -0000
> @@ -1397,6 +1397,8 @@ _bpf_mtap(caddr_t arg, const struct mbuf
>  					if (ISSET(mp->m_pkthdr.csum_flags,
>  					    M_FLOWID))
>  						SET(tbh.bh_flags, BPF_F_FLOWID);
> +					tbh.bh_csumflags =
> +					    mp->m_pkthdr.csum_flags;
>  
>  					m_microtime(mp, &tv);
>  				} else
> Index: sys/net/bpf.h
> ===================================================================
> RCS file: /cvs/src/sys/net/bpf.h,v
> diff -u -p -r1.71 bpf.h
> --- sys/net/bpf.h	9 Mar 2023 05:56:58 -0000	1.71
> +++ sys/net/bpf.h	25 Jan 2024 13:34:17 -0000
> @@ -161,6 +161,7 @@ struct bpf_hdr {
>  #define BPF_F_DIR_IN		(BPF_DIRECTION_IN << BPF_F_DIR_SHIFT)
>  #define BPF_F_DIR_OUT		(BPF_DIRECTION_OUT << BPF_F_DIR_SHIFT)
>  	u_int8_t	bh_drops;
> +	u_int16_t	bh_csumflags;	/* checksum flags */
>  };
>  
>  #ifdef _KERNEL
> Index: sbin/dhcpleased/dhcpleased.h
> ===================================================================
> RCS file: /cvs/src/sbin/dhcpleased/dhcpleased.h,v
> diff -u -p -r1.15 dhcpleased.h
> --- sbin/dhcpleased/dhcpleased.h	25 Nov 2023 12:00:39 -0000	1.15
> +++ sbin/dhcpleased/dhcpleased.h	25 Jan 2024 13:34:18 -0000
> @@ -287,6 +287,7 @@ struct imsg_propose_rdns {
>  struct imsg_dhcp {
>  	uint32_t		if_index;
>  	ssize_t			len;
> +	uint16_t		csumflags;
>  	uint8_t			ether_align[2];
>  	uint8_t			packet[1500];
>  };
> Index: sbin/dhcpleased/engine.c
> ===================================================================
> RCS file: /cvs/src/sbin/dhcpleased/engine.c,v
> diff -u -p -r1.41 engine.c
> --- sbin/dhcpleased/engine.c	14 Dec 2023 09:58:37 -0000	1.41
> +++ sbin/dhcpleased/engine.c	25 Jan 2024 13:34:18 -0000
> @@ -24,6 +24,7 @@
>  #include <sys/socket.h>
>  #include <sys/syslog.h>
>  #include <sys/uio.h>
> +#include <sys/mbuf.h>
>  
>  #include <net/if.h>
>  #include <net/route.h>
> @@ -788,7 +789,8 @@ parse_dhcp(struct dhcpleased_iface *ifac
>  	if (rem < (size_t)ip->ip_hl << 2)
>  		goto too_short;
>  
> -	if (wrapsum(checksum((uint8_t *)ip, ip->ip_hl << 2, 0)) != 0) {
> +	if ((dhcp->csumflags & M_IPV4_CSUM_IN_OK) == 0 &&
> +	    wrapsum(checksum((uint8_t *)ip, ip->ip_hl << 2, 0)) != 0) {
>  		log_warnx("%s: bad IP checksum", __func__);
>  		return;
>  	}
> @@ -834,16 +836,19 @@ parse_dhcp(struct dhcpleased_iface *ifac
>  	p += sizeof(*udp);
>  	rem -= sizeof(*udp);
>  
> -	usum = udp->uh_sum;
> -	udp->uh_sum = 0;
> -
> -	sum = wrapsum(checksum((uint8_t *)udp, sizeof(*udp), checksum(p, rem,
> -	    checksum((uint8_t *)&ip->ip_src, 2 * sizeof(ip->ip_src),
> -	    IPPROTO_UDP + ntohs(udp->uh_ulen)))));
> -
> -	if (usum != 0 && usum != sum) {
> -		log_warnx("%s: bad UDP checksum", __func__);
> -		return;
> +	if ((dhcp->csumflags & M_UDP_CSUM_IN_OK) == 0) {
> +		usum = udp->uh_sum;
> +		udp->uh_sum = 0;
> +
> +		sum = wrapsum(checksum((uint8_t *)udp, sizeof(*udp),
> +		    checksum(p, rem,
> +		    checksum((uint8_t *)&ip->ip_src, 2 * sizeof(ip->ip_src),
> +		    IPPROTO_UDP + ntohs(udp->uh_ulen)))));
> +
> +		if (usum != 0 && usum != sum) {
> +			log_warnx("%s: bad UDP checksum", __func__);
> +			return;
> +		}
>  	}
>  
>  	if (log_getverbose() > 1) {
> Index: sbin/dhcpleased/frontend.c
> ===================================================================
> RCS file: /cvs/src/sbin/dhcpleased/frontend.c,v
> diff -u -p -r1.32 frontend.c
> --- sbin/dhcpleased/frontend.c	14 Dec 2023 09:58:37 -0000	1.32
> +++ sbin/dhcpleased/frontend.c	25 Jan 2024 13:34:18 -0000
> @@ -890,6 +890,7 @@ bpf_receive(int fd, short events, void *
>  		}
>  		memcpy(&imsg_dhcp.packet, p + hdr->bh_hdrlen, hdr->bh_caplen);
>  		imsg_dhcp.len = hdr->bh_caplen;
> +		imsg_dhcp.csumflags = hdr->bh_csumflags;
>  		frontend_imsg_compose_engine(IMSG_DHCP, 0, 0, &imsg_dhcp,
>  		    sizeof(imsg_dhcp));
>   cont: