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
This commit is contained in:
parent
9b4d8bcf8d
commit
a9b20ae6a3
|
@ -1,6 +1,9 @@
|
|||
# This is a cross-platform list tracking distribution packages needed by tests;
|
||||
# see http://docs.openstack.org/infra/bindep/ for additional information.
|
||||
|
||||
zlib1g-dev [platform:dpkg]
|
||||
zlib-devel [platform:rpm]
|
||||
|
||||
build-essential [platform:dpkg]
|
||||
gcc [platform:rpm]
|
||||
make [platform:rpm]
|
||||
|
|
|
@ -11,5 +11,5 @@ Version: @LIBERASURECODE_VERSION@
|
|||
Requires:
|
||||
Conflicts:
|
||||
Libs: -L${libdir} -lerasurecode
|
||||
Libs.private: @ERASURECODE_STATIC_LIBS@
|
||||
Libs.private: @ERASURECODE_STATIC_LIBS@ -lz
|
||||
Cflags: -I${includedir}/ -I${includedir}/liberasurecode
|
||||
|
|
|
@ -57,8 +57,7 @@ alg_sig_t *init_alg_sig(int sig_len, int gf_w);
|
|||
void destroy_alg_sig(alg_sig_t* alg_sig_handle);
|
||||
|
||||
int compute_alg_sig(alg_sig_t* alg_sig_handle, char *buf, int len, char *sig);
|
||||
int crc32_build_fast_table();
|
||||
int crc32(int crc, const void *buf, int size);
|
||||
int liberasurecode_crc32_alt(int crc, const void *buf, int size);
|
||||
|
||||
#endif
|
||||
|
||||
|
|
|
@ -34,9 +34,10 @@ liberasurecode_la_SOURCES = \
|
|||
|
||||
liberasurecode_la_CPPFLAGS = -Werror @GCOV_FLAGS@
|
||||
liberasurecode_la_LIBADD = \
|
||||
builtin/null_code/libnullcode.la -lpthread -lm @GCOV_LDFLAGS@ \
|
||||
builtin/xor_codes/libXorcode.la -lpthread -lm @GCOV_LDFLAGS@ \
|
||||
builtin/rs_vand/liberasurecode_rs_vand.la -lpthread -lm @GCOV_LDFLAGS@
|
||||
builtin/null_code/libnullcode.la \
|
||||
builtin/xor_codes/libXorcode.la \
|
||||
builtin/rs_vand/liberasurecode_rs_vand.la \
|
||||
-lpthread -lm -lz @GCOV_LDFLAGS@
|
||||
|
||||
# Version format (C - A).(A).(R) for C:R:A input
|
||||
liberasurecode_la_LDFLAGS = -rpath '$(libdir)' -version-info @LIBERASURECODE_VERSION_INFO@
|
||||
|
|
|
@ -26,6 +26,7 @@
|
|||
* vi: set noai tw=79 ts=4 sw=4:
|
||||
*/
|
||||
|
||||
#include <zlib.h>
|
||||
#include "assert.h"
|
||||
#include "list.h"
|
||||
#include "erasurecode.h"
|
||||
|
@ -1063,9 +1064,17 @@ int liberasurecode_get_fragment_metadata(char *fragment,
|
|||
uint32_t stored_chksum = fragment_hdr->meta.chksum[0];
|
||||
char *fragment_data = get_data_ptr_from_fragment(fragment);
|
||||
uint64_t fragment_size = fragment_hdr->meta.size;
|
||||
computed_chksum = crc32(0, fragment_data, fragment_size);
|
||||
computed_chksum = crc32(0, (unsigned char *) fragment_data, fragment_size);
|
||||
if (stored_chksum != computed_chksum) {
|
||||
fragment_metadata->chksum_mismatch = 1;
|
||||
// Try again with our "alternative" crc32; see
|
||||
// https://bugs.launchpad.net/liberasurecode/+bug/1666320
|
||||
computed_chksum = liberasurecode_crc32_alt(
|
||||
0, fragment_data, fragment_size);
|
||||
if (stored_chksum != computed_chksum) {
|
||||
fragment_metadata->chksum_mismatch = 1;
|
||||
} else {
|
||||
fragment_metadata->chksum_mismatch = 0;
|
||||
}
|
||||
} else {
|
||||
fragment_metadata->chksum_mismatch = 0;
|
||||
}
|
||||
|
@ -1095,7 +1104,13 @@ int is_invalid_fragment_header(fragment_header_t *header)
|
|||
stored_csum = get_metadata_chksum((char *) header);
|
||||
if (NULL == stored_csum)
|
||||
return 1; /* can't verify, possibly crc32 call error */
|
||||
csum = crc32(0, &header->meta, sizeof(fragment_metadata_t));
|
||||
csum = crc32(0, (unsigned char *) &header->meta, sizeof(fragment_metadata_t));
|
||||
if (*stored_csum == csum) {
|
||||
return 0;
|
||||
}
|
||||
// Else, try again with our "alternative" crc32; see
|
||||
// https://bugs.launchpad.net/liberasurecode/+bug/1666320
|
||||
csum = liberasurecode_crc32_alt(0, &header->meta, sizeof(fragment_metadata_t));
|
||||
return (*stored_csum != csum);
|
||||
}
|
||||
|
||||
|
|
|
@ -28,6 +28,7 @@
|
|||
#include <assert.h>
|
||||
#include <stdio.h>
|
||||
#include <stdarg.h>
|
||||
#include <zlib.h>
|
||||
#include "erasurecode_backend.h"
|
||||
#include "erasurecode_helpers.h"
|
||||
#include "erasurecode_helpers_ext.h"
|
||||
|
@ -474,7 +475,7 @@ inline int set_checksum(ec_checksum_type_t ct, char *buf, int blocksize)
|
|||
|
||||
switch(header->meta.chksum_type) {
|
||||
case CHKSUM_CRC32:
|
||||
header->meta.chksum[0] = crc32(0, data, blocksize);
|
||||
header->meta.chksum[0] = crc32(0, (unsigned char *) data, blocksize);
|
||||
break;
|
||||
case CHKSUM_MD5:
|
||||
break;
|
||||
|
@ -512,7 +513,7 @@ inline int set_metadata_chksum(char *buf)
|
|||
return -1;
|
||||
}
|
||||
|
||||
header->metadata_chksum = crc32(0, &header->meta,
|
||||
header->metadata_chksum = crc32(0, (unsigned char *) &header->meta,
|
||||
sizeof(fragment_metadata_t));
|
||||
return 0;
|
||||
}
|
||||
|
|
|
@ -89,7 +89,7 @@ static int crc32_tab[] = {
|
|||
};
|
||||
|
||||
int
|
||||
crc32(int crc, const void *buf, size_t size)
|
||||
liberasurecode_crc32_alt(int crc, const void *buf, size_t size)
|
||||
{
|
||||
const char *p;
|
||||
|
||||
|
|
|
@ -15,7 +15,7 @@ check_PROGRAMS += alg_sig_test
|
|||
|
||||
liberasurecode_test_SOURCES = liberasurecode_test.c
|
||||
liberasurecode_test_CPPFLAGS = -I$(top_srcdir)/include -I$(top_srcdir)/include/erasurecode @GCOV_FLAGS@
|
||||
liberasurecode_test_LDFLAGS = @GCOV_LDFLAGS@ $(top_builddir)/src/liberasurecode.la -ldl -lpthread
|
||||
liberasurecode_test_LDFLAGS = @GCOV_LDFLAGS@ $(top_builddir)/src/liberasurecode.la -ldl -lpthread -lz
|
||||
check_PROGRAMS += liberasurecode_test
|
||||
|
||||
libec_slap_SOURCES = libec_slap.c
|
||||
|
|
|
@ -28,6 +28,7 @@
|
|||
|
||||
#include <assert.h>
|
||||
#include <stdbool.h>
|
||||
#include <zlib.h>
|
||||
#include "erasurecode.h"
|
||||
#include "erasurecode_helpers.h"
|
||||
#include "erasurecode_helpers_ext.h"
|
||||
|
@ -475,7 +476,7 @@ static void validate_fragment_checksum(struct ec_args *args,
|
|||
assert(false); //currently only have support crc32
|
||||
break;
|
||||
case CHKSUM_CRC32:
|
||||
computed = crc32(0, fragment_data, size);
|
||||
computed = crc32(0, (unsigned char *) fragment_data, size);
|
||||
break;
|
||||
case CHKSUM_NONE:
|
||||
assert(metadata->chksum_mismatch == 0);
|
||||
|
@ -1745,6 +1746,35 @@ static void test_verify_stripe_metadata_frag_idx_invalid(
|
|||
verify_fragment_metadata_mismatch_impl(be_id, args, FRAGIDX_OUT_OF_RANGE);
|
||||
}
|
||||
|
||||
static void test_metadata_crcs()
|
||||
{
|
||||
// We've observed headers like this in the wild, using our busted crc32
|
||||
char header[] =
|
||||
"\x03\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x10\x00"
|
||||
"\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
|
||||
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
|
||||
"\x00\x00\x00\x00\x00\x00\x07\x01\x0e\x02\x00\xcc\x5e\x0c\x0b\x00"
|
||||
"\x04\x01\x00\x22\xee\x45\xb9\x00\x00\x00\x00\x00\x00\x00\x00\x00";
|
||||
|
||||
fragment_metadata_t res;
|
||||
|
||||
assert(liberasurecode_get_fragment_metadata(header, &res) == 0);
|
||||
assert(is_invalid_fragment_header((fragment_header_t *) header) == 0);
|
||||
|
||||
// Switch it to zlib's implementation
|
||||
header[70] = '\x18';
|
||||
header[69] = '\x73';
|
||||
header[68] = '\xf8';
|
||||
header[67] = '\xec';
|
||||
|
||||
assert(liberasurecode_get_fragment_metadata(header, &res) == 0);
|
||||
assert(is_invalid_fragment_header((fragment_header_t *) header) == 0);
|
||||
|
||||
// Write down the wrong thing
|
||||
header[70] = '\xff';
|
||||
assert(liberasurecode_get_fragment_metadata(header, &res) == -EBADHEADER);
|
||||
assert(is_invalid_fragment_header((fragment_header_t *) header) == 1);
|
||||
}
|
||||
|
||||
//static void test_verify_str
|
||||
|
||||
|
@ -1805,6 +1835,10 @@ struct testcase testcases[] = {
|
|||
test_liberasurecode_get_version,
|
||||
EC_BACKENDS_MAX, CHKSUM_TYPES_MAX,
|
||||
.skip = false},
|
||||
{"test_metadata_crcs",
|
||||
test_metadata_crcs,
|
||||
EC_BACKENDS_MAX, 0,
|
||||
.skip = false},
|
||||
// NULL backend test
|
||||
{"create_and_destroy_backend",
|
||||
test_create_and_destroy_backend,
|
||||
|
|
Loading…
Reference in New Issue