Backport of all patches related to INITGROUPS performance

src/ldb_modules/memberof.c

@@ -17,6 +17,9 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE   /* For strchrnul() */
+#endif
 #include <string.h>
 #include <dhash.h>
 
@@ -761,20 +764,77 @@ static int mbof_next_add_callback(struct ldb_request *req,
     return LDB_SUCCESS;
 }
 
+/* based on `ldb_dn_from_ldb_val()` but avoids memcpy */
+static const char *sss_get_linearized_dn_from_ldb_val(const struct ldb_val *strdn)
+{
+    const char *data;
+
+    if (strdn == NULL || strdn->data == NULL || strdn->length == 0) {
+        return NULL;
+    }
+
+    data = (const char *)strdn->data;
+
+    if (data[0] == '<') {
+        const char *p_save = data;
+        const char *p = data;
+        do {
+            p_save = p;
+            p = strstr(p, ">;");
+            if (p) {
+                p = p + 2;
+            }
+        } while (p);
+
+        if (p_save == data) {
+            /* Only extended components, no linearized DN */
+            return NULL;
+        }
+        return p_save;
+    }
+
+    return data;
+}
+
+__attribute__((always_inline))
+static inline bool sss_linearized_dn_match(const char *dn1, const char *dn2)
+{
+    const char *comma = NULL;
+    size_t name_len;
+
+    if ((dn1 == NULL) || (dn2 == NULL)) {
+        return false;
+    }
+
+    if (strcasecmp(dn1, dn2) != 0) {
+        return false;
+    }
+
+    /* Since sysdb cache treats 'name' case-sensitive,
+     * perform additional check to be on a safe side.
+     */
+    if (strncasecmp(dn1, "name=", 5) != 0) {
+        return true;
+    }
+
+    comma = strchrnul(dn1+5, ',');
+    name_len = comma - (dn1 + 5);
+    return (strncmp(dn1+5, dn2+5, name_len) == 0);
+}
+
 /* if it is a group, add all members for cascade effect
  * add memberof attribute to this entry
  */
 static int mbof_add_operation(struct mbof_add_operation *addop)
 {
 
-    TALLOC_CTX *tmp_ctx;
     struct mbof_ctx *ctx;
     struct mbof_add_ctx *add_ctx;
     struct ldb_context *ldb;
     struct ldb_message_element *el;
     struct ldb_request *mod_req;
     struct ldb_message *msg;
-    struct ldb_dn *elval_dn;
+    const char *elval_dn;
     struct ldb_dn *valdn;
     struct mbof_dn_array *parents;
     int i, j, ret;
@@ -799,7 +859,8 @@ static int mbof_add_operation(struct mbof_add_operation *addop)
     /* create new parent set for this entry */
     for (i = 0; i < addop->parents->num; i++) {
         /* never add yourself as memberof */
-        if (ldb_dn_compare(addop->parents->dns[i], addop->entry_dn) == 0) {
+        if (sss_linearized_dn_match(ldb_dn_get_linearized(addop->parents->dns[i]),
+                                    ldb_dn_get_linearized(addop->entry_dn))) {
             continue;
         }
         parents->dns[parents->num] = addop->parents->dns[i];
@@ -810,35 +871,34 @@ static int mbof_add_operation(struct mbof_add_operation *addop)
     el = ldb_msg_find_element(addop->entry, DB_MEMBEROF);
     if (el) {
 
-        tmp_ctx = talloc_new(addop);
-        if (!tmp_ctx) return LDB_ERR_OPERATIONS_ERROR;
-
         for (i = 0; i < el->num_values; i++) {
-            elval_dn = ldb_dn_from_ldb_val(tmp_ctx, ldb, &el->values[i]);
-            if (!elval_dn) {
+            elval_dn = sss_get_linearized_dn_from_ldb_val(&el->values[i]);
+            if (elval_dn == NULL) {
                 ldb_debug(ldb, LDB_DEBUG_TRACE, "Invalid DN in memberof [%s]",
                                             (const char *)el->values[i].data);
-                talloc_free(tmp_ctx);
                 return LDB_ERR_OPERATIONS_ERROR;
             }
             for (j = 0; j < parents->num; j++) {
-                if (ldb_dn_compare(parents->dns[j], elval_dn) == 0) {
+                /* Don't use `ldb_dn_compare()` here -
+                 * it is heavy because when DNs are not equal (vast majority of cases)
+                 * it performs `ldb_dn_casefold_internal()` to return -1 or 1,
+                 * but it's not important in this context.
+                 */
+                if (sss_linearized_dn_match(ldb_dn_get_linearized(parents->dns[j]),
+                                            elval_dn)) {
                     /* duplicate found */
                     break;
                 }
             }
             if (j < parents->num) {
                 /* remove duplicate */
-                for (;j+1 < parents->num; j++) {
-                    parents->dns[j] = parents->dns[j+1];
-                }
+                parents->dns[j] = parents->dns[parents->num - 1];
                 parents->num--;
             }
         }
 
         if (parents->num == 0) {
             /* already contains all parents as memberof, skip to next */
-            talloc_free(tmp_ctx);
             talloc_free(addop->entry);
             addop->entry = NULL;
 
@@ -856,7 +916,6 @@ static int mbof_add_operation(struct mbof_add_operation *addop)
                                        LDB_SUCCESS);
             }
         }
-        talloc_free(tmp_ctx);
     }
 
     /* if it is a group add all members */
@@ -938,7 +997,6 @@ static int mbof_add_operation(struct mbof_add_operation *addop)
         return LDB_ERR_OPERATIONS_ERROR;
     }
     for (i = 0, j = 0; i < parents->num; i++) {
-        if (ldb_dn_compare(parents->dns[i], msg->dn) == 0) continue;
         val = ldb_dn_get_linearized(parents->dns[i]);
         el->values[j].length = strlen(val);
         el->values[j].data = (uint8_t *)talloc_strdup(el->values, val);

src/providers/ldap/sdap.c

@@ -445,7 +445,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
         goto done;
     }
 
-    DEBUG(SSSDBG_TRACE_LIBS, "OriginalDN: [%s].\n", str);
+    DEBUG_CONDITIONAL(SSSDBG_TRACE_LIBS, "OriginalDN: [%s].\n", str);
     PROBE(SDAP_PARSE_ENTRY, "OriginalDN", str, strlen(str));
     ret = sysdb_attrs_add_string(attrs, SYSDB_ORIG_DN, str);
     ldap_memfree(str);
@@ -1824,7 +1824,7 @@ errno_t sdap_get_primary_name(const char *attr_name,
     }
     talloc_free(tmp_ctx);
 
-    DEBUG(SSSDBG_TRACE_FUNC, "Processing object %s\n", orig_name);
+    DEBUG_CONDITIONAL(SSSDBG_TRACE_FUNC, "Processing object %s\n", orig_name);
 
     *_primary_name = orig_name;
 

src/providers/ldap/sdap_async.c

@@ -220,7 +220,7 @@ static void sdap_process_result(struct tevent_context *ev, void *pvt)
      * later in this function once we can match the reply with an operation. */
     old_chain_id = sss_chain_id_set(0);
 
-    DEBUG(SSSDBG_TRACE_INTERNAL,
+    DEBUG_CONDITIONAL(SSSDBG_TRACE_INTERNAL,
           "Trace: sh[%p], connected[%d], ops[%p], ldap[%p]\n",
               sh, (int)sh->connected, sh->ops, sh->ldap);
 
@@ -234,7 +234,7 @@ static void sdap_process_result(struct tevent_context *ev, void *pvt)
     if (ret == 0) {
         /* this almost always means we have reached the end of
          * the list of received messages */
-        DEBUG(SSSDBG_TRACE_INTERNAL, "Trace: end of ldap_result list\n");
+        DEBUG_CONDITIONAL(SSSDBG_TRACE_INTERNAL, "Trace: end of ldap_result list\n");
         return;
     }
 
@@ -360,7 +360,7 @@ static void sdap_process_message(struct tevent_context *ev,
         return;
     }
 
-    DEBUG(SSSDBG_TRACE_ALL,
+    DEBUG_CONDITIONAL(SSSDBG_TRACE_ALL,
           "Message type: [%s]\n", sdap_ldap_result_str(msgtype));
 
     switch (msgtype) {
@@ -471,7 +471,8 @@ static int sdap_op_destructor(void *mem)
     DLIST_REMOVE(op->sh->ops, op);
 
     if (op->done) {
-        DEBUG(SSSDBG_TRACE_INTERNAL, "Operation %d finished\n", op->msgid);
+        DEBUG_CONDITIONAL(SSSDBG_TRACE_INTERNAL,
+                          "Operation %d finished\n", op->msgid);
         return 0;
     }
 
@@ -1629,17 +1630,6 @@ static errno_t sdap_get_generic_ext_step(struct tevent_req *req)
      */
     talloc_zfree(state->op);
 
-    DEBUG(SSSDBG_TRACE_FUNC,
-         "calling ldap_search_ext with [%s][%s].\n",
-          state->filter ? state->filter : "no filter",
-          state->search_base);
-    if (state->attrs) {
-        for (int i = 0; state->attrs[i]; i++) {
-            DEBUG(SSSDBG_TRACE_LIBS,
-                  "Requesting attrs: [%s]\n", state->attrs[i]);
-        }
-    }
-
     disable_paging = dp_opt_get_bool(state->opts->basic, SDAP_DISABLE_PAGING);
 
     if (!disable_paging
@@ -1690,7 +1680,6 @@ static errno_t sdap_get_generic_ext_step(struct tevent_req *req)
         }
         goto done;
     }
-    DEBUG(SSSDBG_TRACE_INTERNAL, "ldap_search_ext called, msgid = %d\n", msgid);
 
     stat_info = talloc_asprintf(state, "server: [%s] filter: [%s] base: [%s]",
                                 sdap_get_server_peer_str_safe(state->sh),
@@ -1699,6 +1688,15 @@ static errno_t sdap_get_generic_ext_step(struct tevent_req *req)
         DEBUG(SSSDBG_OP_FAILURE, "Failed to create info string, ignored.\n");
     }
 
+    DEBUG(SSSDBG_TRACE_FUNC, "ldap_search_ext called: %s; msgid = %d\n",
+          (stat_info ? stat_info : "N/A"), msgid);
+    if (state->attrs) {
+        for (int i = 0; state->attrs[i]; i++) {
+            DEBUG_CONDITIONAL(SSSDBG_TRACE_ALL, "Requesting attrs: [%s]\n",
+                              state->attrs[i]);
+        }
+    }
+
     ret = sdap_op_add(state, state->ev, state->sh, msgid, stat_info,
                       sdap_get_generic_op_finished, req,
                       state->timeout,

src/providers/ldap/sdap_async_groups.c

@@ -1943,7 +1943,7 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
     bool next_base = false;
     size_t count;
     struct sysdb_attrs **groups;
-    char **sysdb_groupnamelist;
+
 
     ret = sdap_get_and_parse_generic_recv(subreq, state,
                                           &count, &groups);
@@ -1999,22 +1999,8 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
     }
 
     if (state->no_members) {
-        ret = sdap_get_primary_fqdn_list(state->dom, state,
-                                state->groups, state->count,
-                                state->opts->group_map[SDAP_AT_GROUP_NAME].name,
-                                state->opts->group_map[SDAP_AT_GROUP_OBJECTSID].name,
-                                state->opts->idmap_ctx,
-                                &sysdb_groupnamelist);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE,
-                  "sysdb_attrs_primary_name_list failed.\n");
-            tevent_req_error(req, ret);
-            return;
-        }
-
         ret = sdap_add_incomplete_groups(state->sysdb, state->dom, state->opts,
-                                         sysdb_groupnamelist, state->groups,
-                                         state->count);
+                                         state->groups, state->count);
         if (ret == EOK) {
             DEBUG(SSSDBG_TRACE_LIBS,
                   "Writing only group data without members was successful.\n");