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

[Michael S. Tsirkin]

> Quoting James Lentini <jlentini at netapp.com>:
> Subject: Re: [PATCHv2] IB/ipoib: S/G and HW checksum support
> 
> 
> 
> On Tue, 4 Sep 2007, Michael S. Tsirkin wrote:
> 
> > > Quoting James Lentini <jlentini at netapp.com>:
> > > Subject: Re: [PATCHv2] IB/ipoib: S/G and HW checksum support
> > > 
> > > 
> > > 
> > > On Tue, 4 Sep 2007, Michael S. Tsirkin wrote:
> > > 
> > > > Add module option hw_csum: when set, IPoIB will report S/G
> > > > support, and rely on hardware end-to-end transport checksum (ICRC)
> > > > instead of software-level protocol checksums.
> > > 
> > > The purpose of this option would be clearer if the parameter name were 
> > > "omit_csum". Calling this "HW checksum" support is misleading because 
> > > the term is already used to describe network adapters that calculate 
> > > TCP/IP checksums in hardware. I realize that you are using the HW 
> > > checksum infrastructure to implement this, but it is really not the 
> > > same thing.
> > 
> > Another reason is that I declare HW_CSUM in the netdev
> > feature list. Yea, someone might get confused,
> > but "omit checksum" is misleading, too, and is likely to
> > scare users away from the feature: the need for end-to-end checksum
> > is a widely recognised requirement. 
> 
> I agree. Since this isn't an end-to-end checksum,

IB spec says:

There are two CRCs in each packet. The Invariant CRC (ICRC) covers
all fields which should not change as the packet traverses the fabric. The
Variant CRC (VCRC) covers all of the fields of the packet. The combination
of the two CRCs allow switches and routers to modify appropriate
fields and still maintain an end to end data integrity for the transport control
and data portion of the packet. The coverage of the ICRC is different
depending on whether the packet is routed to another subnet (i.e. contains
a global route header).

So yes, ICRC is an end-to-end checksum. This is made clear in the
modinfo description of the parameter.

> I recommend that be made clear to the user.

I don't think there's any potential for confusion: ICRC is end to end, it is
not a link level checksum.  The crowd using infiniband seems happy enough to
rely on ICRC for transport and data integrity checks:
SDP, MPI, SRP, and other protocols do so.

> > So I don't have a better name. Hopefully modinfo documents the 
> > option well enough.
> > 
> > > > Since this will not inter-operate with older IPoIB modules, this 
> > > > option is off by default.
> > > > 
> > > > Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>
> > > 
> > > Does the S/G support need to be tied to the checksum changes?
> 
> Can you separate the S/G support and checksum changes into different 
> patches?

Oh, just cut the relevant hunks from the patch, but I don't see why this is useful, since
S/G support in linux does not work without hardware checksumming.

-- 
MST