Signed-off-by: Ben Pfaff <b...@nicira.com> --- ovn/expr.c | 115 ++++++++++++++++++++++++++++++++++++++++++------------- ovn/expr.h | 6 ++- tests/test-ovn.c | 6 +-- 3 files changed, 95 insertions(+), 32 deletions(-)
diff --git a/ovn/expr.c b/ovn/expr.c index cd6f729..83fd866 100644 --- a/ovn/expr.c +++ b/ovn/expr.c @@ -21,6 +21,7 @@ #include "lex.h" #include "match.h" #include "shash.h" +#include "simap.h" #include "openvswitch/vlog.h" VLOG_DEFINE_THIS_MODULE(expr); @@ -1136,9 +1137,14 @@ expr_symtab_add_subfield(struct shash *symtab, const char *name, * 'prereqs'. */ struct expr_symbol * expr_symtab_add_string(struct shash *symtab, const char *name, - const char *prereqs) + enum mf_field_id id, const char *prereqs) { - return add_symbol(symtab, name, 0, prereqs, EXPR_L_NOMINAL, false); + const struct mf_field *field = mf_from_id(id); + struct expr_symbol *symbol; + + symbol = add_symbol(symtab, name, 0, prereqs, EXPR_L_NOMINAL, false); + symbol->field = field; + return symbol; } static enum expr_level @@ -2101,33 +2107,59 @@ expr_match_add(struct hmap *matches, struct expr_match *match) hmap_insert(matches, &match->hmap_node, hash); } -static void -constrain_match(const struct expr *expr, struct match *m) +static bool +constrain_match(const struct expr *expr, const struct simap *ports, + struct match *m) { ovs_assert(expr->type == EXPR_T_CMP); - ovs_assert(expr->cmp.symbol->width); - mf_mask_subfield(expr->cmp.symbol->field, &expr->cmp.value, - &expr->cmp.mask, m); + if (expr->cmp.symbol->width) { + mf_mask_subfield(expr->cmp.symbol->field, &expr->cmp.value, + &expr->cmp.mask, m); + } else { + const struct simap_node *node; + node = ports ? simap_find(ports, expr->cmp.string) : NULL; + if (!node) { + return false; + } + + struct mf_subfield sf; + sf.field = expr->cmp.symbol->field; + sf.ofs = 0; + sf.n_bits = expr->cmp.symbol->field->n_bits; + + union mf_subvalue x; + memset(&x, 0, sizeof x); + x.integer = htonll(node->data); + + mf_write_subfield(&sf, &x, m); + } + return true; } -static void -add_disjunction(const struct expr *or, struct match *m, uint8_t clause, - uint8_t n_clauses, uint32_t conj_id, struct hmap *matches) +static bool +add_disjunction(const struct expr *or, const struct simap *ports, + struct match *m, uint8_t clause, uint8_t n_clauses, + uint32_t conj_id, struct hmap *matches) { struct expr *sub; + int n = 0; ovs_assert(or->type == EXPR_T_OR); LIST_FOR_EACH (sub, node, &or->andor) { struct expr_match *match = expr_match_new(m, clause, n_clauses, conj_id); - constrain_match(sub, &match->match); + n += constrain_match(sub, ports, &match->match); expr_match_add(matches, match); } + + /* If n == 1, then this didn't really need to be a disjunction. Oh well, + * that shouldn't happen much. */ + return n > 0; } static void -add_conjunction(const struct expr *and, uint32_t *n_conjsp, - struct hmap *matches) +add_conjunction(const struct expr *and, const struct simap *ports, + uint32_t *n_conjsp, struct hmap *matches) { struct match match; int n_clauses = 0; @@ -2139,7 +2171,9 @@ add_conjunction(const struct expr *and, uint32_t *n_conjsp, LIST_FOR_EACH (sub, node, &and->andor) { switch (sub->type) { case EXPR_T_CMP: - constrain_match(sub, &match); + if (!constrain_match(sub, ports, &match)) { + return; + } break; case EXPR_T_OR: n_clauses++; @@ -2155,7 +2189,7 @@ add_conjunction(const struct expr *and, uint32_t *n_conjsp, } else if (n_clauses == 1) { LIST_FOR_EACH (sub, node, &and->andor) { if (sub->type == EXPR_T_OR) { - add_disjunction(sub, &match, 0, 0, 0, matches); + add_disjunction(sub, ports, &match, 0, 0, 0, matches); } } } else { @@ -2163,38 +2197,65 @@ add_conjunction(const struct expr *and, uint32_t *n_conjsp, (*n_conjsp)++; LIST_FOR_EACH (sub, node, &and->andor) { if (sub->type == EXPR_T_OR) { - add_disjunction(sub, &match, clause++, n_clauses, *n_conjsp, - matches); + if (!add_disjunction(sub, ports, &match, clause++, + n_clauses, *n_conjsp, matches)) { + /* This clause can't ever match, so we might as well skip + * adding the other clauses--the overall disjunctive flow + * can't ever match. Ideally we would also back out all of + * the clauses we already added, but that seems like a lot + * of trouble for a case that might never occur in + * practice. */ + return; + } } } } } static void -add_cmp_flow(const struct expr *cmp, struct hmap *matches) +add_cmp_flow(const struct expr *cmp, const struct simap *ports, + struct hmap *matches) { struct expr_match *m = expr_match_new(NULL, 0, 0, 0); match_init_catchall(&m->match); - constrain_match(cmp, &m->match); - expr_match_add(matches, m); + if (constrain_match(cmp, ports, &m->match)) { + expr_match_add(matches, m); + } else { + free(m); + } } /* Converts 'expr', which must be in the form returned by expr_normalize(), to * a collection of Open vSwitch flows in 'matches', which this function - * initializes to an hmap of "struct expr_match" structures. */ + * initializes to an hmap of "struct expr_match" structures. Returns the + * number of conjunctive match IDs consumed by 'matches', which uses + * conjunctive match IDs beginning with 0; the caller must offset or remap them + * into the desired range as necessary. + * + * 'ports' must be a map from strings (presumably names of ports) to integers. + * Any comparisons against string fields in 'expr' are translated into integers + * through this map. A comparison against a string that is not in 'ports' acts + * like a Boolean "false"; that is, it will always fail to match. For a simple + * expression, this means that the overall expression always fails to match, + * but an expression with a disjunction on the string field might still match + * on other port names. + * + * (This treatment of string fields might be too simplistic in general, but it + * seems reasonable for now when string fields are used only for ports.) */ uint32_t -expr_to_matches(const struct expr *expr, struct hmap *matches) +expr_to_matches(const struct expr *expr, const struct simap *ports, + struct hmap *matches) { uint32_t n_conjs = 0; hmap_init(matches); switch (expr->type) { case EXPR_T_CMP: - add_cmp_flow(expr, matches); + add_cmp_flow(expr, ports, matches); break; case EXPR_T_AND: - add_conjunction(expr, &n_conjs, matches); + add_conjunction(expr, ports, &n_conjs, matches); break; case EXPR_T_OR: @@ -2202,16 +2263,16 @@ expr_to_matches(const struct expr *expr, struct hmap *matches) struct expr *sub; LIST_FOR_EACH (sub, node, &expr->andor) { - add_cmp_flow(sub, matches); + add_cmp_flow(sub, ports, matches); } } else { struct expr *sub; LIST_FOR_EACH (sub, node, &expr->andor) { if (sub->type == EXPR_T_AND) { - add_conjunction(sub, &n_conjs, matches); + add_conjunction(sub, ports, &n_conjs, matches); } else { - add_cmp_flow(sub, matches); + add_cmp_flow(sub, ports, matches); } } } diff --git a/ovn/expr.h b/ovn/expr.h index ab13c1b..f42d752 100644 --- a/ovn/expr.h +++ b/ovn/expr.h @@ -61,6 +61,7 @@ struct ds; struct shash; +struct simap; /* "Measurement level" of a field. See "Level of Measurement" in the large * comment on struct expr_symbol below for more information. */ @@ -255,7 +256,7 @@ struct expr_symbol *expr_symtab_add_subfield(struct shash *symtab, const char *prereqs, const char *subfield); struct expr_symbol *expr_symtab_add_string(struct shash *symtab, - const char *name, + const char *name, enum mf_field_id, const char *prereqs); struct expr_symbol *expr_symtab_add_predicate(struct shash *symtab, const char *name, @@ -362,6 +363,7 @@ struct expr_match { size_t n, allocated; }; -uint32_t expr_to_matches(const struct expr *, struct hmap *); +uint32_t expr_to_matches(const struct expr *, const struct simap *ports, + struct hmap *matches); #endif /* ovn/expr.h */ diff --git a/tests/test-ovn.c b/tests/test-ovn.c index aef4e45..8048e08 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -130,8 +130,8 @@ create_symtab(struct shash *symtab) { shash_init(symtab); - expr_symtab_add_string(symtab, "inport", NULL); - expr_symtab_add_string(symtab, "outport", NULL); + expr_symtab_add_string(symtab, "inport", MFF_IN_PORT, NULL); + expr_symtab_add_string(symtab, "outport", MFF_ACTSET_OUTPUT, NULL); expr_symtab_add_field(symtab, "xreg0", MFF_XREG0, NULL, false); expr_symtab_add_field(symtab, "xreg1", MFF_XREG1, NULL, false); @@ -858,7 +858,7 @@ test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab, struct test_rule *test_rule; uint32_t n_conjs; - n_conjs = expr_to_matches(modified, &matches); + n_conjs = expr_to_matches(modified, NULL, &matches); classifier_init(&cls, NULL); HMAP_FOR_EACH (m, hmap_node, &matches) { -- 2.1.3 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev