From ae1ee11cea0a90631e14d670883528d6ac6e86b7 Mon Sep 17 00:00:00 2001 From: James Simmons Date: Mon, 17 Apr 2023 21:07:47 -0400 Subject: [PATCH] LU-16462 utils: handle lack of newer nla_attrs Some platforms like SUSE12SP5 have an older version of libnl3. Those versions lack proper support for newer nlattrs like NLA_S64. Without proper support of these newer nlattrs this means nla_validate() will see newer nlattrs as invalid. We need to fill in this missing support on older platforms. The "lctl ping" command will loop forever in jt_ptl_ping() if the netlink yaml parser doesn't work, instead of falling back to the "old_api" interface. However, because IOC_LIBCFS_PING was also deleted, this doesn't work either. Use lustre_lnet_ping_nid() to fallback to IOC_LIBCFS_PING_PEER (since v2_10_52_0-21-g7a36afd9df). Also, jt_ptl_ping() was passing argv[1] directly to the kernel as the NID to ping, without doing name resolution on it first, which broke using "lctl ping HOSTNAME" instead of only numeric IP NIDs. Ensure it returns a non-zero error code in case of failure. Restore IOC_LIBCFS_GET_NI for compatibility until there have been some releases with netlink support, so that "lctl list_nids" works. Also, sanity test_217 that was testing "lctl ping" was always being skipped, because "lctl list_nids" is never returning the hostname with embedded '-', only numeric IP addresses. Change it to prefer testing the hostname if it resolves to a NID, otherwise ping the numeric NID anyway, to confirm that "lctl ping" is still working. Test-Parameters: trivial clientdistro=sles12sp5 testlist=sanity Test-Parameters: clientdistro=sles12sp5 testlist=conf-sanity env=ONLY=43+50+70+91+115+130 Fixes: 86ba46c244 ("LU-9680 obdclass: user netlink to collect devices info") Fixes: d137e9823c ("LU-10003 lnet: use Netlink to support LNet ping commands") Fixes: 3e4061862e ("LU-864 test: Hostname name doesn't equal NID") Change-Id: Ia2dfd84c2d1782578ceff1c2dc6f74d7aa9b458b Signed-off-by: James Simmons Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/49608 Reviewed-by: Andreas Dilger Reviewed-by: Frank Sehr Reviewed-by: Oleg Drokin Tested-by: jenkins Tested-by: Maloo --- lnet/include/uapi/linux/lnet/libcfs_ioctl.h | 2 +- lnet/lnet/api-ni.c | 7 ++ lnet/utils/lnetconfig/liblnetconfig_netlink.c | 126 ++++++++++++++++++++++++-- lustre/tests/sanity.sh | 52 ++++++----- lustre/utils/lctl.c | 2 +- lustre/utils/lustre_cfg.c | 13 +-- lustre/utils/portals.c | 68 ++++++++------ 7 files changed, 207 insertions(+), 63 deletions(-) diff --git a/lnet/include/uapi/linux/lnet/libcfs_ioctl.h b/lnet/include/uapi/linux/lnet/libcfs_ioctl.h index 69c6f6a..88f51bb 100644 --- a/lnet/include/uapi/linux/lnet/libcfs_ioctl.h +++ b/lnet/include/uapi/linux/lnet/libcfs_ioctl.h @@ -91,7 +91,7 @@ struct libcfs_ioctl_data { #define IOC_LIBCFS_MARK_DEBUG _IOWR('e', 32, IOCTL_LIBCFS_TYPE) /* IOC_LIBCFS_MEMHOG obsolete in 2.8.0, was _IOWR('e', 36, IOCTL_LIBCFS_TYPE) */ /* lnet ioctls */ -/* IOC_LIBCFS_GET_NI obsolete in 2.16, was _IOWR('e', 50, IOCTL_LIBCFS_TYPE) */ +#define IOC_LIBCFS_GET_NI _IOWR('e', 50, IOCTL_LIBCFS_TYPE) #define IOC_LIBCFS_FAIL_NID _IOWR('e', 51, IOCTL_LIBCFS_TYPE) #define IOC_LIBCFS_NOTIFY_ROUTER _IOWR('e', 55, IOCTL_LIBCFS_TYPE) #define IOC_LIBCFS_UNCONFIGURE _IOWR('e', 56, IOCTL_LIBCFS_TYPE) diff --git a/lnet/lnet/api-ni.c b/lnet/lnet/api-ni.c index 76433a6..224526d 100644 --- a/lnet/lnet/api-ni.c +++ b/lnet/lnet/api-ni.c @@ -4105,6 +4105,13 @@ LNetCtl(unsigned int cmd, void *arg) sizeof(struct lnet_ioctl_config_data) > LIBCFS_IOC_DATA_MAX); switch (cmd) { + case IOC_LIBCFS_GET_NI: { + struct lnet_processid id = {}; + + rc = LNetGetId(data->ioc_count, &id); + data->ioc_nid = lnet_nid_to_nid4(&id.nid); + return rc; + } case IOC_LIBCFS_FAIL_NID: return lnet_fail_nid(data->ioc_nid, data->ioc_count); diff --git a/lnet/utils/lnetconfig/liblnetconfig_netlink.c b/lnet/utils/lnetconfig/liblnetconfig_netlink.c index 17ab2d4..06830c6 100644 --- a/lnet/utils/lnetconfig/liblnetconfig_netlink.c +++ b/lnet/utils/lnetconfig/liblnetconfig_netlink.c @@ -48,8 +48,8 @@ #define NLM_F_ACK_TLVS 0x200 /* extended ACK TVLs were included */ #endif -#ifndef NLA_NUL_STRING -# define NLA_NUL_STRING 10 +#ifndef NLA_S8 +# define NLA_S8 12 #endif #ifndef NLA_S16 @@ -97,7 +97,121 @@ int64_t nla_get_s64(const struct nlattr *nla) #define NLA_PUT_S64(msg, attrtype, value) \ NLA_PUT_TYPE(msg, int64_t, attrtype, value) -#endif /* ! HAVE_NLA_GET_S64 */ +#ifndef NLA_NUL_STRING +#define NLA_NUL_STRING 10 +#endif + +enum nla_types { + LNET_NLA_UNSPEC = NLA_UNSPEC, + LNET_NLA_U8 = NLA_U8, + LNET_NLA_U16 = NLA_U16, + LNET_NLA_U32 = NLA_U32, + LNET_NLA_U64 = NLA_U64, + LNET_NLA_STRING = NLA_STRING, + LNET_NLA_FLAG = NLA_FLAG, + LNET_NLA_MSECS = NLA_MSECS, + LNET_NLA_NESTED = NLA_NESTED, + LNET_NLA_NESTED_COMPAT = NLA_NESTED + 1, + LNET_NLA_NUL_STRING = NLA_NUL_STRING, + LNET_NLA_BINARY = NLA_NUL_STRING + 1, + LNET_NLA_S8 = NLA_S8, + LNET_NLA_S16 = NLA_S16, + LNET_NLA_S32 = NLA_S32, + LNET_NLA_S64 = NLA_S64, + __LNET_NLA_TYPE_MAX, +}; + +#define LNET_NLA_TYPE_MAX (__LNET_NLA_TYPE_MAX - 1) + +static uint16_t nla_attr_minlen[LNET_NLA_TYPE_MAX+1] = { + [NLA_U8] = sizeof(uint8_t), + [NLA_U16] = sizeof(uint16_t), + [NLA_U32] = sizeof(uint32_t), + [NLA_U64] = sizeof(uint64_t), + [NLA_STRING] = 1, + [NLA_FLAG] = 0, +}; + +static int lnet_validate_nla(const struct nlattr *nla, int maxtype, + const struct nla_policy *policy) +{ + const struct nla_policy *pt; + unsigned int minlen = 0; + int type = nla_type(nla); + + if (type < 0 || type > maxtype) + return 0; + + pt = &policy[type]; + + if (pt->type > NLA_TYPE_MAX) + return -NLE_INVAL; + + if (pt->minlen) + minlen = pt->minlen; + else if (pt->type != NLA_UNSPEC) + minlen = nla_attr_minlen[pt->type]; + + if (nla_len(nla) < minlen) + return -NLE_RANGE; + + if (pt->maxlen && nla_len(nla) > pt->maxlen) + return -NLE_RANGE; + + if (pt->type == NLA_STRING) { + const char *data = nla_data(nla); + + if (data[nla_len(nla) - 1] != '\0') + return -NLE_INVAL; + } + + return 0; +} + +int lnet_nla_parse(struct nlattr *tb[], int maxtype, struct nlattr *head, + int len, const struct nla_policy *policy) +{ + struct nlattr *nla; + int rem, err; + + memset(tb, 0, sizeof(struct nlattr *) * (maxtype + 1)); + + nla_for_each_attr(nla, head, len, rem) { + int type = nla_type(nla); + + if (type > maxtype) + continue; + + if (policy) { + err = lnet_validate_nla(nla, maxtype, policy); + if (err < 0) + return err; + } + + tb[type] = nla; + } + + return 0; +} + +int lnet_genlmsg_parse(struct nlmsghdr *nlh, int hdrlen, struct nlattr *tb[], + int maxtype, const struct nla_policy *policy) +{ + struct genlmsghdr *ghdr; + + if (!genlmsg_valid_hdr(nlh, hdrlen)) + return -NLE_MSG_TOOSHORT; + + ghdr = nlmsg_data(nlh); + return lnet_nla_parse(tb, maxtype, genlmsg_attrdata(ghdr, hdrlen), + genlmsg_attrlen(ghdr, hdrlen), policy); +} + +#else /* !HAVE_NLA_GET_S64 */ + +#define lnet_genlmsg_parse genlmsg_parse + +#endif /* HAVE_NLA_GET_S64 */ /** * Set NETLINK_BROADCAST_ERROR flags on socket to report ENOBUFS errors. @@ -760,8 +874,8 @@ static int yaml_netlink_msg_parse(struct nl_msg *msg, void *arg) struct genlmsghdr *ghdr = genlmsg_hdr(nlh); struct nlattr *attrs[LN_SCALAR_MAX + 1]; - if (genlmsg_parse(nlh, 0, attrs, LN_SCALAR_MAX, - scalar_attr_policy)) + if (lnet_genlmsg_parse(nlh, 0, attrs, LN_SCALAR_MAX, + scalar_attr_policy)) return NL_SKIP; if (attrs[LN_SCALAR_ATTR_LIST]) { @@ -796,7 +910,7 @@ static int yaml_netlink_msg_parse(struct nl_msg *msg, void *arg) for (i = 1; i < maxtype; i++) policy[i].type = data->cur->keys.lkl_list[i].lkp_data_type; - if (genlmsg_parse(nlh, 0, attrs, maxtype, policy)) + if (lnet_genlmsg_parse(nlh, 0, attrs, maxtype, policy)) return NL_SKIP; size = data->parser->raw_buffer.end - diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index c91cc3f..d0fc80a 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -20206,34 +20206,42 @@ test_217() { # bug 22430 [ $PARALLEL == "yes" ] && skip "skip parallel run" local node - local nid for node in $(nodes_list); do - nid=$(host_nids_address $node $NETTYPE) - if [[ $nid = *-* ]] ; then - echo "lctl ping $(h2nettype $nid)" + local nid=$(host_nids_address $node $NETTYPE) + local node_ip=$(do_node $node getent ahostsv4 $node | + awk '{ print $1; exit; }') + + echo "node: '$node', nid: '$nid', node_ip='$node_ip'" + # if hostname matches any NID, use hostname for better testing + if [[ -z "$nid" || "$nid" =~ "$node_ip" ]]; then + echo "lctl ping node $node@$NETTYPE" + lctl ping $node@$NETTYPE + else # otherwise, at least test 'lctl ping' is working + echo "lctl ping nid $(h2nettype $nid)" lctl ping $(h2nettype $nid) - else echo "skipping $node (no hyphen detected)" fi done } -run_test 217 "check lctl ping for hostnames with hiphen ('-')" +run_test 217 "check lctl ping for hostnames with embedded hyphen ('-')" test_218() { - # do directio so as not to populate the page cache - log "creating a 10 Mb file" - $MULTIOP $DIR/$tfile oO_CREAT:O_DIRECT:O_RDWR:w$((10*1048576))c || error "multiop failed while creating a file" - log "starting reads" - dd if=$DIR/$tfile of=/dev/null bs=4096 & - log "truncating the file" - $MULTIOP $DIR/$tfile oO_TRUNC:c || error "multiop failed while truncating the file" - log "killing dd" - kill %+ || true # reads might have finished - echo "wait until dd is finished" - wait - log "removing the temporary file" - rm -rf $DIR/$tfile || error "tmp file removal failed" + # do directio so as not to populate the page cache + log "creating a 10 Mb file" + $MULTIOP $DIR/$tfile oO_CREAT:O_DIRECT:O_RDWR:w$((10*1048576))c || + error "multiop failed while creating a file" + log "starting reads" + dd if=$DIR/$tfile of=/dev/null bs=4096 & + log "truncating the file" + $MULTIOP $DIR/$tfile oO_TRUNC:c || + error "multiop failed while truncating the file" + log "killing dd" + kill %+ || true # reads might have finished + echo "wait until dd is finished" + wait + log "removing the temporary file" + rm -rf $DIR/$tfile || error "tmp file removal failed" } run_test 218 "parallel read and truncate should not deadlock" @@ -28381,10 +28389,11 @@ test_430c() { # cp version 8.33+ prefers lseek over fiemap local ver=$(cp --version | awk '{ print $4; exit; }') + echo "cp $ver installed" if (( $(version_code $ver) >= $(version_code 8.33) )); then start=$SECONDS - time cp -v $file $file.tmp + time cp -v $file $file.tmp || error "cp $file failed" (( SECONDS - start < 5 )) || { strace cp $file $file.tmp |& grep -E "open|read|seek|FIEMAP" | @@ -28394,12 +28403,13 @@ test_430c() { else echo "cp test skipped due to $ver < 8.33" fi + # tar version 1.29+ supports SEEK_HOLE/DATA ver=$(tar --version | awk '{ print $4; exit; }') echo "tar $ver installed" if (( $(version_code $ver) >= $(version_code 1.29) )); then start=$SECONDS - time tar cvf $file.tmp --sparse $file || error "tar $file failed" + time tar cvf $file.tmp --sparse $file || error "tar $file error" (( SECONDS - start < 5 )) || { strace tar cf $file.tmp --sparse $file |& grep -E "open|read|seek|FIEMAP" | diff --git a/lustre/utils/lctl.c b/lustre/utils/lctl.c index 134e6fe..eee572f 100644 --- a/lustre/utils/lctl.c +++ b/lustre/utils/lctl.c @@ -143,7 +143,7 @@ command_t cmdlist[] = { "print the LNet routing table, same as 'route_list'\n" "usage: show_route"}, {"ping", jt_ptl_ping, 0, "Check LNET connectivity\n" - "usage: ping nid [timeout] [pid]"}, + "usage: ping nid [timeout [pid]]"}, {"net_drop_add", jt_ptl_drop_add, 0, "Add LNet drop rule\n" "usage: net_drop_add <-s | --source NID>\n" " <-d | --dest NID>\n" diff --git a/lustre/utils/lustre_cfg.c b/lustre/utils/lustre_cfg.c index 8adddcd..44571e1 100644 --- a/lustre/utils/lustre_cfg.c +++ b/lustre/utils/lustre_cfg.c @@ -1329,13 +1329,13 @@ int jt_lcfg_listparam(int argc, char **argv) rc2 = param_display(&popt, path, NULL, LIST_PARAM); if (rc2 < 0) { - if (rc2 == -ENOENT && getuid() != 0) - rc2 = llapi_param_display_value(path, 0, 0, - stdout); if (rc == 0) rc = rc2; - if (rc < 0) { + if (rc2 == -ENOENT && getuid() != 0) + rc2 = llapi_param_display_value(path, 0, 0, + stdout); + if (rc2 < 0) { fprintf(stderr, "error: %s: listing '%s': %s\n", jt_cmdname(argv[0]), path, strerror(-rc2)); @@ -1417,11 +1417,12 @@ int jt_lcfg_getparam(int argc, char **argv) rc2 = param_display(&popt, path, NULL, mode); if (rc2 < 0) { + if (rc == 0) + rc = rc2; + if (rc2 == -ENOENT && getuid() != 0) rc2 = llapi_param_display_value(path, version, flags, stdout); - if (rc == 0) - rc = rc2; continue; } } diff --git a/lustre/utils/portals.c b/lustre/utils/portals.c index ab04be6..3fba44c 100644 --- a/lustre/utils/portals.c +++ b/lustre/utils/portals.c @@ -346,7 +346,9 @@ int jt_ptl_network(int argc, char **argv) return 0; } +#ifndef IOC_LIBCFS_GET_NI #define IOC_LIBCFS_GET_NI _IOWR('e', 50, IOCTL_LIBCFS_TYPE) +#endif int jt_ptl_list_nids(int argc, char **argv) @@ -1072,7 +1074,9 @@ int jt_ptl_push_connection(int argc, char **argv) return 0; } -#define IOC_LIBCFS_PING _IOWR('e', 61, IOCTL_LIBCFS_TYPE) +#ifndef IOC_LIBCFS_PING_PEER +#define IOC_LIBCFS_PING_PEER _IOWR('e', 62, IOCTL_LIBCFS_TYPE) +#endif int jt_ptl_ping(int argc, char **argv) { @@ -1080,9 +1084,10 @@ int jt_ptl_ping(int argc, char **argv) int rc; int timeout; struct lnet_process_id id; - struct lnet_process_id ids[16]; + struct lnet_process_id ids[LNET_INTERFACES_MAX_DEFAULT]; + lnet_nid_t src = LNET_NID_ANY; int maxids = sizeof(ids) / sizeof(ids[0]); - struct libcfs_ioctl_data data; + struct lnet_ioctl_ping_data ping = { { 0 } }; yaml_emitter_t request; yaml_parser_t reply; yaml_event_t event; @@ -1092,14 +1097,14 @@ int jt_ptl_ping(int argc, char **argv) if (argc < 2) { fprintf(stderr, "usage: %s id [timeout (secs)]\n", argv[0]); - return 0; + return -EINVAL; } sep = strchr(argv[1], '-'); if (!sep) { rc = lnet_parse_nid(argv[1], &id); if (rc != 0) - return -1; + return -EINVAL; } else { char *end; @@ -1112,7 +1117,7 @@ int jt_ptl_ping(int argc, char **argv) if (end != sep) { /* assuming '-' is part of hostname */ rc = lnet_parse_nid(argv[1], &id); if (rc != 0) - return -1; + return -EINVAL; } else { id.nid = libcfs_str2nid(sep + 1); @@ -1120,7 +1125,7 @@ int jt_ptl_ping(int argc, char **argv) fprintf(stderr, "Can't parse process id \"%s\"\n", argv[1]); - return -1; + return -EINVAL; } } } @@ -1130,7 +1135,7 @@ int jt_ptl_ping(int argc, char **argv) if (timeout > 120 * 1000) { fprintf(stderr, "Timeout %s is to large\n", argv[2]); - return -1; + return -EINVAL; } } else { timeout = 1000; /* default 1 second timeout */ @@ -1143,10 +1148,8 @@ int jt_ptl_ping(int argc, char **argv) /* Setup parser to recieve Netlink packets */ rc = yaml_parser_initialize(&reply); - if (rc == 0) { - nl_socket_free(sk); + if (rc == 0) goto old_api; - } rc = yaml_parser_set_input_netlink(&reply, sk, false); if (rc == 0) @@ -1228,10 +1231,11 @@ int jt_ptl_ping(int argc, char **argv) if (rc == 0) goto emitter_error; + /* convert NID to string, in case libcfs_str2nid() did name lookup */ yaml_scalar_event_initialize(&event, NULL, (yaml_char_t *)YAML_STR_TAG, - (yaml_char_t *)argv[1], - strlen(argv[1]), 1, 0, + (yaml_char_t *)libcfs_nid2str(id.nid), + strlen(libcfs_nid2str(id.nid)), 1, 0, YAML_PLAIN_SCALAR_STYLE); rc = yaml_emitter_emit(&request, &event); if (rc == 0) @@ -1262,6 +1266,7 @@ emitter_error: if (rc == 0) { yaml_emitter_log_error(&request, stderr); rc = -EINVAL; + goto old_api; } yaml_emitter_delete(&request); @@ -1298,6 +1303,7 @@ emitter_error: rc = strtol((char *)event.data.scalar.value, NULL, 10); fprintf(stdout, "failed to ping %s: %s\n", argv[1], strerror(-rc)); + break; /* "rc" is clobbered if loop is run again */ } skip: done = (event.type == YAML_STREAM_END_EVENT); @@ -1305,9 +1311,10 @@ skip: } free_reply: if (rc == 0) { + /* yaml_* functions return 0 for error */ const char *msg = yaml_parser_get_reader_error(&reply); - rc = -errno; + rc = errno ? -errno : -EHOSTUNREACH; if (strcmp(msg, "Unspecific failure") != 0) { fprintf(stdout, "failed to ping %s: %s\n", argv[1], msg); @@ -1315,32 +1322,37 @@ free_reply: fprintf(stdout, "failed to ping %s: %s\n", argv[1], strerror(errno)); } + } else if (rc == 1) { + /* yaml_* functions return 1 for success */ + rc = 0; } yaml_parser_delete(&reply); nl_socket_free(sk); return rc; old_api: - LIBCFS_IOC_INIT(data); - data.ioc_nid = id.nid; - data.ioc_u32[0] = id.pid; - data.ioc_u32[1] = timeout; - data.ioc_plen1 = sizeof(ids); - data.ioc_pbuf1 = (char *)ids; + if (sk) + nl_socket_free(sk); - rc = l_ioctl(LNET_DEV_ID, IOC_LIBCFS_PING, &data); + LIBCFS_IOC_INIT_V2(ping, ping_hdr); + ping.ping_hdr.ioc_len = sizeof(ping); + ping.ping_id = id; + ping.ping_src = src; + ping.op_param = timeout; + ping.ping_count = maxids; + ping.ping_buf = ids; + + rc = l_ioctl(LNET_DEV_ID, IOC_LIBCFS_PING_PEER, &ping); if (rc != 0) { - fprintf(stderr, "failed to ping %s: %s\n", - id.pid == LNET_PID_ANY ? - libcfs_nid2str(id.nid) : libcfs_id2str(id), + fprintf(stderr, "failed to ping %s: %s\n", argv[1], strerror(errno)); - return -1; + return rc; } - for (i = 0; i < data.ioc_count && i < maxids; i++) + for (i = 0; i < ping.ping_count && i < maxids; i++) printf("%s\n", libcfs_id2str(ids[i])); - if (data.ioc_count > maxids) - printf("%d out of %d ids listed\n", maxids, data.ioc_count); + if (ping.ping_count > maxids) + printf("%d out of %d ids listed\n", maxids, ping.ping_count); return 0; } -- 1.8.3.1