AUTOFS: speed up the client by requesting multiple entries at once

https://fedorahosted.org/sssd/ticket/1166

3 files changed, 239 insertions, 78 deletions

diff --git a/src/responder/autofs/autofs_private.h b/src/responder/autofs/autofs_private.h
index b29b3bb0..bb0c6189 100644
--- a/src/responder/autofs/autofs_private.h
+++ b/src/responder/autofs/autofs_private.h
@@ -39,6 +39,7 @@ struct autofs_cmd_ctx {
     char *mapname;
     char *key;
     uint32_t cursor;
+    uint32_t max_entries;
     bool check_next;
 };
 
diff --git a/src/responder/autofs/autofssrv_cmd.c b/src/responder/autofs/autofssrv_cmd.c
index 58ffce1a..8123db2a 100644
--- a/src/responder/autofs/autofssrv_cmd.c
+++ b/src/responder/autofs/autofssrv_cmd.c
@@ -814,9 +814,12 @@ setautomntent_recv(struct tevent_req *req)
 static errno_t
 getautomntent_process(struct autofs_cmd_ctx *cmdctx,
                       struct autofs_map_ctx *map,
-                      uint32_t cursor);
+                      uint32_t cursor, uint32_t max_entries);
 static void
 getautomntent_implicit_done(struct tevent_req *req);
+static errno_t
+fill_autofs_entry(struct ldb_message *entry, struct sss_packet *packet, size_t *rp);
+
 
 static int
 sss_autofs_cmd_getautomntent(struct cli_ctx *client)
@@ -870,9 +873,11 @@ sss_autofs_cmd_getautomntent(struct cli_ctx *client)
     }
 
     SAFEALIGN_COPY_UINT32_CHECK(&cmdctx->cursor, body+c+namelen+1, blen, &c);
+    SAFEALIGN_COPY_UINT32_CHECK(&cmdctx->max_entries, body+c+namelen+1, blen, &c);
 
     DEBUG(SSSDBG_TRACE_FUNC,
-          ("Requested data of map %s cursor %d\n", cmdctx->mapname, cmdctx->cursor));
+          ("Requested data of map %s cursor %d max entries %d\n",
+           cmdctx->mapname, cmdctx->cursor, cmdctx->max_entries));
 
     ret = get_autofs_map(actx, cmdctx->mapname, &map);
     if (ret == ENOENT) {
@@ -915,7 +920,7 @@ sss_autofs_cmd_getautomntent(struct cli_ctx *client)
     DEBUG(SSSDBG_TRACE_INTERNAL,
           ("returning entries for [%s]\n", map->mapname));
 
-    ret = getautomntent_process(cmdctx, map, cmdctx->cursor);
+    ret = getautomntent_process(cmdctx, map, cmdctx->cursor, cmdctx->max_entries);
 
 done:
     return autofs_cmd_done(cmdctx, ret);
@@ -955,7 +960,8 @@ getautomntent_implicit_done(struct tevent_req *req)
         goto done;
     }
 
-    ret = getautomntent_process(cmdctx, map, cmdctx->cursor);
+    ret = getautomntent_process(cmdctx, map,
+                                cmdctx->cursor, cmdctx->max_entries);
 done:
     autofs_cmd_done(cmdctx, ret);
     return;
@@ -964,18 +970,15 @@ done:
 static errno_t
 getautomntent_process(struct autofs_cmd_ctx *cmdctx,
                       struct autofs_map_ctx *map,
-                      uint32_t cursor)
+                      uint32_t cursor, uint32_t max_entries)
 {
     struct cli_ctx *client = cmdctx->cctx;
     errno_t ret;
-    const char *key;
-    size_t keylen;
-    const char *value;
-    size_t valuelen;
     struct ldb_message *entry;
-    size_t len;
+    size_t rp;
+    uint32_t i, stop, left, nentries;
     uint8_t *body;
-    size_t blen, rp;
+    size_t blen;
 
     /* create response packet */
     ret = sss_packet_new(client->creq, 0,
@@ -994,51 +997,92 @@ getautomntent_process(struct autofs_cmd_ctx *cmdctx,
         }
         goto done;
     }
-    entry = map->entries[cursor];
+
+    ret = sss_packet_grow(client->creq->out, sizeof(uint32_t));
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Cannot grow packet\n"));
+        goto done;
+    }
+
+    sss_packet_get_body(client->creq->out, &body, &blen);
+    rp = sizeof(uint32_t);  /* We'll write the number of entries here */
+
+    left = map->entry_count - cursor;
+    stop = max_entries < left ? max_entries : left;
+
+    nentries = 0;
+    for (i=0; i < stop; i++) {
+        entry = map->entries[cursor];
+        cursor++;
+
+        ret = fill_autofs_entry(entry, client->creq->out, &rp);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  ("Cannot fill entry %d/%d, skipping\n", i, stop));
+            continue;
+        }
+        nentries++;
+    }
+
+    rp = 0;
+    SAFEALIGN_SET_UINT32(&body[rp], nentries, &rp);
+
+    ret = EOK;
+done:
+    sss_packet_set_error(client->creq->out, ret);
+    sss_cmd_done(client, cmdctx);
+
+    return EOK;
+}
+
+static errno_t
+fill_autofs_entry(struct ldb_message *entry, struct sss_packet *packet, size_t *rp)
+{
+    errno_t ret;
+    const char *key;
+    size_t keylen;
+    const char *value;
+    size_t valuelen;
+    uint8_t *body;
+    size_t blen;
+    size_t len;
 
     key = ldb_msg_find_attr_as_string(entry, SYSDB_AUTOFS_ENTRY_KEY, NULL);
     value = ldb_msg_find_attr_as_string(entry, SYSDB_AUTOFS_ENTRY_VALUE, NULL);
     if (!key || !value) {
-        ret = EAGAIN;
         DEBUG(SSSDBG_MINOR_FAILURE, ("Incomplete entry\n"));
-        goto done;
+        return EINVAL;
     }
 
-    /* FIXME - split below into a separate function */
     keylen = 1 + strlen(key);
     valuelen = 1 + strlen(value);
-    len = sizeof(uint32_t) + sizeof(uint32_t) + keylen  + sizeof(uint32_t)+ valuelen;
+    len = sizeof(uint32_t) + sizeof(uint32_t) + keylen + sizeof(uint32_t) + valuelen;
 
-    ret = sss_packet_grow(client->creq->out, len);
+    ret = sss_packet_grow(packet, len);
     if (ret != EOK) {
-        goto done;
+        DEBUG(SSSDBG_OP_FAILURE, ("Cannot grow packet\n"));
+        return ret;
     }
 
-    sss_packet_get_body(client->creq->out, &body, &blen);
+    sss_packet_get_body(packet, &body, &blen);
 
-    rp = 0;
-    SAFEALIGN_SET_UINT32(&body[rp], len, &rp);
-    SAFEALIGN_SET_UINT32(&body[rp], keylen, &rp);
+    SAFEALIGN_SET_UINT32(&body[*rp], len, rp);
+    SAFEALIGN_SET_UINT32(&body[*rp], keylen, rp);
 
     if (keylen == 1) {
-        body[rp] = '\0';
+        body[*rp] = '\0';
     } else {
-        memcpy(&body[rp], key, keylen);
+        memcpy(&body[*rp], key, keylen);
     }
-    rp += keylen;
+    *rp += keylen;
 
-    SAFEALIGN_SET_UINT32(&body[rp], valuelen, &rp);
+    SAFEALIGN_SET_UINT32(&body[*rp], valuelen, rp);
     if (valuelen == 1) {
-        body[rp] = '\0';
+        body[*rp] = '\0';
     } else {
-        memcpy(&body[rp], value, valuelen);
+        memcpy(&body[*rp], value, valuelen);
     }
-    rp += valuelen;
-
-    ret = EOK;
-done:
-    sss_packet_set_error(client->creq->out, ret);
-    sss_cmd_done(client, cmdctx);
+    *rp += valuelen;
 
     return EOK;
 }
diff --git a/src/sss_client/autofs/sss_autofs.c b/src/sss_client/autofs/sss_autofs.c
index 6195c0fc..218c1ed7 100644
--- a/src/sss_client/autofs/sss_autofs.c
+++ b/src/sss_client/autofs/sss_autofs.c
@@ -28,11 +28,29 @@
 #define MAX_AUTOMNTMAPNAME_LEN  NAME_MAX
 #define MAX_AUTOMNTKEYNAME_LEN  NAME_MAX
 
+/* How many entries shall _sss_getautomntent_r retreive at once */
+#define GETAUTOMNTENT_MAX_ENTRIES   512
+
 struct automtent {
     char *mapname;
     size_t cursor;
 };
 
+static struct sss_getautomntent_data {
+    char *mapname;
+    size_t len;
+    size_t ptr;
+    uint8_t *data;
+} sss_getautomntent_data;
+
+static void
+sss_getautomntent_data_clean(void)
+{
+    free(sss_getautomntent_data.data);
+    free(sss_getautomntent_data.mapname);
+    memset(&sss_getautomntent_data, 0, sizeof(struct sss_getautomntent_data));
+}
+
 errno_t
 _sss_setautomntent(const char *mapname, void **context)
 {
@@ -49,6 +67,9 @@ _sss_setautomntent(const char *mapname, void **context)
 
     sss_nss_lock();
 
+    /* Make sure there are no leftovers from previous runs */
+    sss_getautomntent_data_clean();
+
     ret = sss_strnlen(mapname, MAX_AUTOMNTMAPNAME_LEN, &name_len);
     if (ret != 0) {
         ret = EINVAL;
@@ -106,6 +127,118 @@ out:
     return ret;
 }
 
+static errno_t
+sss_getautomntent_data_return(const char *mapname, char **_key, char **_value)
+{
+    size_t dp;
+    uint32_t len = 0;
+    char *key = NULL;
+    uint32_t keylen;
+    char *value = NULL;
+    uint32_t vallen;
+    errno_t ret;
+
+    if (sss_getautomntent_data.mapname == NULL ||
+        sss_getautomntent_data.data == NULL ||
+        sss_getautomntent_data.ptr >= sss_getautomntent_data.len) {
+        /* We're done with this buffer */
+        ret = ENOENT;
+        goto done;
+    }
+
+    ret = strcmp(mapname, sss_getautomntent_data.mapname);
+    if (ret != EOK) {
+        /* The map we're looking for is not cached. Let responder
+         * do an implicit setautomntent */
+        ret = ENOENT;
+        goto done;
+    }
+
+    dp = sss_getautomntent_data.ptr;
+
+    SAFEALIGN_COPY_UINT32(&len, sss_getautomntent_data.data+dp, &dp);
+    if (len + sss_getautomntent_data.ptr > sss_getautomntent_data.len) {
+        /* len is bigger than the buffer */
+        ret = EIO;
+        goto done;
+    }
+
+    if (len == 0) {
+        /* There are no more records. */
+        *_key = NULL;
+        *_value = NULL;
+        ret = ENOENT;
+        goto done;
+    }
+
+    SAFEALIGN_COPY_UINT32(&keylen, sss_getautomntent_data.data+dp, &dp);
+    if (keylen + dp > sss_getautomntent_data.len) {
+        ret = EIO;
+        goto done;
+    }
+
+    key = malloc(keylen);
+    if (!key) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    safealign_memcpy(key, sss_getautomntent_data.data+dp, keylen, &dp);
+
+    SAFEALIGN_COPY_UINT32(&vallen, sss_getautomntent_data.data+dp, &dp);
+    if (vallen + dp > sss_getautomntent_data.len) {
+        ret = EIO;
+        goto done;
+    }
+
+    value = malloc(vallen);
+    if (!value) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    safealign_memcpy(value, sss_getautomntent_data.data+dp, vallen, &dp);
+
+    sss_getautomntent_data.ptr = dp;
+    *_key = key;
+    *_value = value;
+    return EOK;
+
+done:
+    free(key);
+    free(value);
+    sss_getautomntent_data_clean();
+    return ret;
+}
+
+/* The repbuf is owned by the sss_getautomntent_data once this
+ * function is called */
+static errno_t
+sss_getautomntent_data_save(const char *mapname, uint8_t **repbuf, size_t replen)
+{
+    size_t rp;
+    uint32_t num;
+
+    rp = 0;
+    SAFEALIGN_COPY_UINT32(&num, repbuf+rp, &rp);
+    if (num == 0) {
+        free(repbuf);
+        return ENOENT;
+    }
+
+    sss_getautomntent_data.mapname = strdup(mapname);
+    if (sss_getautomntent_data.mapname == NULL) {
+        free(repbuf);
+        return ENOENT;
+    }
+
+    sss_getautomntent_data.data = *repbuf;
+    sss_getautomntent_data.len = replen;
+    sss_getautomntent_data.ptr = rp;
+    *repbuf = NULL;
+    return EOK;
+}
+
 errno_t
 _sss_getautomntent_r(char **key, char **value, void *context)
 {
@@ -121,12 +254,6 @@ _sss_getautomntent_r(char **key, char **value, void *context)
     uint8_t *data;
     uint32_t v;
 
-    char *buf;
-    uint32_t len;
-    uint32_t keylen;
-    uint32_t vallen;
-    size_t rp;
-
     sss_nss_lock();
 
     ctx = (struct automtent *) context;
@@ -142,9 +269,21 @@ _sss_getautomntent_r(char **key, char **value, void *context)
         goto out;
     }
 
+    ret = sss_getautomntent_data_return(ctx->mapname, key, value);
+    if (ret == EOK) {
+        /* The results are available from cache. Just advance the
+         * cursor and return. */
+        ctx->cursor++;
+        ret = 0;
+        goto out;
+    }
+    /* Don't try to handle any error codes, just go to the responder again */
+
+    ret = 0;
     data_len = sizeof(uint32_t) +            /* mapname len */
                name_len + 1 +                /* mapname\0   */
-               sizeof(uint32_t);             /* index into the map */
+               sizeof(uint32_t) +            /* index into the map */
+               sizeof(uint32_t);             /* num entries to retreive */
 
     data = malloc(data_len);
     if (!data) {
@@ -152,12 +291,13 @@ _sss_getautomntent_r(char **key, char **value, void *context)
         goto out;
     }
 
-    v = name_len;
-    SAFEALIGN_COPY_UINT32(data, &v, &ctr);
+    SAFEALIGN_COPY_UINT32(data, &name_len, &ctr);
 
     safealign_memcpy(data+ctr, ctx->mapname, name_len + 1, &ctr);
 
-    v = ctx->cursor;
+    SAFEALIGN_COPY_UINT32(data+ctr, &ctx->cursor, &ctr);
+
+    v = GETAUTOMNTENT_MAX_ENTRIES;
     SAFEALIGN_COPY_UINT32(data+ctr, &v, &ctr);
 
     rd.data = data;
@@ -171,54 +311,28 @@ _sss_getautomntent_r(char **key, char **value, void *context)
         goto out;
     }
 
-    /* Got reply, let's parse it */
-    rp = 0;
-    SAFEALIGN_COPY_UINT32(&len, repbuf+rp, &rp);
-    if (len == 0) {
-        /* End of iteration */
+    /* Got reply, let's save it and return from "cache" */
+    ret = sss_getautomntent_data_save(ctx->mapname, &repbuf, replen);
+    if (ret == ENOENT) {
+        /* No results */
         *key = NULL;
         *value = NULL;
-        ret = ENOENT;
-        goto out;
-    }
-
-    SAFEALIGN_COPY_UINT32(&keylen, repbuf+rp, &rp);
-    if (keylen > len-rp) {
-        ret = EIO;
-        goto out;
-    }
-
-    buf = malloc(keylen);
-    if (!buf) {
-        ret = ENOMEM;
         goto out;
-    }
-
-    safealign_memcpy(buf, repbuf+rp, keylen, &rp);
-    *key = buf;
-
-    SAFEALIGN_COPY_UINT32(&vallen, repbuf+rp, &rp);
-    if (vallen > len-rp) {
-        ret = EIO;
+    } else if (ret != EOK) {
+        /* Unexpected error */
         goto out;
     }
 
-    buf = malloc(vallen);
-    if (!buf) {
-        free(*key);
-        ret = ENOMEM;
+    ret = sss_getautomntent_data_return(ctx->mapname, key, value);
+    if (ret != EOK) {
         goto out;
     }
 
-    safealign_memcpy(buf, repbuf+rp, vallen, &rp);
-    *value = buf;
-
     /* Advance the cursor so that we'll fetch the next map
      * next time getautomntent is called */
     ctx->cursor++;
     ret = 0;
 out:
-    free(repbuf);
     sss_nss_unlock();
     return ret;
 }
@@ -341,6 +455,8 @@ _sss_endautomntent(void **context)
 
     sss_nss_lock();
 
+    sss_getautomntent_data_clean();
+
     fctx = (struct automtent *) *context;
 
     free(fctx->mapname);