[ofa-general] [PATCHv2] IB/ipoib: S/G and HW checksum support

[Jason Gunthorpe]

On Tue, Sep 04, 2007 at 08:48:43PM +0300, Michael S. Tsirkin wrote:
> > Aren't you already summing all UD packets (inclusing multicast) in the
> > driver before sending? Though, to be honest, I don't see this in your
> > patch, so maybe not..
> 
> no

Yuk. Sending invalid UD packets is horrible. So it truely is all or
nothing. Every gateway, embedded IP device, etc must support this or
you cannot use it..

> > > > What if the RXing Linux IB side is acting as a forwarder to ethernet?
> > > > It will forward corrupt packets if this option is set, right?
> > > 
> > > No. The checksum will be calculated by the gateway before being sent on the
> > > ethernet interface.
> > 
> > I thought linux only recomputed the checksum on the forwarding path if
> > the skb was marked as needing checksum. Since you set the skb as
> > already summed, there should be cases where invalid packets will be
> > forwarded..
> 
> I don't set CHECKSUM_UNECESSARY, so linux will have to recompute the checksum.

Eh? You set IPOIB_HEADER_F_HWCSUM on the TX path if the csum is
invalid and the test that on the rx path to set CHECKSUM_UNNECESSARY.
So, all badly csumed packets have CHECKSUM_UNECESSARY set.

> > I'd be surprised if a real, hardware, IPoIB to XX device recomputed
> > the checksum unconditionally. The typical approach is to do an
> > incremental update of the checksum if you are changing the packet
> > headers. This preserves the end-to-end-ness and also does not require
> > buffering the entire packet before updating it.

I was talking about real, existing HW gateways here, not Linux. I
don't know of any reason a gateway would even touch the payload and
require a L4 checksum update, let alone doing it non-incrementally..

> Since skb is not marked with CHECKSUM_COMPLETE, linux will
> recompute the checksum IMO.

MM, no, I don't think so. This checksum stuff is all about the L4
TCP/UDP checksum. If on RX the checksum is invalid the packet is
dumped and it never gets into the forwarding code (ip_forward
routine). Up until very recently ip_fowrard just unconditionally set
ip_summed to CHECKSUM_NONE to reflect this. Of course CHECKSUM_NONE
disables all checksum updates on the driver TX path.

FWIW, CHECKSUM_COMPLETE is listed as an rx path option, no in tree
driver tests it on the TX path. You should probably be using
ip_summed != CHECKSUM_PARTIAL as a test in hard_header.

With the new changes to ip_forward, maybe you could get away with
setting CHECKSUM_PARTIAL in your RX path to get the TX of the final
output device to regenerate the L4 checksum?

Even so, sending out malformed UD packets strikes me as a
compatability killer.. This would be much better as a RC only
negotiated at CM feature.

Jason