[ovs-dev] [PATCH v7 2/7] ovn: add is_chassis_resident match expression component
Ben Pfaff
blp at ovn.org
Fri Jan 6 23:14:00 UTC 2017
On Fri, Jan 06, 2017 at 12:00:29PM -0800, Mickey Spiegel wrote:
> 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>
Here's an incremental I suggest folding in. Some of the differences are
style, but I also added documentation and changed the details of the
expr_simplify_condition() interface.
I suggest calling these "functions" instead of "conditions", but I
didn't make that change, except in the documentation.
One change I made is that is_chassis_resident is considered to be a
function name only if it is followed by a left parenthesis. In fact, I
think that this is a good marker of a function invocation in any case,
so I generalized it so that any identifier followed by a left
parenthesis has to be a function invocation otherwise a
function-specific syntax error is given.
Acked-by: Ben Pfaff <blp at ovn.org>
--8<--------------------------cut here-------------------------->8--
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index c0655c3..3d7633e 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -97,11 +97,7 @@ is_chassis_resident_cb(const void *c_aux_, const char *port_name)
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;
+ return pb && pb->chassis && pb->chassis == c_aux->chassis;
}
static bool
@@ -239,11 +235,6 @@ 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, addr_sets, &error);
if (!error) {
if (prereqs) {
@@ -262,6 +253,7 @@ consider_logical_flow(const struct lport_index *lports,
return;
}
+ struct condition_aux cond_aux = { lports, chassis };
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,
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 95430e6..d57fac2 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2015, 2016, 2017 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -1002,9 +1002,6 @@ expr_addr_sets_destroy(struct shash *addr_sets)
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;
@@ -1044,9 +1041,14 @@ 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 (lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) {
+ if (lexer_match_id(ctx->lexer, "is_chassis_resident")) {
+ lexer_get(ctx->lexer); /* Skip "(". */
+ *atomic = true;
+ return parse_chassis_resident(ctx);
+ }
+ lexer_error(ctx->lexer, "parsing function name");
+ return NULL;
}
if (!parse_field(ctx, &f)) {
@@ -1865,26 +1867,20 @@ expr_simplify_condition(struct expr *expr,
const char *port_name),
const void *c_aux)
{
- bool cond_initial_result;
+ bool 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);
- }
+ result = is_chassis_resident(c_aux, expr->cond.string);
break;
default:
OVS_NOT_REACHED();
}
- if (expr->cond.not) {
- cond_initial_result = !cond_initial_result;
- }
+
+ result ^= expr->cond.not;
expr_destroy(expr);
- return expr_create_boolean(cond_initial_result);
+ return expr_create_boolean(result);
}
/* Takes ownership of 'expr' and returns an equivalent expression whose
@@ -3153,6 +3149,15 @@ expr_resolve_field(const struct expr_field *f)
return (struct mf_subfield) { symbol->field, ofs, n_bits };
}
+static bool
+microflow_is_chassis_resident_cb(const void *c_aux OVS_UNUSED,
+ const char *port_name OVS_UNUSED)
+{
+ /* Assume tests calling expr_parse_microflow are not chassis specific, so
+ * is_chassis_resident need not be supplied and should return true. */
+ return true;
+}
+
static struct expr *
expr_parse_microflow__(struct lexer *lexer,
const struct shash *symtab,
@@ -3173,9 +3178,7 @@ expr_parse_microflow__(struct lexer *lexer,
struct ds annotated = DS_EMPTY_INITIALIZER;
expr_format(e, &annotated);
- /* 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_simplify(e, microflow_is_chassis_resident_cb, NULL);
e = expr_normalize(e);
struct match m = MATCH_CATCHALL_INITIALIZER;
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 2f35079..f78f040 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -534,6 +534,20 @@
packet.
</p>
+ <p>
+ Match expressions also support a kind of function syntax. The
+ following functions are supported:
+ </p>
+
+ <dl>
+ <dt><code>is_chassis_resident(<var>lport</var>)</code></dt>
+ <dd>
+ Evaluates to true on a chassis on which logical port <var>lport</var>
+ (a quoted string) resides, and to false elsewhere. This function was
+ introduced in OVN 2.7.
+ </dd>
+ </dl>
+
<p><em>Symbols</em></p>
<p>
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 7106bed..149471c 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -586,7 +586,7 @@ read_address_sets(void)
}
static bool
-ovntrace_is_chassis_resident(const void *ports_ OVS_UNUSED,
+ovntrace_is_chassis_resident(const void *aux OVS_UNUSED,
const char *port_name)
{
if (port_name[0] == '\0') {
@@ -681,7 +681,7 @@ read_flows(void)
continue;
}
if (match) {
- match = expr_simplify(match, ovntrace_is_chassis_resident, &ports);
+ match = expr_simplify(match, ovntrace_is_chassis_resident, NULL);
}
struct ovntrace_flow *flow = xzalloc(sizeof *flow);
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index daec9ac..e64f12f 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -803,6 +803,13 @@ free_rule(struct test_rule *test_rule)
free(test_rule);
}
+static bool
+tree_shape_is_chassis_resident_cb(const void *c_aux OVS_UNUSED,
+ const char *port_name OVS_UNUSED)
+{
+ return true;
+}
+
static int
test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab,
struct expr *terminals[], int n_terminals,
@@ -849,7 +856,9 @@ test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab,
exit(EXIT_FAILURE);
}
} else if (operation >= OP_SIMPLIFY) {
- modified = expr_simplify(expr_clone(expr), NULL, NULL);
+ modified = expr_simplify(expr_clone(expr),
+ tree_shape_is_chassis_resident_cb,
+ NULL);
ovs_assert(expr_honors_invariants(modified));
if (operation >= OP_NORMALIZE) {
More information about the dev
mailing list