From 4f23b242f316fcdc2ee924c2da49c7dd2bfdc36f Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Mon, 30 Mar 2026 01:49:51 +0300 Subject: [PATCH] Do not use scrap_buffer in the client (it would block protocol checksum support) --- src/client/cluster_client.cpp | 32 +++++++++------------------ src/client/cluster_client.h | 3 --- src/client/cluster_client_impl.h | 1 - src/client/messenger.h | 1 + src/client/msgr_encrypt.cpp | 18 ++++++++++----- src/client/msgr_receive.cpp | 38 +++++++++++++++++++++++++++++++- 6 files changed, 61 insertions(+), 32 deletions(-) diff --git a/src/client/cluster_client.cpp b/src/client/cluster_client.cpp index f309ec45..ae1364f4 100644 --- a/src/client/cluster_client.cpp +++ b/src/client/cluster_client.cpp @@ -71,9 +71,6 @@ cluster_client_t::cluster_client_t(ring_loop_t *ringloop, timerfd_manager_t *tfd st_cli.infinite_start = config["client_infinite_start"].bool_value(); } st_cli.load_global_config(); - - scrap_buffer_size = SCRAP_BUFFER_SIZE; - scrap_buffer = malloc_or_die(scrap_buffer_size); } cluster_client_t::~cluster_client_t() @@ -96,7 +93,6 @@ cluster_client_t::~cluster_client_t() { ringloop->unregister_consumer(&consumer); } - free(scrap_buffer); delete wb; wb = NULL; } @@ -1245,7 +1241,7 @@ resume_2: return 0; } -static void add_iov(int size, bool skip, cluster_op_t *op, int &iov_idx, size_t &iov_pos, osd_op_buf_list_t &iov, void *scrap, int scrap_len) +static void add_iov(int size, int skip, cluster_op_t *op, int &iov_idx, size_t &iov_pos, osd_op_buf_list_t &iov) { int left = size; while (left > 0 && iov_idx < op->iov.count) @@ -1253,7 +1249,7 @@ static void add_iov(int size, bool skip, cluster_op_t *op, int &iov_idx, size_t int cur_left = op->iov.buf[iov_idx].iov_len - iov_pos; if (cur_left < left) { - if (!skip) + if (skip == 0) { iov.push_back((uint8_t*)op->iov.buf[iov_idx].iov_base + iov_pos, cur_left); } @@ -1263,7 +1259,7 @@ static void add_iov(int size, bool skip, cluster_op_t *op, int &iov_idx, size_t } else { - if (!skip) + if (skip == 0) { iov.push_back((uint8_t*)op->iov.buf[iov_idx].iov_base + iov_pos, left); } @@ -1272,16 +1268,10 @@ static void add_iov(int size, bool skip, cluster_op_t *op, int &iov_idx, size_t } } assert(left == 0); - if (skip && scrap_len > 0) + if (skip == 1) { - // All skipped ranges are read into the same useless buffer - left = size; - while (left > 0) - { - int cur_left = scrap_len < left ? scrap_len : left; - iov.push_back(scrap, cur_left); - left -= cur_left; - } + // data read into a NULL buffer will be discarded by messenger + iov.push_back(NULL, size); } } @@ -1347,10 +1337,10 @@ void cluster_client_t::slice_rw(cluster_op_t *op) { begin = cur; // Just advance iov_idx & iov_pos - add_iov(cur-prev, true, op, iov_idx, iov_pos, op->parts[i].iov, NULL, 0); + add_iov(cur-prev, 2, op, iov_idx, iov_pos, op->parts[i].iov); } else - add_iov(cur-prev, skip_prev, op, iov_idx, iov_pos, op->parts[i].iov, scrap_buffer, scrap_buffer_size); + add_iov(cur-prev, skip_prev ? 1 : 0, op, iov_idx, iov_pos, op->parts[i].iov); } skip_prev = skip; prev = cur; @@ -1361,11 +1351,11 @@ void cluster_client_t::slice_rw(cluster_op_t *op) if (skip_prev) { // Just advance iov_idx & iov_pos - add_iov(end-prev, true, op, iov_idx, iov_pos, op->parts[i].iov, NULL, 0); + add_iov(end-prev, 2, op, iov_idx, iov_pos, op->parts[i].iov); end = prev; } else - add_iov(cur-prev, skip_prev, op, iov_idx, iov_pos, op->parts[i].iov, scrap_buffer, scrap_buffer_size); + add_iov(cur-prev, skip_prev ? 1 : 0, op, iov_idx, iov_pos, op->parts[i].iov); if (end == begin) { op->done_count++; @@ -1374,7 +1364,7 @@ void cluster_client_t::slice_rw(cluster_op_t *op) } else if (op->opcode != OSD_OP_READ_BITMAP && op->opcode != OSD_OP_READ_CHAIN_BITMAP && op->opcode != OSD_OP_DELETE) { - add_iov(end-begin, false, op, iov_idx, iov_pos, op->parts[i].iov, NULL, 0); + add_iov(end-begin, 0, op, iov_idx, iov_pos, op->parts[i].iov); } op->parts[i].parent = op; op->parts[i].offset = begin; diff --git a/src/client/cluster_client.h b/src/client/cluster_client.h index 33862d45..d9d3fe8d 100644 --- a/src/client/cluster_client.h +++ b/src/client/cluster_client.h @@ -153,9 +153,6 @@ public: std::set dirty_osds; uint64_t dirty_bytes = 0, dirty_ops = 0; - void *scrap_buffer = NULL; - unsigned scrap_buffer_size = 0; - // inodes require some extra state for read/write, it's stored here. // moreover, robin_hood access is slightly faster than std::map :) robin_hood::unordered_flat_map> inode_cache; diff --git a/src/client/cluster_client_impl.h b/src/client/cluster_client_impl.h index 2e3a3c87..35bd2c5f 100644 --- a/src/client/cluster_client_impl.h +++ b/src/client/cluster_client_impl.h @@ -5,7 +5,6 @@ #include "cluster_client.h" -#define SCRAP_BUFFER_SIZE 4*1024*1024 #define PART_SENT 1 #define PART_DONE 2 #define PART_ERROR 4 diff --git a/src/client/messenger.h b/src/client/messenger.h index f8d4974f..8999869f 100644 --- a/src/client/messenger.h +++ b/src/client/messenger.h @@ -304,6 +304,7 @@ protected: bool allocate_reply_buffers(osd_client_t *cl, osd_op_t *op); size_t op_copy_from(osd_client_t *cl, uint8_t *src, size_t src_len, size_t & done); size_t op_get_read_buffers(osd_client_t *cl, std::vector & lst); + void op_alloc_temp_buffers(osd_op_t *op, int i); void handle_finished_op(osd_client_t *cl); void handle_immediate_ops(); diff --git a/src/client/msgr_encrypt.cpp b/src/client/msgr_encrypt.cpp index d25824ab..b71e9253 100644 --- a/src/client/msgr_encrypt.cpp +++ b/src/client/msgr_encrypt.cpp @@ -81,7 +81,6 @@ void op_aes_xts_encrypt_t::encrypt_block(uint8_t *in, uint8_t *out) #endif } -// FIXME: Copy-paste void op_aes_xts_encrypt_t::update(uint8_t *in, size_t max_in, uint8_t *out, size_t max_out, size_t & done_in, size_t & done_out) { // Fucking AES-XTS implementations (all of them) don't have streaming support, @@ -245,6 +244,8 @@ void op_aes_xts_decrypt_t::decrypt_block(uint8_t *in, uint8_t *out) #endif } +// out may be NULL, in this case all input is still decrypted to calculate checksums, +// but part of it is skipped and not copied to out void op_aes_xts_decrypt_t::update(uint8_t *in, size_t max_in, uint8_t *out, size_t max_out, size_t & done_in, size_t & done_out) { // Fucking AES-XTS implementations (all of them) don't have streaming support, @@ -258,7 +259,8 @@ void op_aes_xts_decrypt_t::update(uint8_t *in, size_t max_in, uint8_t *out, size assert(tmp); if (max_out > block_size - tmp_pos) max_out = block_size - tmp_pos; - memcpy(out, tmp + tmp_pos, max_out); + if (out) + memcpy(out, tmp + tmp_pos, max_out); done_out += max_out; tmp_pos += max_out; if (tmp_pos >= block_size) @@ -276,7 +278,7 @@ void op_aes_xts_decrypt_t::update(uint8_t *in, size_t max_in, uint8_t *out, size done_in += max_in; offset += max_in; } - else if (max_out < block_size) + else if (max_out < block_size || !out) { // Accumulate and decrypt input in , then copy part of it to if (!tmp) @@ -288,7 +290,8 @@ void op_aes_xts_decrypt_t::update(uint8_t *in, size_t max_in, uint8_t *out, size memcpy(tmp + offset%block_size, in, max_in); decrypt_block(tmp, tmp); decrypted = true; - memcpy(out, tmp, max_out); + if (out) + memcpy(out, tmp, max_out); tmp_pos = max_out; done_in += max_in; offset += max_in; @@ -297,7 +300,8 @@ void op_aes_xts_decrypt_t::update(uint8_t *in, size_t max_in, uint8_t *out, size else if (!(offset%block_size)) { // Full block - simplest case - decrypt_block(in, out); + if (out) + decrypt_block(in, out); done_in += block_size; offset += block_size; done_out += block_size; @@ -308,6 +312,7 @@ void op_aes_xts_decrypt_t::update(uint8_t *in, size_t max_in, uint8_t *out, size assert(tmp); max_in = block_size - offset%block_size; memcpy(tmp + offset%block_size, in, max_in); + assert(out); decrypt_block(tmp, out); done_in += max_in; offset += max_in; @@ -383,7 +388,8 @@ bool osd_messenger_t::op_decrypted_copy_data_from(osd_client_t* cl, uint8_t *enc return false; size_t done_in = 0; size_t done_out = 0; - cl->decrypt_ctx->update(enc_buf+done, enc_len-done, plain+from, plain_len-from, done_in, done_out); + // plain == NULL means skip output + cl->decrypt_ctx->update(enc_buf+done, enc_len-done, plain ? plain+from : NULL, plain_len-from, done_in, done_out); done += done_in; cl->read_op_pos += done_out; cl->read_op_inline_decrypt_in += done_in; diff --git a/src/client/msgr_receive.cpp b/src/client/msgr_receive.cpp index 98720e74..ccc349d9 100644 --- a/src/client/msgr_receive.cpp +++ b/src/client/msgr_receive.cpp @@ -388,7 +388,10 @@ size_t osd_messenger_t::op_copy_from(osd_client_t *cl, uint8_t *src, size_t src_ size_t n = dst_len-from; if (n > src_len-done) n = src_len-done; - memcpy(dst+from, src+done, n); + if (dst) + memcpy(dst+from, src+done, n); + else + assert(!this->osd_num); // NULL buffers are only used by clients done += n; cl->read_op_pos += n; from += n; @@ -575,8 +578,17 @@ size_t osd_messenger_t::op_get_read_buffers(osd_client_t *cl, std::vector from = cl->read_op_inline_decrypt_in; } for (int i = 0; i < op->iov.count; i++) + { + if (!op->iov.buf[i].iov_base) + { + // When we recvmsg directly into the operation without copying, + // we need some place for all buffers, so we allocate temporary + // buffers for all skipped parts + op_alloc_temp_buffers(op, i); + } if (!op_read_buf((uint8_t*)op->iov.buf[i].iov_base, op->iov.buf[i].iov_len)) return done; + } } } else if (op->reply.hdr.opcode == OSD_OP_SEC_LIST && op->reply.hdr.retval > 0) @@ -599,6 +611,30 @@ size_t osd_messenger_t::op_get_read_buffers(osd_client_t *cl, std::vector return done; } +void osd_messenger_t::op_alloc_temp_buffers(osd_op_t *op, int i) +{ + size_t total_skip = 0; + for (int j = i; j < op->iov.count; j++) + { + if (!op->iov.buf[j].iov_base) + { + total_skip += op->iov.buf[j].iov_len; + } + } + assert(total_skip); + assert(!op->rmw_buf); + op->rmw_buf = malloc_or_die(total_skip); + total_skip = 0; + for (int j = i; j < op->iov.count; j++) + { + if (!op->iov.buf[j].iov_base) + { + op->iov.buf[j].iov_base = (uint8_t*)op->rmw_buf + total_skip; + total_skip += op->iov.buf[j].iov_len; + } + } +} + void osd_messenger_t::handle_finished_op(osd_client_t *cl) { osd_op_t *op = cl->read_op;