Index | Thread | Search

From:
Jeremie Courreges-Anglas <jca@wxcvbn.org>
Subject:
Re: dhcpleased(8): fix pseudo csum from vio(4) offloading
To:
jan@openbsd.org
Cc:
David Gwynne <loki@animata.net>, Alexander Bluhm <alexander.bluhm@gmx.net>, Openbsd Tech <tech@openbsd.org>
Date:
Fri, 26 Jan 2024 22:17:40 +0100

Download raw body.

Thread
On Fri, Jan 26 2024, jan@openbsd.org wrote:
> 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?

Sorry to chime in a bit late...  I think it would be cleaner to avoid
changing the bpf_hdr structure size and exporting the kernel mbuf flag
values to userland.  David introduced a bh_flags member in bpf.h rev
1.69.  Is there a reason why you can't use one of the two remaining
bh_flags bit?  Something like

	#define BPF_F_CKSUM_OK 0x40

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE