[ovs-dev] [PATCH 1/6] ovn: add is_chassis_resident match expression component
Mickey Spiegel
mickeys.dev at gmail.com
Fri Dec 23 10:31:23 UTC 2016
This patch introduces a new match expression component
is_chassis_resident(). Unlike match expression comparisons,
is_chassis_resident is not pushed down to OpenFlow. It is a
conditional that is evaluated in the controller during expr_simplify(),
when it is replaced by a boolean expression. The is_chassis_resident
conditional evaluates to "true" when the specified string identifies a
port name that is resident on this controller chassis, i.e., the
corresponding southbound database Port_Binding has a chassis column
that matches this chassis. Otherwise it evaluates to "false".
This allows higher level features to specify flows that are only
installed on some chassis rather than on all chassis with the
corresponding datapath.
Suggested-by: Ben Pfaff <blp at ovn.org>
Signed-off-by: Mickey Spiegel <mickeys.dev at gmail.com>
---
include/ovn/expr.h | 22 +++++-
ovn/controller/lflow.c | 39 ++++++++--
ovn/controller/lflow.h | 5 +-
ovn/controller/ovn-controller.c | 5 +-
ovn/lib/expr.c | 155 ++++++++++++++++++++++++++++++++++++++--
ovn/utilities/ovn-trace.c | 21 +++++-
tests/ovn.at | 14 ++++
tests/test-ovn.c | 15 +++-
8 files changed, 260 insertions(+), 16 deletions(-)
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index 371ba20..d3749fa 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -292,6 +292,15 @@ enum expr_type {
EXPR_T_AND, /* Logical AND of 2 or more subexpressions. */
EXPR_T_OR, /* Logical OR of 2 or more subexpressions. */
EXPR_T_BOOLEAN, /* True or false constant. */
+ EXPR_T_CONDITION, /* Conditional to be evaluated in the
+ * controller during expr_simplify(),
+ * prior to constructing OpenFlow matches. */
+};
+
+/* Expression condition type. */
+enum expr_cond_type {
+ EXPR_COND_CHASSIS_RESIDENT, /* Check if specified logical port name is
+ * resident on the controller chassis. */
};
/* Relational operator. */
@@ -349,6 +358,14 @@ struct expr {
/* EXPR_T_BOOLEAN. */
bool boolean;
+
+ /* EXPR_T_CONDITION. */
+ struct {
+ enum expr_cond_type type;
+ bool not;
+ /* XXX Should arguments for conditions be generic? */
+ char *string;
+ } cond;
};
};
@@ -375,7 +392,10 @@ void expr_destroy(struct expr *);
struct expr *expr_annotate(struct expr *, const struct shash *symtab,
char **errorp);
-struct expr *expr_simplify(struct expr *);
+struct expr *expr_simplify(struct expr *,
+ bool (*is_chassis_resident)(const void *c_aux,
+ const char *port_name),
+ const void *c_aux);
struct expr *expr_normalize(struct expr *);
bool expr_honors_invariants(const struct expr *);
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index d913998..0384c8d 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -64,12 +64,18 @@ struct lookup_port_aux {
const struct sbrec_datapath_binding *dp;
};
+struct condition_aux {
+ const struct lport_index *lports;
+ const struct sbrec_chassis *chassis;
+};
+
static void consider_logical_flow(const struct lport_index *lports,
const struct mcgroup_index *mcgroups,
const struct sbrec_logical_flow *lflow,
const struct hmap *local_datapaths,
struct group_table *group_table,
const struct simap *ct_zones,
+ const struct sbrec_chassis *chassis,
struct hmap *dhcp_opts_p,
struct hmap *dhcpv6_opts_p,
uint32_t *conj_id_ofs_p,
@@ -99,6 +105,20 @@ lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
}
static bool
+is_chassis_resident_cb(const void *c_aux_, const char *port_name)
+{
+ const struct condition_aux *c_aux = c_aux_;
+
+ const struct sbrec_port_binding *pb
+ = lport_lookup_by_name(c_aux->lports, port_name);
+ if (pb && pb->chassis && pb->chassis == c_aux->chassis) {
+ return true;
+ }
+
+ return false;
+}
+
+static bool
is_switch(const struct sbrec_datapath_binding *ldp)
{
return smap_get(&ldp->external_ids, "logical-switch") != NULL;
@@ -112,6 +132,7 @@ add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
const struct hmap *local_datapaths,
struct group_table *group_table,
const struct simap *ct_zones,
+ const struct sbrec_chassis *chassis,
struct hmap *flow_table,
struct shash *expr_address_sets_p)
{
@@ -135,7 +156,7 @@ add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->ovnsb_idl) {
consider_logical_flow(lports, mcgroups, lflow, local_datapaths,
- group_table, ct_zones,
+ group_table, ct_zones, chassis,
&dhcp_opts, &dhcpv6_opts, &conj_id_ofs,
flow_table, expr_address_sets_p);
}
@@ -151,6 +172,7 @@ consider_logical_flow(const struct lport_index *lports,
const struct hmap *local_datapaths,
struct group_table *group_table,
const struct simap *ct_zones,
+ const struct sbrec_chassis *chassis,
struct hmap *dhcp_opts_p,
struct hmap *dhcpv6_opts_p,
uint32_t *conj_id_ofs_p,
@@ -231,6 +253,11 @@ consider_logical_flow(const struct lport_index *lports,
struct hmap matches;
struct expr *expr;
+ struct condition_aux cond_aux = {
+ .lports = lports,
+ .chassis = chassis
+ };
+
expr = expr_parse_string(lflow->match, &symtab,
expr_address_sets_p, &error);
if (!error) {
@@ -250,7 +277,7 @@ consider_logical_flow(const struct lport_index *lports,
return;
}
- expr = expr_simplify(expr);
+ expr = expr_simplify(expr, is_chassis_resident_cb, &cond_aux);
expr = expr_normalize(expr);
uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, &aux,
&matches);
@@ -371,7 +398,9 @@ add_neighbor_flows(struct controller_ctx *ctx,
/* Translates logical flows in the Logical_Flow table in the OVN_SB database
* into OpenFlow flows. See ovn-architecture(7) for more information. */
void
-lflow_run(struct controller_ctx *ctx, const struct lport_index *lports,
+lflow_run(struct controller_ctx *ctx,
+ const struct sbrec_chassis *chassis,
+ const struct lport_index *lports,
const struct mcgroup_index *mcgroups,
const struct hmap *local_datapaths,
struct group_table *group_table,
@@ -381,8 +410,8 @@ lflow_run(struct controller_ctx *ctx, const struct lport_index *lports,
struct shash expr_address_sets = SHASH_INITIALIZER(&expr_address_sets);
update_address_sets(ctx, &expr_address_sets);
- add_logical_flows(ctx, lports, mcgroups, local_datapaths,
- group_table, ct_zones, flow_table, &expr_address_sets);
+ add_logical_flows(ctx, lports, mcgroups, local_datapaths, group_table,
+ ct_zones, chassis, flow_table, &expr_address_sets);
add_neighbor_flows(ctx, lports, flow_table);
expr_macros_destroy(&expr_address_sets);
diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index 6305ce0..2ddf55d 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -40,6 +40,7 @@ struct group_table;
struct hmap;
struct lport_index;
struct mcgroup_index;
+struct sbrec_chassis;
struct simap;
struct uuid;
@@ -61,7 +62,9 @@ struct uuid;
#define LOG_PIPELINE_LEN 16
void lflow_init(void);
-void lflow_run(struct controller_ctx *, const struct lport_index *,
+void lflow_run(struct controller_ctx *,
+ const struct sbrec_chassis *chassis,
+ const struct lport_index *,
const struct mcgroup_index *,
const struct hmap *local_datapaths,
struct group_table *group_table,
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 368ccc9..5bddcf3 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -592,8 +592,9 @@ main(int argc, char *argv[])
commit_ct_zones(br_int, &pending_ct_zones);
struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
- lflow_run(&ctx, &lports, &mcgroups, &local_datapaths,
- &group_table, &ct_zones, &flow_table);
+ lflow_run(&ctx, chassis, &lports, &mcgroups,
+ &local_datapaths, &group_table, &ct_zones,
+ &flow_table);
physical_run(&ctx, mff_ovn_geneve,
br_int, chassis, &ct_zones, &lports,
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 5399985..3034456 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -233,6 +233,11 @@ expr_not(struct expr *expr)
case EXPR_T_BOOLEAN:
expr->boolean = !expr->boolean;
break;
+
+ case EXPR_T_CONDITION:
+ expr->cond.not = !expr->cond.not;
+ break;
+
default:
OVS_NOT_REACHED();
}
@@ -290,6 +295,9 @@ expr_fix(struct expr *expr)
case EXPR_T_BOOLEAN:
return expr;
+ case EXPR_T_CONDITION:
+ return expr;
+
default:
OVS_NOT_REACHED();
}
@@ -394,6 +402,21 @@ expr_format_andor(const struct expr *e, const char *op, struct ds *s)
}
}
+static void
+expr_format_condition(const struct expr *e, struct ds *s)
+{
+ if (e->cond.not) {
+ ds_put_char(s, '!');
+ }
+ switch (e->cond.type) {
+ case EXPR_COND_CHASSIS_RESIDENT:
+ ds_put_format(s, "is_chassis_resident(");
+ json_string_escape(e->cond.string, s);
+ ds_put_char(s, ')');
+ break;
+ }
+}
+
/* Appends a string form of 'e' to 's'. The string form is acceptable for
* parsing back into an equivalent expression. */
void
@@ -415,6 +438,10 @@ expr_format(const struct expr *e, struct ds *s)
case EXPR_T_BOOLEAN:
ds_put_char(s, e->boolean ? '1' : '0');
break;
+
+ case EXPR_T_CONDITION:
+ expr_format_condition(e, s);
+ break;
}
}
@@ -973,6 +1000,32 @@ expr_macros_destroy(struct shash *macros)
}
static struct expr *
+parse_chassis_resident(struct expr_context *ctx)
+{
+ if (!lexer_force_match(ctx->lexer, LEX_T_LPAREN)) {
+ return NULL;
+ }
+ if (ctx->lexer->token.type != LEX_T_STRING) {
+ lexer_syntax_error(ctx->lexer, "expecting string");
+ return NULL;
+ }
+
+ struct expr *e = xzalloc(sizeof *e);
+ e->type = EXPR_T_CONDITION;
+ e->cond.type = EXPR_COND_CHASSIS_RESIDENT;
+ e->cond.not = false;
+ e->cond.string = xstrdup(ctx->lexer->token.s);
+
+ lexer_get(ctx->lexer);
+ if (!lexer_force_match(ctx->lexer, LEX_T_RPAREN)) {
+ expr_destroy(e);
+ return NULL;
+ }
+
+ return e;
+}
+
+static struct expr *
expr_parse_primary(struct expr_context *ctx, bool *atomic)
{
*atomic = false;
@@ -991,6 +1044,11 @@ expr_parse_primary(struct expr_context *ctx, bool *atomic)
enum expr_relop r;
struct expr_constant_set c;
+ if (lexer_match_id(ctx->lexer, "is_chassis_resident")) {
+ *atomic = true;
+ return parse_chassis_resident(ctx);
+ }
+
if (!parse_field(ctx, &f)) {
return NULL;
}
@@ -1383,6 +1441,7 @@ expr_get_level(const struct expr *expr)
return level;
case EXPR_T_BOOLEAN:
+ case EXPR_T_CONDITION:
return EXPR_L_BOOLEAN;
default:
@@ -1465,6 +1524,14 @@ expr_clone_andor(struct expr *expr)
return new;
}
+static struct expr *
+expr_clone_condition(struct expr *expr)
+{
+ struct expr *new = xmemdup(expr, sizeof *expr);
+ new->cond.string = xstrdup(new->cond.string);
+ return new;
+}
+
/* Returns a clone of 'expr'. This is a "deep copy": neither the returned
* expression nor any of its substructure will be shared with 'expr'. */
struct expr *
@@ -1480,6 +1547,9 @@ expr_clone(struct expr *expr)
case EXPR_T_BOOLEAN:
return expr_create_boolean(expr->boolean);
+
+ case EXPR_T_CONDITION:
+ return expr_clone_condition(expr);
}
OVS_NOT_REACHED();
}
@@ -1511,6 +1581,10 @@ expr_destroy(struct expr *expr)
case EXPR_T_BOOLEAN:
break;
+
+ case EXPR_T_CONDITION:
+ free(expr->cond.string);
+ break;
}
free(expr);
}
@@ -1639,6 +1713,7 @@ expr_annotate__(struct expr *expr, const struct shash *symtab,
}
case EXPR_T_BOOLEAN:
+ case EXPR_T_CONDITION:
*errorp = NULL;
return expr;
@@ -1781,10 +1856,42 @@ expr_simplify_relational(struct expr *expr)
return new ? new : expr_create_boolean(false);
}
+/* Resolves condition and replaces the expression with a boolean. */
+static struct expr *
+expr_simplify_condition(struct expr *expr,
+ bool (*is_chassis_resident)(const void *c_aux,
+ const char *port_name),
+ const void *c_aux)
+{
+ bool cond_initial_result;
+ switch (expr->cond.type) {
+ case EXPR_COND_CHASSIS_RESIDENT:
+ if (!is_chassis_resident || !c_aux) {
+ /* For test cases where chassis is not considered. */
+ cond_initial_result = true;
+ } else {
+ cond_initial_result =
+ is_chassis_resident(c_aux, expr->cond.string);
+ }
+ break;
+
+ default:
+ OVS_NOT_REACHED();
+ }
+ if (expr->cond.not) {
+ cond_initial_result = !cond_initial_result;
+ }
+ expr_destroy(expr);
+ return expr_create_boolean(cond_initial_result);
+}
+
/* Takes ownership of 'expr' and returns an equivalent expression whose
* EXPR_T_CMP nodes use only tests for equality (EXPR_R_EQ). */
struct expr *
-expr_simplify(struct expr *expr)
+expr_simplify(struct expr *expr,
+ bool (*is_chassis_resident)(const void *c_aux,
+ const char *port_name),
+ const void *c_aux)
{
struct expr *sub, *next;
@@ -1799,12 +1906,16 @@ expr_simplify(struct expr *expr)
case EXPR_T_OR:
LIST_FOR_EACH_SAFE (sub, next, node, &expr->andor) {
ovs_list_remove(&sub->node);
- expr_insert_andor(expr, next, expr_simplify(sub));
+ expr_insert_andor(expr, next,
+ expr_simplify(sub, is_chassis_resident, c_aux));
}
return expr_fix(expr);
case EXPR_T_BOOLEAN:
return expr;
+
+ case EXPR_T_CONDITION:
+ return expr_simplify_condition(expr, is_chassis_resident, c_aux);
}
OVS_NOT_REACHED();
}
@@ -1832,6 +1943,7 @@ expr_is_cmp(const struct expr *expr)
}
case EXPR_T_BOOLEAN:
+ case EXPR_T_CONDITION:
return NULL;
default:
@@ -1928,6 +2040,8 @@ crush_and_string(struct expr *expr, const struct expr_symbol *symbol)
}
free(new);
break;
+ case EXPR_T_CONDITION:
+ OVS_NOT_REACHED();
}
}
@@ -2022,6 +2136,8 @@ crush_and_numeric(struct expr *expr, const struct expr_symbol *symbol)
}
expr_destroy(new);
break;
+ case EXPR_T_CONDITION:
+ OVS_NOT_REACHED();
}
}
if (ovs_list_is_empty(&expr->andor)) {
@@ -2234,6 +2350,10 @@ crush_cmps(struct expr *expr, const struct expr_symbol *symbol)
case EXPR_T_BOOLEAN:
return expr;
+ /* Should not hit expression type condition, since crush_cmps is only
+ * called during expr_normalize, after expr_simplify which resolves
+ * all conditions. */
+ case EXPR_T_CONDITION:
default:
OVS_NOT_REACHED();
}
@@ -2441,8 +2561,13 @@ expr_normalize(struct expr *expr)
case EXPR_T_BOOLEAN:
return expr;
+
+ /* Should not hit expression type condition, since expr_normalize is
+ * only called after expr_simplify, which resolves all conditions. */
+ case EXPR_T_CONDITION:
+ default:
+ OVS_NOT_REACHED();
}
- OVS_NOT_REACHED();
}
/* Creates, initializes, and returns a new 'struct expr_match'. If 'm' is
@@ -2609,6 +2734,8 @@ add_conjunction(const struct expr *and,
break;
case EXPR_T_AND:
case EXPR_T_BOOLEAN:
+ case EXPR_T_CONDITION:
+ default:
OVS_NOT_REACHED();
}
}
@@ -2740,6 +2867,12 @@ expr_to_matches(const struct expr *expr,
/* No match. */
}
break;
+
+ /* Should not hit expression type condition, since expr_to_matches is
+ * only called after expr_simplify, which resolves all conditions. */
+ case EXPR_T_CONDITION:
+ default:
+ OVS_NOT_REACHED();
}
return n_conjs;
}
@@ -2816,6 +2949,7 @@ expr_honors_invariants(const struct expr *expr)
return true;
case EXPR_T_BOOLEAN:
+ case EXPR_T_CONDITION:
return true;
default:
@@ -2864,6 +2998,9 @@ expr_is_normalized(const struct expr *expr)
case EXPR_T_BOOLEAN:
return true;
+ case EXPR_T_CONDITION:
+ return false;
+
default:
OVS_NOT_REACHED();
}
@@ -2956,6 +3093,11 @@ expr_evaluate(const struct expr *e, const struct flow *uflow,
case EXPR_T_BOOLEAN:
return e->boolean;
+ case EXPR_T_CONDITION:
+ /* Assume tests calling expr_evaluate are not chassis specific, so
+ * is_chassis_resident evaluates as true. */
+ return (e->cond.not ? false : true);
+
default:
OVS_NOT_REACHED();
}
@@ -3029,7 +3171,9 @@ expr_parse_microflow__(struct lexer *lexer,
struct ds annotated = DS_EMPTY_INITIALIZER;
expr_format(e, &annotated);
- e = expr_simplify(e);
+ /* Assume tests calling expr_parse_microflow are not chassis specific, so
+ * is_chassis_resident need not be supplied and should return true. */
+ e = expr_simplify(e, NULL, NULL);
e = expr_normalize(e);
struct match m = MATCH_CATCHALL_INITIALIZER;
@@ -3065,6 +3209,9 @@ expr_parse_microflow__(struct lexer *lexer,
}
break;
+ /* Should not hit expression type condition, since
+ * expr_simplify was called above. */
+ case EXPR_T_CONDITION:
default:
OVS_NOT_REACHED();
}
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 6d8e514..b5100ba 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -563,6 +563,25 @@ read_address_sets(void)
}
}
+static bool
+ovntrace_is_chassis_resident(const void *ports_ OVS_UNUSED,
+ const char *port_name)
+{
+ if (port_name[0] == '\0') {
+ return true;
+ }
+
+ const struct ovntrace_port *port = shash_find_data(&ports, port_name);
+ if (port) {
+ /* Since ovntrace is not chassis specific, assume any port
+ * that exists is resident. */
+ return true;
+ }
+
+ VLOG_WARN("%s: unknown logical port\n", port_name);
+ return false;
+}
+
static int
compare_flow(const void *a_, const void *b_)
{
@@ -640,7 +659,7 @@ read_flows(void)
continue;
}
if (match) {
- match = expr_simplify(match);
+ match = expr_simplify(match, ovntrace_is_chassis_resident, &ports);
}
struct ovntrace_flow *flow = xzalloc(sizeof *flow);
diff --git a/tests/ovn.at b/tests/ovn.at
index 557b2ca..55b7e9a 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -453,6 +453,20 @@ AT_CHECK([simplify 'tcp.dst < 65535'], [0],
]])
AT_CLEANUP
+AT_SETUP([ovn -- is_chassis_resident simplification])
+simplify() {
+ echo "$1" | ovstest test-ovn simplify-expr
+}
+AT_CHECK([simplify 'is_chassis_resident("eth1")'], [0], [1
+])
+AT_CHECK([simplify 'is_chassis_resident("eth2")'], [0], [0
+])
+AT_CHECK([simplify '!is_chassis_resident("eth1")'], [0], [0
+])
+AT_CHECK([simplify '!is_chassis_resident("eth2")'], [0], [1
+])
+AT_CLEANUP
+
AT_SETUP([ovn -- 4-term numeric expression normalization])
AT_CHECK([ovstest test-ovn exhaustive --operation=normalize --nvars=3 --svars=0 --bits=1 4], [0],
[Tested normalizing 1874026 expressions of 4 terminals with 3 numeric vars (each 1 bits) in terms of operators == != < <= > >=.
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index 2e82a6f..3d80954 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -219,6 +219,17 @@ lookup_port_cb(const void *ports_, const char *port_name, unsigned int *portp)
return true;
}
+static bool
+is_chassis_resident_cb(const void *ports_, const char *port_name)
+{
+ const struct simap *ports = ports_;
+ const struct simap_node *node = simap_find(ports, port_name);
+ if (node) {
+ return true;
+ }
+ return false;
+}
+
static void
test_parse_expr__(int steps)
{
@@ -246,7 +257,7 @@ test_parse_expr__(int steps)
}
if (!error) {
if (steps > 1) {
- expr = expr_simplify(expr);
+ expr = expr_simplify(expr, is_chassis_resident_cb, &ports);
}
if (steps > 2) {
expr = expr_normalize(expr);
@@ -837,7 +848,7 @@ test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab,
exit(EXIT_FAILURE);
}
} else if (operation >= OP_SIMPLIFY) {
- modified = expr_simplify(expr_clone(expr));
+ modified = expr_simplify(expr_clone(expr), NULL, NULL);
ovs_assert(expr_honors_invariants(modified));
if (operation >= OP_NORMALIZE) {
--
1.9.1
More information about the dev
mailing list