From: Or Ozeri Date: Thu, 14 Oct 2021 09:56:09 +0000 (+0300) Subject: msg/async: support disabling data crc for protocol v2 X-Git-Tag: v17.1.0~571^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f86c146cda26d1eb9b1334c1f8339ba622d00b80;p=ceph.git msg/async: support disabling data crc for protocol v2 In protocol v1, the ms_crc_data config option allowed for globally disabling the crc calculation and verification for data segments. This option was so far ignored in protocol v2 implementation. This commit extends v2 to respect this configuration option. When the data crc is disabled, zeros will be sent instead of the supposed calculated crc. Signed-off-by: Or Ozeri --- diff --git a/src/crimson/net/ProtocolV2.h b/src/crimson/net/ProtocolV2.h index ab6ad86c1bf..2ad0e494136 100644 --- a/src/crimson/net/ProtocolV2.h +++ b/src/crimson/net/ProtocolV2.h @@ -124,8 +124,12 @@ class ProtocolV2 final : public Protocol { ceph::crypto::onwire::rxtx_t session_stream_handlers; ceph::compression::onwire::rxtx_t session_comp_handlers; - ceph::msgr::v2::FrameAssembler tx_frame_asm{&session_stream_handlers, false, &session_comp_handlers}; - ceph::msgr::v2::FrameAssembler rx_frame_asm{&session_stream_handlers, false, &session_comp_handlers}; + ceph::msgr::v2::FrameAssembler tx_frame_asm{ + &session_stream_handlers, false, common::local_conf()->ms_crc_data, + &session_comp_handlers}; + ceph::msgr::v2::FrameAssembler rx_frame_asm{ + &session_stream_handlers, false, common::local_conf()->ms_crc_data, + &session_comp_handlers}; ceph::bufferlist rx_preamble; ceph::msgr::v2::segment_bls_t rx_segments_data; diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc index d13f0ead3ac..a176fc2c808 100644 --- a/src/msg/async/ProtocolV2.cc +++ b/src/msg/async/ProtocolV2.cc @@ -96,8 +96,10 @@ ProtocolV2::ProtocolV2(AsyncConnection *connection) replacing(false), can_write(false), bannerExchangeCallback(nullptr), - tx_frame_asm(&session_stream_handlers, false, &session_compression_handlers), - rx_frame_asm(&session_stream_handlers, false, &session_compression_handlers), + tx_frame_asm(&session_stream_handlers, false, cct->_conf->ms_crc_data, + &session_compression_handlers), + rx_frame_asm(&session_stream_handlers, false, cct->_conf->ms_crc_data, + &session_compression_handlers), next_tag(static_cast(0)), keepalive(false) { } diff --git a/src/msg/async/frames_v2.cc b/src/msg/async/frames_v2.cc index 8a297534763..e0c41fdb64c 100644 --- a/src/msg/async/frames_v2.cc +++ b/src/msg/async/frames_v2.cc @@ -108,7 +108,7 @@ bufferlist FrameAssembler::asm_crc_rev0(const preamble_block_t& preamble, frame_bl.append(reinterpret_cast(&preamble), sizeof(preamble)); for (size_t i = 0; i < m_descs.size(); i++) { ceph_assert(segment_bls[i].length() == m_descs[i].logical_len); - epilogue.crc_values[i] = segment_bls[i].crc32c(-1); + epilogue.crc_values[i] = m_with_data_crc ? segment_bls[i].crc32c(-1) : 0; if (segment_bls[i].length() > 0) { frame_bl.claim_append(segment_bls[i]); } @@ -161,7 +161,7 @@ bufferlist FrameAssembler::asm_crc_rev1(const preamble_block_t& preamble, ceph_assert(segment_bls[0].length() == m_descs[0].logical_len); if (segment_bls[0].length() > 0) { - uint32_t crc = segment_bls[0].crc32c(-1); + uint32_t crc = m_with_data_crc ? segment_bls[0].crc32c(-1) : 0; frame_bl.claim_append(segment_bls[0]); encode(crc, frame_bl); } @@ -171,7 +171,8 @@ bufferlist FrameAssembler::asm_crc_rev1(const preamble_block_t& preamble, for (size_t i = 1; i < m_descs.size(); i++) { ceph_assert(segment_bls[i].length() == m_descs[i].logical_len); - epilogue.crc_values[i - 1] = segment_bls[i].crc32c(-1); + epilogue.crc_values[i - 1] = + m_with_data_crc ? segment_bls[i].crc32c(-1) : 0; if (segment_bls[i].length() > 0) { frame_bl.claim_append(segment_bls[i]); } @@ -341,7 +342,9 @@ bool FrameAssembler::disasm_all_crc_rev0(bufferlist segment_bls[], for (size_t i = 0; i < m_descs.size(); i++) { ceph_assert(segment_bls[i].length() == m_descs[i].logical_len); - check_segment_crc(segment_bls[i], epilogue->crc_values[i]); + if (m_with_data_crc) { + check_segment_crc(segment_bls[i], epilogue->crc_values[i]); + } } return !(epilogue->late_flags & FRAME_LATE_FLAG_ABORTED); } @@ -374,7 +377,9 @@ void FrameAssembler::disasm_first_crc_rev1(bufferlist& preamble_bl, uint32_t expected_crc; decode(expected_crc, it); segment_bl.splice(m_descs[0].logical_len, FRAME_CRC_SIZE); - check_segment_crc(segment_bl, expected_crc); + if (m_with_data_crc) { + check_segment_crc(segment_bl, expected_crc); + } } else { ceph_assert(segment_bl.length() == 0); } @@ -388,7 +393,9 @@ bool FrameAssembler::disasm_remaining_crc_rev1(bufferlist segment_bls[], for (size_t i = 1; i < m_descs.size(); i++) { ceph_assert(segment_bls[i].length() == m_descs[i].logical_len); - check_segment_crc(segment_bls[i], epilogue->crc_values[i - 1]); + if (m_with_data_crc) { + check_segment_crc(segment_bls[i], epilogue->crc_values[i - 1]); + } } return check_epilogue_late_status(epilogue->late_status); } diff --git a/src/msg/async/frames_v2.h b/src/msg/async/frames_v2.h index 5b786e1aff8..9431d6e2db3 100644 --- a/src/msg/async/frames_v2.h +++ b/src/msg/async/frames_v2.h @@ -188,8 +188,9 @@ class FrameAssembler { public: // crypto must be non-null FrameAssembler(const ceph::crypto::onwire::rxtx_t* crypto, bool is_rev1, - const ceph::compression::onwire::rxtx_t* compression) - : m_crypto(crypto), m_is_rev1(is_rev1), m_compression(compression) {} + bool with_data_crc, const ceph::compression::onwire::rxtx_t* compression) + : m_crypto(crypto), m_is_rev1(is_rev1), m_with_data_crc(with_data_crc), + m_compression(compression) {} void set_is_rev1(bool is_rev1) { m_descs.clear(); @@ -401,6 +402,7 @@ private: __u8 m_flags; const ceph::crypto::onwire::rxtx_t* m_crypto; bool m_is_rev1; // msgr2.1? + bool m_with_data_crc; const ceph::compression::onwire::rxtx_t* m_compression; }; diff --git a/src/test/msgr/test_frames_v2.cc b/src/test/msgr/test_frames_v2.cc index b164ceb4ad2..c5be32a9c21 100644 --- a/src/test/msgr/test_frames_v2.cc +++ b/src/test/msgr/test_frames_v2.cc @@ -176,8 +176,10 @@ class RoundTripTestBase : public ::testing::TestWithParam< std::tuple> { protected: RoundTripTestBase() - : m_tx_frame_asm(&m_tx_crypto, std::get<1>(GetParam()).is_rev1, &m_tx_comp), - m_rx_frame_asm(&m_rx_crypto, std::get<1>(GetParam()).is_rev1, &m_rx_comp), + : m_tx_frame_asm(&m_tx_crypto, std::get<1>(GetParam()).is_rev1, true, + &m_tx_comp), + m_rx_frame_asm(&m_rx_crypto, std::get<1>(GetParam()).is_rev1, true, + &m_rx_comp), m_header(make_bufferlist(std::get<0>(GetParam()).header_len, 'H')), m_front(make_bufferlist(std::get<0>(GetParam()).front_len, 'F')), m_middle(make_bufferlist(std::get<0>(GetParam()).middle_len, 'M')),