From ae780515cd4db1e6f23db9f75a628ce9c39bdd49 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 9 Jun 2017 12:35:36 -0500

Subject: [PATCH 1/8] Refactor: libcrmcommon: remember when IPC client is root

 or cluster user

will allow using a different eviction threshold

---

 include/crm/common/ipcs.h |  3 ++-

 lib/common/ipc.c          | 13 ++++++++++---

 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/crm/common/ipcs.h b/include/crm/common/ipcs.h

index 52338e3..43b7b60 100644

--- a/include/crm/common/ipcs.h

+++ b/include/crm/common/ipcs.h

@@ -60,7 +60,8 @@ struct crm_remote_s {

 enum crm_client_flags

 {

-    crm_client_flag_ipc_proxied = 0x00001, /* ipc_proxy code only */

+    crm_client_flag_ipc_proxied    = 0x00001, /* ipc_proxy code only */

+    crm_client_flag_ipc_privileged = 0x00002, /* root or cluster user */

 };

 struct crm_client_s {

diff --git a/lib/common/ipc.c b/lib/common/ipc.c

index 50980ec..e0a7a5c 100644

--- a/lib/common/ipc.c

+++ b/lib/common/ipc.c

@@ -314,6 +314,7 @@ crm_client_alloc(void *key)

 crm_client_t *

 crm_client_new(qb_ipcs_connection_t * c, uid_t uid_client, gid_t gid_client)

 {

+    static gid_t uid_cluster = 0;

     static gid_t gid_cluster = 0;

     crm_client_t *client = NULL;

@@ -323,11 +324,12 @@ crm_client_new(qb_ipcs_connection_t * c, uid_t uid_client, gid_t gid_client)

         return NULL;

     }

-    if (gid_cluster == 0) {

-        if(crm_user_lookup(CRM_DAEMON_USER, NULL, &gid_cluster) < 0) {

+    if (uid_cluster == 0) {

+        if (crm_user_lookup(CRM_DAEMON_USER, &uid_cluster, &gid_cluster) < 0) {

             static bool have_error = FALSE;

             if(have_error == FALSE) {

-                crm_warn("Could not find group for user %s", CRM_DAEMON_USER);

+                crm_warn("Could not find user and group IDs for user %s",

+                         CRM_DAEMON_USER);

                 have_error = TRUE;

             }

         }

@@ -347,6 +349,11 @@ crm_client_new(qb_ipcs_connection_t * c, uid_t uid_client, gid_t gid_client)

     client->kind = CRM_CLIENT_IPC;

     client->pid = crm_ipcs_client_pid(c);

+    if ((uid_client == 0) || (uid_client == uid_cluster)) {

+        /* Remember when a connection came from root or hacluster */

+        set_bit(client->flags, crm_client_flag_ipc_privileged);

+    }

+

     crm_debug("Connecting %p for uid=%d gid=%d pid=%u id=%s", c, uid_client, gid_client, client->pid, client->id);

 #if ENABLE_ACL

-- 

1.8.3.1

From f93ce6bdd6be5d2670ab0cd8dd10d3b8b9972a65 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 9 Jun 2017 14:46:16 -0500

Subject: [PATCH 2/8] Low: libcrmcommon: add function for testing daemon name

---

 include/crm/common/util.h |  2 ++

 lib/common/utils.c        | 21 +++++++++++++++++++++

 2 files changed, 23 insertions(+)

diff --git a/include/crm/common/util.h b/include/crm/common/util.h

index 682b346..aa192f9 100644

--- a/include/crm/common/util.h

+++ b/include/crm/common/util.h

@@ -26,6 +26,7 @@

 #  include <sys/types.h>

 #  include <stdlib.h>

+#  include <stdbool.h>

 #  include <limits.h>

 #  include <signal.h>

 #  include <sysexits.h>

@@ -127,6 +128,7 @@ gboolean did_rsc_op_fail(lrmd_event_data_t * event, int target_rc);

 char *crm_md5sum(const char *buffer);

 char *crm_generate_uuid(void);

+bool crm_is_daemon_name(const char *name);

 int crm_user_lookup(const char *name, uid_t * uid, gid_t * gid);

diff --git a/lib/common/utils.c b/lib/common/utils.c

index 27ed60d..a652197 100644

--- a/lib/common/utils.c

+++ b/lib/common/utils.c

@@ -1959,6 +1959,27 @@ crm_generate_uuid(void)

     return buffer;

 }

+/*!

+ * \brief Check whether a string represents a cluster daemon name

+ *

+ * \param[in] name  String to check

+ *

+ * \return TRUE if name is standard client name used by daemons, FALSE otherwise

+ */

+bool

+crm_is_daemon_name(const char *name)

+{

+    return (name &&

+            (!strcmp(name, CRM_SYSTEM_CRMD)

+            || !strcmp(name, CRM_SYSTEM_STONITHD)

+            || !strcmp(name, T_ATTRD)

+            || !strcmp(name, CRM_SYSTEM_CIB)

+            || !strcmp(name, CRM_SYSTEM_MCP)

+            || !strcmp(name, CRM_SYSTEM_DC)

+            || !strcmp(name, CRM_SYSTEM_TENGINE)

+            || !strcmp(name, CRM_SYSTEM_LRMD)));

+}

+

 #include <md5.h>

 char *

-- 

1.8.3.1

From 19497fa9785c854084253167b7dc54a8c026e1ad Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 9 Jun 2017 11:11:50 -0500

Subject: [PATCH 3/8] Low: libcrmcommon: support setting max queue length per

 IPC client

---

 include/crm/common/ipcs.h |  3 +++

 lib/common/ipc.c          | 39 ++++++++++++++++++++++++++-------------

 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/include/crm/common/ipcs.h b/include/crm/common/ipcs.h

index 43b7b60..d2db212 100644

--- a/include/crm/common/ipcs.h

+++ b/include/crm/common/ipcs.h

@@ -19,6 +19,7 @@

 #ifndef CRM_COMMON_IPCS__H

 #  define CRM_COMMON_IPCS__H

+#  include <stdbool.h>

 #  include <qb/qbipcs.h>

 #  ifdef HAVE_GNUTLS_GNUTLS_H

 #    undef KEYFILE

@@ -95,6 +96,7 @@ struct crm_client_s {

     struct crm_remote_s *remote;        /* TCP/TLS */

     unsigned int queue_backlog; /* IPC queue length after last flush */

+    unsigned int queue_max;     /* Evict client whose queue grows this big */

 };

 extern GHashTable *client_connections;

@@ -110,6 +112,7 @@ crm_client_t *crm_client_alloc(void *key);

 crm_client_t *crm_client_new(qb_ipcs_connection_t * c, uid_t uid, gid_t gid);

 void crm_client_destroy(crm_client_t * c);

 void crm_client_disconnect_all(qb_ipcs_service_t *s);

+bool crm_set_client_queue_max(crm_client_t *client, const char *qmax);

 void crm_ipcs_send_ack(crm_client_t * c, uint32_t request, uint32_t flags,

                        const char *tag, const char *function, int line);

diff --git a/lib/common/ipc.c b/lib/common/ipc.c

index e0a7a5c..3110334 100644

--- a/lib/common/ipc.c

+++ b/lib/common/ipc.c

@@ -37,8 +37,9 @@

 #define PCMK_IPC_VERSION 1

-/* Evict clients whose event queue grows this large */

-#define PCMK_IPC_MAX_QUEUE 500

+/* Evict clients whose event queue grows this large (by default) */

+#define PCMK_IPC_DEFAULT_QUEUE_MAX 500

+#define PCMK_IPC_DEFAULT_QUEUE_MAX_S "500"

 struct crm_ipc_response_header {

     struct qb_ipc_response_header qb;

@@ -409,6 +410,24 @@ crm_client_destroy(crm_client_t * c)

     free(c);

 }

+/*!

+ * \brief Raise IPC eviction threshold for a client, if allowed

+ *

+ * \param[in,out] client     Client to modify

+ * \param[in]     queue_max  New threshold (as string)

+ *

+ * \return TRUE if change was allowed, FALSE otherwise

+ */

+bool

+crm_set_client_queue_max(crm_client_t *client, const char *qmax)

+{

+    if (is_set(client->flags, crm_client_flag_ipc_privileged)) {

+        client->queue_max = crm_parse_int(qmax, PCMK_IPC_DEFAULT_QUEUE_MAX_S);

+        return TRUE;

+    }

+    return FALSE;

+}

+

 int

 crm_ipcs_client_pid(qb_ipcs_connection_t * c)

 {

@@ -553,18 +572,12 @@ crm_ipcs_flush_events(crm_client_t * c)

     }

     if (queue_len) {

-        /* We want to allow clients to briefly fall behind on processing

-         * incoming messages, but drop completely unresponsive clients so the

-         * connection doesn't consume resources indefinitely.

-         *

-         * @TODO It is possible that the queue could reasonably grow large in a

-         * short time. An example is a reprobe of hundreds of resources on many

-         * nodes resulting in a surge of CIB replies to the crmd. We could

-         * possibly give cluster daemons a higher threshold here, and/or prevent

-         * such a surge by throttling LRM history writes in the crmd.

-         */

-        if (queue_len > PCMK_IPC_MAX_QUEUE) {

+        /* Allow clients to briefly fall behind on processing incoming messages,

+         * but drop completely unresponsive clients so the connection doesn't

+         * consume resources indefinitely.

+         */

+        if (queue_len > QB_MAX(c->queue_max, PCMK_IPC_DEFAULT_QUEUE_MAX)) {

             if ((c->queue_backlog <= 1) || (queue_len < c->queue_backlog)) {

                 /* Don't evict for a new or shrinking backlog */

                 crm_warn("Client with process ID %u has a backlog of %u messages "

-- 

1.8.3.1

From 324e241d9d9666dfe6543ddf88d965b12d5a164f Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Mon, 12 Jun 2017 17:47:12 -0500

Subject: [PATCH 4/8] Low: libcib: always use current values when unpacking

 config

Previously, cib_read_config() called unpack_instance_attributes() with

overwrite=FALSE. This meant that changes to an option would not take effect

unless the option was not set before.

The only significant use of cib_read_config() was in cib_acl_enabled(), which

used a new, empty hash table for every call, so the issue didn't matter.

The cib daemon also used cib_read_config() to maintain a global config_hash,

which was affected by the issue, but didn't matter because it was never used.

This change will allow config_hash to be used.

---

 lib/cib/cib_utils.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c

index ab48f16..8aeed67 100644

--- a/lib/cib/cib_utils.c

+++ b/lib/cib/cib_utils.c

@@ -743,7 +743,7 @@ cib_read_config(GHashTable * options, xmlNode * current_cib)

     config = get_object_root(XML_CIB_TAG_CRMCONFIG, current_cib);

     if (config) {

         unpack_instance_attributes(current_cib, config, XML_CIB_TAG_PROPSET, NULL, options,

-                                   CIB_OPTIONS_FIRST, FALSE, now);

+                                   CIB_OPTIONS_FIRST, TRUE, now);

     }

     verify_cib_options(options);

-- 

1.8.3.1

From b986acbe131216aaa372681e97dfc0f2ef8f70ad Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Tue, 13 Jun 2017 16:03:36 -0500

Subject: [PATCH 5/8] Low: libcib: correctly search for v2 patchset changes

cib_internal_config_changed() was never updated for v2 patch format

---

 lib/cib/cib_utils.c | 23 ++++++++++++-----------

 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c

index 8aeed67..f639ada 100644

--- a/lib/cib/cib_utils.c

+++ b/lib/cib/cib_utils.c

@@ -794,23 +794,24 @@ cib_apply_patch_event(xmlNode * event, xmlNode * input, xmlNode ** output, int l

     return rc;

 }

+/* v2 and v2 patch formats */

+#define XPATH_CONFIG_CHANGE \

+    "//" XML_CIB_TAG_CRMCONFIG " | " \

+    "//" XML_DIFF_CHANGE "[contains(@" XML_DIFF_PATH ",'/" XML_CIB_TAG_CRMCONFIG "/')]"

+

 gboolean

-cib_internal_config_changed(xmlNode * diff)

+cib_internal_config_changed(xmlNode *diff)

 {

     gboolean changed = FALSE;

-    xmlXPathObject *xpathObj = NULL;

-    if (diff == NULL) {

-        return FALSE;

-    }

+    if (diff) {

+        xmlXPathObject *xpathObj = xpath_search(diff, XPATH_CONFIG_CHANGE);

-    xpathObj = xpath_search(diff, "//" XML_CIB_TAG_CRMCONFIG);

-    if (numXpathResults(xpathObj) > 0) {

-        changed = TRUE;

+        if (numXpathResults(xpathObj) > 0) {

+            changed = TRUE;

+        }

+        freeXpathObject(xpathObj);

     }

-

-    freeXpathObject(xpathObj);

-

     return changed;

 }

-- 

1.8.3.1

From 3e5cdd9b329b3328a88912db48c4e4a4d6f94563 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 9 Jun 2017 15:33:27 -0500

Subject: [PATCH 6/8] Feature: cib,libcib: support option for IPC eviction

 threshold

Only the cib utilizes the option currently, but the other daemons

could easily do the same, if they are subject to large IPC bursts.

---

 cib/callbacks.c     | 13 +++++++++++++

 cib/callbacks.h     | 10 ++++++++++

 lib/cib/cib_utils.c | 24 ++++++++++++++++++++----

 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/cib/callbacks.c b/cib/callbacks.c

index 4708f10..544a920 100644

--- a/cib/callbacks.c

+++ b/cib/callbacks.c

@@ -281,6 +281,19 @@ cib_common_callback(qb_ipcs_connection_t * c, void *data, size_t size, gboolean

             cib_client->name = crm_itoa(cib_client->pid);

         } else {

             cib_client->name = strdup(value);

+            if (crm_is_daemon_name(value)) {

+                set_bit(cib_client->options, cib_is_daemon);

+            }

+        }

+    }

+

+    /* Allow cluster daemons more leeway before being evicted */

+    if (is_set(cib_client->options, cib_is_daemon)) {

+        const char *qmax = cib_config_lookup("cluster-ipc-limit");

+

+        if (crm_set_client_queue_max(cib_client, qmax)) {

+            crm_trace("IPC threshold for %s[%u] is now %u",

+                      cib_client->name, cib_client->pid, cib_client->queue_max);

         }

     }

diff --git a/cib/callbacks.h b/cib/callbacks.h

index b4d48d6..bddff09 100644

--- a/cib/callbacks.h

+++ b/cib/callbacks.h

@@ -19,6 +19,7 @@

 #include <stdio.h>

 #include <sys/types.h>

 #include <unistd.h>

+#include <glib.h>

 #include <crm/crm.h>

 #include <crm/cib.h>

@@ -43,6 +44,9 @@ enum cib_notifications

     cib_notify_replace = 0x0004,

     cib_notify_confirm = 0x0008,

     cib_notify_diff    = 0x0010,

+

+    /* not a notification, but uses the same IPC bitmask */

+    cib_is_daemon      = 0x1000, /* whether client is another cluster daemon */

 };

 /* *INDENT-ON* */

@@ -80,3 +84,9 @@ extern void cib_ha_peer_callback(HA_Message * msg, void *private_data);

 extern int cib_ccm_dispatch(gpointer user_data);

 extern void cib_ccm_msg_callback(oc_ed_t event, void *cookie, size_t size, const void *data);

 #endif

+

+static inline const char *

+cib_config_lookup(const char *opt)

+{

+    return g_hash_table_lookup(config_hash, opt);

+}

diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c

index f639ada..adbf9c8 100644

--- a/lib/cib/cib_utils.c

+++ b/lib/cib/cib_utils.c

@@ -699,10 +699,26 @@ cib_native_notify(gpointer data, gpointer user_data)

 }

 pe_cluster_option cib_opts[] = {

-    /* name, old-name, validate, default, description */

-    {"enable-acl", NULL, "boolean", NULL, "false", &check_boolean,

-     "Enable CIB ACL", NULL}

-    ,

+    /*

+     * name, legacy name,

+     * type, allowed values, default, validator,

+     * short description,

+     * long description

+     */

+    {

+        "enable-acl", NULL,

+        "boolean", NULL, "false", &check_boolean,

+        "Enable CIB ACL",

+        NULL

+    },

+    {

+        "cluster-ipc-limit", NULL,

+        "integer", NULL, "500", &check_positive_number,

+        "Maximum IPC message backlog before disconnecting a cluster daemon",

+        "Raise this if log has \"Evicting client\" messages for cluster daemon"

+            " PIDs (a good value is the number of resources in the cluster"

+            " multiplied by the number of nodes)"

+    },

 };

 void

-- 

1.8.3.1

From 3f855794adbe9ad46aaca3848d591f5dc26d3371 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Tue, 13 Jun 2017 18:07:45 -0500

Subject: [PATCH 7/8] Doc: Pacemaker Explained: document cluster-ipc-limit

---

 doc/Pacemaker_Explained/en-US/Ch-Options.txt | 9 +++++++++

 1 file changed, 9 insertions(+)

diff --git a/doc/Pacemaker_Explained/en-US/Ch-Options.txt b/doc/Pacemaker_Explained/en-US/Ch-Options.txt

index f4e1af7..ec0c6b9 100644

--- a/doc/Pacemaker_Explained/en-US/Ch-Options.txt

+++ b/doc/Pacemaker_Explained/en-US/Ch-Options.txt

@@ -267,6 +267,15 @@ take effect, we can optionally poll the cluster's status for changes. A value

 of 0 disables polling. Positive values are an interval (in seconds unless other

 SI units are specified, e.g. 5min).

+| cluster-ipc-limit | 500 |

+indexterm:[cluster-ipc-limit,Cluster Option]

+indexterm:[Cluster,Option,cluster-ipc-limit]

+The maximum IPC message backlog before one cluster daemon will disconnect

+another. This is of use in large clusters, for which a good value is the number

+of resources in the cluster multiplied by the number of nodes. The default of

+500 is also the minimum. Raise this if you see "Evicting client" messages for

+cluster daemon PIDs in the logs.

+

 | pe-error-series-max | -1 |

 indexterm:[pe-error-series-max,Cluster Option]

 indexterm:[Cluster,Option,pe-error-series-max]

-- 

1.8.3.1

From bc6d723a94221419b15650a52e8ed1d843a32ff1 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 15 Jun 2017 14:20:27 -0500

Subject: [PATCH 8/8] Refactor: libcrmcommon: avoid redundant constant

 definitions

---

 lib/common/ipc.c | 9 ++++++---

 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/common/ipc.c b/lib/common/ipc.c

index 3110334..c238bca 100644

--- a/lib/common/ipc.c

+++ b/lib/common/ipc.c

@@ -39,7 +39,6 @@

 /* Evict clients whose event queue grows this large (by default) */

 #define PCMK_IPC_DEFAULT_QUEUE_MAX 500

-#define PCMK_IPC_DEFAULT_QUEUE_MAX_S "500"

 struct crm_ipc_response_header {

     struct qb_ipc_response_header qb;

@@ -422,8 +421,12 @@ bool

 crm_set_client_queue_max(crm_client_t *client, const char *qmax)

 {

     if (is_set(client->flags, crm_client_flag_ipc_privileged)) {

-        client->queue_max = crm_parse_int(qmax, PCMK_IPC_DEFAULT_QUEUE_MAX_S);

-        return TRUE;

+        int qmax_int = crm_int_helper(qmax, NULL);

+

+        if ((errno == 0) && (qmax_int > 0)) {

+            client->queue_max = qmax_int;

+            return TRUE;

+        }

     }

     return FALSE;

 }

-- 

1.8.3.1