summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTim Burke <tim.burke@gmail.com>2017-07-06 00:21:01 +0000
committerTim Burke <tim.burke@gmail.com>2017-07-06 17:40:38 +0000
commita9b20ae6a38073afe91ae2b7d789ddfb7dabade8 (patch)
tree5453dc1088cf5504c64dbb6e90d68500090fbef3
parent9b4d8bcf8dc97d7edad3dc1443b317ecb5a0a254 (diff)
Use zlib for CRC-32
Previously, we had our own CRC that was almost but not quite like zlib's implementation. However, * it hasn't been subjected to the same rigor with regard to error-detection properties and * it may not even get used, depending upon whether zlib happens to get loaded before or after liberasurecode. Now, we'll use zlib's CRC-32 when writing new frags, while still tolerating frags that were created with the old implementation. Change-Id: Ib5ea2a830c7c23d66bf2ca404a3eb84ad00c5bc5 Closes-Bug: 1666320
Notes
Notes (review): Code-Review+2: Pete Zaitcev <zaitcev@kotori.zaitcev.us> Code-Review+2: Kota Tsuyuzaki <tsuyuzaki.kota@lab.ntt.co.jp> Workflow+1: Kota Tsuyuzaki <tsuyuzaki.kota@lab.ntt.co.jp> Verified+2: Jenkins Submitted-by: Jenkins Submitted-at: Thu, 13 Jul 2017 19:28:53 +0000 Reviewed-on: https://review.openstack.org/459023 Project: openstack/liberasurecode Branch: refs/heads/master
-rw-r--r--bindep.txt3
-rw-r--r--erasurecode.pc.in2
-rw-r--r--include/erasurecode/alg_sig.h3
-rw-r--r--src/Makefile.am7
-rw-r--r--src/erasurecode.c21
-rw-r--r--src/erasurecode_helpers.c5
-rw-r--r--src/utils/chksum/crc32.c2
-rw-r--r--test/Makefile.am2
-rw-r--r--test/liberasurecode_test.c36
9 files changed, 67 insertions, 14 deletions
diff --git a/bindep.txt b/bindep.txt
index aaef309..7593290 100644
--- a/bindep.txt
+++ b/bindep.txt
@@ -1,6 +1,9 @@
1# This is a cross-platform list tracking distribution packages needed by tests; 1# This is a cross-platform list tracking distribution packages needed by tests;
2# see http://docs.openstack.org/infra/bindep/ for additional information. 2# see http://docs.openstack.org/infra/bindep/ for additional information.
3 3
4zlib1g-dev [platform:dpkg]
5zlib-devel [platform:rpm]
6
4build-essential [platform:dpkg] 7build-essential [platform:dpkg]
5gcc [platform:rpm] 8gcc [platform:rpm]
6make [platform:rpm] 9make [platform:rpm]
diff --git a/erasurecode.pc.in b/erasurecode.pc.in
index ee6d82b..175367c 100644
--- a/erasurecode.pc.in
+++ b/erasurecode.pc.in
@@ -11,5 +11,5 @@ Version: @LIBERASURECODE_VERSION@
11Requires: 11Requires:
12Conflicts: 12Conflicts:
13Libs: -L${libdir} -lerasurecode 13Libs: -L${libdir} -lerasurecode
14Libs.private: @ERASURECODE_STATIC_LIBS@ 14Libs.private: @ERASURECODE_STATIC_LIBS@ -lz
15Cflags: -I${includedir}/ -I${includedir}/liberasurecode 15Cflags: -I${includedir}/ -I${includedir}/liberasurecode
diff --git a/include/erasurecode/alg_sig.h b/include/erasurecode/alg_sig.h
index 52554a9..e250fb3 100644
--- a/include/erasurecode/alg_sig.h
+++ b/include/erasurecode/alg_sig.h
@@ -57,8 +57,7 @@ alg_sig_t *init_alg_sig(int sig_len, int gf_w);
57void destroy_alg_sig(alg_sig_t* alg_sig_handle); 57void destroy_alg_sig(alg_sig_t* alg_sig_handle);
58 58
59int compute_alg_sig(alg_sig_t* alg_sig_handle, char *buf, int len, char *sig); 59int compute_alg_sig(alg_sig_t* alg_sig_handle, char *buf, int len, char *sig);
60int crc32_build_fast_table(); 60int liberasurecode_crc32_alt(int crc, const void *buf, int size);
61int crc32(int crc, const void *buf, int size);
62 61
63#endif 62#endif
64 63
diff --git a/src/Makefile.am b/src/Makefile.am
index 8312dd0..693809e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -34,9 +34,10 @@ liberasurecode_la_SOURCES = \
34 34
35liberasurecode_la_CPPFLAGS = -Werror @GCOV_FLAGS@ 35liberasurecode_la_CPPFLAGS = -Werror @GCOV_FLAGS@
36liberasurecode_la_LIBADD = \ 36liberasurecode_la_LIBADD = \
37 builtin/null_code/libnullcode.la -lpthread -lm @GCOV_LDFLAGS@ \ 37 builtin/null_code/libnullcode.la \
38 builtin/xor_codes/libXorcode.la -lpthread -lm @GCOV_LDFLAGS@ \ 38 builtin/xor_codes/libXorcode.la \
39 builtin/rs_vand/liberasurecode_rs_vand.la -lpthread -lm @GCOV_LDFLAGS@ 39 builtin/rs_vand/liberasurecode_rs_vand.la \
40 -lpthread -lm -lz @GCOV_LDFLAGS@
40 41
41# Version format (C - A).(A).(R) for C:R:A input 42# Version format (C - A).(A).(R) for C:R:A input
42liberasurecode_la_LDFLAGS = -rpath '$(libdir)' -version-info @LIBERASURECODE_VERSION_INFO@ 43liberasurecode_la_LDFLAGS = -rpath '$(libdir)' -version-info @LIBERASURECODE_VERSION_INFO@
diff --git a/src/erasurecode.c b/src/erasurecode.c
index d4a06c2..20da457 100644
--- a/src/erasurecode.c
+++ b/src/erasurecode.c
@@ -26,6 +26,7 @@
26 * vi: set noai tw=79 ts=4 sw=4: 26 * vi: set noai tw=79 ts=4 sw=4:
27 */ 27 */
28 28
29#include <zlib.h>
29#include "assert.h" 30#include "assert.h"
30#include "list.h" 31#include "list.h"
31#include "erasurecode.h" 32#include "erasurecode.h"
@@ -1063,9 +1064,17 @@ int liberasurecode_get_fragment_metadata(char *fragment,
1063 uint32_t stored_chksum = fragment_hdr->meta.chksum[0]; 1064 uint32_t stored_chksum = fragment_hdr->meta.chksum[0];
1064 char *fragment_data = get_data_ptr_from_fragment(fragment); 1065 char *fragment_data = get_data_ptr_from_fragment(fragment);
1065 uint64_t fragment_size = fragment_hdr->meta.size; 1066 uint64_t fragment_size = fragment_hdr->meta.size;
1066 computed_chksum = crc32(0, fragment_data, fragment_size); 1067 computed_chksum = crc32(0, (unsigned char *) fragment_data, fragment_size);
1067 if (stored_chksum != computed_chksum) { 1068 if (stored_chksum != computed_chksum) {
1068 fragment_metadata->chksum_mismatch = 1; 1069 // Try again with our "alternative" crc32; see
1070 // https://bugs.launchpad.net/liberasurecode/+bug/1666320
1071 computed_chksum = liberasurecode_crc32_alt(
1072 0, fragment_data, fragment_size);
1073 if (stored_chksum != computed_chksum) {
1074 fragment_metadata->chksum_mismatch = 1;
1075 } else {
1076 fragment_metadata->chksum_mismatch = 0;
1077 }
1069 } else { 1078 } else {
1070 fragment_metadata->chksum_mismatch = 0; 1079 fragment_metadata->chksum_mismatch = 0;
1071 } 1080 }
@@ -1095,7 +1104,13 @@ int is_invalid_fragment_header(fragment_header_t *header)
1095 stored_csum = get_metadata_chksum((char *) header); 1104 stored_csum = get_metadata_chksum((char *) header);
1096 if (NULL == stored_csum) 1105 if (NULL == stored_csum)
1097 return 1; /* can't verify, possibly crc32 call error */ 1106 return 1; /* can't verify, possibly crc32 call error */
1098 csum = crc32(0, &header->meta, sizeof(fragment_metadata_t)); 1107 csum = crc32(0, (unsigned char *) &header->meta, sizeof(fragment_metadata_t));
1108 if (*stored_csum == csum) {
1109 return 0;
1110 }
1111 // Else, try again with our "alternative" crc32; see
1112 // https://bugs.launchpad.net/liberasurecode/+bug/1666320
1113 csum = liberasurecode_crc32_alt(0, &header->meta, sizeof(fragment_metadata_t));
1099 return (*stored_csum != csum); 1114 return (*stored_csum != csum);
1100} 1115}
1101 1116
diff --git a/src/erasurecode_helpers.c b/src/erasurecode_helpers.c
index fd14298..4a49786 100644
--- a/src/erasurecode_helpers.c
+++ b/src/erasurecode_helpers.c
@@ -28,6 +28,7 @@
28#include <assert.h> 28#include <assert.h>
29#include <stdio.h> 29#include <stdio.h>
30#include <stdarg.h> 30#include <stdarg.h>
31#include <zlib.h>
31#include "erasurecode_backend.h" 32#include "erasurecode_backend.h"
32#include "erasurecode_helpers.h" 33#include "erasurecode_helpers.h"
33#include "erasurecode_helpers_ext.h" 34#include "erasurecode_helpers_ext.h"
@@ -474,7 +475,7 @@ inline int set_checksum(ec_checksum_type_t ct, char *buf, int blocksize)
474 475
475 switch(header->meta.chksum_type) { 476 switch(header->meta.chksum_type) {
476 case CHKSUM_CRC32: 477 case CHKSUM_CRC32:
477 header->meta.chksum[0] = crc32(0, data, blocksize); 478 header->meta.chksum[0] = crc32(0, (unsigned char *) data, blocksize);
478 break; 479 break;
479 case CHKSUM_MD5: 480 case CHKSUM_MD5:
480 break; 481 break;
@@ -512,7 +513,7 @@ inline int set_metadata_chksum(char *buf)
512 return -1; 513 return -1;
513 } 514 }
514 515
515 header->metadata_chksum = crc32(0, &header->meta, 516 header->metadata_chksum = crc32(0, (unsigned char *) &header->meta,
516 sizeof(fragment_metadata_t)); 517 sizeof(fragment_metadata_t));
517 return 0; 518 return 0;
518} 519}
diff --git a/src/utils/chksum/crc32.c b/src/utils/chksum/crc32.c
index 6bc844d..b11dec9 100644
--- a/src/utils/chksum/crc32.c
+++ b/src/utils/chksum/crc32.c
@@ -89,7 +89,7 @@ static int crc32_tab[] = {
89}; 89};
90 90
91int 91int
92crc32(int crc, const void *buf, size_t size) 92liberasurecode_crc32_alt(int crc, const void *buf, size_t size)
93{ 93{
94 const char *p; 94 const char *p;
95 95
diff --git a/test/Makefile.am b/test/Makefile.am
index d8ffa79..2e33bdc 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -15,7 +15,7 @@ check_PROGRAMS += alg_sig_test
15 15
16liberasurecode_test_SOURCES = liberasurecode_test.c 16liberasurecode_test_SOURCES = liberasurecode_test.c
17liberasurecode_test_CPPFLAGS = -I$(top_srcdir)/include -I$(top_srcdir)/include/erasurecode @GCOV_FLAGS@ 17liberasurecode_test_CPPFLAGS = -I$(top_srcdir)/include -I$(top_srcdir)/include/erasurecode @GCOV_FLAGS@
18liberasurecode_test_LDFLAGS = @GCOV_LDFLAGS@ $(top_builddir)/src/liberasurecode.la -ldl -lpthread 18liberasurecode_test_LDFLAGS = @GCOV_LDFLAGS@ $(top_builddir)/src/liberasurecode.la -ldl -lpthread -lz
19check_PROGRAMS += liberasurecode_test 19check_PROGRAMS += liberasurecode_test
20 20
21libec_slap_SOURCES = libec_slap.c 21libec_slap_SOURCES = libec_slap.c
diff --git a/test/liberasurecode_test.c b/test/liberasurecode_test.c
index 4791ca9..68c1c13 100644
--- a/test/liberasurecode_test.c
+++ b/test/liberasurecode_test.c
@@ -28,6 +28,7 @@
28 28
29#include <assert.h> 29#include <assert.h>
30#include <stdbool.h> 30#include <stdbool.h>
31#include <zlib.h>
31#include "erasurecode.h" 32#include "erasurecode.h"
32#include "erasurecode_helpers.h" 33#include "erasurecode_helpers.h"
33#include "erasurecode_helpers_ext.h" 34#include "erasurecode_helpers_ext.h"
@@ -475,7 +476,7 @@ static void validate_fragment_checksum(struct ec_args *args,
475 assert(false); //currently only have support crc32 476 assert(false); //currently only have support crc32
476 break; 477 break;
477 case CHKSUM_CRC32: 478 case CHKSUM_CRC32:
478 computed = crc32(0, fragment_data, size); 479 computed = crc32(0, (unsigned char *) fragment_data, size);
479 break; 480 break;
480 case CHKSUM_NONE: 481 case CHKSUM_NONE:
481 assert(metadata->chksum_mismatch == 0); 482 assert(metadata->chksum_mismatch == 0);
@@ -1745,6 +1746,35 @@ static void test_verify_stripe_metadata_frag_idx_invalid(
1745 verify_fragment_metadata_mismatch_impl(be_id, args, FRAGIDX_OUT_OF_RANGE); 1746 verify_fragment_metadata_mismatch_impl(be_id, args, FRAGIDX_OUT_OF_RANGE);
1746} 1747}
1747 1748
1749static void test_metadata_crcs()
1750{
1751 // We've observed headers like this in the wild, using our busted crc32
1752 char header[] =
1753 "\x03\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x10\x00"
1754 "\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
1755 "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
1756 "\x00\x00\x00\x00\x00\x00\x07\x01\x0e\x02\x00\xcc\x5e\x0c\x0b\x00"
1757 "\x04\x01\x00\x22\xee\x45\xb9\x00\x00\x00\x00\x00\x00\x00\x00\x00";
1758
1759 fragment_metadata_t res;
1760
1761 assert(liberasurecode_get_fragment_metadata(header, &res) == 0);
1762 assert(is_invalid_fragment_header((fragment_header_t *) header) == 0);
1763
1764 // Switch it to zlib's implementation
1765 header[70] = '\x18';
1766 header[69] = '\x73';
1767 header[68] = '\xf8';
1768 header[67] = '\xec';
1769
1770 assert(liberasurecode_get_fragment_metadata(header, &res) == 0);
1771 assert(is_invalid_fragment_header((fragment_header_t *) header) == 0);
1772
1773 // Write down the wrong thing
1774 header[70] = '\xff';
1775 assert(liberasurecode_get_fragment_metadata(header, &res) == -EBADHEADER);
1776 assert(is_invalid_fragment_header((fragment_header_t *) header) == 1);
1777}
1748 1778
1749//static void test_verify_str 1779//static void test_verify_str
1750 1780
@@ -1805,6 +1835,10 @@ struct testcase testcases[] = {
1805 test_liberasurecode_get_version, 1835 test_liberasurecode_get_version,
1806 EC_BACKENDS_MAX, CHKSUM_TYPES_MAX, 1836 EC_BACKENDS_MAX, CHKSUM_TYPES_MAX,
1807 .skip = false}, 1837 .skip = false},
1838 {"test_metadata_crcs",
1839 test_metadata_crcs,
1840 EC_BACKENDS_MAX, 0,
1841 .skip = false},
1808 // NULL backend test 1842 // NULL backend test
1809 {"create_and_destroy_backend", 1843 {"create_and_destroy_backend",
1810 test_create_and_destroy_backend, 1844 test_create_and_destroy_backend,