From 9e6c0915bb73a04e288d85e4ceb22869ed48359c Mon Sep 17 00:00:00 2001 From: Chris Horn Date: Tue, 9 May 2023 09:58:15 -0600 Subject: [PATCH] LU-16574 udsp: lnetctl udsp improvements lnet_udsp_del_policy() did not previously return non-zero, but its single caller would check for a non-zero and call lnet_udsp_apply_policies(). This code is removed. lnet_udsp_del_policy() will now return non-zero but only in the case where there is no matching policy index. In this case the policies are not modified and thus we needn't re-apply them. Modify some error checking for lnetctl udsp commands to provide better error messages. Correct typos in lustre_lnet_add_udsp() error messages. Correct lustre_lnet_del_udsp()'s handling of errno. Update help text of udsp commands. Use parse_long() in jt_show_udsp() to parse the idx argument. Sanity check priority and idx arguments for udsp add/del rather than silently modifying them when the user passes in bad values. Implement 'lnetctl udsp del --all' to provide easy way for admin to delete all configured policies (this is equivalent to 'lnetctl udsp del --idx -1', but it is more user friendly). The udsp del command now requires either --all or --idx be specified. Test-Parameters: trivial HPE-bug-id: LUS-11490 Signed-off-by: Chris Horn Change-Id: Ie5e91d8ac1c810473768566593e993e47070e14c Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/51087 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Cyril Bordage Reviewed-by: Frank Sehr Reviewed-by: Oleg Drokin --- lnet/lnet/api-ni.c | 6 --- lnet/lnet/udsp.c | 3 ++ lnet/utils/lnetconfig/liblnetconfig_udsp.c | 30 ++++++++----- lnet/utils/lnetctl.c | 69 ++++++++++++++++++++++-------- 4 files changed, 74 insertions(+), 34 deletions(-) diff --git a/lnet/lnet/api-ni.c b/lnet/lnet/api-ni.c index b83f664..c268ff7 100644 --- a/lnet/lnet/api-ni.c +++ b/lnet/lnet/api-ni.c @@ -4622,12 +4622,6 @@ report_ping_err: mutex_lock(&the_lnet.ln_api_mutex); rc = lnet_udsp_del_policy(idx); - if (!rc) { - rc = lnet_udsp_apply_policies(NULL, false); - CDEBUG(D_NET, "policy re-application returned %d\n", - rc); - rc = 0; - } mutex_unlock(&the_lnet.ln_api_mutex); return rc; diff --git a/lnet/lnet/udsp.c b/lnet/lnet/udsp.c index e073d12..a22a611 100644 --- a/lnet/lnet/udsp.c +++ b/lnet/lnet/udsp.c @@ -982,6 +982,9 @@ lnet_udsp_del_policy(int idx) } } + if (!removed) + return -ENOENT; + return 0; } diff --git a/lnet/utils/lnetconfig/liblnetconfig_udsp.c b/lnet/utils/lnetconfig/liblnetconfig_udsp.c index 0c388c1..2167aaa 100644 --- a/lnet/utils/lnetconfig/liblnetconfig_udsp.c +++ b/lnet/utils/lnetconfig/liblnetconfig_udsp.c @@ -481,19 +481,31 @@ int lustre_lnet_add_udsp(char *src, char *dst, char *rte, goto out; } + if (!(src || rte || dst)) { + snprintf(err_str, sizeof(err_str), + "\"Missing required argument(s)\""); + rc = LUSTRE_CFG_RC_BAD_PARAM; + goto out; + } + /* sanitize parameters: * src-dst can be simultaneously present * dst-rte can be simultaneously present */ - if ((!src && !rte && !dst) || - (src && rte && dst) || - (src && rte && !dst)) { + if (src && rte && dst) { snprintf(err_str, sizeof(err_str), "\"The combination of src, dst and rte is not supported\""); rc = LUSTRE_CFG_RC_BAD_PARAM; goto out; } + if (src && rte) { + snprintf(err_str, sizeof(err_str), + "\"src and rte cannot be combined\""); + rc = LUSTRE_CFG_RC_BAD_PARAM; + goto out; + } + udsp = lnet_udsp_alloc(); if (!udsp) { snprintf(err_str, sizeof(err_str), "\"out of memory\""); @@ -523,7 +535,7 @@ int lustre_lnet_add_udsp(char *src, char *dst, char *rte, if (rc < 0) { snprintf(err_str, sizeof(err_str), - "\failed to parse src parameter\""); + "\"failed to parse src parameter\""); goto out; } } @@ -534,7 +546,7 @@ int lustre_lnet_add_udsp(char *src, char *dst, char *rte, if (rc < 0) { snprintf(err_str, sizeof(err_str), - "\failed to parse dst parameter\""); + "\"failed to parse dst parameter\""); goto out; } } @@ -545,7 +557,7 @@ int lustre_lnet_add_udsp(char *src, char *dst, char *rte, if (rc < 0) { snprintf(err_str, sizeof(err_str), - "\failed to parse rte parameter\""); + "\"failed to parse rte parameter\""); goto out; } } @@ -605,11 +617,9 @@ int lustre_lnet_del_udsp(unsigned int idx, int seq_no, struct cYAML **err_rc) udsp_bulk.iou_idx = idx; rc = l_ioctl(LNET_DEV_ID, IOC_LIBCFS_DEL_UDSP, &udsp_bulk); - if (rc < 0) { - rc = -errno; + if (rc < 0) snprintf(err_str, sizeof(err_str), - "\"cannot del udsp: %s\"", strerror(rc)); - } + "\"cannot del udsp: %s\"", strerror(errno)); cYAML_build_error(rc, seq_no, ADD_CMD, "udsp", err_str, err_rc); return rc; diff --git a/lnet/utils/lnetctl.c b/lnet/utils/lnetctl.c index f22103a..2e1dd5d 100644 --- a/lnet/utils/lnetctl.c +++ b/lnet/utils/lnetctl.c @@ -283,16 +283,19 @@ command_t peer_cmds[] = { command_t udsp_cmds[] = { {"add", jt_add_udsp, 0, "add a udsp\n" - "\t--src: ip2nets syntax specifying the local NID to match\n" - "\t--dst: ip2nets syntax specifying the remote NID to match\n" - "\t--rte: ip2nets syntax specifying the router NID to match\n" - "\t--priority: priority value (0 - highest priority)\n" - "\t--idx: index of where to insert the rule.\n" - "\t By default, appends to the end of the rule list.\n"}, + "\t--src nid|net: ip2nets syntax specifying the local NID or network to match.\n" + "\t--dst nid: ip2nets syntax specifying the remote NID to match.\n" + "\t--rte nid: ip2nets syntax specifying the router NID to match.\n" + "\t--priority p: Assign priority value p where p >= 0.\n" + "\t Note: 0 is the highest priority.\n" + "\t--idx n: Insert the rule in the n'th position on the list of rules.\n" + "\t By default, rules are appended to the end of the rule list.\n"}, {"del", jt_del_udsp, 0, "delete a udsp\n" - "\t--idx: index of the Policy.\n"}, + "\t--all: Delete all rules.\n" + "\t--idx n: Delete the rule at index n.\n"}, {"show", jt_show_udsp, 0, "show udsps\n" - "\t --idx: index of the policy to show.\n"}, + "\t--idx n: Show the rule at at index n.\n" + "\t By default, all rules are shown.\n"}, { 0, 0, 0, NULL } }; @@ -2709,7 +2712,7 @@ static int jt_show_stats(int argc, char **argv) static int jt_show_udsp(int argc, char **argv) { - int idx = -1; + long int idx = -1; int rc, opt; struct cYAML *err_rc = NULL, *show_rc = NULL; @@ -2727,7 +2730,11 @@ static int jt_show_udsp(int argc, char **argv) long_options, NULL)) != -1) { switch (opt) { case 'i': - idx = atoi(optarg); + rc = parse_long(optarg, &idx); + if (rc != 0 || idx < -1) { + printf("Invalid index \"%s\"\n", optarg); + return -EINVAL; + } break; case '?': print_help(net_cmds, "net", "show"); @@ -3516,15 +3523,19 @@ static int jt_add_udsp(int argc, char **argv) break; case 'p': rc = parse_long(optarg, &priority); - if (rc != 0) - priority = -1; + if (rc != 0 || priority < 0) { + printf("Invalid priority \"%s\"\n", optarg); + return -EINVAL; + } action_type = "priority"; udsp_action.udsp_priority = priority; break; case 'i': rc = parse_long(optarg, &idx); - if (rc != 0) - idx = 0; + if (rc != 0 || idx < 0) { + printf("Invalid index \"%s\"\n", optarg); + return -EINVAL; + } break; case '?': print_help(udsp_cmds, "udsp", "add"); @@ -3533,6 +3544,11 @@ static int jt_add_udsp(int argc, char **argv) } } + if (!(src || dst || rte)) { + print_help(udsp_cmds, "udsp", "add"); + return 0; + } + rc = lustre_lnet_add_udsp(src, dst, rte, action_type, &udsp_action, idx, -1, &err_rc); @@ -3547,11 +3563,13 @@ static int jt_add_udsp(int argc, char **argv) static int jt_del_udsp(int argc, char **argv) { struct cYAML *err_rc = NULL; - long int idx = 0; + long int idx = -2; int opt, rc = 0; + bool all = false; - const char *const short_options = "i:"; + const char *const short_options = "ai:"; static const struct option long_options[] = { + { .name = "all", .has_arg = no_argument, .val = 'a' }, { .name = "idx", .has_arg = required_argument, .val = 'i' }, { .name = NULL } }; @@ -3562,10 +3580,15 @@ static int jt_del_udsp(int argc, char **argv) while ((opt = getopt_long(argc, argv, short_options, long_options, NULL)) != -1) { switch (opt) { + case 'a': + all = true; + break; case 'i': rc = parse_long(optarg, &idx); - if (rc != 0) - idx = 0; + if (rc != 0 || idx < -1) { + printf("Invalid index \"%s\"\n", optarg); + return -EINVAL; + } break; case '?': print_help(udsp_cmds, "udsp", "del"); @@ -3574,6 +3597,16 @@ static int jt_del_udsp(int argc, char **argv) } } + if (all && idx != -2) { + printf("Cannot combine --all with --idx\n"); + return -EINVAL; + } else if (all) { + idx = -1; + } else if (idx == -2) { + printf("Must specify --idx or --all\n"); + return -EINVAL; + } + rc = lustre_lnet_del_udsp(idx, -1, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) cYAML_print_tree2file(stderr, err_rc); -- 1.8.3.1