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 3df071d27992838752ebd1897192b6afe39bc3e5..23f5179a9e4bd4f188447c199882d195c8b9994c 100644 (file)
--- 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 963c9446a6ca9968db1703f1a61d2d231c257758..e1aa98d66ab1d11bd79956ac7ffc68e735c4b9db 100644 (file)
--- 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 85d480d0eb6bd924dd2117a4ba795c4d20501142..cc6430741c1a4fba4ec4853b2e09a708910c50dc 100644 (file)
--- 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 c52f9c71872e885649f6c68b333f55c0b819d393..b28fc33f809f0a3416651c89917d0ee0345abdd4 100644 (file)
--- 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);