From: David Gwynne Subject: Re: dhcpleased(8): fix pseudo csum from vio(4) offloading To: Alexander Bluhm Cc: Jan Klemkow , Openbsd Tech Date: Fri, 26 Jan 2024 06:44:26 +1000 Has this been tested on any strict alignment architectures? bpf is supposed to pad the header, but it's worth checking. On Fri, 26 Jan 2024, 00:08 Alexander Bluhm, 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. > > > > 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 > > #include > > #include > > +#include > > > > #include > > #include > > @@ -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: > >