From 175d213bf833624c501188ca8f2dd7fe4bf599b3 Mon Sep 17 00:00:00 2001 From: Cristian Dumitrescu Date: Tue, 14 Sep 2021 20:00:05 +0100 Subject: [PATCH] pipeline: improve handling of learner action arguments 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 --- examples/pipeline/examples/learner.spec | 6 +-- lib/pipeline/rte_swx_pipeline.c | 55 +++++++++--------------- lib/pipeline/rte_swx_pipeline.h | 10 ----- lib/pipeline/rte_swx_pipeline_internal.h | 8 ++-- lib/pipeline/rte_swx_pipeline_spec.c | 35 ++------------- 5 files changed, 31 insertions(+), 83 deletions(-) diff --git a/examples/pipeline/examples/learner.spec b/examples/pipeline/examples/learner.spec index d635422282..4ee52da7ac 100644 --- a/examples/pipeline/examples/learner.spec +++ b/examples/pipeline/examples/learner.spec @@ -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 diff --git a/lib/pipeline/rte_swx_pipeline.c b/lib/pipeline/rte_swx_pipeline.c index 31f0029404..1cd09a4b44 100644 --- a/lib/pipeline/rte_swx_pipeline.c +++ b/lib/pipeline/rte_swx_pipeline.c @@ -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); } diff --git a/lib/pipeline/rte_swx_pipeline.h b/lib/pipeline/rte_swx_pipeline.h index 2f18a820b9..490ff60c0d 100644 --- a/lib/pipeline/rte_swx_pipeline.h +++ b/lib/pipeline/rte_swx_pipeline.h @@ -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*. */ diff --git a/lib/pipeline/rte_swx_pipeline_internal.h b/lib/pipeline/rte_swx_pipeline_internal.h index 3baae55737..4361c535a0 100644 --- a/lib/pipeline/rte_swx_pipeline_internal.h +++ b/lib/pipeline/rte_swx_pipeline_internal.h @@ -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, diff --git a/lib/pipeline/rte_swx_pipeline_spec.c b/lib/pipeline/rte_swx_pipeline_spec.c index d9cd1d0595..5c21a7ae4d 100644 --- a/lib/pipeline/rte_swx_pipeline_spec.c +++ b/lib/pipeline/rte_swx_pipeline_spec.c @@ -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; -- 2.20.1