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 <[email protected]> > Signed-off-by: Mickey Spiegel <[email protected]>
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 <[email protected]> --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) { _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
