pipeline: improve handling of learner action arguments
authorCristian Dumitrescu <cristian.dumitrescu@intel.com>
Tue, 14 Sep 2021 19:00:05 +0000 (20:00 +0100)
committerThomas Monjalon <thomas@monjalon.net>
Mon, 27 Sep 2021 10:18:49 +0000 (12:18 +0200)
The arguments of actions that are learned are now specified as part of
the learn instruction as opposed to being statically specified as part
of the learner table configuration.

Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
examples/pipeline/examples/learner.spec
lib/pipeline/rte_swx_pipeline.c
lib/pipeline/rte_swx_pipeline.h
lib/pipeline/rte_swx_pipeline_internal.h
lib/pipeline/rte_swx_pipeline_spec.c

index d635422..4ee52da 100644 (file)
@@ -84,7 +84,7 @@ action learn_action args none {
        // Add the current lookup key to the table with fwd_action as the key action. The action
        // arguments are read from the packet meta-data (the m.fwd_action_arg_port_out field). These
        // packet meta-data fields have to be written before the "learn" instruction is invoked.
-       learn fwd_action
+       learn fwd_action m.fwd_action_arg_port_out
 
        // Send the current packet to the same output port.
        mov m.port_out m.fwd_action_arg_port_out
@@ -101,9 +101,9 @@ learner fwd_table {
        }
 
        actions {
-               fwd_action args m.fwd_action_arg_port_out
+               fwd_action
 
-               learn_action args none
+               learn_action
        }
 
        default_action learn_action args none
index 31f0029..1cd09a4 100644 (file)
@@ -2359,6 +2359,9 @@ action_find(struct rte_swx_pipeline *p, const char *name);
 static int
 action_has_nbo_args(struct action *a);
 
+static int
+learner_action_args_check(struct rte_swx_pipeline *p, struct action *a, const char *mf_name);
+
 static int
 instr_learn_translate(struct rte_swx_pipeline *p,
                      struct action *action,
@@ -2368,16 +2371,31 @@ instr_learn_translate(struct rte_swx_pipeline *p,
                      struct instruction_data *data __rte_unused)
 {
        struct action *a;
+       const char *mf_name;
+       uint32_t mf_offset = 0;
 
        CHECK(action, EINVAL);
-       CHECK(n_tokens == 2, EINVAL);
+       CHECK((n_tokens == 2) || (n_tokens == 3), EINVAL);
 
        a = action_find(p, tokens[1]);
        CHECK(a, EINVAL);
        CHECK(!action_has_nbo_args(a), EINVAL);
 
+       mf_name = (n_tokens > 2) ? tokens[2] : NULL;
+       CHECK(!learner_action_args_check(p, a, mf_name), EINVAL);
+
+       if (mf_name) {
+               struct field *mf;
+
+               mf = metadata_field_parse(p, mf_name);
+               CHECK(mf, EINVAL);
+
+               mf_offset = mf->offset / 8;
+       }
+
        instr->type = INSTR_LEARNER_LEARN;
        instr->learn.action_id = a->id;
+       instr->learn.mf_offset = mf_offset;
 
        return 0;
 }
@@ -8165,7 +8183,6 @@ rte_swx_pipeline_learner_config(struct rte_swx_pipeline *p,
        CHECK(params->action_names, EINVAL);
        for (i = 0; i < params->n_actions; i++) {
                const char *action_name = params->action_names[i];
-               const char *action_field_name = params->action_field_names[i];
                struct action *a;
                uint32_t action_data_size;
 
@@ -8174,10 +8191,6 @@ rte_swx_pipeline_learner_config(struct rte_swx_pipeline *p,
                a = action_find(p, action_name);
                CHECK(a, EINVAL);
 
-               status = learner_action_args_check(p, a, action_field_name);
-               if (status)
-                       return status;
-
                status = learner_action_learning_check(p,
                                                       a,
                                                       params->action_names,
@@ -8218,10 +8231,6 @@ rte_swx_pipeline_learner_config(struct rte_swx_pipeline *p,
        if (!l->actions)
                goto nomem;
 
-       l->action_arg = calloc(params->n_actions, sizeof(struct field *));
-       if (!l->action_arg)
-               goto nomem;
-
        if (action_data_size_max) {
                l->default_action_data = calloc(1, action_data_size_max);
                if (!l->default_action_data)
@@ -8243,14 +8252,9 @@ rte_swx_pipeline_learner_config(struct rte_swx_pipeline *p,
 
        l->header = header;
 
-       for (i = 0; i < params->n_actions; i++) {
-               const char *mf_name = params->action_field_names[i];
-
+       for (i = 0; i < params->n_actions; i++)
                l->actions[i] = action_find(p, params->action_names[i]);
 
-               l->action_arg[i] = mf_name ? metadata_field_parse(p, mf_name) : NULL;
-       }
-
        l->default_action = default_action;
 
        if (default_action->st)
@@ -8280,7 +8284,6 @@ nomem:
        if (!l)
                return -ENOMEM;
 
-       free(l->action_arg);
        free(l->actions);
        free(l->fields);
        free(l);
@@ -8375,7 +8378,6 @@ learner_build_free(struct rte_swx_pipeline *p)
                        struct learner_runtime *r = &t->learners[j];
 
                        free(r->mailbox);
-                       free(r->action_data);
                }
 
                free(t->learners);
@@ -8419,7 +8421,6 @@ learner_build(struct rte_swx_pipeline *p)
                TAILQ_FOREACH(l, &p->learners, node) {
                        struct learner_runtime *r = &t->learners[l->id];
                        uint64_t size;
-                       uint32_t j;
 
                        /* r->mailbox. */
                        size = rte_swx_table_learner_mailbox_size_get();
@@ -8435,21 +8436,6 @@ learner_build(struct rte_swx_pipeline *p)
                        r->key = l->header ?
                                &t->structs[l->header->struct_id] :
                                &t->structs[p->metadata_struct_id];
-
-                       /* r->action_data. */
-                       r->action_data = calloc(p->n_actions, sizeof(uint8_t *));
-                       if (!r->action_data) {
-                               status = -ENOMEM;
-                               goto error;
-                       }
-
-                       for (j = 0; j < l->n_actions; j++) {
-                               struct action *a = l->actions[j];
-                               struct field *mf = l->action_arg[j];
-                               uint8_t *m = t->structs[p->metadata_struct_id];
-
-                               r->action_data[a->id] = mf ? &m[mf->offset / 8] : NULL;
-                       }
                }
        }
 
@@ -8476,7 +8462,6 @@ learner_free(struct rte_swx_pipeline *p)
                TAILQ_REMOVE(&p->learners, l, node);
                free(l->fields);
                free(l->actions);
-               free(l->action_arg);
                free(l->default_action_data);
                free(l);
        }
index 2f18a82..490ff60 100644 (file)
@@ -696,16 +696,6 @@ struct rte_swx_pipeline_learner_params {
         */
        uint32_t n_actions;
 
-       /** This table type allows adding the latest lookup key (typically done
-        * only in the case of lookup miss) to the table with a given action.
-        * The action arguments are picked up from the packet meta-data: for
-        * each action, a set of successive meta-data fields (with the name of
-        * the first such field provided here) is 1:1 mapped to the action
-        * arguments. These meta-data fields must be set with the actual values
-        * of the action arguments before the key add operation.
-        */
-       const char **action_field_names;
-
        /** The default table action that gets executed on lookup miss. Must be
         * one of the table actions included in the *action_names*.
         */
index 3baae55..4361c53 100644 (file)
@@ -448,7 +448,7 @@ enum instruction_type {
        INSTR_LEARNER,
        INSTR_LEARNER_AF,
 
-       /* learn LEARNER ACTION_NAME */
+       /* learn LEARNER ACTION_NAME [ m.action_first_arg ] */
        INSTR_LEARNER_LEARN,
 
        /* forget */
@@ -583,6 +583,7 @@ struct instr_table {
 
 struct instr_learn {
        uint8_t action_id;
+       uint8_t mf_offset;
 };
 
 struct instr_extern_obj {
@@ -809,7 +810,6 @@ struct learner {
 
        /* Action. */
        struct action **actions;
-       struct field **action_arg;
        struct action *default_action;
        uint8_t *default_action_data;
        uint32_t n_actions;
@@ -826,7 +826,6 @@ TAILQ_HEAD(learner_tailq, learner);
 struct learner_runtime {
        void *mailbox;
        uint8_t **key;
-       uint8_t **action_data;
 };
 
 struct learner_statistics {
@@ -2017,6 +2016,7 @@ __instr_learn_exec(struct rte_swx_pipeline *p,
                   const struct instruction *ip)
 {
        uint64_t action_id = ip->learn.action_id;
+       uint32_t mf_offset = ip->learn.mf_offset;
        uint32_t learner_id = t->learner_id;
        struct rte_swx_table_state *ts = &t->table_state[p->n_tables +
                p->n_selectors + learner_id];
@@ -2029,7 +2029,7 @@ __instr_learn_exec(struct rte_swx_pipeline *p,
                                           l->mailbox,
                                           t->time,
                                           action_id,
-                                          l->action_data[action_id]);
+                                          &t->metadata[mf_offset]);
 
        TRACE("[Thread %2u] learner %u learn %s\n",
              p->thread_id,
index d9cd1d0..5c21a7a 100644 (file)
@@ -1293,7 +1293,7 @@ selector_block_parse(struct selector_spec *s,
  *             ...
  *     }
  *     actions {
- *             ACTION_NAME args METADATA_FIELD_NAME
+ *             ACTION_NAME
  *             ...
  *     }
  *     default_action ACTION_NAME args none | ARGS_BYTE_ARRAY [ const ]
@@ -1340,15 +1340,6 @@ learner_spec_free(struct learner_spec *s)
        free(s->params.action_names);
        s->params.action_names = NULL;
 
-       for (i = 0; i < s->params.n_actions; i++) {
-               uintptr_t name = (uintptr_t)s->params.action_field_names[i];
-
-               free((void *)name);
-       }
-
-       free(s->params.action_field_names);
-       s->params.action_field_names = NULL;
-
        s->params.n_actions = 0;
 
        default_action_name = (uintptr_t)s->params.default_action_name;
@@ -1468,9 +1459,7 @@ learner_actions_block_parse(struct learner_spec *s,
                            const char **err_msg)
 {
        const char **new_action_names = NULL;
-       const char **new_action_field_names = NULL;
-       char *action_name = NULL, *action_field_name = NULL;
-       int has_args = 1;
+       char *action_name = NULL;
 
        /* Handle end of block. */
        if ((n_tokens == 1) && !strcmp(tokens[0], "}")) {
@@ -1479,7 +1468,7 @@ learner_actions_block_parse(struct learner_spec *s,
        }
 
        /* Check input arguments. */
-       if ((n_tokens != 3) || strcmp(tokens[1], "args")) {
+       if (n_tokens != 1) {
                if (err_line)
                        *err_line = n_lines;
                if (err_msg)
@@ -1487,28 +1476,14 @@ learner_actions_block_parse(struct learner_spec *s,
                return -EINVAL;
        }
 
-       if (!strcmp(tokens[2], "none"))
-               has_args = 0;
-
        action_name = strdup(tokens[0]);
 
-       if (has_args)
-               action_field_name = strdup(tokens[2]);
-
        new_action_names = realloc(s->params.action_names,
                                   (s->params.n_actions + 1) * sizeof(char *));
 
-       new_action_field_names = realloc(s->params.action_field_names,
-                                        (s->params.n_actions + 1) * sizeof(char *));
-
-       if (!action_name ||
-           (has_args && !action_field_name) ||
-           !new_action_names ||
-           !new_action_field_names) {
+       if (!action_name || !new_action_names) {
                free(action_name);
-               free(action_field_name);
                free(new_action_names);
-               free(new_action_field_names);
 
                if (err_line)
                        *err_line = n_lines;
@@ -1519,8 +1494,6 @@ learner_actions_block_parse(struct learner_spec *s,
 
        s->params.action_names = new_action_names;
        s->params.action_names[s->params.n_actions] = action_name;
-       s->params.action_field_names = new_action_field_names;
-       s->params.action_field_names[s->params.n_actions] = action_field_name;
        s->params.n_actions++;
 
        return 0;