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 <chris.horn@hpe.com>
Change-Id: Ie5e91d8ac1c810473768566593e993e47070e14c
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/51087
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Cyril Bordage <cbordage@whamcloud.com>
Reviewed-by: Frank Sehr <fsehr@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lnet/lnet/api-ni.c b/lnet/lnet/api-ni.c
index b83f664..c268ff7 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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);