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

Reply via email to