Re: [PATCH] iscsi_iser: Re-enable 'iser_pi_guard' module parameter

Martin K. Petersen
2018-01-17T05:09:38Z

Hannes,

> Do we need to keep the parameter?
> If so we really should be re-enable it; ATM it's just a no-op leading
> the user to believe something has actually happened.

The driver module parameters existed mainly for the purpose of hw
testing. One could disable IP checksum and revert to CRC to ensure both
code paths were working properly. But it wasn't really meant to be
something ordinary users would ever muck with. Why would anybody use the
much slower CRC when IP checksum was supported by the hardware? (*)

The reason the kernel interface changed from a per-driver to a per-I/O
flag for checksum selection was that there are a few things you can't
express when checksum conversion is taking place. For instance, say
you're writing a RAID parity disk. The 8 additional bytes on the parity
disk are not valid T10 PI but rather the XOR of the PI across the
stripe. So you need to be able to disable checking and conversion on a
per-I/O basis when accessing the parity disk. Deliberately writing "bad"
PI for test purposes is another example where conversion must be
disabled.

Anyway. I think Sagi's iser_pi_guard patch was a result of that
transition from driver global checksum setting to per-I/O ditto.

I don't have a problem with having a block-level preference knob
(slightly convoluted to implement since they are different integrity
profiles). But I do question whether it is worth the hassle. What
exactly is your use case that requires twiddling this?

(*) The modern PCLMULQDQ-optimized CRC calculation blurs that picture
slightly.

--
Martin K. Petersen Oracle Linux Engineering