Re: [PATCHv4] blk-integrity: support arbitrary buffer alignment

[Keith Busch <kbusch@kernel.org>]

On Thu, Nov 13, 2025 at 07:20:22PM +0000, Eric Biggers wrote:
> On Thu, Nov 13, 2025 at 01:14:13PM -0500, Keith Busch wrote:
> > On Thu, Nov 13, 2025 at 09:31:35AM -0800, Eric Biggers wrote:
> > > On Thu, Nov 13, 2025 at 07:26:21AM -0800, Keith Busch wrote:
> > > > +static void blk_set_ip_pi(struct t10_pi_tuple *pi,
> > > > +			  struct blk_integrity_iter *iter)
> > > >  {
> > > > -	u8 offset = bi->pi_offset;
> > > > -	unsigned int i;
> > > > -
> > > > -	for (i = 0 ; i < iter->data_size ; i += iter->interval) {
> > > > -		struct crc64_pi_tuple *pi = iter->prot_buf + offset;
> > > > +	__be16 csum = (__force __be16)~(lower_16_bits(iter->crc));
> > > 
> > > This just throws away half of the checksum instead of properly combining
> > > the two halves.  How is this being tested?
> > 
> > Yeah, this is the only guard type I've never seen a device subscribe to,
> > so not particularly easily tested on my side. I just forced the code
> > path down here anyway and checked if the result matches the result from
> > the existing code calling "ip_compute_csum()". Maybe I can just continue
> > using that as I suspect devices using that can't handle split data
> > intervals that I'm trying to enable.
> 
> Wouldn't csum_fold() combine the halves correctly?

It doesn't look like that would be correct if we assume the existing
code is correct. The current result from ip_compute_csum() just
downcasts the result without folding, just like I'm doing here. My new
code is produces the same result as the existing code, so worst case
scenario, this isn't introducing a regression.

> Anyway, it needs to be tested.

Like on real hardware? I'm a bit at a loss as to how, I've never seen
anything subscribe to this format, not even in emulation. The only thing
I can readily do to test this is run random data through the old code,
print the result, then run the same data through the new code and see if
they're the same. That test is successful. Not good enough?