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 <oro@il.ibm.com>

diff --git a/src/crimson/net/ProtocolV2.h b/src/crimson/net/ProtocolV2.h
index ab6ad86c1bfb9deb59022dbf106c4b4daae4c988..2ad0e4941366fe40a4456f3fcd478c81b8c3bb1f 100644 (file)
--- 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 d13f0ead3aced0fd8fc7b5901744871bbcf9d015..a176fc2c808ac9eb8d465310781daf4eacc20b98 100644 (file)
--- 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<Tag>(0)),
       keepalive(false) {
 }

diff --git a/src/msg/async/frames_v2.cc b/src/msg/async/frames_v2.cc
index 8a297534763645433b3c8fedb27044fcaca3ea0f..e0c41fdb64c173b1de4a0832c0bf43211a50e87c 100644 (file)
--- 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<const char*>(&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 5b786e1aff881cd056ebc9de47dc8b391b9b1011..9431d6e2db3eda72c73a199ed3161e19135c2086 100644 (file)
--- 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 b164ceb4ad2411eaef4d544c10ac2ae9821405b7..c5be32a9c21422a2506977b2b946aea2652dddfb 100644 (file)
--- 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<round_trip_instance_t, mode_t>> {
 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')),