On Fri, Jan 26, 2024 at 06:44:26AM +1000, David Gwynne wrote:
> On Fri, 26 Jan 2024, 00:08 Alexander Bluhm, <alexander.bluhm@gmx.net> wrote:
> > 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.
> 
> Has this been tested on any strict alignment architectures? bpf is supposed
> to pad the header, but it's worth checking.

Your are totally right here.  I get a SIGBUS on sparc64 for messing up
the ether_align[2].  I fixed and tested it with the diff below.

ok?

Thanks,
Jan

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	26 Jan 2024 09:50:46 -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	26 Jan 2024 09:50:46 -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	26 Jan 2024 19:21:09 -0000
@@ -287,7 +287,7 @@ struct imsg_propose_rdns {
 struct imsg_dhcp {
 	uint32_t		if_index;
 	ssize_t			len;
-	uint8_t			ether_align[2];
+	uint16_t		csumflags;
 	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	26 Jan 2024 19:27:36 -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	26 Jan 2024 13:55:58 -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: