Whamcloud - gitweb
LU-17015 build: rework upcall cache
authorSebastien Buisson <sbuisson@ddn.com>
Thu, 12 Oct 2023 14:45:29 +0000 (16:45 +0200)
committerAndreas Dilger <adilger@whamcloud.com>
Sat, 14 Oct 2023 10:47:49 +0000 (10:47 +0000)
EX-4333 introduced in upcall_cache.c a dependency on md_object.h for
struct lu_ucred. Rework files to move this dependency to a differnt
file, so that upcall_cache.c can be built in client-only mode.

Fixes: fb0082bba1 ("EX-4333 sec: support supplementary groups from client")
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
Change-Id: I4bcc7e07a4f4886c5994d17cbef72ea09eb1be1d
Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/52670
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
lustre/include/upcall_cache.h
lustre/obdclass/Makefile.in
lustre/obdclass/upcall_cache.c
lustre/obdclass/upcall_cache_internal.c [new file with mode: 0644]
lustre/obdclass/upcall_cache_internal.h [new file with mode: 0644]

index 4f553d9..8c2cdf0 100644 (file)
 #include <libcfs/libcfs.h>
 #include <uapi/linux/lnet/lnet-types.h>
 
+/* The special identity_upcall value "INTERNAL" implements a particular behavior
+ * which does not involve an actual upcall. Instead, the cache is filled with
+ * supplementary groups read from the user's credentials provided as input
+ * (usually got from the client request), cumulatively at each request.
+ */
+#define IDENTITY_UPCALL_INTERNAL       "INTERNAL"
+
 /** \defgroup ucache ucache
  *
  * @{
index 53401d5..4694d3e 100644 (file)
@@ -19,6 +19,7 @@ obdclass-all-objs += lustre_compr.o
 @SERVER_TRUE@obdclass-all-objs += acl.o
 @SERVER_TRUE@obdclass-all-objs += idmap.o
 @SERVER_TRUE@obdclass-all-objs += upcall_cache.o
+@SERVER_TRUE@obdclass-all-objs += upcall_cache_internal.o
 @SERVER_TRUE@obdclass-all-objs += lprocfs_jobstats.o
 @SERVER_TRUE@obdclass-all-objs += lprocfs_status_server.o
 @SERVER_TRUE@obdclass-all-objs += lu_ucred.o
@@ -32,12 +33,13 @@ obdclass-objs := $(obdclass-all-objs)
 EXTRA_PRE_CFLAGS := -I@LINUX@/fs -I@LDISKFS_DIR@ -I@LDISKFS_DIR@/ldiskfs
 
 EXTRA_DIST = $(obdclass-all-objs:.o=.c) llog_test.c llog_internal.h
-EXTRA_DIST += cl_internal.h local_storage.h
+EXTRA_DIST += cl_internal.h local_storage.h upcall_cache_internal.h
 EXTRA_DIST += range_lock.c interval_tree.c
 
 @SERVER_FALSE@EXTRA_DIST += acl.c
 @SERVER_FALSE@EXTRA_DIST += idmap.c
 @SERVER_FALSE@EXTRA_DIST += upcall_cache.c
+@SERVER_FALSE@EXTRA_DIST += upcall_cache_internal.c
 @SERVER_FALSE@EXTRA_DIST += lprocfs_jobstats.c
 @SERVER_FALSE@EXTRA_DIST += lprocfs_status_server.c
 @SERVER_FALSE@EXTRA_DIST += lu_ucred.c
index af0d7f2..38ada78 100644 (file)
 
 #include <libcfs/libcfs.h>
 #include <uapi/linux/lnet/lnet-types.h>
-#include <lustre_idmap.h>
-#include <md_object.h>
 #include <upcall_cache.h>
-
-/* The special identity_upcall value "INTERNAL" implements a particular behavior
- * which does not involve an actual upcall. Instead, the cache is filled with
- * supplementary groups read from the user's credentials provided as input
- * (usually got from the client request), cumulatively at each request.
- */
-#define IDENTITY_UPCALL_INTERNAL       "INTERNAL"
+#include "upcall_cache_internal.h"
 
 static struct upcall_cache_entry *alloc_entry(struct upcall_cache *cache,
                                              __u64 key, void *args)
@@ -67,19 +59,6 @@ static struct upcall_cache_entry *alloc_entry(struct upcall_cache *cache,
        return entry;
 }
 
-/* protected by cache lock */
-static void free_entry(struct upcall_cache *cache,
-                      struct upcall_cache_entry *entry)
-{
-       if (cache->uc_ops->free_entry)
-               cache->uc_ops->free_entry(cache, entry);
-
-       list_del(&entry->ue_hash);
-       CDEBUG(D_OTHER, "destroy cache entry %p for key %llu\n",
-               entry, entry->ue_key);
-       LIBCFS_FREE(entry, sizeof(*entry));
-}
-
 static inline int upcall_compare(struct upcall_cache *cache,
                                 struct upcall_cache_entry *entry,
                                 __u64 key, void *args)
@@ -106,20 +85,6 @@ static inline int downcall_compare(struct upcall_cache *cache,
        return 0;
 }
 
-static inline void get_entry(struct upcall_cache_entry *entry)
-{
-       atomic_inc(&entry->ue_refcount);
-}
-
-static inline void put_entry(struct upcall_cache *cache,
-                            struct upcall_cache_entry *entry)
-{
-       if (atomic_dec_and_test(&entry->ue_refcount) &&
-           (UC_CACHE_IS_INVALID(entry) || UC_CACHE_IS_EXPIRED(entry))) {
-               free_entry(cache, entry);
-       }
-}
-
 static int check_unlink_entry(struct upcall_cache *cache,
                              struct upcall_cache_entry *entry)
 {
@@ -149,39 +114,9 @@ static inline int refresh_entry(struct upcall_cache *cache,
                                struct upcall_cache_entry *entry,
                                __u32 fsgid, int ngroups, gid_t *grouplist)
 {
-       if (strcmp(cache->uc_upcall, IDENTITY_UPCALL_INTERNAL) == 0) {
-               struct group_info *ginfo = NULL;
-
-               if (ngroups && grouplist) {
-                       ginfo = groups_alloc(ngroups);
-                       if (!ginfo) {
-                               CDEBUG(D_OTHER,
-                                      "failed to alloc %d groups: rc = %d\n",
-                                      ngroups, -ENOMEM);
-                               return -ENOMEM;
-                       }
-                       lustre_groups_from_list(ginfo, grouplist);
-                       lustre_groups_sort(ginfo);
-               }
-
-               spin_lock(&cache->uc_lock);
-               get_entry(entry);
-               entry->u.identity.mi_uid = entry->ue_key;
-               entry->u.identity.mi_gid = fsgid;
-               entry->u.identity.mi_ginfo = ginfo;
-               entry->u.identity.mi_nperms = 0;
-               entry->u.identity.mi_perms = NULL;
-               entry->ue_expire = ktime_get_seconds() + cache->uc_entry_expire;
-               UC_CACHE_SET_VALID(entry);
-               UC_CACHE_CLEAR_ACQUIRING(entry);
-               spin_unlock(&cache->uc_lock);
-               put_entry(cache, entry);
-
-               CDEBUG(D_OTHER,
-                      "%s: INTERNAL refreshed entry for '%llu' with %d groups\n",
-                      cache->uc_name, entry->ue_key, ngroups);
-               return 0;
-       }
+       if (strcmp(cache->uc_upcall, IDENTITY_UPCALL_INTERNAL) == 0)
+               return refresh_entry_internal(cache, entry, fsgid,
+                                             ngroups, grouplist);
 
        LASSERT(cache->uc_ops->do_upcall);
        return cache->uc_ops->do_upcall(cache, entry);
@@ -191,12 +126,12 @@ struct upcall_cache_entry *upcall_cache_get_entry(struct upcall_cache *cache,
                                                  __u64 key, void *args)
 {
        struct upcall_cache_entry *entry = NULL, *new = NULL, *next;
-       struct lu_ucred *uc = (struct lu_ucred *)args;
-       gid_t *grouplist = NULL, *glist_p;
+       gid_t fsgid = (__u32)__kgid_val(INVALID_GID);
+       gid_t *grouplist = NULL;
        bool failedacquiring = false;
        struct list_head *head;
        wait_queue_entry_t wait;
-       int rc, found, i, ngroups = 0;
+       int rc, found, ngroups = 0;
        ENTRY;
 
        LASSERT(cache);
@@ -238,78 +173,18 @@ find_again:
        }
        get_entry(entry);
 
-       spin_unlock(&cache->uc_lock);
-       if (!grouplist && !strcmp(cache->uc_upcall, IDENTITY_UPCALL_INTERNAL) &&
-           uc && uc->uc_suppgids[0] != -1) {
-               struct md_identity *identity = &entry->u.identity;
-               bool supp_in_ginfo[2] = { false, uc->uc_suppgids[1] == -1 };
-
-               if (!identity->mi_ginfo ||
-                   !identity->mi_ginfo->ngroups)
-                       ngroups = 0;
-               else
-                       ngroups = identity->mi_ginfo->ngroups;
-
-               /* check if provided supp groups are already in cache */
-               for (i = 0; i < 2 && uc->uc_suppgids[i] != -1; i++) {
-                       if (unlikely(uc->uc_suppgids[i] == uc->uc_fsuid)) {
-                               /* Do not place user's group ID in group list */
-                               supp_in_ginfo[i] = true;
-                       } else if (ngroups) {
-                               atomic_inc(&identity->mi_ginfo->usage);
-                               supp_in_ginfo[i] =
-                                       lustre_groups_search(identity->mi_ginfo,
-                                                           uc->uc_suppgids[i]);
-                               atomic_dec(&identity->mi_ginfo->usage);
-                       }
-               }
-
-               /* build new list of groups, which is a merge of provided supp
-                * groups and all other groups already in cache
-                */
-               if (!supp_in_ginfo[0] || !supp_in_ginfo[1]) {
-                       CDEBUG(D_OTHER,
-                              "%s: INTERNAL might add suppgids %d,%d for entry '%llu'\n",
-                              cache->uc_name, uc->uc_suppgids[0],
-                              uc->uc_suppgids[1], entry->ue_key);
-
-                       ngroups += !supp_in_ginfo[0] + !supp_in_ginfo[1];
-                       CFS_ALLOC_PTR_ARRAY(grouplist, ngroups);
-                       if (grouplist == NULL)
-                               GOTO(out, entry = ERR_PTR(-ENOMEM));
-
-                       glist_p = grouplist;
-                       for (i = 0; i < 2; i++) {
-                               if (!supp_in_ginfo[i])
-                                       *(glist_p++) = uc->uc_suppgids[i];
-                       }
-
-                       if (identity->mi_ginfo && identity->mi_ginfo->ngroups) {
-                               atomic_inc(&identity->mi_ginfo->usage);
-                               lustre_list_from_groups(glist_p,
-                                                       identity->mi_ginfo);
-                               atomic_dec(&identity->mi_ginfo->usage);
-                       }
-
-                       if (!UC_CACHE_IS_NEW(entry)) {
-                               /* force refresh as an existing cache entry
-                                * cannot be modified
-                                */
-                               spin_lock(&cache->uc_lock);
-                               entry->ue_expire = ktime_get_seconds();
-                               spin_unlock(&cache->uc_lock);
-                       }
-               }
+       /* special processing of supp groups for identity upcall */
+       if (strcmp(cache->uc_upcall, IDENTITY_UPCALL_INTERNAL) == 0) {
+               spin_unlock(&cache->uc_lock);
+               rc = upcall_cache_get_entry_internal(cache, entry, args, &fsgid,
+                                                    &grouplist, &ngroups);
+               spin_lock(&cache->uc_lock);
+               if (rc)
+                       GOTO(out, entry = ERR_PTR(rc));
        }
 
-       spin_lock(&cache->uc_lock);
        /* acquire for new one */
        if (UC_CACHE_IS_NEW(entry)) {
-               gid_t fsgid = (__u32)__kgid_val(INVALID_GID);
-
-               if (uc)
-                       fsgid = uc->uc_fsgid;
-
                UC_CACHE_SET_ACQUIRING(entry);
                UC_CACHE_CLEAR_NEW(entry);
                spin_unlock(&cache->uc_lock);
diff --git a/lustre/obdclass/upcall_cache_internal.c b/lustre/obdclass/upcall_cache_internal.c
new file mode 100644 (file)
index 0000000..d999a0c
--- /dev/null
@@ -0,0 +1,152 @@
+/*
+ * GPL HEADER START
+ *
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 only,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 for more details (a copy is included
+ * in the LICENSE file that accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 along with this program; If not, see
+ * http://www.gnu.org/licenses/gpl-2.0.html
+ *
+ * GPL HEADER END
+ */
+/*
+ * Copyright (c) 2023, Whamcloud.
+ */
+/*
+ * This file is part of Lustre, http://www.lustre.org/
+ *
+ */
+#define DEBUG_SUBSYSTEM S_SEC
+
+#include <lustre_idmap.h>
+#include <md_object.h>
+#include <upcall_cache.h>
+#include "upcall_cache_internal.h"
+
+inline int refresh_entry_internal(struct upcall_cache *cache,
+                                 struct upcall_cache_entry *entry,
+                                 __u32 fsgid, int ngroups,
+                                 gid_t *grouplist)
+{
+       struct group_info *ginfo = NULL;
+
+       if (ngroups && grouplist) {
+               ginfo = groups_alloc(ngroups);
+               if (!ginfo) {
+                       CDEBUG(D_OTHER,
+                              "failed to alloc %d groups: rc = %d\n",
+                              ngroups, -ENOMEM);
+                       return -ENOMEM;
+               }
+               lustre_groups_from_list(ginfo, grouplist);
+               lustre_groups_sort(ginfo);
+       }
+
+       spin_lock(&cache->uc_lock);
+       get_entry(entry);
+       entry->u.identity.mi_uid = entry->ue_key;
+       entry->u.identity.mi_gid = fsgid;
+       entry->u.identity.mi_ginfo = ginfo;
+       entry->u.identity.mi_nperms = 0;
+       entry->u.identity.mi_perms = NULL;
+       entry->ue_expire = ktime_get_seconds() + cache->uc_entry_expire;
+       UC_CACHE_SET_VALID(entry);
+       UC_CACHE_CLEAR_ACQUIRING(entry);
+       spin_unlock(&cache->uc_lock);
+       put_entry(cache, entry);
+
+       CDEBUG(D_OTHER,
+              "%s: INTERNAL refreshed entry for '%llu' with %d groups\n",
+              cache->uc_name, entry->ue_key, ngroups);
+       return 0;
+}
+
+int upcall_cache_get_entry_internal(struct upcall_cache *cache,
+                                   struct upcall_cache_entry *entry,
+                                   void *args, gid_t *fsgid,
+                                   gid_t **grouplist, int *ngroups)
+{
+       struct lu_ucred *uc = (struct lu_ucred *)args;
+       struct md_identity *identity;
+       bool supp_in_ginfo[2];
+       gid_t *glist_p;
+       int i;
+
+       if (*grouplist || !uc || uc->uc_suppgids[0] == -1)
+               /* grouplist already built, or no supp groups
+                * => return immediately
+                */
+               return 0;
+
+       identity = &entry->u.identity;
+       *fsgid = uc->uc_fsgid;
+       supp_in_ginfo[0] = false;
+       supp_in_ginfo[1] = (uc->uc_suppgids[1] == -1);
+
+       if (!identity->mi_ginfo || !identity->mi_ginfo->ngroups)
+               *ngroups = 0;
+       else
+               *ngroups = identity->mi_ginfo->ngroups;
+
+       /* check if provided supp groups are already in cache */
+       for (i = 0; i < 2 && uc->uc_suppgids[i] != -1; i++) {
+               if (unlikely(uc->uc_suppgids[i] == uc->uc_fsuid)) {
+                       /* Do not place user's group ID in group list */
+                       supp_in_ginfo[i] = true;
+               } else if (*ngroups) {
+                       atomic_inc(&identity->mi_ginfo->usage);
+                       supp_in_ginfo[i] =
+                               lustre_groups_search(identity->mi_ginfo,
+                                                    uc->uc_suppgids[i]);
+                       atomic_dec(&identity->mi_ginfo->usage);
+               }
+       }
+
+       /* build new list of groups, which is a merge of provided supp
+        * groups and all other groups already in cache
+        */
+       if (!supp_in_ginfo[0] || !supp_in_ginfo[1]) {
+               CDEBUG(D_OTHER,
+                      "%s: INTERNAL might add suppgids %d,%d for entry '%llu'\n",
+                      cache->uc_name, uc->uc_suppgids[0],
+                      uc->uc_suppgids[1], entry->ue_key);
+
+               *ngroups += !supp_in_ginfo[0] + !supp_in_ginfo[1];
+               CFS_ALLOC_PTR_ARRAY(*grouplist, *ngroups);
+               if (*grouplist == NULL)
+                       return -ENOMEM;
+
+               glist_p = *grouplist;
+               for (i = 0; i < 2; i++) {
+                       if (!supp_in_ginfo[i])
+                               *(glist_p++) = uc->uc_suppgids[i];
+               }
+
+               if (identity->mi_ginfo && identity->mi_ginfo->ngroups) {
+                       atomic_inc(&identity->mi_ginfo->usage);
+                       lustre_list_from_groups(glist_p, identity->mi_ginfo);
+                       atomic_dec(&identity->mi_ginfo->usage);
+               }
+
+               if (!UC_CACHE_IS_NEW(entry)) {
+                       /* force refresh as an existing cache entry
+                        * cannot be modified
+                        */
+                       spin_lock(&cache->uc_lock);
+                       entry->ue_expire = ktime_get_seconds();
+                       spin_unlock(&cache->uc_lock);
+               }
+       }
+
+       return 0;
+}
diff --git a/lustre/obdclass/upcall_cache_internal.h b/lustre/obdclass/upcall_cache_internal.h
new file mode 100644 (file)
index 0000000..cab97b2
--- /dev/null
@@ -0,0 +1,88 @@
+/*
+ * GPL HEADER START
+ *
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 only,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 for more details (a copy is included
+ * in the LICENSE file that accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 along with this program; If not, see
+ * http://www.gnu.org/licenses/gpl-2.0.html
+ *
+ * GPL HEADER END
+ */
+/*
+ * Copyright (c) 2023, Whamcloud.
+ */
+/*
+ * This file is part of Lustre, http://www.lustre.org/
+ *
+ */
+
+#ifndef _UPCALL_CACHE_INTERNAL_H
+#define _UPCALL_CACHE_INTERNAL_H
+
+#include <upcall_cache.h>
+
+/* protected by cache lock */
+static void free_entry(struct upcall_cache *cache,
+                      struct upcall_cache_entry *entry)
+{
+       if (cache->uc_ops->free_entry)
+               cache->uc_ops->free_entry(cache, entry);
+
+       list_del(&entry->ue_hash);
+       CDEBUG(D_OTHER, "destroy cache entry %p for key %llu\n",
+              entry, entry->ue_key);
+       LIBCFS_FREE(entry, sizeof(*entry));
+}
+
+static inline void get_entry(struct upcall_cache_entry *entry)
+{
+       atomic_inc(&entry->ue_refcount);
+}
+
+static inline void put_entry(struct upcall_cache *cache,
+                            struct upcall_cache_entry *entry)
+{
+       if (atomic_dec_and_test(&entry->ue_refcount) &&
+           (UC_CACHE_IS_INVALID(entry) || UC_CACHE_IS_EXPIRED(entry))) {
+               free_entry(cache, entry);
+       }
+}
+
+#ifdef HAVE_SERVER_SUPPORT
+int refresh_entry_internal(struct upcall_cache *cache,
+                          struct upcall_cache_entry *entry,
+                          __u32 fsgid, int ngroups,
+                          gid_t *grouplist);
+int upcall_cache_get_entry_internal(struct upcall_cache *cache,
+                                   struct upcall_cache_entry *entry,
+                                   void *args, gid_t *fsgid,
+                                   gid_t **grouplist, int *ngroups);
+#else /* HAVE_SERVER_SUPPORT */
+static inline int refresh_entry_internal(struct upcall_cache *cache,
+                                        struct upcall_cache_entry *entry,
+                                        __u32 fsgid, int ngroups,
+                                        gid_t *grouplist)
+{
+       return -EOPNOTSUPP;
+}
+static inline int upcall_cache_get_entry_internal(struct upcall_cache *cache,
+                                              struct upcall_cache_entry *entry,
+                                              void *args, gid_t *fsgid,
+                                              gid_t **grouplist, int *ngroups)
+{
+       return -EOPNOTSUPP;
+}
+#endif
+
+#endif /* _UPCALL_CACHE_INTERNAL_H */