crimson/net: misc fixes in v1 read path

* abort when decoding/signature-check failed;
* change log-level to warn;
* free the decoded message when failed;
* add unlikely to read path;

Signed-off-by: Yingxin Cheng <yingxincheng@gmail.com>

diff --git a/src/crimson/net/Errors.cc b/src/crimson/net/Errors.cc
index 62d60ce1c7bd5a42d7039b2a5f7c537461afc24b..76a32527d14aa3a15fabb7bd20163518d0e61035 100644 (file)
--- a/src/crimson/net/Errors.cc
+++ b/src/crimson/net/Errors.cc
@@ -41,6 +41,8 @@ const std::error_category& net_category()
           return "connection refused";
         case error::connection_reset:
           return "connection reset";
+        case error::corrupted_message:
+          return "corrupted message";
         default:
           return "unknown";
       }

diff --git a/src/crimson/net/Errors.h b/src/crimson/net/Errors.h
index d75082fd864a7348fb7dcbf5d6e48371c5bb9e94..c5c17dc0e3bd1f23d4927ed569445e62c9bba41d 100644 (file)
--- a/src/crimson/net/Errors.h
+++ b/src/crimson/net/Errors.h
@@ -28,6 +28,7 @@ enum class error {
   connection_aborted,
   connection_refused,
   connection_reset,
+  corrupted_message,
 };
 
 /// net error category

diff --git a/src/crimson/net/ProtocolV1.cc b/src/crimson/net/ProtocolV1.cc
index 408f7f0d0fecce440c6eccad8513348639686016..22242e95a35be1d9bc49a304432773d0a8d3502e 100644 (file)
--- a/src/crimson/net/ProtocolV1.cc
+++ b/src/crimson/net/ProtocolV1.cc
@@ -777,27 +777,28 @@ seastar::future<> ProtocolV1::read_message()
       ::decode(m.footer, p);
       auto msg = ::decode_message(nullptr, 0, m.header, m.footer,
                                   m.front, m.middle, m.data, nullptr);
-      if (!msg) {
-       logger().debug("decode message failed");
-       return;
+      if (unlikely(!msg)) {
+        logger().warn("{} decode message failed", conn);
+        throw std::system_error{make_error_code(error::corrupted_message)};
       }
+      constexpr bool add_ref = false; // Message starts with 1 ref
+      // TODO: change MessageRef with foreign_ptr
+      auto msg_ref = MessageRef{msg, add_ref};
+
       if (session_security) {
-       if (session_security->check_message_signature(msg)) {
-         logger().debug("signature check failed");
-         return;
-       }
+        if (unlikely(session_security->check_message_signature(msg))) {
+          logger().warn("{} message signature check failed", conn);
+          throw std::system_error{make_error_code(error::corrupted_message)};
+        }
       }
       // TODO: set time stamps
       msg->set_byte_throttler(conn.policy.throttler_bytes);
 
-      if (!conn.update_rx_seq(msg->get_seq())) {
+      if (unlikely(!conn.update_rx_seq(msg->get_seq()))) {
         // skip this message
         return;
       }
 
-      constexpr bool add_ref = false; // Message starts with 1 ref
-      // TODO: change MessageRef with foreign_ptr
-      auto msg_ref = MessageRef{msg, add_ref};
       // start dispatch, ignoring exceptions from the application layer
       seastar::with_gate(pending_dispatch, [this, msg = std::move(msg_ref)] {
           logger().debug("{} <= {}@{} === {}", messenger,