Re: [PATCH 0/2] lio-target: Add support for libcrypto crc32c and crc32-intel offload

Previous thread: [Bug 18252] spinlock lockup in __make_request <- submit_bio <- ondemand_readahead by Stefan Richter on Saturday, September 11, 2010 - 2:50 am. (5 messages)

Next thread: [PATCH 1/2] lio-target: Convert to use libcrypto crc32c by Nicholas A. Bellinger on Saturday, September 11, 2010 - 3:31 am. (1 message)
From: Nicholas A. Bellinger
Date: Saturday, September 11, 2010 - 3:31 am

From: Nicholas Bellinger &lt;nab@linux-iscsi.org&gt;

Greetings all,

This patch series converts the LIO-Target fabric module from using a legacy
internal slicing by 1x CRC32C algorithm to the slicing by 1x CRC32C available in
crc32c.ko, as well as initial support for the Nehalem series crc32c-intel.ko
instruction offload available since v2.6.27 in late 2008.

So far this series has been lightly tested with a handful of Open-iSCSI client VMs
with the optimized crc32c-intel.ko offload case, and there appears to be some
HeaderDigest failures for one case with RHEL6 B2 x86_64, while Ubuntu i686 on
v2.6.27 and OpenSuse 11.2 x86_64 on v2.6.31 work as expected with HeaderDigest=CRC32C
and DataDigest=CRC32C using the offload on the LIO-Target side.

Currently this patch disables the new iSCSI TPG attribute crc32c_x86_offload
to force the crc32c.ko slicing by x1 CRC32C algorithm until these compatibility
issues with existing libcrc32c clients can be properly resolved with the Nehalem
CRC32C offload instructions running on the LIO-Target side.

Comments are welcome!

Signed-off-by: Nicholas A. Bellinger &lt;nab@linux-iscsi.org&gt;

Nicholas Bellinger (2):
  lio-target: Convert to use libcrypto crc32c
  lio-target: Add support for Intel Nehalem crc32-intel libcrypto
    offload

 drivers/target/lio-target/Kbuild                  |    1 -
 drivers/target/lio-target/iscsi_crc.c             |  171 ------------
 drivers/target/lio-target/iscsi_crc.h             |    9 -
 drivers/target/lio-target/iscsi_target.c          |  302 ++++++++++++++++-----
 drivers/target/lio-target/iscsi_target_configfs.c |    6 +
 drivers/target/lio-target/iscsi_target_core.h     |    7 +
 drivers/target/lio-target/iscsi_target_erl1.c     |    1 -
 drivers/target/lio-target/iscsi_target_erl2.c     |    1 -
 drivers/target/lio-target/iscsi_target_login.c    |   76 +++++-
 drivers/target/lio-target/iscsi_target_login.h    |    1 +
 drivers/target/lio-target/iscsi_target_nego.c     |   18 +-
 ...
From: Hannes Reinecke
Date: Monday, September 13, 2010 - 2:21 am

Due to a customer issue I had to revisit the CRC32 issue recently.
And according to you the main reason for using your own CRC routines
had been endianness issues.
IE the in-kernel crc32c routines apparently weren't able to
calculate the checksum in an endianness-independent manner.
So a CRC calculated on a BE machine would fail to be validated by a
LE machine and vice versa.

Has this been fixed / verified?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries &amp; Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--

From: Nicholas A. Bellinger
Date: Monday, September 13, 2010 - 12:50 pm

Indeed, this has historically been the case for LIO-Target and
Core-iSCSI.  I noticed the issue with libcrypto while porting the latter
big endian safe.  I was planning to test this patch on some powerpc/ppc
hardware with v2.6.36-rc4 in the next days, but it looks like
lio-core-2.6.git will need a seperate crypto/crc32c.c patch to function
properly on big endian arches.

Thanks for mentioning this point Hannes!

--nab

--

From: Mike Christie
Date: Monday, September 13, 2010 - 1:35 pm

There was this bug that was fixed a couple years ago:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=ef1945...
since then I think we have not had problems.
--

From: Nicholas A. Bellinger
Date: Monday, September 13, 2010 - 1:40 pm

Hi Mike,

Thanks for the pointer on this, I will give it a shot on powerpc and see
what happens.

FYI, I was under the assumption that for BIG_ENDIAN that crc32c() still
needed an bitshift for the returned value.  This is what we had been
doing with our old internal do_crc() code here:

http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=blob;f=drivers/target...

AFAICT there is still nothing that makes crypto/crc32c.c big endian
safe, unless I am overlooking something obvious..?

Best,

--nab


--

From: Nicholas A. Bellinger
Date: Monday, September 13, 2010 - 3:20 pm

Hi Mike and Hannes,

So after a quick test using Open-iSCSI 32-bit x86 VM w/ v2.6.27 and
LIO-Target 64-bit powerpc w/v2.6.36-rc3 with this libcrypto patch I do
not see any HeaderDigest or DataDigest failures using the exiting
slicing by 1x crypto/crc32c.c code on 64-bit BIG_ENDIAN.  While I am at
it I will also try OpenSuse 11.2 and RHEL6 B2 x86_64 VMs for good
measure.

So the appropiate endian conversion does occur in chksum_final() and
__chksum_finup() code.  My apologies for the earier confusion.

Thanks again,

--nab


--

Previous thread: [Bug 18252] spinlock lockup in __make_request <- submit_bio <- ondemand_readahead by Stefan Richter on Saturday, September 11, 2010 - 2:50 am. (5 messages)

Next thread: [PATCH 1/2] lio-target: Convert to use libcrypto crc32c by Nicholas A. Bellinger on Saturday, September 11, 2010 - 3:31 am. (1 message)