From: Joao Eduardo Luis <joao.luis@inktank.com>
Date: Fri, 12 Sep 2014 16:21:45 +0000 (+0100)
Subject: mon: Monitor: create logical divisions on dispatch() based on msg nature
X-Git-Tag: v0.87~41^2~8
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3d78285dfa6eac4098f4a1d2e6d657d8089f08d2;p=ceph.git

mon: Monitor: create logical divisions on dispatch() based on msg nature

Instead of a single switch(), have multiple switch() and order them by
increasing necessity of privileges.

This patch thus divides the big switch into:

- messages not requiring auth/caps checks at all
- messages which caps shall be checked somewhere else
- messages the Monitor class needs to deal with but only require a
  client to have enough caps for the monitor to consider handling them
- messages that only a monitor is allowed to send.

Backport: firefly

Signed-off-by: Joao Eduardo Luis <joao.luis@inktank.com>
---

diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index 58267fe05f85..49939f11d4a9 100644
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -3200,41 +3200,30 @@ void Monitor::dispatch(MonSession *s, Message *m, const bool src_is_mon)
 {
   assert(m != NULL);
 
+  /* deal with all messages that do not necessarily need caps */
+  bool dealt_with = true;
   switch (m->get_type()) {
-
-    case MSG_ROUTE:
-      handle_route(static_cast<MRoute*>(m));
-      break;
-
-    // misc
-    case CEPH_MSG_MON_GET_MAP:
-      handle_mon_get_map(static_cast<MMonGetMap*>(m));
-      break;
-
-    case CEPH_MSG_MON_GET_VERSION:
-      handle_get_version(static_cast<MMonGetVersion*>(m));
-      break;
-
-    case MSG_MON_COMMAND:
-      handle_command(static_cast<MMonCommand*>(m));
+    // auth
+    case MSG_MON_GLOBAL_ID:
+    case CEPH_MSG_AUTH:
+      /* no need to check caps here */
+      paxos_service[PAXOS_AUTH]->dispatch((PaxosServiceMessage*)m);
       break;
 
-    case CEPH_MSG_MON_SUBSCRIBE:
-      /* FIXME: check what's being subscribed, filter accordingly */
-      handle_subscribe(static_cast<MMonSubscribe*>(m));
+    case CEPH_MSG_PING:
+      handle_ping(static_cast<MPing*>(m));
       break;
 
-    case MSG_MON_PROBE:
-      handle_probe(static_cast<MMonProbe*>(m));
+    default:
+      dealt_with = false;
       break;
+  }
+  if (dealt_with)
+    return;
 
-    // Sync (i.e., the new slurp, but on steroids)
-    case MSG_MON_SYNC:
-      handle_sync(static_cast<MMonSync*>(m));
-      break;
-    case MSG_MON_SCRUB:
-      handle_scrub(static_cast<MMonScrub*>(m));
-      break;
+  /* deal with all messages which caps should be checked somewhere else */
+  dealt_with = true;
+  switch (m->get_type()) {
 
     // OSDs
     case MSG_OSD_MARK_ME_DOWN:
@@ -3255,12 +3244,6 @@ void Monitor::dispatch(MonSession *s, Message *m, const bool src_is_mon)
       paxos_service[PAXOS_MDSMAP]->dispatch((PaxosServiceMessage*)m);
       break;
 
-    // auth
-    case MSG_MON_GLOBAL_ID:
-    case CEPH_MSG_AUTH:
-      /* no need to check caps here */
-      paxos_service[PAXOS_AUTH]->dispatch((PaxosServiceMessage*)m);
-      break;
 
     // pg
     case CEPH_MSG_STATFS:
@@ -3278,6 +3261,65 @@ void Monitor::dispatch(MonSession *s, Message *m, const bool src_is_mon)
       paxos_service[PAXOS_LOG]->dispatch((PaxosServiceMessage*)m);
       break;
 
+    default:
+      dealt_with = false;
+      break;
+  }
+  if (dealt_with)
+    return;
+
+  /* messages we, the Monitor class, need to deal with
+   * but may be sent by clients. */
+  dealt_with = true;
+  switch (m->get_type()) {
+
+    // misc
+    case CEPH_MSG_MON_GET_MAP:
+      handle_mon_get_map(static_cast<MMonGetMap*>(m));
+      break;
+
+    case CEPH_MSG_MON_GET_VERSION:
+      handle_get_version(static_cast<MMonGetVersion*>(m));
+      break;
+
+    case MSG_MON_COMMAND:
+      handle_command(static_cast<MMonCommand*>(m));
+      break;
+
+    case CEPH_MSG_MON_SUBSCRIBE:
+      /* FIXME: check what's being subscribed, filter accordingly */
+      handle_subscribe(static_cast<MMonSubscribe*>(m));
+      break;
+
+    default:
+      dealt_with = false;
+      break;
+  }
+  if (dealt_with)
+    return;
+
+  /* messages that should only be sent by another monitor */
+  dealt_with = true;
+  switch (m->get_type()) {
+
+    case MSG_ROUTE:
+      handle_route(static_cast<MRoute*>(m));
+      break;
+
+    case MSG_MON_PROBE:
+      handle_probe(static_cast<MMonProbe*>(m));
+      break;
+
+    // Sync (i.e., the new slurp, but on steroids)
+    case MSG_MON_SYNC:
+      handle_sync(static_cast<MMonSync*>(m));
+      break;
+    case MSG_MON_SCRUB:
+      handle_scrub(static_cast<MMonScrub*>(m));
+      break;
+
+    /* log acks are sent from a monitor we sent the MLog to, and are
+       never sent by clients to us. */
     case MSG_LOGACK:
       log_client.handle_log_ack((MLogAck*)m);
       m->put();
@@ -3352,15 +3394,14 @@ void Monitor::dispatch(MonSession *s, Message *m, const bool src_is_mon)
       health_monitor->dispatch(static_cast<MMonHealth *>(m));
       break;
 
-    case CEPH_MSG_PING:
-      handle_ping(static_cast<MPing*>(m));
-      break;
-
     default:
-      dout(1) << "dropping unexpected " << *m << dendl;
-      m->put();
+      dealt_with = false;
       break;
   }
+  if (!dealt_with) {
+    dout(1) << "dropping unexpected " << *m << dendl;
+    m->put();
+  }
 }
 
 void Monitor::handle_ping(MPing *m)