lio-target: Add support for libcrypto crc32c and crc32-intel offload
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
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.2x86_64on 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_offloadto 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
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.2x86_64on 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_offloadto 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.cit 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
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.cit still appears AFAICT to not bebig 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.cpatch 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.
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.cit still appears AFAICT to not bebig 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.cpatch 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:
AFAICT there is still nothing that makes crypto/crc32c.c big endian safe, unless I am overlooking something obvious..?
Best,
--nab
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.cit still appears AFAICT to not bebig 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.cpatch 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_ENDIANthatcrc32c()still needed an bitshift for the returned value. This is what we had been doing with our old internaldo_crc()code here:AFAICT there is still nothing that makes
crypto/crc32c.cbig 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