From: Mykola Golub <mgolub@mirantis.com>
Date: Fri, 23 Jan 2015 09:47:45 +0000 (+0200)
Subject: msg: fixup for 2ffacbe (crc configuration in messenger)
X-Git-Tag: v0.93~106
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=71c6d98979ac26fb37b94172316cb65c475bc6e4;p=ceph.git

msg: fixup for 2ffacbe (crc configuration in messenger)

* Update the remaining Message::encode() calls, which now expect crc
  flags to be passed as a flags parameter instead of a bool (this e.g.
  fixes the issue with routing messages forwarded from an older client).

* In Message::encode() data crc is calculated when MSG_CRC_DATA is
  set, but in decode_message(), Pipe::read/write_message() data crc is
  calculated when MSG_CRC_HEADER is set. Fix this.

Note, 2ffacbe changed the behavior of Pipe::read_message/write_message():
previously the methods always calculated crc, now they calculate it
only if crc is enabled in the config. This means crc can not be disabled
if there are monitors of older version in the cluster.

Signed-off-by: Mykola Golub <mgolub@mirantis.com>
Reviewed-by: Sage Weil <sage@redhat.com>
---

diff --git a/src/msg/Message.cc b/src/msg/Message.cc
index 3df071d279928..23f5179a9e4bd 100644
--- a/src/msg/Message.cc
+++ b/src/msg/Message.cc
@@ -277,7 +277,8 @@ Message *decode_message(CephContext *cct, int crcflags,
       }
       return 0;
     }
-
+  }
+  if (crcflags & MSG_CRC_DATA) {
     if ((footer.flags & CEPH_MSG_FOOTER_NOCRC) == 0) {
       __u32 data_crc = data.crc32c(0);
       if (data_crc != footer.data_crc) {
@@ -290,7 +291,7 @@ Message *decode_message(CephContext *cct, int crcflags,
 	return 0;
       }
     }
-  } 
+  }
 
   // make message
   Message *m = 0;
@@ -775,7 +776,7 @@ void encode_message(Message *msg, uint64_t features, bufferlist& payload)
   bufferlist front, middle, data;
   ceph_msg_footer_old old_footer;
   ceph_msg_footer footer;
-  msg->encode(features, true);
+  msg->encode(features, MSG_CRC_ALL);
   ::encode(msg->get_header(), payload);
 
   // Here's where we switch to the old footer format.  PLR
diff --git a/src/msg/Message.h b/src/msg/Message.h
index 963c9446a6ca9..e1aa98d66ab1d 100644
--- a/src/msg/Message.h
+++ b/src/msg/Message.h
@@ -170,8 +170,9 @@
 #define MSG_MON_HEALTH            0x601
 
 // *** Message::encode() crcflags bits ***
-#define MSG_CRC_DATA           1
-#define MSG_CRC_HEADER         2
+#define MSG_CRC_DATA           (1 << 0)
+#define MSG_CRC_HEADER         (1 << 1)
+#define MSG_CRC_ALL            (MSG_CRC_DATA | MSG_CRC_HEADER)
 
 // Xio Testing
 #define MSG_DATA_PING		  0x602
diff --git a/src/msg/async/AsyncMessenger.cc b/src/msg/async/AsyncMessenger.cc
index 85d480d0eb6bd..cc6430741c1a4 100644
--- a/src/msg/async/AsyncMessenger.cc
+++ b/src/msg/async/AsyncMessenger.cc
@@ -616,7 +616,7 @@ void AsyncMessenger::submit_message(Message *m, AsyncConnectionRef con,
                                     const entity_addr_t& dest_addr, int dest_type)
 {
   if (cct->_conf->ms_dump_on_send) {
-    m->encode(-1, true);
+    m->encode(-1, MSG_CRC_ALL);
     ldout(cct, 0) << __func__ << "submit_message " << *m << "\n";
     m->get_payload().hexdump(*_dout);
     if (m->get_data().length() > 0) {
diff --git a/src/msg/simple/Pipe.cc b/src/msg/simple/Pipe.cc
index c52f9c71872e8..b28fc33f809f0 100644
--- a/src/msg/simple/Pipe.cc
+++ b/src/msg/simple/Pipe.cc
@@ -1925,8 +1925,7 @@ int Pipe::read_message(Message **pm, AuthSessionHandler* auth_handler)
            << dendl;
 
   // verify header crc
-  if (!(msgr->crcflags & MSG_CRC_HEADER)) {
-  } else if (header_crc != header.crc) {
+  if ((msgr->crcflags & MSG_CRC_HEADER) && header_crc != header.crc) {
     ldout(msgr->cct,0) << "reader got bad header crc " << header_crc << " != " << header.crc << dendl;
     return -1;
   }
@@ -2052,11 +2051,9 @@ int Pipe::read_message(Message **pm, AuthSessionHandler* auth_handler)
     ceph_msg_footer_old old_footer;
     if (tcp_read((char*)&old_footer, sizeof(old_footer)) < 0)
       goto out_dethrottle;
-    if (msgr->crcflags & MSG_CRC_HEADER) {
-      footer.front_crc = old_footer.front_crc;
-      footer.middle_crc = old_footer.middle_crc;
-      footer.data_crc = old_footer.data_crc;
-    }
+    footer.front_crc = old_footer.front_crc;
+    footer.middle_crc = old_footer.middle_crc;
+    footer.data_crc = old_footer.data_crc;
     footer.sig = 0;
     footer.flags = old_footer.flags;
   }
@@ -2339,10 +2336,10 @@ int Pipe::write_message(ceph_msg_header& header, ceph_msg_footer& footer, buffer
     if (msgr->crcflags & MSG_CRC_HEADER) {
       old_footer.front_crc = footer.front_crc;
       old_footer.middle_crc = footer.middle_crc;
-      old_footer.data_crc = footer.data_crc;
     } else {
-	old_footer.front_crc = old_footer.middle_crc = old_footer.data_crc = 0;
+	old_footer.front_crc = old_footer.middle_crc = 0;
     }
+    old_footer.data_crc = msgr->crcflags & MSG_CRC_DATA ? footer.data_crc : 0;
     old_footer.flags = footer.flags;   
     msgvec[msg.msg_iovlen].iov_base = (char*)&old_footer;
     msgvec[msg.msg_iovlen].iov_len = sizeof(old_footer);