Re: NPF: broken checksums

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

Maxime Villard <max%m00nbsd.net@localhost> wrote:
> > <...>
> > 
> > Not sure what your point is. In all cases the check should be
> > !nbuf_cksum_barrier, no?
> 
> So I ended up committing my patch.
> 
> My patch makes sense to me, and with it in place, the packets aren't
> dropped anymore.
> 
> If somehow the original code was intentional and functional, then please
> explain how and why. As I said, my tests showed that the packets were
> systematically dropped, so it didn't work.

It is not the first time we mess up with checksum fixup.  This is because
the "delayed checksum" (i.e. partially pre-computed) logic in our network
stack is convoluted and undocumented.

The right thing to do would be:

- Remove the delayed checksum logic from the network stack.  It's useless
with modern CPUs (let alone hardware checksum offloading by NIC).  AFAIK,
other BSDs removed/simplified it years ago.

- The network stack should provide a clear and simple API for the packet
filters (well, updaters) to check whether they should fixup the checksum
on change or leave the checksum handling for the network stack.

- The idea was to abstract the current mess within nbuf_cksum_barrier(),
but it clearly needs a big comment explaining what's going on with the
network stack logic..

> 
> It looks like there is a 16byte alignment problem somewhere in NPF. RFC793
> says that "An option may begin on any octet boundary", so NPF should
> handle that.
> 

That is odd.  Does not sound like an alignment problem per se, though.

-- 
Mindaugas