On Fri, Jan 26, 2024 at 10:17:40PM +0100, Jeremie Courreges-Anglas wrote:
> 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

If we go this way, we have to define three macros:

#define BPF_F_IP4_CKSUM_OK	0x040
#define BPF_F_TCP_CKSUM_OK	0x080
#define BPF_F_UDP_CKSUM_OK	0x100

To differentiate between them, cause not every driver supports all
checksums.  And we also need code to translate the mbuf macros into this
new one.

I would suggest to keep it simple an just use the mbuf flags and macros.

bye,
Jan