LU-8006 ptlrpc: cleanup codes of TBF command

The codes of parsing TBF command are improved so as to make it
easier to add new commands. And command format has been changed
to:
For NRS "tbf nid" policy:
lctl set_param ost.OSS.ost_io.nrs_tbf_rule=
	"start $RULE_NAME nid={$NID} rate=$RATE"
For NRS "tbf jobid" policy:
lctl set_param ost.OSS.ost_io.nrs_tbf_rule=
	"start $RULE_NAME jobid={$JOBID} rate=$RATE"

lctl set_param ost.OSS.ost_io.nrs_tbf_rule=
	"change $RULE_NAME rate=$RATE"

Signed-off-by: Li Xi <lixi@ddn.com>
Signed-off-by: Qian Yingjin <qian@ddn.com>
Change-Id: Id9861f4c3d894bea614288089622c11124e7f4de
Reviewed-on: http://review.whamcloud.com/19499
Tested-by: Jenkins
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Wang Shilong <wshilong@ddn.com>
Reviewed-by: Gu Zheng <gzheng@ddn.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/include/lustre_nrs_tbf.h b/lustre/include/lustre_nrs_tbf.h
index 3d52f78..4d93c23 100644 (file)
--- a/lustre/include/lustre_nrs_tbf.h
+++ b/lustre/include/lustre_nrs_tbf.h
@@ -150,6 +150,7 @@ struct nrs_tbf_ops {
 #define NRS_TBF_TYPE_JOBID     "jobid"
 #define NRS_TBF_TYPE_NID       "nid"
 #define NRS_TBF_TYPE_MAX_LEN   20
+#define NRS_TBF_FLAG_INVALID   0
 #define NRS_TBF_FLAG_JOBID     0x0000001
 #define NRS_TBF_FLAG_NID       0x0000002
 
@@ -226,19 +227,26 @@ struct nrs_tbf_head {
 enum nrs_tbf_cmd_type {
        NRS_CTL_TBF_START_RULE = 0,
        NRS_CTL_TBF_STOP_RULE,
-       NRS_CTL_TBF_CHANGE_RATE,
+       NRS_CTL_TBF_CHANGE_RULE,
 };
 
 struct nrs_tbf_cmd {
-       enum nrs_tbf_cmd_type    tc_cmd;
-       char                    *tc_name;
-       __u64                    tc_rpc_rate;
-       struct list_head         tc_nids;
-       char                    *tc_nids_str;
-       struct list_head         tc_jobids;
-       char                    *tc_jobids_str;
-       __u32                    tc_valid_types;
-       __u32                    tc_rule_flags;
+       enum nrs_tbf_cmd_type                    tc_cmd;
+       char                                    *tc_name;
+       union {
+               struct nrs_tbf_cmd_start {
+                       __u64                    ts_rpc_rate;
+                       struct list_head         ts_nids;
+                       char                    *ts_nids_str;
+                       struct list_head         ts_jobids;
+                       char                    *ts_jobids_str;
+                       __u32                    ts_valid_type;
+                       __u32                    ts_rule_flags;
+               } tc_start;
+               struct nrs_tbf_cmd_change {
+                       __u64                    tc_rpc_rate;
+               } tc_change;
+       } u;
 };
 
 struct nrs_tbf_req {
@@ -264,6 +272,10 @@ enum nrs_ctl_tbf {
         * Write the the data of a TBF policy.
         */
        NRS_CTL_TBF_WR_RULE,
+       /**
+        * Read the TBF policy type preset by proc entry "nrs_policies".
+        */
+       NRS_CTL_TBF_RD_TYPE_FLAG,
 };
 
 /** @} tbf */

diff --git a/lustre/ptlrpc/nrs_tbf.c b/lustre/ptlrpc/nrs_tbf.c
index 282eb8a..db9a29d 100644 (file)
--- a/lustre/ptlrpc/nrs_tbf.c
+++ b/lustre/ptlrpc/nrs_tbf.c
@@ -295,7 +295,7 @@ nrs_tbf_rule_start(struct ptlrpc_nrs_policy *policy,
                return -ENOMEM;
 
        memcpy(rule->tr_name, start->tc_name, strlen(start->tc_name));
-       rule->tr_rpc_rate = start->tc_rpc_rate;
+       rule->tr_rpc_rate = start->u.tc_start.ts_rpc_rate;
        rule->tr_nsecs = NSEC_PER_SEC;
        do_div(rule->tr_nsecs, rule->tr_rpc_rate);
        rule->tr_depth = tbf_depth;
@@ -324,7 +324,7 @@ nrs_tbf_rule_start(struct ptlrpc_nrs_policy *policy,
        list_add(&rule->tr_linkage, &head->th_list);
        spin_unlock(&head->th_rule_lock);
        atomic_inc(&head->th_rule_sequence);
-       if (start->tc_rule_flags & NTRS_DEFAULT) {
+       if (start->u.tc_start.ts_rule_flags & NTRS_DEFAULT) {
                rule->tr_flags |= NTRS_DEFAULT;
                LASSERT(head->th_rule == NULL);
                head->th_rule = rule;
@@ -346,7 +346,7 @@ nrs_tbf_rule_change(struct ptlrpc_nrs_policy *policy,
        if (rule == NULL)
                return -ENOENT;
 
-       rule->tr_rpc_rate = change->tc_rpc_rate;
+       rule->tr_rpc_rate = change->u.tc_change.tc_rpc_rate;
        rule->tr_nsecs = NSEC_PER_SEC;
        do_div(rule->tr_nsecs, rule->tr_rpc_rate);
        rule->tr_generation++;
@@ -390,14 +390,14 @@ nrs_tbf_command(struct ptlrpc_nrs_policy *policy,
 
        switch (cmd->tc_cmd) {
        case NRS_CTL_TBF_START_RULE:
-               if (!(cmd->tc_valid_types & head->th_type_flag))
+               if (cmd->u.tc_start.ts_valid_type != head->th_type_flag)
                        return -EINVAL;
 
                spin_unlock(&policy->pol_nrs->nrs_lock);
                rc = nrs_tbf_rule_start(policy, head, cmd);
                spin_lock(&policy->pol_nrs->nrs_lock);
                return rc;
-       case NRS_CTL_TBF_CHANGE_RATE:
+       case NRS_CTL_TBF_CHANGE_RULE:
                rc = nrs_tbf_rule_change(policy, head, cmd);
                return rc;
        case NRS_CTL_TBF_STOP_RULE:
@@ -689,12 +689,12 @@ nrs_tbf_jobid_startup(struct ptlrpc_nrs_policy *policy,
        }
 
        memset(&start, 0, sizeof(start));
-       start.tc_jobids_str = "*";
+       start.u.tc_start.ts_jobids_str = "*";
 
-       start.tc_rpc_rate = tbf_rate;
-       start.tc_rule_flags = NTRS_DEFAULT;
+       start.u.tc_start.ts_rpc_rate = tbf_rate;
+       start.u.tc_start.ts_rule_flags = NTRS_DEFAULT;
        start.tc_name = NRS_TBF_DEFAULT_RULE;
-       INIT_LIST_HEAD(&start.tc_jobids);
+       INIT_LIST_HEAD(&start.u.tc_start.ts_jobids);
        rc = nrs_tbf_rule_start(policy, head, &start);
 
        return rc;
@@ -776,26 +776,53 @@ nrs_tbf_jobid_list_parse(char *str, int len, struct list_head *jobid_list)
 
 static void nrs_tbf_jobid_cmd_fini(struct nrs_tbf_cmd *cmd)
 {
-       if (!list_empty(&cmd->tc_jobids))
-               nrs_tbf_jobid_list_free(&cmd->tc_jobids);
-       if (cmd->tc_jobids_str)
-               OBD_FREE(cmd->tc_jobids_str, strlen(cmd->tc_jobids_str) + 1);
+       if (!list_empty(&cmd->u.tc_start.ts_jobids))
+               nrs_tbf_jobid_list_free(&cmd->u.tc_start.ts_jobids);
+       if (cmd->u.tc_start.ts_jobids_str)
+               OBD_FREE(cmd->u.tc_start.ts_jobids_str,
+                        strlen(cmd->u.tc_start.ts_jobids_str) + 1);
 }
 
-static int nrs_tbf_jobid_parse(struct nrs_tbf_cmd *cmd, const char *id)
+static int nrs_tbf_check_id_value(struct cfs_lstr *src, char *key)
 {
+       struct cfs_lstr res;
+       int keylen = strlen(key);
+       int rc;
+
+       rc = cfs_gettok(src, '=', &res);
+       if (rc == 0 || res.ls_len != keylen ||
+           strncmp(res.ls_str, key, keylen) != 0 ||
+           src->ls_len <= 2 || src->ls_str[0] != '{' ||
+           src->ls_str[src->ls_len - 1] != '}')
+               return -EINVAL;
+
+       /* Skip '{' and '}' */
+       src->ls_str++;
+       src->ls_len -= 2;
+       return 0;
+}
+
+static int nrs_tbf_jobid_parse(struct nrs_tbf_cmd *cmd, char *id)
+{
+       struct cfs_lstr src;
        int rc;
 
-       OBD_ALLOC(cmd->tc_jobids_str, strlen(id) + 1);
-       if (cmd->tc_jobids_str == NULL)
+       src.ls_str = id;
+       src.ls_len = strlen(id);
+       rc = nrs_tbf_check_id_value(&src, "jobid");
+       if (rc)
+               return rc;
+
+       OBD_ALLOC(cmd->u.tc_start.ts_jobids_str, src.ls_len + 1);
+       if (cmd->u.tc_start.ts_jobids_str == NULL)
                return -ENOMEM;
 
-       memcpy(cmd->tc_jobids_str, id, strlen(id));
+       memcpy(cmd->u.tc_start.ts_jobids_str, src.ls_str, src.ls_len);
 
        /* parse jobid list */
-       rc = nrs_tbf_jobid_list_parse(cmd->tc_jobids_str,
-                                     strlen(cmd->tc_jobids_str),
-                                     &cmd->tc_jobids);
+       rc = nrs_tbf_jobid_list_parse(cmd->u.tc_start.ts_jobids_str,
+                                     strlen(cmd->u.tc_start.ts_jobids_str),
+                                     &cmd->u.tc_start.ts_jobids);
        if (rc)
                nrs_tbf_jobid_cmd_fini(cmd);
 
@@ -808,18 +835,18 @@ static int nrs_tbf_jobid_rule_init(struct ptlrpc_nrs_policy *policy,
 {
        int rc = 0;
 
-       LASSERT(start->tc_jobids_str);
+       LASSERT(start->u.tc_start.ts_jobids_str);
        OBD_ALLOC(rule->tr_jobids_str,
-                 strlen(start->tc_jobids_str) + 1);
+                 strlen(start->u.tc_start.ts_jobids_str) + 1);
        if (rule->tr_jobids_str == NULL)
                return -ENOMEM;
 
        memcpy(rule->tr_jobids_str,
-              start->tc_jobids_str,
-              strlen(start->tc_jobids_str));
+              start->u.tc_start.ts_jobids_str,
+              strlen(start->u.tc_start.ts_jobids_str));
 
        INIT_LIST_HEAD(&rule->tr_jobids);
-       if (!list_empty(&start->tc_jobids)) {
+       if (!list_empty(&start->u.tc_start.ts_jobids)) {
                rc = nrs_tbf_jobid_list_parse(rule->tr_jobids_str,
                                              strlen(rule->tr_jobids_str),
                                              &rule->tr_jobids);
@@ -828,7 +855,7 @@ static int nrs_tbf_jobid_rule_init(struct ptlrpc_nrs_policy *policy,
        }
        if (rc)
                OBD_FREE(rule->tr_jobids_str,
-                        strlen(start->tc_jobids_str) + 1);
+                        strlen(start->u.tc_start.ts_jobids_str) + 1);
        return rc;
 }
 
@@ -836,8 +863,8 @@ static int
 nrs_tbf_jobid_rule_dump(struct nrs_tbf_rule *rule, struct seq_file *m)
 {
        seq_printf(m, "%s {%s} %llu, ref %d\n", rule->tr_name,
-                         rule->tr_jobids_str, rule->tr_rpc_rate,
-                         atomic_read(&rule->tr_ref) - 1);
+                  rule->tr_jobids_str, rule->tr_rpc_rate,
+                  atomic_read(&rule->tr_ref) - 1);
        return 0;
 }
 
@@ -991,12 +1018,12 @@ nrs_tbf_nid_startup(struct ptlrpc_nrs_policy *policy,
                return -ENOMEM;
 
        memset(&start, 0, sizeof(start));
-       start.tc_nids_str = "*";
+       start.u.tc_start.ts_nids_str = "*";
 
-       start.tc_rpc_rate = tbf_rate;
-       start.tc_rule_flags = NTRS_DEFAULT;
+       start.u.tc_start.ts_rpc_rate = tbf_rate;
+       start.u.tc_start.ts_rule_flags = NTRS_DEFAULT;
        start.tc_name = NRS_TBF_DEFAULT_RULE;
-       INIT_LIST_HEAD(&start.tc_nids);
+       INIT_LIST_HEAD(&start.u.tc_start.ts_nids);
        rc = nrs_tbf_rule_start(policy, head, &start);
 
        return rc;
@@ -1013,25 +1040,25 @@ static int nrs_tbf_nid_rule_init(struct ptlrpc_nrs_policy *policy,
                                 struct nrs_tbf_rule *rule,
                                 struct nrs_tbf_cmd *start)
 {
-       LASSERT(start->tc_nids_str);
+       LASSERT(start->u.tc_start.ts_nids_str);
        OBD_ALLOC(rule->tr_nids_str,
-                 strlen(start->tc_nids_str) + 1);
+                 strlen(start->u.tc_start.ts_nids_str) + 1);
        if (rule->tr_nids_str == NULL)
                return -ENOMEM;
 
        memcpy(rule->tr_nids_str,
-              start->tc_nids_str,
-              strlen(start->tc_nids_str));
+              start->u.tc_start.ts_nids_str,
+              strlen(start->u.tc_start.ts_nids_str));
 
        INIT_LIST_HEAD(&rule->tr_nids);
-       if (!list_empty(&start->tc_nids)) {
+       if (!list_empty(&start->u.tc_start.ts_nids)) {
                if (cfs_parse_nidlist(rule->tr_nids_str,
                                      strlen(rule->tr_nids_str),
                                      &rule->tr_nids) <= 0) {
                        CERROR("nids {%s} illegal\n",
                               rule->tr_nids_str);
                        OBD_FREE(rule->tr_nids_str,
-                                strlen(start->tc_nids_str) + 1);
+                                strlen(start->u.tc_start.ts_nids_str) + 1);
                        return -EINVAL;
                }
        }
@@ -1042,8 +1069,8 @@ static int
 nrs_tbf_nid_rule_dump(struct nrs_tbf_rule *rule, struct seq_file *m)
 {
        seq_printf(m, "%s {%s} %llu, ref %d\n", rule->tr_name,
-                         rule->tr_nids_str, rule->tr_rpc_rate,
-                         atomic_read(&rule->tr_ref) - 1);
+                  rule->tr_nids_str, rule->tr_rpc_rate,
+                  atomic_read(&rule->tr_ref) - 1);
        return 0;
 }
 
@@ -1064,24 +1091,34 @@ static void nrs_tbf_nid_rule_fini(struct nrs_tbf_rule *rule)
 
 static void nrs_tbf_nid_cmd_fini(struct nrs_tbf_cmd *cmd)
 {
-       if (!list_empty(&cmd->tc_nids))
-               cfs_free_nidlist(&cmd->tc_nids);
-       if (cmd->tc_nids_str)
-               OBD_FREE(cmd->tc_nids_str, strlen(cmd->tc_nids_str) + 1);
+       if (!list_empty(&cmd->u.tc_start.ts_nids))
+               cfs_free_nidlist(&cmd->u.tc_start.ts_nids);
+       if (cmd->u.tc_start.ts_nids_str)
+               OBD_FREE(cmd->u.tc_start.ts_nids_str,
+                        strlen(cmd->u.tc_start.ts_nids_str) + 1);
 }
 
-static int nrs_tbf_nid_parse(struct nrs_tbf_cmd *cmd, const char *id)
+static int nrs_tbf_nid_parse(struct nrs_tbf_cmd *cmd, char *id)
 {
-       OBD_ALLOC(cmd->tc_nids_str, strlen(id) + 1);
-       if (cmd->tc_nids_str == NULL)
+       struct cfs_lstr src;
+       int rc;
+
+       src.ls_str = id;
+       src.ls_len = strlen(id);
+       rc = nrs_tbf_check_id_value(&src, "nid");
+       if (rc)
+               return rc;
+
+       OBD_ALLOC(cmd->u.tc_start.ts_nids_str, src.ls_len + 1);
+       if (cmd->u.tc_start.ts_nids_str == NULL)
                return -ENOMEM;
 
-       memcpy(cmd->tc_nids_str, id, strlen(id));
+       memcpy(cmd->u.tc_start.ts_nids_str, src.ls_str, src.ls_len);
 
        /* parse NID list */
-       if (cfs_parse_nidlist(cmd->tc_nids_str,
-                             strlen(cmd->tc_nids_str),
-                             &cmd->tc_nids) <= 0) {
+       if (cfs_parse_nidlist(cmd->u.tc_start.ts_nids_str,
+                             strlen(cmd->u.tc_start.ts_nids_str),
+                             &cmd->u.tc_start.ts_nids) <= 0) {
                nrs_tbf_nid_cmd_fini(cmd);
                return -EINVAL;
        }
@@ -1257,6 +1294,15 @@ static int nrs_tbf_ctl(struct ptlrpc_nrs_policy *policy,
                                     cmd);
                }
                break;
+       /**
+        * Read the TBF policy type of a policy instance.
+        */
+       case NRS_CTL_TBF_RD_TYPE_FLAG: {
+               struct nrs_tbf_head *head = policy->pol_private;
+
+               *(__u32 *)arg = head->th_type_flag;
+               }
+               break;
        }
 
        RETURN(rc);
@@ -1628,66 +1674,123 @@ no_hp:
        return rc;
 }
 
-static int nrs_tbf_id_parse(struct nrs_tbf_cmd *cmd, char **val)
+static int nrs_tbf_id_parse(struct nrs_tbf_cmd *cmd, char *token)
 {
        int rc;
-       char *token;
-
-       token = strsep(val, "}");
-       if (*val == NULL)
-               GOTO(out, rc = -EINVAL);
-
-       if (strlen(token) <= 1 ||
-           token[0] != '{')
-               GOTO(out, rc = -EINVAL);
-       /* Skip '{' */
-       token++;
-
-       /* Should be followed by ' ' or nothing */
-       if ((*val)[0] == '\0')
-               *val = NULL;
-       else if ((*val)[0] == ' ')
-               (*val)++;
-       else
-               GOTO(out, rc = -EINVAL);
-
-       rc = nrs_tbf_jobid_parse(cmd, token);
-       if (!rc)
-               cmd->tc_valid_types |= NRS_TBF_FLAG_JOBID;
 
-       rc = nrs_tbf_nid_parse(cmd, token);
-       if (!rc)
-               cmd->tc_valid_types |= NRS_TBF_FLAG_NID;
-
-       if (!cmd->tc_valid_types)
+       if (cmd->u.tc_start.ts_valid_type & NRS_TBF_FLAG_JOBID)
+               rc = nrs_tbf_jobid_parse(cmd, token);
+       else if (cmd->u.tc_start.ts_valid_type & NRS_TBF_FLAG_NID)
+               rc = nrs_tbf_nid_parse(cmd, token);
+       else if (cmd->u.tc_start.ts_valid_type == NRS_TBF_FLAG_INVALID)
                rc = -EINVAL;
        else
                rc = 0;
-out:
+
        return rc;
 }
 
 
 static void nrs_tbf_cmd_fini(struct nrs_tbf_cmd *cmd)
 {
-       if (cmd->tc_valid_types & NRS_TBF_FLAG_JOBID)
-               nrs_tbf_jobid_cmd_fini(cmd);
-       if (cmd->tc_valid_types & NRS_TBF_FLAG_NID)
-               nrs_tbf_nid_cmd_fini(cmd);
+       if (cmd->tc_cmd == NRS_CTL_TBF_START_RULE) {
+               if (cmd->u.tc_start.ts_valid_type & NRS_TBF_FLAG_JOBID)
+                       nrs_tbf_jobid_cmd_fini(cmd);
+               else if (cmd->u.tc_start.ts_valid_type & NRS_TBF_FLAG_NID)
+                       nrs_tbf_nid_cmd_fini(cmd);
+       }
+}
+
+static bool name_is_valid(const char *name)
+{
+       int i;
+
+       for (i = 0; i < strlen(name); i++) {
+               if ((!isalnum(name[i])) &&
+                   (name[i] != '_'))
+                       return false;
+       }
+       return true;
+}
+
+static int
+nrs_tbf_parse_value_pair(struct nrs_tbf_cmd *cmd, char *buffer)
+{
+       char    *key;
+       char    *val;
+       int      rc;
+       __u64    rate;
+
+       val = buffer;
+       key = strsep(&val, "=");
+       if (val == NULL || strlen(val) == 0)
+               return -EINVAL;
+
+       /* Key of the value pair */
+       if (strcmp(key, "rate") == 0) {
+               rc = kstrtoull(val, 10, &rate);
+               if (rc)
+                       return rc;
+
+               if (rate <= 0 || rate >= LPROCFS_NRS_RATE_MAX)
+                       return -EINVAL;
+
+               if (cmd->tc_cmd == NRS_CTL_TBF_START_RULE)
+                       cmd->u.tc_start.ts_rpc_rate = rate;
+               else if (cmd->tc_cmd == NRS_CTL_TBF_CHANGE_RULE)
+                       cmd->u.tc_change.tc_rpc_rate = rate;
+               else
+                       return -EINVAL;
+       } else {
+               return -EINVAL;
+       }
+       return 0;
+}
+
+static int
+nrs_tbf_parse_value_pairs(struct nrs_tbf_cmd *cmd, char *buffer)
+{
+       char    *val;
+       char    *token;
+       int      rc;
+
+       val = buffer;
+       while (val != NULL && strlen(val) != 0) {
+               token = strsep(&val, " ");
+               rc = nrs_tbf_parse_value_pair(cmd, token);
+               if (rc)
+                       return rc;
+       }
+
+       switch (cmd->tc_cmd) {
+       case NRS_CTL_TBF_START_RULE:
+               if (cmd->u.tc_start.ts_rpc_rate == 0)
+                       cmd->u.tc_start.ts_rpc_rate = tbf_rate;
+               break;
+       case NRS_CTL_TBF_CHANGE_RULE:
+               if (cmd->u.tc_change.tc_rpc_rate == 0)
+                       return -EINVAL;
+               break;
+       case NRS_CTL_TBF_STOP_RULE:
+               break;
+       default:
+               return -EINVAL;
+       }
+       return 0;
 }
 
 static struct nrs_tbf_cmd *
-nrs_tbf_parse_cmd(char *buffer, unsigned long count)
+nrs_tbf_parse_cmd(char *buffer, unsigned long count, __u32 type_flag)
 {
-       static struct nrs_tbf_cmd *cmd;
-       char                      *token;
-       char                      *val;
-       int                        i;
-       int                        rc = 0;
+       static struct nrs_tbf_cmd       *cmd;
+       char                            *token;
+       char                            *val;
+       int                              rc = 0;
 
        OBD_ALLOC_PTR(cmd);
        if (cmd == NULL)
                GOTO(out, rc = -ENOMEM);
+       memset(cmd, 0, sizeof(*cmd));
 
        val = buffer;
        token = strsep(&val, " ");
@@ -1695,58 +1798,51 @@ nrs_tbf_parse_cmd(char *buffer, unsigned long count)
                GOTO(out_free_cmd, rc = -EINVAL);
 
        /* Type of the command */
-       if (strcmp(token, "start") == 0)
+       if (strcmp(token, "start") == 0) {
                cmd->tc_cmd = NRS_CTL_TBF_START_RULE;
-       else if (strcmp(token, "stop") == 0)
+               cmd->u.tc_start.ts_valid_type = type_flag;
+       } else if (strcmp(token, "stop") == 0)
                cmd->tc_cmd = NRS_CTL_TBF_STOP_RULE;
        else if (strcmp(token, "change") == 0)
-               cmd->tc_cmd = NRS_CTL_TBF_CHANGE_RATE;
+               cmd->tc_cmd = NRS_CTL_TBF_CHANGE_RULE;
        else
                GOTO(out_free_cmd, rc = -EINVAL);
 
        /* Name of the rule */
        token = strsep(&val, " ");
-       if (val == NULL) {
-               /**
-                * Stop comand only need name argument,
-                * But other commands need ID or rate argument.
-                */
-               if (cmd->tc_cmd != NRS_CTL_TBF_STOP_RULE)
-                       GOTO(out_free_cmd, rc = -EINVAL);
-       }
-
-       for (i = 0; i < strlen(token); i++) {
-               if ((!isalnum(token[i])) &&
-                   (token[i] != '_'))
-                       GOTO(out_free_cmd, rc = -EINVAL);
-       }
+       if ((val == NULL && cmd->tc_cmd != NRS_CTL_TBF_STOP_RULE) ||
+           !name_is_valid(token))
+               GOTO(out_free_cmd, rc = -EINVAL);
        cmd->tc_name = token;
 
        if (cmd->tc_cmd == NRS_CTL_TBF_START_RULE) {
                /* List of ID */
                LASSERT(val);
-               rc = nrs_tbf_id_parse(cmd, &val);
+               token = val;
+               val = strrchr(token, '}');
+               if (!val)
+                       GOTO(out_free_cmd, rc = -EINVAL);
+
+               /* Skip '}' */
+               val++;
+               if (*val == '\0') {
+                       val = NULL;
+               } else if (*val == ' ') {
+                       *val = '\0';
+                       val++;
+               } else
+                       GOTO(out_free_cmd, rc = -EINVAL);
+
+               rc = nrs_tbf_id_parse(cmd, token);
                if (rc)
                        GOTO(out_free_cmd, rc);
        }
 
-       if (val != NULL) {
-               if (cmd->tc_cmd == NRS_CTL_TBF_STOP_RULE ||
-                   strlen(val) == 0 || !isdigit(val[0]))
-                       GOTO(out_free_nid, rc = -EINVAL);
-
-               cmd->tc_rpc_rate = simple_strtoull(val, NULL, 10);
-               if (cmd->tc_rpc_rate <= 0 ||
-                   cmd->tc_rpc_rate >= LPROCFS_NRS_RATE_MAX)
-                       GOTO(out_free_nid, rc = -EINVAL);
-       } else {
-               if (cmd->tc_cmd == NRS_CTL_TBF_CHANGE_RATE)
-                       GOTO(out_free_nid, rc = -EINVAL);
-               /* No RPC rate given */
-               cmd->tc_rpc_rate = tbf_rate;
-       }
+       rc = nrs_tbf_parse_value_pairs(cmd, val);
+       if (rc)
+               GOTO(out_cmd_fini, rc = -EINVAL);
        goto out;
-out_free_nid:
+out_cmd_fini:
        nrs_tbf_cmd_fini(cmd);
 out_free_cmd:
        OBD_FREE_PTR(cmd);
@@ -1756,6 +1852,31 @@ out:
        return cmd;
 }
 
+/**
+ * Get the TBF policy type (nid, jobid, etc) preset by
+ * proc entry 'nrs_policies' for command buffer parsing.
+ *
+ * \param[in] svc the PTLRPC service
+ * \param[in] queue the NRS queue type
+ *
+ * \retval the preset TBF policy type flag
+ */
+static __u32
+nrs_tbf_type_flag(struct ptlrpc_service *svc, enum ptlrpc_nrs_queue_type queue)
+{
+       __u32   type;
+       int     rc;
+
+       rc = ptlrpc_nrs_policy_control(svc, queue,
+                                      NRS_POL_NAME_TBF,
+                                      NRS_CTL_TBF_RD_TYPE_FLAG,
+                                      true, &type);
+       if (rc != 0)
+               type = NRS_TBF_FLAG_INVALID;
+
+       return type;
+}
+
 extern struct nrs_core nrs_core;
 #define LPROCFS_WR_NRS_TBF_MAX_CMD (4096)
 static ssize_t
@@ -1806,7 +1927,7 @@ ptlrpc_lprocfs_nrs_tbf_rule_seq_write(struct file *file,
        else if (queue == PTLRPC_NRS_QUEUE_BOTH && !nrs_svc_has_hp(svc))
                queue = PTLRPC_NRS_QUEUE_REG;
 
-       cmd = nrs_tbf_parse_cmd(val, length);
+       cmd = nrs_tbf_parse_cmd(val, length, nrs_tbf_type_flag(svc, queue));
        if (IS_ERR(cmd))
                GOTO(out_free_kernbuff, rc = PTR_ERR(cmd));
 

diff --git a/lustre/tests/sanityn.sh b/lustre/tests/sanityn.sh
index 738c837..2b65a30 100755 (executable)
--- a/lustre/tests/sanityn.sh
+++ b/lustre/tests/sanityn.sh
@@ -3053,7 +3053,7 @@ tbf_rule_operate()
        shift 1
 
        do_facet $facet lctl set_param \
-               ost.OSS.ost_io.nrs_tbf_rule="$@"
+               ost.OSS.ost_io.nrs_tbf_rule="$*"
        [ $? -ne 0 ] &&
                error "failed to operate on TBF rules"
 }
@@ -3067,19 +3067,26 @@ test_77e() {
                        error "failed to set TBF policy"
        done
 
+       local idis
+       local rateis
+       if [ $(lustre_version_code ost1) -ge $(version_code 2.8.54) ]; then
+               idis="nid="
+               rateis="rate="
+       fi
+
        # Only operate rules on ost1 since OSTs might run on the same OSS
        # Add some rules
-       tbf_rule_operate ost1 "start\ localhost\ {0@lo}\ 1000"
+       tbf_rule_operate ost1 "start\ localhost\ ${idis}{0@lo}\ ${rateis}1000"
        local address=$(comma_list "$(host_nids_address $CLIENTS $NETTYPE)")
        local client_nids=$(nids_list $address "\\")
-       tbf_rule_operate ost1 "start\ clients\ {$client_nids}\ 100"
-       tbf_rule_operate ost1 "start\ others\ {*.*.*.*@$NETTYPE}\ 50"
+       tbf_rule_operate ost1 "start\ clients\ ${idis}{$client_nids}\ ${rateis}100"
+       tbf_rule_operate ost1 "start\ others\ ${idis}{*.*.*.*@$NETTYPE}\ ${rateis}50"
        nrs_write_read
 
        # Change the rules
-       tbf_rule_operate ost1 "change\ localhost\ 1001"
-       tbf_rule_operate ost1 "change\ clients\ 101"
-       tbf_rule_operate ost1 "change\ others\ 51"
+       tbf_rule_operate ost1 "change\ localhost\ ${rateis}1001"
+       tbf_rule_operate ost1 "change\ clients\ ${rateis}101"
+       tbf_rule_operate ost1 "change\ others\ ${rateis}51"
        nrs_write_read
 
        # Stop the rules
@@ -3118,17 +3125,24 @@ test_77f() {
                        error "failed to set TBF policy"
        done
 
+       local idis
+       local rateis
+       if [ $(lustre_version_code ost1) -ge $(version_code 2.8.54) ]; then
+               idis="jobid="
+               rateis="rate="
+       fi
+
        # Only operate rules on ost1 since OSTs might run on the same OSS
        # Add some rules
-       tbf_rule_operate ost1 "start\ runas\ {iozone.$RUNAS_ID\ dd.$RUNAS_ID\ tiotest.$RUNAS_ID}\ 1000"
-       tbf_rule_operate ost1 "start\ iozone_runas\ {iozone.$RUNAS_ID}\ 100"
-       tbf_rule_operate ost1 "start\ dd_runas\ {dd.$RUNAS_ID}\ 50"
+       tbf_rule_operate ost1 "start\ runas\ ${idis}{iozone.$RUNAS_ID\ dd.$RUNAS_ID\ tiotest.$RUNAS_ID}\ ${rateis}1000"
+       tbf_rule_operate ost1 "start\ iozone_runas\ ${idis}{iozone.$RUNAS_ID}\ ${rateis}100"
+       tbf_rule_operate ost1 "start\ dd_runas\ ${idis}{dd.$RUNAS_ID}\ ${rateis}50"
        nrs_write_read "$RUNAS"
 
        # Change the rules
-       tbf_rule_operate ost1 "change\ runas\ 1001"
-       tbf_rule_operate ost1 "change\ iozone_runas\ 101"
-       tbf_rule_operate ost1 "change\ dd_runas\ 51"
+       tbf_rule_operate ost1 "change\ runas\ ${rateis}1001"
+       tbf_rule_operate ost1 "change\ iozone_runas\ ${rateis}101"
+       tbf_rule_operate ost1 "change\ dd_runas\ ${rateis}51"
        nrs_write_read "$RUNAS"
 
        # Stop the rules
@@ -3174,9 +3188,16 @@ test_77g() {
                        error "failed to set TBF policy"
        done
 
+       local idis
+       local rateis
+       if [ $(lustre_version_code ost1) -ge $(version_code 2.8.54) ]; then
+               idis="jobid="
+               rateis="rate="
+       fi
+
        # Add a rule that only valid for Jobid TBF. If direct change between
        # TBF types is not supported, this operation will fail.
-       tbf_rule_operate ost1 "start\ dd_runas\ {dd.$RUNAS_ID}\ 50"
+       tbf_rule_operate ost1 "start\ dd_runas\ ${idis}{dd.$RUNAS_ID}\ ${rateis}50"
 
        # Cleanup the TBF policy
        for i in $(seq 1 $OSTCOUNT)

diff --git a/lustre/utils/lustre_cfg.c b/lustre/utils/lustre_cfg.c
index f9d4a16..92edd33 100644 (file)
--- a/lustre/utils/lustre_cfg.c
+++ b/lustre/utils/lustre_cfg.c
@@ -1457,15 +1457,6 @@ int jt_lcfg_setparam(int argc, char **argv)
                        continue;
                }
 
-               /* A value containing '=' is indicative of user error, e.g.:
-                *     lctl set_param param1 param2=value2
-                *     lctl set_param param1=param2=value2
-                */
-               if (strchr(value, '=') != NULL)
-                       fprintf(stderr,
-                               "warning: %s: value '%s' contains '='\n",
-                               jt_cmdname(argv[0]), value);
-
                rc2 = param_display(&popt, path, value, SET_PARAM);
                if (rc == 0)
                        rc = rc2;