From 70f7b7e9bef06160296e23ad75270125266c3c3d Mon Sep 17 00:00:00 2001 From: Serguei Smirnov Date: Tue, 20 Jun 2023 12:21:42 -0700 Subject: [PATCH] LU-16709 lnet: fix locking multiple peer NIDs If Lustre identifies the same peer with multiple NIDs, as a result of peer discovery it is possible that the discovered peer is found to contain a NID which is locked as primary by a different existing peer record. In this case it is safe to merge the peer records, but the NID which got locked the earliest should be kept as primary. This allows for the first of the two locked NIDs to stay primary as intended for the purpose of communicating with Lustre even if peer discovery succeeded using a different NID of MR peer. This patch adds updates to the original port because master version of of this moment evolved after it was landed. Lustre-change: https://review.whamcloud.com/50530 Lustre-commit: 3b7a02ee4d656b7b3e044713681da2f56dddb152 Test-parameters: trivial testlist=sanity-lnet Fixes: 1a2db3e14b78 ("EX-7251 lnet: fix locking multiple NIDs") Signed-off-by: Serguei Smirnov Change-Id: I3303e618b37a76c30be6426972e7853bb31ae497 Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/51384 Reviewed-by: Andreas Dilger Reviewed-by: Cyril Bordage Tested-by: jenkins Tested-by: Maloo --- lnet/include/uapi/linux/lnet/libcfs_ioctl.h | 3 +- lnet/lnet/api-ni.c | 23 ++++++++++ lnet/lnet/peer.c | 16 ++++--- lnet/utils/lnetconfig/liblnetconfig.c | 38 ++++++++++++++++ lnet/utils/lnetconfig/liblnetconfig.h | 11 +++++ lnet/utils/lnetctl.c | 16 +++++-- lustre/tests/sanity-lnet.sh | 69 +++++++++++++++++++++++++++++ 7 files changed, 165 insertions(+), 11 deletions(-) diff --git a/lnet/include/uapi/linux/lnet/libcfs_ioctl.h b/lnet/include/uapi/linux/lnet/libcfs_ioctl.h index 492bec8..36c0ec2 100644 --- a/lnet/include/uapi/linux/lnet/libcfs_ioctl.h +++ b/lnet/include/uapi/linux/lnet/libcfs_ioctl.h @@ -150,7 +150,8 @@ struct libcfs_ioctl_data { #define IOC_LIBCFS_GET_LOCAL_HSTATS _IOWR(IOC_LIBCFS_TYPE, 103, IOCTL_CONFIG_SIZE) #define IOC_LIBCFS_GET_RECOVERY_QUEUE _IOWR(IOC_LIBCFS_TYPE, 104, IOCTL_CONFIG_SIZE) #define IOC_LIBCFS_SET_CONNS_PER_PEER _IOWR(IOC_LIBCFS_TYPE, 105, IOCTL_CONFIG_SIZE) -#define IOC_LIBCFS_MAX_NR 105 +#define IOC_LIBCFS_SET_PEER _IOWR(IOC_LIBCFS_TYPE, 106, IOCTL_CONFIG_SIZE) +#define IOC_LIBCFS_MAX_NR 106 extern int libcfs_ioctl_data_adjust(struct libcfs_ioctl_data *data); diff --git a/lnet/lnet/api-ni.c b/lnet/lnet/api-ni.c index 3ecf65c..1cc2e07 100644 --- a/lnet/lnet/api-ni.c +++ b/lnet/lnet/api-ni.c @@ -4025,6 +4025,29 @@ LNetCtl(unsigned int cmd, void *arg) return 0; } + case IOC_LIBCFS_SET_PEER: { + struct lnet_ioctl_peer_cfg *cfg = arg; + struct lnet_peer *lp; + + if (cfg->prcfg_hdr.ioc_len < sizeof(*cfg)) + return -EINVAL; + + mutex_lock(&the_lnet.ln_api_mutex); + lp = lnet_find_peer(cfg->prcfg_prim_nid); + if (!lp) { + mutex_unlock(&the_lnet.ln_api_mutex); + return -ENOENT; + } + spin_lock(&lp->lp_lock); + lp->lp_state = cfg->prcfg_state; + spin_unlock(&lp->lp_lock); + lnet_peer_decref_locked(lp); + mutex_unlock(&the_lnet.ln_api_mutex); + CDEBUG(D_NET, "Set peer %s state to %u\n", + libcfs_nid2str(cfg->prcfg_prim_nid), cfg->prcfg_state); + return 0; + } + case IOC_LIBCFS_SET_CONNS_PER_PEER: { struct lnet_ioctl_reset_conns_per_peer_cfg *cfg = arg; int value; diff --git a/lnet/lnet/peer.c b/lnet/lnet/peer.c index dcb12ed..45c74f6 100644 --- a/lnet/lnet/peer.c +++ b/lnet/lnet/peer.c @@ -1310,8 +1310,10 @@ LNetPrimaryNID(lnet_nid_t nid) */ again: spin_lock(&lp->lp_lock); - if (!(lp->lp_state & LNET_PEER_LOCK_PRIMARY) && lock_prim_nid) + if (!(lp->lp_state & LNET_PEER_LOCK_PRIMARY) && lock_prim_nid) { lp->lp_state |= LNET_PEER_LOCK_PRIMARY; + lp->lp_prim_lock_ts = ktime_get_ns(); + } /* DD disabled, nothing to do */ if (lnet_peer_discovery_disabled) { @@ -1630,7 +1632,7 @@ lnet_peer_add_nid(struct lnet_peer *lp, lnet_nid_t nid, unsigned flags) struct lnet_peer *lp2 = lpni->lpni_peer_net->lpn_peer; int rtr_refcount = lp2->lp_rtr_refcount; - unsigned peer2_state; + unsigned int peer2_state; __u64 peer2_prim_lock_ts; /* If there's another peer that this NID belongs to @@ -1659,8 +1661,7 @@ lnet_peer_add_nid(struct lnet_peer *lp, lnet_nid_t nid, unsigned flags) peer2_prim_lock_ts = lp2->lp_prim_lock_ts; spin_unlock(&lp2->lp_lock); - /* If both peers have their primary NIDs locked, - * the NID which got locked the earliest should be + /* NID which got locked the earliest should be * kept as primary. In case if the peers were * created by Lustre, this allows the * first listed NID to stay primary as intended @@ -1670,13 +1671,14 @@ lnet_peer_add_nid(struct lnet_peer *lp, lnet_nid_t nid, unsigned flags) */ spin_lock(&lp->lp_lock); if (peer2_state & LNET_PEER_LOCK_PRIMARY && - lp->lp_state & LNET_PEER_LOCK_PRIMARY && - peer2_prim_lock_ts < lp->lp_prim_lock_ts) { + ((lp->lp_state & LNET_PEER_LOCK_PRIMARY && + peer2_prim_lock_ts < lp->lp_prim_lock_ts) || + !(lp->lp_state & LNET_PEER_LOCK_PRIMARY))) { lp->lp_prim_lock_ts = peer2_prim_lock_ts; lp->lp_primary_nid = nid; + lp->lp_state |= LNET_PEER_LOCK_PRIMARY; } spin_unlock(&lp->lp_lock); - /* * if we're trying to delete a router it means * we're moving this peer NI to a new peer so must diff --git a/lnet/utils/lnetconfig/liblnetconfig.c b/lnet/utils/lnetconfig/liblnetconfig.c index dcdabdf..4516f0f 100644 --- a/lnet/utils/lnetconfig/liblnetconfig.c +++ b/lnet/utils/lnetconfig/liblnetconfig.c @@ -1852,6 +1852,31 @@ lustre_lnet_config_healthv(int value, bool all, lnet_nid_t nid, return rc; } +static int +lustre_lnet_config_peer(int state, lnet_nid_t nid, char *name, + int seq_no, struct cYAML **err_rc) +{ + struct lnet_ioctl_peer_cfg data; + int rc = LUSTRE_CFG_RC_NO_ERR; + char err_str[LNET_MAX_STR_LEN] = "\"success\""; + + LIBCFS_IOC_INIT_V2(data, prcfg_hdr); + data.prcfg_state = state; + data.prcfg_prim_nid = nid; + data.prcfg_cfg_nid = LNET_NID_ANY; + + rc = l_ioctl(LNET_DEV_ID, IOC_LIBCFS_SET_PEER, &data); + if (rc != 0) { + rc = -errno; + snprintf(err_str, + sizeof(err_str), "Can not set peer property: %s", + strerror(errno)); + } + + cYAML_build_error(rc, seq_no, ADD_CMD, name, err_str, err_rc); + + return rc; +} static int lustre_lnet_config_conns_per_peer(int value, bool all, lnet_nid_t nid, @@ -1914,6 +1939,19 @@ int lustre_lnet_config_peer_ni_healthv(int value, bool all, char *lpni_nid, "peer_ni healthv", seq_no, err_rc); } +int lustre_lnet_set_peer_state(int state, char *lpni_nid, int seq_no, + struct cYAML **err_rc) +{ + lnet_nid_t nid; + + if (lpni_nid) + nid = libcfs_str2nid(lpni_nid); + else + nid = LNET_NID_ANY; + return lustre_lnet_config_peer(state, nid, "peer state", seq_no, + err_rc); +} + int lustre_lnet_config_ni_conns_per_peer(int value, bool all, char *ni_nid, int seq_no, struct cYAML **err_rc) { diff --git a/lnet/utils/lnetconfig/liblnetconfig.h b/lnet/utils/lnetconfig/liblnetconfig.h index 1cae395..ec93398 100644 --- a/lnet/utils/lnetconfig/liblnetconfig.h +++ b/lnet/utils/lnetconfig/liblnetconfig.h @@ -746,4 +746,15 @@ int lustre_lnet_parse_nidstr(char *nidstr, lnet_nid_t *lnet_nidlist, */ int lustre_lnet_show_peer_debug_info(char *peer_nid, int seq_no, struct cYAML **err_rc); + +/* lustre_lnet_set_peer_state + * set peer state + * lpni_nid - primary nid of the peer + * seq_no - sequence number of the request + * err_rc - [OUT] struct cYAML tree describing the error. Freed by + * caller + */ +int lustre_lnet_set_peer_state(int state, char *lpni_nid, int seq_no, + struct cYAML **err_rc); + #endif /* LIB_LNET_CONFIG_API_H */ diff --git a/lnet/utils/lnetctl.c b/lnet/utils/lnetctl.c index 313b1d2..720c4fe 100644 --- a/lnet/utils/lnetctl.c +++ b/lnet/utils/lnetctl.c @@ -270,7 +270,8 @@ command_t peer_cmds[] = { {"set", jt_set_peer_ni_value, 0, "set peer ni specific parameter\n" "\t--nid: Peer NI NID to set the\n" "\t--health: specify health value to set\n" - "\t--all: set all peer_nis values to the one specified\n"}, + "\t--all: set all peer_nis values to the one specified\n" + "\t--state: set peer state (DANGEROUS: for test/debug only)"}, { 0, 0, 0, NULL } }; @@ -1300,13 +1301,15 @@ static int set_value_helper(int argc, char **argv, char *nid = NULL; long int healthv = -1; bool all = false; + long int state = -1; int rc, opt; struct cYAML *err_rc = NULL; - const char *const short_options = "t:n:a"; + const char *const short_options = "t:n:s:a"; static const struct option long_options[] = { { .name = "nid", .has_arg = required_argument, .val = 'n' }, { .name = "health", .has_arg = required_argument, .val = 't' }, + { .name = "state", .has_arg = required_argument, .val = 's' }, { .name = "all", .has_arg = no_argument, .val = 'a' }, { .name = NULL } }; @@ -1324,6 +1327,10 @@ static int set_value_helper(int argc, char **argv, if (parse_long(optarg, &healthv) != 0) healthv = -1; break; + case 's': + if (parse_long(optarg, &state) != 0) + state = -1; + break; case 'a': all = true; break; @@ -1332,7 +1339,10 @@ static int set_value_helper(int argc, char **argv, } } - rc = cb(healthv, all, nid, -1, &err_rc); + if (state > -1) + rc = lustre_lnet_set_peer_state(state, nid, -1, &err_rc); + else + rc = cb(healthv, all, nid, -1, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) cYAML_print_tree2file(stderr, err_rc); diff --git a/lustre/tests/sanity-lnet.sh b/lustre/tests/sanity-lnet.sh index e13ce1f..5d46512 100755 --- a/lustre/tests/sanity-lnet.sh +++ b/lustre/tests/sanity-lnet.sh @@ -2196,6 +2196,75 @@ test_302() { } run_test 302 "Check that peer debug info can be dumped" +test_304() { + [[ ${NETTYPE} == tcp* ]] || skip "Need tcp NETTYPE" + + cleanup_netns || error "Failed to cleanup netns before test execution" + cleanup_lnet || error "Failed to unload modules before test execution" + + setup_fakeif || error "Failed to add fake IF" + have_interface "$FAKE_IF" || + error "Expect $FAKE_IF configured but not found" + + reinit_dlc || return $? + + add_net "tcp" "${INTERFACES[0]}" || return $? + add_net "tcp" "$FAKE_IF" || return $? + + local nid1=$(lctl list_nids | head -n 1) + local nid2=$(lctl list_nids | tail --lines 1) + + check_ni_status "$nid1" up + check_ni_status "$nid2" up + + do_lnetctl peer add --prim_nid ${nid2} --lock_prim || + error "peer add failed $?" + local locked_peer_state=($(do_lnetctl peer show -v 4 --nid ${nid2} | + awk '/peer state/{print $NF}')) + + # Expect peer state bits: + # LNET_PEER_MULTI_RAIL(0) | LNET_PEER_CONFIGURED(3) | + # LNET_PEER_LOCK_PRIMARY(20) + (( $locked_peer_state != "1048585")) && + error "Wrong peer state \"$locked_peer_state\" expected 1048585" + + # Clear LNET_PEER_CONFIGURED bit and verify + do_lnetctl peer set --nid ${nid2} --state 1048577 || + error "peer add failed $?" + locked_peer_state=($(do_lnetctl peer show -v 4 --nid ${nid2} | + awk '/peer state/{print $NF}')) + (( $locked_peer_state != "1048577")) && + error "Wrong peer state \"$locked_peer_state\" expected 1048577" + do_lnetctl discover ${nid1} || + error "Failed to discover peer" + + # Expect nid2 and nid1 peer entries to be consolidated, + # nid2 to stay primary + cat <> $TMP/sanity-lnet-$testnum-expected.yaml +peer: + - primary nid: ${nid2} + Multi-Rail: True + peer ni: + - nid: ${nid1} + state: NA + - nid: ${nid2} + state: NA +EOF + $LNETCTL peer show > $TMP/sanity-lnet-$testnum-actual.yaml + compare_yaml_files || + error "Unexpected peer configuration" + + locked_peer_state=($(do_lnetctl peer show -v 4 --nid ${nid2} | + awk '/peer state/{print $NF}')) + # Expect peer state bits to be added: + # LNET_PEER_DISCOVERED(4) | LNET_PEER_NIDS_UPTODATE(8) + (( $locked_peer_state != "1048849")) && + error "Wrong peer state \"$locked_peer_state\" expected 1048849" + return 0 +} +run_test 304 "Check locked primary peer nid consolidation" + complete_test $SECONDS + cleanup_testsuite exit_status -- 1.8.3.1