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

[James Lentini]

On Wed, 5 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:
> > 
> > > > 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:
<snip> 
> So yes, ICRC is an end-to-end checksum. This is made clear in the
> modinfo description of the parameter.

The ICRC checksum is a fine checksum. Your defining end-to-end as one 
end of an IB network to another. End-to-end in Internet terms is from 
one host to another over many potential networks. The source of a TCP 
packet could be on a IB network and be communicating with a node 
across the globe on a token ring. The TCP checksum is from source to 
destination, end-to-end. If you don't perform the TCP checksum at the 
source, there is no end-to-end checksum.

> > I recommend that be made clear to the user.
> 
> I don't think there's any potential for confusion

There is a potential for confusion. The threads on this topic show 
that. How about naming the module parameter "omit_inet_csums"?

> > > 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.

Ok. Given that, there's no reason to separate them.