From f21c507fbca2afab5a5d97d4e816696a69d1c593 Mon Sep 17 00:00:00 2001 From: Andreas Dilger Date: Wed, 23 Jun 2021 02:20:24 -0600 Subject: [PATCH] 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..*" and "*.MGC.*"). 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 Change-Id: I51f865e4ce3a7bc4879f9d688c4b3a68d731810f Reviewed-on: https://review.whamcloud.com/44056 Tested-by: jenkins Reviewed-by: John L. Hammond Tested-by: Maloo Reviewed-by: Emoly Liu Reviewed-by: Oleg Drokin --- lustre/tests/sanity.sh | 9 +++++++ lustre/utils/lustre_cfg.c | 62 +++++++++++++++-------------------------------- 2 files changed, 28 insertions(+), 43 deletions(-) diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 0f3c56a..60416c6 100755 --- 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 --- 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; -- 1.8.3.1