From: Amir Shehata Date: Tue, 14 Jul 2020 17:51:51 +0000 (-0700) Subject: LU-13750 lnet: Fix peer add command X-Git-Tag: 2.13.56~82 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=30f6c3d601fb9e7bc5af8dfc7a6a4abd404aea18 LU-13750 lnet: Fix peer add command The peer add command is suppose to add one peer per command. The primary NID can be specified followed by a set of constituent NIDs. This patch restores this behavior and ensures that for peer add the primary NID must be specified to make the command syntax more consistent with the peer del command. And behave in a similar way as net add/del commands. The APIs have been changed as well to make it more easily testable from the LUTF. There a few cleanups to avoid having to do unnecessary parsing. Test-Parameters: trivial testlist=sanity-lnet Fixes: 892f675e660 (LU-12410 lnet: Convert lnetctl peer add and del) Signed-off-by: Amir Shehata Change-Id: I32ed53742f2c379abb47f7acd3f7f336062e0458 Reviewed-on: https://review.whamcloud.com/39392 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Chris Horn Reviewed-by: James Simmons Reviewed-by: Oleg Drokin --- diff --git a/lnet/utils/lnetconfig/liblnetconfig.c b/lnet/utils/lnetconfig/liblnetconfig.c index 8e7cfd8..f7c7929 100644 --- a/lnet/utils/lnetconfig/liblnetconfig.c +++ b/lnet/utils/lnetconfig/liblnetconfig.c @@ -256,57 +256,6 @@ void lustre_lnet_init_nw_descr(struct lnet_dlc_network_descr *nw_descr) } } -int tokenize_nidstr(char *nidstr, char *out[LNET_MAX_STR_LEN], char *err_str) -{ - int bracket = 0, num_str = 0; - char *strstart, *chptr; - - if (!nidstr) { - snprintf(err_str, LNET_MAX_STR_LEN, "supplied nidstr is NULL"); - return LUSTRE_CFG_RC_BAD_PARAM; - } - - /* comma-separated nidranges -> space-separated nidranges */ - chptr = &nidstr[0]; - strstart = chptr; - while (true) { - if (*chptr == '\0') { - out[num_str] = strstart; - num_str++; - break; - } - - if (*chptr == '[') - bracket++; - else if (*chptr == ']') - bracket--; - - if (bracket < 0) { - snprintf(err_str, LNET_MAX_STR_LEN, - "Mismatched brackets in nidstring \"%s\"", - nidstr); - return LUSTRE_CFG_RC_BAD_PARAM; - } - - if (bracket == 0 && *chptr == ',') { - out[num_str] = strstart; - *chptr = '\0'; - num_str++; - strstart = chptr + 1; - } - - chptr++; - } - - if (bracket != 0) { - snprintf(err_str, LNET_MAX_STR_LEN, - "Mismatched brackets in nidstring \"%s\"", nidstr); - return LUSTRE_CFG_RC_BAD_PARAM; - } - - return num_str; -} - int lustre_lnet_parse_nidstr(char *nidstr, lnet_nid_t *lnet_nidlist, int max_nids, char *err_str) { @@ -339,6 +288,7 @@ int lustre_lnet_parse_nidstr(char *nidstr, lnet_nid_t *lnet_nidlist, } num_nids = cfs_expand_nidlist(&nidlist, lnet_nidlist, max_nids); + cfs_free_nidlist(&nidlist); if (num_nids == -1) { snprintf(err_str, LNET_MAX_STR_LEN, @@ -730,95 +680,106 @@ static int lustre_lnet_handle_peer_nidlist(lnet_nid_t *nidlist, int num_nids, return rc; } -int lustre_lnet_config_peer_nidlist(char *pnidstr, lnet_nid_t *lnet_nidlist, - int num_nids, bool is_mr, int seq_no, - struct cYAML **err_rc) +static int +lustre_lnet_mod_peer_nidlist(lnet_nid_t pnid, lnet_nid_t *lnet_nidlist, + int cmd, int num_nids, bool is_mr, int seq_no, + struct cYAML **err_rc) { int rc = LUSTRE_CFG_RC_NO_ERR; char err_str[LNET_MAX_STR_LEN]; lnet_nid_t *lnet_nidlist2 = NULL; - lnet_nid_t pnid; + int ioc_cmd = (cmd == LNETCTL_ADD_CMD) ? IOC_LIBCFS_ADD_PEER_NI : + IOC_LIBCFS_DEL_PEER_NI; + char *cmd_str = (cmd == LNETCTL_ADD_CMD) ? ADD_CMD : DEL_CMD; - if (pnidstr) { - pnid = libcfs_str2nid(pnidstr); - if (pnid == LNET_NID_ANY) { - snprintf(err_str, LNET_MAX_STR_LEN, - "bad primary NID: '%s'", pnidstr); - rc = LUSTRE_CFG_RC_MISSING_PARAM; - goto out; - } - - num_nids++; - - lnet_nidlist2 = calloc(sizeof(*lnet_nidlist2), num_nids); - if (!lnet_nidlist2) { - snprintf(err_str, LNET_MAX_STR_LEN, "out of memory"); - rc = LUSTRE_CFG_RC_OUT_OF_MEM; - goto out; - } - lnet_nidlist2[0] = pnid; - memcpy(&lnet_nidlist2[1], lnet_nidlist, sizeof(*lnet_nidlist) * - (num_nids - 1)); + num_nids++; + lnet_nidlist2 = calloc(sizeof(*lnet_nidlist2), num_nids); + if (!lnet_nidlist2) { + snprintf(err_str, LNET_MAX_STR_LEN, "out of memory"); + rc = LUSTRE_CFG_RC_OUT_OF_MEM; + goto out; } + lnet_nidlist2[0] = pnid; + memcpy(&lnet_nidlist2[1], lnet_nidlist, sizeof(*lnet_nidlist) * + (num_nids - 1)); - rc = lustre_lnet_handle_peer_nidlist((pnidstr) ? lnet_nidlist2 : - lnet_nidlist, - num_nids, is_mr, - IOC_LIBCFS_ADD_PEER_NI, ADD_CMD, - err_str); + rc = lustre_lnet_handle_peer_nidlist(lnet_nidlist2, + num_nids, is_mr, ioc_cmd, + cmd_str, err_str); out: if (lnet_nidlist2) free(lnet_nidlist2); - cYAML_build_error(rc, seq_no, ADD_CMD, "peer_ni", err_str, err_rc); + cYAML_build_error(rc, seq_no, cmd_str, "peer_ni", err_str, err_rc); return rc; } -int lustre_lnet_del_peer_nidlist(char *pnidstr, lnet_nid_t *lnet_nidlist, - int num_nids, int seq_no, - struct cYAML **err_rc) +static void +replace_sep(char *str, char sep, char newsep) { - int rc = LUSTRE_CFG_RC_NO_ERR; - char err_str[LNET_MAX_STR_LEN]; - lnet_nid_t *lnet_nidlist2 = NULL; - lnet_nid_t pnid; + int bracket = 0; + int i; + if (!str) + return; + for (i = 0; i < strlen(str); i++) { + /* don't replace ',' within [] */ + if (str[i] == '[') + bracket++; + else if (str[i] == ']') + bracket--; + else if (str[i] == sep && bracket == 0) + str[i] = newsep; + } +} - if (!pnidstr) { - snprintf(err_str, sizeof(err_str), - "\"Primary nid is not provided\""); - rc = LUSTRE_CFG_RC_MISSING_PARAM; +int lustre_lnet_modify_peer(char *prim_nid, char *nids, bool is_mr, + int cmd, int seq_no, struct cYAML **err_rc) +{ + int num_nids, rc; + char err_str[LNET_MAX_STR_LEN] = "Error"; + lnet_nid_t lnet_nidlist[LNET_MAX_NIDS_PER_PEER]; + lnet_nid_t pnid = LNET_NID_ANY; + + if (!prim_nid) { + rc = LUSTRE_CFG_RC_BAD_PARAM; + snprintf(err_str, LNET_MAX_STR_LEN, + "--prim_nid must be specified"); goto out; } - pnid = libcfs_str2nid(pnidstr); + pnid = libcfs_str2nid(prim_nid); if (pnid == LNET_NID_ANY) { rc = LUSTRE_CFG_RC_BAD_PARAM; - snprintf(err_str, sizeof(err_str), - "bad key NID: '%s'", - pnidstr); + snprintf(err_str, LNET_MAX_STR_LEN, + "badly formatted primary NID: %s", prim_nid); goto out; } - num_nids++; - lnet_nidlist2 = calloc(sizeof(*lnet_nidlist2), num_nids); - if (!lnet_nidlist2) { - snprintf(err_str, sizeof(err_str), - "out of memory"); - rc = LUSTRE_CFG_RC_OUT_OF_MEM; - goto out; + num_nids = 0; + if (nids) { + /* + * if there is no primary nid we need to make the first nid in the + * nids list the primary nid + */ + replace_sep(nids, ',', ' '); + rc = lustre_lnet_parse_nidstr(nids, lnet_nidlist, + LNET_MAX_NIDS_PER_PEER, err_str); + if (rc < 0) + goto out; + + num_nids = rc; } - lnet_nidlist2[0] = pnid; - memcpy(&lnet_nidlist2[1], lnet_nidlist, sizeof(*lnet_nidlist) * - (num_nids - 1)); - rc = lustre_lnet_handle_peer_nidlist(lnet_nidlist2, num_nids, false, - IOC_LIBCFS_DEL_PEER_NI, DEL_CMD, - err_str); + rc = lustre_lnet_mod_peer_nidlist(pnid, lnet_nidlist, + cmd, num_nids, is_mr, + -1, err_rc); + out: - if (lnet_nidlist2) - free(lnet_nidlist2); + if (rc != LUSTRE_CFG_RC_NO_ERR) + cYAML_build_error(rc, -1, "peer", + cmd == LNETCTL_ADD_CMD ? "add" : "del", + err_str, err_rc); - cYAML_build_error(rc, seq_no, DEL_CMD, "peer_ni", err_str, err_rc); return rc; } @@ -826,10 +787,9 @@ int lustre_lnet_route_common(char *nw, char *nidstr, int hops, int prio, int sen, int seq_no, struct cYAML **err_rc, int cmd) { - int rc, num_nids, num_nidstrs, nid_idx, str_idx; + int rc, num_nids, idx; __u32 rnet; char err_str[LNET_MAX_STR_LEN]; - char *nidstrarray[LNET_MAX_STR_LEN]; struct lnet_ioctl_config_data data; lnet_nid_t lnet_nidlist[LNET_MAX_NIDS_PER_PEER]; @@ -853,21 +813,15 @@ int lustre_lnet_route_common(char *nw, char *nidstr, int hops, int prio, goto out; } - rc = tokenize_nidstr(nidstr, &nidstrarray[0], err_str); + replace_sep(nidstr, ',', ' '); + rc = lustre_lnet_parse_nidstr(nidstr, lnet_nidlist, + LNET_MAX_NIDS_PER_PEER, err_str); if (rc < 0) goto out; - num_nidstrs = rc; - - for (str_idx = 0; str_idx < num_nidstrs; str_idx++) { - rc = lustre_lnet_parse_nidstr(nidstrarray[str_idx], - lnet_nidlist, - LNET_MAX_NIDS_PER_PEER, err_str); - if (rc < 0) - goto out; - - num_nids = rc; + num_nids = rc; + for (idx = 0; idx < num_nids; idx++) { LIBCFS_IOC_INIT_V2(data, cfg_hdr); data.cfg_net = rnet; if (cmd == LNETCTL_ADD_CMD) { @@ -876,24 +830,28 @@ int lustre_lnet_route_common(char *nw, char *nidstr, int hops, int prio, data.cfg_config_u.cfg_route.rtr_sensitivity = sen; } - for (nid_idx = 0; nid_idx < num_nids; nid_idx++) { - data.cfg_nid = lnet_nidlist[nid_idx]; + data.cfg_nid = lnet_nidlist[idx]; - if (cmd == LNETCTL_ADD_CMD) - rc = l_ioctl(LNET_DEV_ID, IOC_LIBCFS_ADD_ROUTE, - &data); - else - rc = l_ioctl(LNET_DEV_ID, IOC_LIBCFS_DEL_ROUTE, - &data); + if (cmd == LNETCTL_ADD_CMD) + rc = l_ioctl(LNET_DEV_ID, IOC_LIBCFS_ADD_ROUTE, + &data); + else + rc = l_ioctl(LNET_DEV_ID, IOC_LIBCFS_DEL_ROUTE, + &data); - if (rc != 0 && errno != EEXIST && - errno != EHOSTUNREACH) { - rc = -errno; - snprintf(err_str, LNET_MAX_STR_LEN, - "route operation failed: %s", - strerror(errno)); - goto out; - } + if (rc != 0 && errno != EEXIST && + errno != EHOSTUNREACH) { + rc = -errno; + snprintf(err_str, LNET_MAX_STR_LEN, + "route operation failed: %s", + strerror(errno)); + goto out; + } else if (errno == EEXIST) { + /* + * continue chugging along if one of the + * routes already exists + */ + rc = 0; } } @@ -4281,23 +4239,33 @@ static int handle_yaml_peer_common(struct cYAML *tree, struct cYAML **show_rc, struct cYAML **err_rc, int cmd) { int rc, num_nids = 0, seqn; - bool mr_value; + bool mr_value = false; char *nidstr = NULL, *prim_nidstr; char err_str[LNET_MAX_STR_LEN]; struct cYAML *seq_no, *prim_nid, *mr, *peer_nis; lnet_nid_t lnet_nidlist[LNET_MAX_NIDS_PER_PEER]; + lnet_nid_t pnid = LNET_NID_ANY; seq_no = cYAML_get_object_item(tree, "seq_no"); seqn = seq_no ? seq_no->cy_valueint : -1; prim_nid = cYAML_get_object_item(tree, "primary nid"); - prim_nidstr = prim_nid ? prim_nid->cy_valuestring : NULL; - peer_nis = cYAML_get_object_item(tree, "peer ni"); - if (!(prim_nid || peer_nis)) { + if (!prim_nid) { rc = LUSTRE_CFG_RC_BAD_PARAM; snprintf(err_str, LNET_MAX_STR_LEN, - "Neither \"primary nid\" nor \"peer ni\" are defined"); + "\"primary nid\" must be specified"); + goto failed; + } + + prim_nidstr = prim_nid->cy_valuestring; + + /* if the provided primary NID is bad, no need to go any further */ + pnid = libcfs_str2nid(prim_nidstr); + if (pnid == LNET_NID_ANY) { + rc = LUSTRE_CFG_RC_BAD_PARAM; + snprintf(err_str, LNET_MAX_STR_LEN, + "badly formatted primary NID: %s", prim_nidstr); goto failed; } @@ -4340,13 +4308,11 @@ static int handle_yaml_peer_common(struct cYAML *tree, struct cYAML **show_rc, goto failed; } } + } - rc = lustre_lnet_config_peer_nidlist(prim_nidstr, lnet_nidlist, - num_nids, mr_value, seqn, - err_rc); - } else - rc = lustre_lnet_del_peer_nidlist(prim_nidstr, lnet_nidlist, - num_nids, seqn, err_rc); + rc = lustre_lnet_mod_peer_nidlist(pnid, lnet_nidlist, cmd, + num_nids, mr_value, seqn, + err_rc); failed: if (nidstr) diff --git a/lnet/utils/lnetconfig/liblnetconfig.h b/lnet/utils/lnetconfig/liblnetconfig.h index 2d34038..af4456b 100644 --- a/lnet/utils/lnetconfig/liblnetconfig.h +++ b/lnet/utils/lnetconfig/liblnetconfig.h @@ -75,8 +75,6 @@ struct lnet_dlc_intf_descr { /* forward declaration of the cYAML structure. */ struct cYAML; -int tokenize_nidstr(char *nidstr, char *out[LNET_MAX_STR_LEN], char *err_str); - /* * lustre_lnet_config_lib_init() * Initialize the Library to enable communication with the LNET kernel @@ -521,45 +519,35 @@ int lustre_lnet_show_stats(int seq_no, struct cYAML **show_rc, struct cYAML **err_rc); /* - * lustre_lnet_config_peer_nidlist - * Add a peer NID to a peer with primary NID pnid. If a pnid is not provided - * then the first NID in the NID list becomes the primary NID for a newly - * created peer. - * Otherwise, if the provided primary NID is unique, then a new peer is + * lustre_lnet_modify_peer + * Handle a peer config or delete operation. + * + * Config Operation: + * Add a peer NID to a peer with primary NID pnid. + * If the provided primary NID is unique, then a new peer is * created with this primary NID, and the NIDs in the NID list are added as * secondary NIDs to this new peer. * If any of the NIDs in the NID list are not unique then the operation * fails. Some peer NIDs might have already been added. It's the responsibility * of the caller of this API to remove the added NIDs if so desired. * - * pnid - The desired primary NID of a new peer, or the primary NID of - * an existing peer. - * lnet_nidlist - List of LNet NIDs to add to the peer - * num_nids - The number of LNet NIDs in the lnet_nidlist array - * mr - Specifies whether this peer is MR capable. - * seq_no - sequence number of the command - * err_rc - YAML structure of the resultant return code - */ -int lustre_lnet_config_peer_nidlist(char *pnid, lnet_nid_t *lnet_nidlist, - int num_nids, bool mr, int seq_no, - struct cYAML **err_rc); - -/* - * lustre_lnet_del_peer_nidlist + * Delete Operation: * Delete the NIDs given in the NID list from the peer with the primary NID * pnid. If pnid is NULL, or it doesn't identify a peer, the operation fails, * and no change happens to the system. * The operation is aborted on the first NID that fails to be deleted. * - * pnid - The primary NID of the peer to be modified - * lnet_nidlist - The list of LNet NIDs to delete from the peer - * num_nids - the number of nids in the lnet_nidlist array + * prim_nid - The desired primary NID of a new peer, or the primary NID of + * an existing peer. + * nids - a comma separated string of nids + * is_mr - Specifies whether this peer is MR capable. + * cmd - CONFIG or DELETE * seq_no - sequence number of the command * err_rc - YAML structure of the resultant return code */ -int lustre_lnet_del_peer_nidlist(char *pnid, lnet_nid_t *lnet_nidlist, - int num_nids, int seq_no, - struct cYAML **err_rc); +int lustre_lnet_modify_peer(char *prim_nid, char *nids, bool is_mr, + int cmd, int seq_no, struct cYAML **err_rc); + /* * lustre_lnet_show_peer * Show the peer identified by nid, knid. If knid is NULL all diff --git a/lnet/utils/lnetctl.c b/lnet/utils/lnetctl.c index b75f684..40deca5 100644 --- a/lnet/utils/lnetctl.c +++ b/lnet/utils/lnetctl.c @@ -221,9 +221,7 @@ command_t set_cmds[] = { command_t peer_cmds[] = { {"add", jt_add_peer_nid, 0, "add a peer NID\n" - "\t--prim_nid: Primary NID of the peer. If not provided then the first\n" - "\t NID in the list becomes the Primary NID of a newly created\n" - "\t peer. \n" + "\t--prim_nid: Primary NID of the peer.\n" "\t--nid: one or more peer NIDs\n" "\t--non_mr: create this peer as not Multi-Rail capable\n" "\t--ip2nets: specify a range of nids per peer"}, @@ -1682,13 +1680,11 @@ static int jt_export(int argc, char **argv) static int jt_peer_nid_common(int argc, char **argv, int cmd) { - int rc = LUSTRE_CFG_RC_NO_ERR, opt, num_nids, num_nidstrs, i; + int rc = LUSTRE_CFG_RC_NO_ERR, opt; bool is_mr = true; char *prim_nid = NULL, *nidstr = NULL; char err_str[LNET_MAX_STR_LEN] = "Error"; - char *nidstrarray[LNET_MAX_STR_LEN]; struct cYAML *err_rc = NULL; - lnet_nid_t lnet_nidlist[LNET_MAX_NIDS_PER_PEER]; const char *const short_opts = "k:mn:"; const struct option long_opts[] = { @@ -1715,7 +1711,7 @@ static int jt_peer_nid_common(int argc, char **argv, int cmd) rc = LUSTRE_CFG_RC_BAD_PARAM; snprintf(err_str, LNET_MAX_STR_LEN, "Unrecognized option '-%c'", opt); - goto out; + goto build_error; } is_mr = false; break; @@ -1727,63 +1723,19 @@ static int jt_peer_nid_common(int argc, char **argv, int cmd) } } - if (!(nidstr || prim_nid)) { - rc = LUSTRE_CFG_RC_BAD_PARAM; - snprintf(err_str, LNET_MAX_STR_LEN, - "--prim_nid or --nid (or both) must be specified"); - goto out; - } - - if (!nidstr) { - /* We were only provided a primary nid */ - num_nids = 0; - if (cmd == LNETCTL_ADD_CMD) - rc = lustre_lnet_config_peer_nidlist(prim_nid, - lnet_nidlist, - num_nids, is_mr, - -1, &err_rc); - else - rc = lustre_lnet_del_peer_nidlist(prim_nid, - lnet_nidlist, - num_nids, -1, - &err_rc); - - goto out; - } - - rc = tokenize_nidstr(nidstr, &nidstrarray[0], err_str); - if (rc <= 0) + rc = lustre_lnet_modify_peer(prim_nid, nidstr, is_mr, cmd, + -1, &err_rc); + if (rc != LUSTRE_CFG_RC_NO_ERR) goto out; - num_nidstrs = rc; - - for (i = 0; i < num_nidstrs; i++) { - rc = lustre_lnet_parse_nidstr(nidstrarray[i], lnet_nidlist, - LNET_MAX_NIDS_PER_PEER, err_str); - if (rc < 0) - goto out; - - num_nids = rc; - - if (cmd == LNETCTL_ADD_CMD) - rc = lustre_lnet_config_peer_nidlist(prim_nid, - lnet_nidlist, - num_nids, is_mr, - -1, &err_rc); - else - rc = lustre_lnet_del_peer_nidlist(prim_nid, - lnet_nidlist, - num_nids, -1, - &err_rc); - } +build_error: + cYAML_build_error(rc, -1, "peer", + cmd == LNETCTL_ADD_CMD ? "add" : "del", + err_str, &err_rc); out: - if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_build_error(rc, -1, "peer", - cmd == LNETCTL_ADD_CMD ? "add" : "del", - err_str, &err_rc); + if (rc != LUSTRE_CFG_RC_NO_ERR) cYAML_print_tree2file(stderr, err_rc); - } cYAML_free_tree(err_rc); diff --git a/lustre/doc/lnetctl.8 b/lustre/doc/lnetctl.8 index 401d03d..8ae8cf7 100644 --- a/lustre/doc/lnetctl.8 +++ b/lustre/doc/lnetctl.8 @@ -111,7 +111,7 @@ parameter\. .SS "Peer Configuration" .TP \fBlnetctl peer\fR add -Configure an LNET peer with at least one supplied NID\. By default, peers are marked as multi-rail capable\. If prim_nid is not specified, the first NID in this list is assumed to be the primary NID for the peer. +Configure an LNET peer with at least one supplied NID\. The primary NID must be specified. By default, peers are marked as multi-rail capable\. . .br . diff --git a/lustre/tests/sanity-lnet.sh b/lustre/tests/sanity-lnet.sh index c97dd3d..483e669 100755 --- a/lustre/tests/sanity-lnet.sh +++ b/lustre/tests/sanity-lnet.sh @@ -951,10 +951,14 @@ test_99a() { do_lnetctl peer del --prim_nid 1.1.1.1@o2ib && error "Command should have failed" - echo "Don't provide mandatory arguments peer del" + echo "Don't provide mandatory argument for peer del" do_lnetctl peer del --nid 1.1.1.1@tcp && error "Command should have failed" + echo "Don't provide mandatory argument for peer add" + do_lnetctl peer add --nid 1.1.1.1@tcp && + error "Command should have failed" + echo "Don't provide mandatory arguments peer add" do_lnetctl peer add && error "Command should have failed" @@ -988,7 +992,7 @@ test_99a() { local nidstr for nidstr in ${invalid_strs}; do echo "Check invalid nidstring - '$nidstr'" - do_lnetctl peer add --nid $nidstr && + do_lnetctl peer add --prim_nid 1.1.1.1@tcp --nid $nidstr && error "Command should have failed" done