LU-14779 utils: no DNS lookups for NID in get_param

Calling libcfs_str2nid() speculatively in "lctl get_param" to see
if there is a NID in the parameter name results in multiple DNS
lookups for invalid hostnames (e.g. "exports.192.168.0.10"). That
may take a very long time if there are a large number of connected
clients, and if the DNS server overloaded or is having problems.

Instead of doing these speculative NID conversions, skip the whole
NID string in the parameter name for the two known parameters that
may contain a NID ("*.exports.<NID>.*" and "*.MGC<NID>.*").  This
is considerably faster since it is only working on a local string.

If new parameters are added that contain a NID (unlikely, but
possible), then "clean_path()" would need to be updated as part
of that change.

Fixes: 85cbe1a3ee69 ("LU-5030 util: migrate lctl params functions to use cfs_get_paths()")
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Change-Id: I51f865e4ce3a7bc4879f9d688c4b3a68d731810f
Reviewed-on: https://review.whamcloud.com/44056
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Emoly Liu <emoly@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index 0f3c56a..60416c6 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -24167,6 +24167,15 @@ test_401d() {
 }
 run_test 401d "Verify 'lctl set_param' accepts values containing '='"
 
+test_401e() { # LU-14779
+       $LCTL list_param -R "ldlm.namespaces.MGC*" ||
+               error "lctl list_param MGC* failed"
+       $LCTL get_param "ldlm.namespaces.MGC*" || error "lctl get_param failed"
+       $LCTL get_param "ldlm.namespaces.MGC*.lru_size" ||
+               error "lctl get_param lru_size failed"
+}
+run_test 401e "verify 'lctl get_param' works with NID in parameter"
+
 test_402() {
        [[ $MDS1_VERSION -ge $(version_code 2.7.66) ]] ||
        [[ $MDS1_VERSION -ge $(version_code 2.7.18.4) &&

diff --git a/lustre/utils/lustre_cfg.c b/lustre/utils/lustre_cfg.c
index 4900766..665e7f8 100644 (file)
--- a/lustre/utils/lustre_cfg.c
+++ b/lustre/utils/lustre_cfg.c
@@ -707,7 +707,8 @@ static char *strnchr(const char *p, char c, size_t n)
 static int
 clean_path(struct param_opts *popt, char *path)
 {
-       char *nidstr = NULL;
+       char *nidstart = NULL;
+       char *nidend = NULL;
        char *tmp;
 
        if (popt == NULL || path == NULL || strlen(path) == 0)
@@ -742,63 +743,38 @@ clean_path(struct param_opts *popt, char *path)
                }
        }
 
-       /* Does this path contain a NID string ? */
+       /* Does path contain a NID string?  Skip '.->/' replacement for it. */
        tmp = strchr(path, '@');
        if (tmp) {
-               char *find_nid = strdup(path);
-               lnet_nid_t nid;
-
-               if (!find_nid)
-                       return -ENOMEM;
-
-               /*
-                * First we need to chop off rest after nid string.
-                * Since find_nid is a clone of path it better have '@'
+               /* First find the NID start.  NIDs may have variable (0-4) '.',
+                * so find the common NID prefixes instead of trying to count
+                * the dots.  Not great, but there are only two, and faster
+                * than multiple speculative NID parses and bad DNS lookups.
                 */
-               tmp = strchr(find_nid, '@');
-               tmp = strchr(tmp, '.');
-               if (tmp)
-                       *tmp = '\0';
-
-               /* Now chop off the front. */
-               for (tmp = strchr(find_nid, '.'); tmp != NULL;
-                    tmp = strchr(tmp, '.')) {
-                       /* Remove MGC to make it NID format */
-                       if (!strncmp(++tmp, "MGC", 3))
-                               tmp += 3;
-
-                       nid = libcfs_str2nid(tmp);
-                       if (nid != LNET_NID_ANY) {
-                               nidstr = libcfs_nid2str(nid);
-                               if (!nidstr)
-                                       return -EINVAL;
-                               break;
-                       }
-               }
-               free(find_nid);
+               if ((tmp = strstr(path, ".exports.")))
+                       nidstart = tmp + strlen(".exports.");
+               else if ((tmp = strstr(path, ".MGC")))
+                       nidstart = tmp + 1;
+
+               /* Next, find the end of the NID string. */
+               if (nidstart)
+                       nidend = strchrnul(strchr(nidstart, '@'), '.');
        }
 
        /* replace param '.' with '/' */
        for (tmp = strchr(path, '.'); tmp != NULL; tmp = strchr(tmp, '.')) {
                *tmp++ = '/';
 
-               /* Remove MGC to make it NID format */
-               if (!strncmp(tmp, "MGC", 3))
-                       tmp += 3;
-
                /*
                 * There exist cases where some of the subdirectories of the
                 * the parameter tree has embedded in its name a NID string.
                 * This means that it is possible that these subdirectories
                 * could have actual '.' in its name. If this is the case we
-                * don't want to blindly replace the '.' with '/'.
+                * don't want to blindly replace the '.' with '/', so skip
+                * over the part of the parameter containing the NID.
                 */
-               if (nidstr) {
-                       char *match = strstr(tmp, nidstr);
-
-                       if (tmp == match)
-                               tmp += strlen(nidstr);
-               }
+               if (tmp == nidstart)
+                       tmp = nidend;
        }
 
        return 0;