-
Notifications
You must be signed in to change notification settings - Fork 75
Combine per-part CRCs for whole-object download checksum validation #663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d5dce07
cc9b306
c481e28
761b6cb
7c4b05d
fdc5b14
c7cd314
4dfa66c
d1307ac
fb4b8b9
2a416d3
438f4ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -253,6 +253,22 @@ static void s_crc_destroy(struct aws_s3_checksum *checksum) { | |
| aws_mem_release(checksum->allocator, checksum); | ||
| } | ||
|
|
||
| static int s_crc32_combine(struct aws_s3_checksum *head, uint64_t tail_value, uint64_t tail_length) { | ||
| head->impl.crc_val_32bit = aws_checksums_crc32_combine(head->impl.crc_val_32bit, (uint32_t)tail_value, tail_length); | ||
| return AWS_OP_SUCCESS; | ||
| } | ||
|
|
||
| static int s_crc32c_combine(struct aws_s3_checksum *head, uint64_t tail_value, uint64_t tail_length) { | ||
| head->impl.crc_val_32bit = | ||
| aws_checksums_crc32c_combine(head->impl.crc_val_32bit, (uint32_t)tail_value, tail_length); | ||
| return AWS_OP_SUCCESS; | ||
| } | ||
|
|
||
| static int s_crc64nvme_combine(struct aws_s3_checksum *head, uint64_t tail_value, uint64_t tail_length) { | ||
| head->impl.crc_val_64bit = aws_checksums_crc64nvme_combine(head->impl.crc_val_64bit, tail_value, tail_length); | ||
| return AWS_OP_SUCCESS; | ||
| } | ||
|
|
||
| static struct aws_checksum_vtable hash_vtable = { | ||
| .update = s_hash_update, | ||
| .finalize = s_hash_finalize, | ||
|
|
@@ -269,16 +285,19 @@ static struct aws_checksum_vtable crc32_vtable = { | |
| .update = s_crc32_checksum_update, | ||
| .finalize = s_crc32_finalize, | ||
| .destroy = s_crc_destroy, | ||
| .combine = s_crc32_combine, | ||
| }; | ||
| static struct aws_checksum_vtable crc32c_vtable = { | ||
| .update = s_crc32c_checksum_update, | ||
| .finalize = s_crc32_finalize, | ||
| .destroy = s_crc_destroy, | ||
| .combine = s_crc32c_combine, | ||
| }; | ||
| static struct aws_checksum_vtable crc64nvme_vtable = { | ||
| .update = s_crc64nvme_checksum_update, | ||
| .finalize = s_crc64_finalize, | ||
| .destroy = s_crc_destroy, | ||
| .combine = s_crc64nvme_combine, | ||
| }; | ||
|
|
||
| struct aws_s3_checksum *aws_hash_new(struct aws_allocator *allocator, aws_hash_new_fn hash_fn) { | ||
|
|
@@ -405,6 +424,49 @@ int aws_checksum_finalize(struct aws_s3_checksum *checksum, struct aws_byte_buf | |
| return checksum->vtable->finalize(checksum, output); | ||
| } | ||
|
|
||
| bool aws_checksum_algorithm_is_combinable(enum aws_s3_checksum_algorithm algorithm) { | ||
| switch (algorithm) { | ||
| case AWS_SCA_CRC32: | ||
| case AWS_SCA_CRC32C: | ||
| case AWS_SCA_CRC64NVME: | ||
| return true; | ||
| default: | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| int aws_checksum_combine_digest( | ||
| struct aws_s3_checksum *head, | ||
| struct aws_byte_cursor tail_digest, | ||
| uint64_t tail_length) { | ||
|
|
||
| if (head == NULL || head->vtable == NULL || head->vtable->combine == NULL) { | ||
| return aws_raise_error(AWS_ERROR_UNSUPPORTED_OPERATION); | ||
| } | ||
| if (!head->good) { | ||
| return aws_raise_error(AWS_ERROR_INVALID_STATE); | ||
| } | ||
| if (tail_digest.len != head->digest_size) { | ||
| return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT); | ||
| } | ||
|
|
||
| /* The CRC finalizers write digests big-endian, so read them back the same way. */ | ||
| uint64_t tail_value = 0; | ||
| if (head->digest_size == AWS_CRC32_LEN) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. potential optimization idea for future: but for gets we never need to finalize to be and then back. thats needed purely for puts
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, mostly because our current aws_s3_checksum interface is a pointer tied with the request itself, so I want to keep the parts level checksum independent from the request without affecting the lifetime of the request itself to keep it simple. And the interface only has the checksum_finalize that provides the encoded checksum. and I keep a list of the finalized one with the meta request. But, the extra encode/decode should be trivial, we can optimize it if we need to. |
||
| uint32_t value_32 = 0; | ||
| if (!aws_byte_cursor_read_be32(&tail_digest, &value_32)) { | ||
| return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT); | ||
| } | ||
| tail_value = value_32; | ||
| } else { | ||
| if (!aws_byte_cursor_read_be64(&tail_digest, &tail_value)) { | ||
| return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT); | ||
| } | ||
| } | ||
|
|
||
| return head->vtable->combine(head, tail_value, tail_length); | ||
| } | ||
|
|
||
| static int s_checksum_compute_fn( | ||
| struct aws_allocator *allocator, | ||
| const struct aws_byte_cursor *input, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we know the timings on this? if needed we can probably speed it up further
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
~350ns per combine operations. So, for the case of small part size on large object, it took 20ms, but not i'd say to be a concern here.