From 9c0d244e2321904434a4d33d55318e697ad77849 Mon Sep 17 00:00:00 2001
From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Mon, 11 Feb 2019 06:42:24 +0100
Subject: [PATCH] msg/async: slightly rework ProtocolV2 preamble crafting.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
---
 src/msg/async/ProtocolV2.cc | 68 +++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 33 deletions(-)

diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc
index 6bdcdc5f819..fda4575ecf2 100644
--- a/src/msg/async/ProtocolV2.cc
+++ b/src/msg/async/ProtocolV2.cc
@@ -102,6 +102,7 @@ static void alloc_aligned_buffer(bufferlist &data, unsigned len, unsigned off) {
  * Protocol V2 Frame Structures
  **/
 
+static constexpr uint32_t FRAME_PREAMBLE_SIZE = 4 * sizeof(__le32);
 template <class T>
 struct Frame {
 protected:
@@ -109,14 +110,14 @@ protected:
   ceph::bufferlist::contiguous_filler preamble_filler;
 
 public:
-  Frame() : preamble_filler(payload.append_hole(8)) {}
+  Frame() : preamble_filler(payload.append_hole(FRAME_PREAMBLE_SIZE)) {}
 
   bufferlist &get_buffer(const uint32_t extra_payload_len = 0) {
-    __le32 msg_len = payload.length() + extra_payload_len - sizeof(std::uint32_t);
-    preamble_filler.copy_in(sizeof(msg_len),
-			    reinterpret_cast<const char*>(&msg_len));
+    __le32 rest_len = frame_size - FRAME_PREAMBLE_SIZE + extra_payload_len;
+    preamble_filler.copy_in(sizeof(rest_len),
+			    reinterpret_cast<const char*>(&rest_len));
 
-    __le32 tag = static_cast<uint32_t>(static_cast<T *>(this)->tag);
+    __le32 tag = static_cast<uint32_t>(T::tag);
     preamble_filler.copy_in(sizeof(tag),
 			    reinterpret_cast<const char*>(&tag));
     ceph_assert(tag != 0);
@@ -211,7 +212,7 @@ public:
 struct HelloFrame : public PayloadFrame<HelloFrame,
                                         uint8_t,          // entity type
                                         entity_addr_t> {  // peer_addr
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::HELLO;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::HELLO;
   using PayloadFrame::PayloadFrame;
 
   inline uint8_t &entity_type() { return get_val<0>(); }
@@ -221,7 +222,7 @@ struct HelloFrame : public PayloadFrame<HelloFrame,
 struct AuthRequestFrame
   : public PayloadFrame<AuthRequestFrame,
 			uint32_t, vector<uint32_t>, bufferlist> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::AUTH_REQUEST;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::AUTH_REQUEST;
   using PayloadFrame::PayloadFrame;
 
   inline uint32_t &method() { return get_val<0>(); }
@@ -235,7 +236,7 @@ struct AuthBadMethodFrame
 			int32_t,  // result
 			std::vector<uint32_t>,   // allowed_methods
 			std::vector<uint32_t>> { // allowed_modes
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::AUTH_BAD_METHOD;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::AUTH_BAD_METHOD;
   using PayloadFrame::PayloadFrame;
 
   inline uint32_t &method() { return get_val<0>(); }
@@ -246,7 +247,7 @@ struct AuthBadMethodFrame
 
 struct AuthReplyMoreFrame
     : public PayloadFrame<AuthReplyMoreFrame, bufferlist> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::AUTH_REPLY_MORE;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::AUTH_REPLY_MORE;
   using PayloadFrame::PayloadFrame;
 
   inline bufferlist &auth_payload() { return get_val<0>(); }
@@ -254,7 +255,7 @@ struct AuthReplyMoreFrame
 
 struct AuthRequestMoreFrame
     : public PayloadFrame<AuthRequestMoreFrame, bufferlist> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::AUTH_REQUEST_MORE;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::AUTH_REQUEST_MORE;
   using PayloadFrame::PayloadFrame;
 
   inline bufferlist &auth_payload() { return get_val<0>(); }
@@ -265,7 +266,7 @@ struct AuthDoneFrame
 			uint64_t, // global_id
 			uint32_t, // con_mode
 			bufferlist> { // auth method payload
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::AUTH_DONE;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::AUTH_DONE;
   using PayloadFrame::PayloadFrame;
 
   inline uint64_t &global_id() { return get_val<0>(); }
@@ -311,14 +312,14 @@ struct SignedEncryptedFrame : public PayloadFrame<T, Args...> {
 struct ClientIdentFrame
     : public SignedEncryptedFrame<ClientIdentFrame, 
                                   entity_addrvec_t,  // my addresses
-				                          entity_addr_t,  // target address
-				                          int64_t,  // global_id
+                                  entity_addr_t,  // target address
+                                  int64_t,  // global_id
                                   uint64_t,  // global seq
                                   uint64_t,  // supported features
                                   uint64_t,  // required features
                                   uint64_t,  // flags
                                   uint64_t> {  // client cookie
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::CLIENT_IDENT;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::CLIENT_IDENT;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline entity_addrvec_t &addrs() { return get_val<0>(); }
@@ -335,7 +336,7 @@ struct ServerIdentFrame
     : public SignedEncryptedFrame<ServerIdentFrame, entity_addrvec_t,
                                   int64_t, uint64_t, uint64_t,
                                   uint64_t, uint64_t, uint64_t> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::SERVER_IDENT;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::SERVER_IDENT;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline entity_addrvec_t &addrs() { return get_val<0>(); }
@@ -350,12 +351,12 @@ struct ServerIdentFrame
 struct ReconnectFrame
     : public SignedEncryptedFrame<ReconnectFrame, 
                                   entity_addrvec_t,  // my addresses
-				                          uint64_t,  // client cookie
+                                  uint64_t,  // client cookie
                                   uint64_t,  // server cookie
                                   uint64_t,  // global sequence
                                   uint64_t,  // connect sequence
-				                          uint64_t> { // message sequence
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::SESSION_RECONNECT;
+                                  uint64_t> { // message sequence
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::SESSION_RECONNECT;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline entity_addrvec_t &addrs() { return get_val<0>(); }
@@ -367,14 +368,14 @@ struct ReconnectFrame
 };
 
 struct ResetFrame : public SignedEncryptedFrame<ResetFrame, bool> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::SESSION_RESET;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::SESSION_RESET;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline bool &full() { return get_val<0>(); }
 };
 
 struct RetryFrame : public SignedEncryptedFrame<RetryFrame, uint64_t> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::SESSION_RETRY;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::SESSION_RETRY;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline uint64_t &connect_seq() { return get_val<0>(); }
@@ -382,19 +383,19 @@ struct RetryFrame : public SignedEncryptedFrame<RetryFrame, uint64_t> {
 
 struct RetryGlobalFrame
     : public SignedEncryptedFrame<RetryGlobalFrame, uint64_t> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::SESSION_RETRY_GLOBAL;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::SESSION_RETRY_GLOBAL;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline uint64_t &global_seq() { return get_val<0>(); }
 };
 
 struct WaitFrame : public Frame<WaitFrame> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::WAIT;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::WAIT;
 };
 
 struct ReconnectOkFrame
     : public SignedEncryptedFrame<ReconnectOkFrame, uint64_t> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::SESSION_RECONNECT_OK;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::SESSION_RECONNECT_OK;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline uint64_t &msg_seq() { return get_val<0>(); }
@@ -402,14 +403,14 @@ struct ReconnectOkFrame
 
 struct IdentMissingFeaturesFrame
     : public SignedEncryptedFrame<IdentMissingFeaturesFrame, uint64_t> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::IDENT_MISSING_FEATURES;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::IDENT_MISSING_FEATURES;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline uint64_t &features() { return get_val<0>(); }
 };
 
 struct KeepAliveFrame : public SignedEncryptedFrame<KeepAliveFrame, utime_t> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::KEEPALIVE2;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::KEEPALIVE2;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   KeepAliveFrame(ProtocolV2 &protocol)
@@ -420,14 +421,14 @@ struct KeepAliveFrame : public SignedEncryptedFrame<KeepAliveFrame, utime_t> {
 
 struct KeepAliveFrameAck
     : public SignedEncryptedFrame<KeepAliveFrame, utime_t> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::KEEPALIVE2_ACK;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::KEEPALIVE2_ACK;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline utime_t &timestamp() { return get_val<0>(); }
 };
 
 struct AckFrame : public SignedEncryptedFrame<AckFrame, uint64_t> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::ACK;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::ACK;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline uint64_t &seq() { return get_val<0>(); }
@@ -438,7 +439,7 @@ struct AckFrame : public SignedEncryptedFrame<AckFrame, uint64_t> {
 // being the `extra_payload_len` passed to get_buffer().
 struct MessageHeaderFrame
     : public PayloadFrame<MessageHeaderFrame, ceph_msg_header2> {
-  const ProtocolV2::Tag tag = ProtocolV2::Tag::MESSAGE;
+  static const ProtocolV2::Tag tag = ProtocolV2::Tag::MESSAGE;
 
   // XXX, TODO: MessageHeaderFrame needs to be aware about `protocol` only
   // because lacking preamble encryption. This will be dropped, altogether
@@ -1321,7 +1322,7 @@ CtPtr ProtocolV2::read_frame() {
   }
 
   ldout(cct, 20) << __func__ << dendl;
-  return READ(sizeof(__le32) * 2, handle_read_frame_length_and_tag);
+  return READ(FRAME_PREAMBLE_SIZE, handle_read_frame_length_and_tag);
 }
 
 CtPtr ProtocolV2::handle_read_frame_length_and_tag(char *buffer, int r) {
@@ -1333,18 +1334,19 @@ CtPtr ProtocolV2::handle_read_frame_length_and_tag(char *buffer, int r) {
     return _fault();
   }
 
+  ceph::bufferlist preamble;
+  preamble.push_back(buffer::create_static(FRAME_PREAMBLE_SIZE, buffer));
+
   if (auth_meta->is_mode_secure()) {
     ceph_assert(session_stream_handlers.rx);
     session_stream_handlers.rx->reset_rx_handler();
   }
 
-  bufferlist bl;
-  bl.push_back(buffer::create_static(sizeof(uint32_t) * 2, buffer));
   try {
-    auto ti = bl.cbegin();
+    auto ti = preamble.cbegin();
     uint32_t frame_len;
     decode(frame_len, ti);
-    next_payload_len = frame_len - sizeof(uint32_t);
+    next_payload_len = frame_len;
     uint32_t tag;
     decode(tag, ti);
     next_tag = static_cast<Tag>(tag);
-- 
2.39.5