Skip to contentSkip to conversation

lio-target: Add support for libcrypto crc32c and crc32-intel offload

PATCH In review Nicholas A. Bellinger 3 participants 6 messages Sep 11, 2010 – Sep 13, 2010 SCSI

lore
Share & tools
lore
#0 lore raw

cover lio-target: Add support for libcrypto crc32c and crc32-intel offload

From: Nicholas Bellinger <[email protected]>

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!

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 +-
 drivers/target/lio-target/iscsi_target_tmr.c      |    1 -
 drivers/target/lio-target/iscsi_target_tpg.c      |   19 ++
 drivers/target/lio-target/iscsi_target_tpg.h      |    1 +
 14 files changed, 356 insertions(+), 258 deletions(-)
 delete mode 100644 drivers/target/lio-target/iscsi_crc.c
 delete mode 100644 drivers/target/lio-target/iscsi_crc.h
#1 lore raw
Quoted 15 lines · Nicholas A. Bellinger

From: Nicholas Bellinger <[email protected]>

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.

Quoted 5 lines

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.

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

Signature · 8 lines
Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#2 lore raw
Quoted 16 lines · Hannes Reinecke
Quoted 15 lines · Nicholas A. Bellinger

From: Nicholas Bellinger <[email protected]>

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.

Quoted 8 lines
Quoted 5 lines

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.

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.

Indeed, this has historically been the case for LIO-Target and Core-iSCSI. I noticed the issue with libcrypto while porting the latter on a old G4 TI-book waaay back during v2.6.10-rc3 days.

Quoted 9 lines

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?

From taking a look at crypto/crc32c.c it still appears AFAICT to not be

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

#3 lore raw
Quoted 5 lines · Nicholas A. Bellinger
Quoted 5 lines

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.

Quoted 9 lines
Quoted 4 lines

Has this been fixed / verified?

From taking a look at crypto/crc32c.c it still appears AFAICT to not be

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.

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=ef19454bd437b2ba14c9cda1de85debd9f383484
since then I think we have not had problems.

#4 lore raw
Quoted 16 lines · Mike Christie
Quoted 5 lines · Nicholas A. Bellinger
Quoted 5 lines

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.

Quoted 9 lines
Quoted 4 lines

Has this been fixed / verified?

From taking a look at crypto/crc32c.c it still appears AFAICT to not be

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.

Quoted 3 lines

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=ef19454bd437b2ba14c9cda1de85debd9f383484
since then I think we have not had problems.

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/lio-target/iscsi_crc.c;hb=83559072b27eb8cd92abb9a3fc9be85018c5db06#l138

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

Best,

--nab

#5 lore raw
Quoted 26 lines · Nicholas A. Bellinger
Quoted 16 lines · Mike Christie
Quoted 5 lines · Nicholas A. Bellinger
Quoted 5 lines

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.

Quoted 9 lines
Quoted 4 lines

Has this been fixed / verified?

From taking a look at crypto/crc32c.c it still appears AFAICT to not be

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.

Quoted 3 lines

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=ef19454bd437b2ba14c9cda1de85debd9f383484
since then I think we have not had problems.

Hi Mike,

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

Quoted 9 lines

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/lio-target/iscsi_crc.c;hb=83559072b27eb8cd92abb9a3fc9be85018c5db06#l138

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

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