On Thu, Jun 23, 2016 at 1:05 AM, <bscha...@redhat.com> wrote: > From: Russel Bryant <russ...@ovn.org> > > Update the OVN expression parser to support address sets. Previously, > you could have a set of IP or MAC addresses in this form: > > {addr1, addr2, ..., addrN} > > This patch adds support for a bit of indirection where we can define a > set of addresses and refer to them by name. > > $name > > This '$name' can be used in the expresssions like > > {addr1, addr2, $name, ... } > {$name} > $name > > A future patch will expose the ability to define address sets for use. > > Signed-off-by: Russell Bryant <russ...@ovn.org> > Co-authored-by: Babu Shanmugam <bscha...@redhat.com> > Signed-off-by: Babu Shanmugam <bscha...@redhat.com> > --- > ovn/controller/lflow.c | 2 +- > ovn/lib/actions.c | 2 +- > ovn/lib/expr.c | 121 > +++++++++++++++++++++++++++++++++++++++++++++++-- > ovn/lib/expr.h | 17 +++++++ > ovn/lib/lex.c | 16 +++++++ > ovn/lib/lex.h | 1 + > tests/ovn.at | 50 ++++++++++++++++++++ > tests/test-ovn.c | 31 +++++++++++-- > 8 files changed, 230 insertions(+), 10 deletions(-) > > Please see tests I suggest adding in this commit:
https://github.com/anbu-enovance/ovs/pull/2/commits/f463a43451b85b4896f1be78e1a2ee7066b3b465 > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c > index efc427d..263cba6 100644 > --- a/ovn/controller/lflow.c > +++ b/ovn/controller/lflow.c > @@ -297,7 +297,7 @@ add_logical_flows(struct controller_ctx *ctx, const > struct lport_index *lports, > struct hmap matches; > struct expr *expr; > > - expr = expr_parse_string(lflow->match, &symtab, &error); > + expr = expr_parse_string(lflow->match, &symtab, NULL, &error); > if (!error) { > if (prereqs) { > expr = expr_combine(EXPR_T_AND, expr, prereqs); > diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c > index 4a486a0..0771b61 100644 > --- a/ovn/lib/actions.c > +++ b/ovn/lib/actions.c > @@ -180,7 +180,7 @@ add_prerequisite(struct action_context *ctx, const > char *prerequisite) > struct expr *expr; > char *error; > > - expr = expr_parse_string(prerequisite, ctx->ap->symtab, &error); > + expr = expr_parse_string(prerequisite, ctx->ap->symtab, NULL, &error); > ovs_assert(!error); > ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr); > } > diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c > index f274ab4..f87ece0 100644 > --- a/ovn/lib/expr.c > +++ b/ovn/lib/expr.c > @@ -451,6 +451,7 @@ struct expr_field { > struct expr_context { > struct lexer *lexer; /* Lexer for pulling more tokens. */ > const struct shash *symtab; /* Symbol table. */ > + const struct shash *macros; /* Table of macros. */ > char *error; /* Error, if any, otherwise NULL. */ > bool not; /* True inside odd number of NOT > operators. */ > }; > @@ -772,6 +773,41 @@ assign_constant_set_type(struct expr_context *ctx, > } > > static bool > +parse_macros(struct expr_context *ctx, struct expr_constant_set *cs, > + size_t *allocated_values) > +{ > + if (!ctx->macros) { > + expr_syntax_error(ctx, "No macros defined."); > + return false; > + } > + > + struct expr_constant_set *addr_set > + = shash_find_data(ctx->macros, ctx->lexer->token.s); > + if (!addr_set) { > + expr_syntax_error(ctx, "Unknown macro: '%s'", > ctx->lexer->token.s); > + return false; > + } > + > > + cs->type = EXPR_C_INTEGER; > // call use assign_constant_set_type() instead of blindly // setting expr_constant_set's type. This way, we can stop // cases when type is unxecpected by the macro parsing: if (!assign_constant_set_type(ctx, cs, EXPR_C_INTEGER)) { return false; } > + size_t n_values = cs->n_values + addr_set->n_values; > + if (n_values >= *allocated_values) { > + cs->values = xrealloc(cs->values, n_values * sizeof *cs->values); > + *allocated_values = n_values; > + } > + size_t i; > + for (i = 0; i < addr_set->n_values; i++) { > + union expr_constant *c1 = &cs->values[cs->n_values++]; > + union expr_constant *c2 = &addr_set->values[i]; > + c1->value = c2->value; > + c1->format = c2->format; > + c1->masked = c2->masked; > + c1->mask = c2->mask; > + } > + > + return true; > +} > + > +static bool > parse_constant(struct expr_context *ctx, struct expr_constant_set *cs, > size_t *allocated_values) > { > @@ -802,6 +838,12 @@ parse_constant(struct expr_context *ctx, struct > expr_constant_set *cs, > } > lexer_get(ctx->lexer); > return true; > + } else if (ctx->lexer->token.type == LEX_T_MACRO) { > + if (!parse_macros(ctx, cs, allocated_values)) { > + return false; > + } > + lexer_get(ctx->lexer); > + return true; > } else { > expr_syntax_error(ctx, "expecting constant."); > return false; > @@ -851,6 +893,72 @@ expr_constant_set_destroy(struct expr_constant_set > *cs) > } > } > > +void > +expr_macros_add(struct shash *macros, const char *name, > + const char * const *values, size_t n_values) > +{ > + /* Replace any existing entry for this name. */ > + expr_macros_remove(macros, name); > + > + struct expr_constant_set *cset = xzalloc(sizeof *cset); > + cset->type = EXPR_C_INTEGER; > + cset->in_curlies = true; > + cset->n_values = n_values; > + cset->values = xmalloc(cset->n_values * sizeof *cset->values); > + size_t i, errors = 0; > + for (i = 0; i < n_values; i++) { > + /* Use the lexer to convert each macro into the proper > + * integer format. */ > + struct lexer lex; > + lexer_init(&lex, values[i]); > + lexer_get(&lex); > + if (lex.token.type != LEX_T_INTEGER > + && lex.token.type != LEX_T_MASKED_INTEGER) { > + VLOG_WARN("Invalid address set entry: '%s', token type: %d", > + values[i], lex.token.type); > + errors += 1; > + } else { > + union expr_constant *c = &cset->values[i - errors]; > + c->value = lex.token.value; > + c->format = lex.token.format; > + c->masked = lex.token.type == LEX_T_MASKED_INTEGER; > + if (c->masked) { > + c->mask = lex.token.mask; > + } > + } > + lexer_destroy(&lex); > + } > + cset->n_values -= errors; > + > + shash_add(macros, name, cset); > +} > + > +void > +expr_macros_remove(struct shash *macros, const char *name) > +{ > + struct expr_constant_set *cset > + = shash_find_and_delete(macros, name); > + > + if (cset) { > + expr_constant_set_destroy(cset); > + free(cset); > + } > +} > + > +/* Destroy all contents of macros. */ > +void > +expr_macros_destroy(struct shash *macros) > +{ > + struct shash_node *node, *next; > + > + SHASH_FOR_EACH_SAFE (node, next, macros) { > + struct expr_constant_set *cset = node->data; > + > + shash_delete(macros, node); > + expr_constant_set_destroy(cset); > + } > +} > + > static struct expr * > expr_parse_primary(struct expr_context *ctx, bool *atomic) > { > @@ -1023,12 +1131,14 @@ expr_parse__(struct expr_context *ctx) > * The caller must eventually free the returned expression (with > * expr_destroy()) or error (with free()). */ > struct expr * > -expr_parse(struct lexer *lexer, const struct shash *symtab, char **errorp) > +expr_parse(struct lexer *lexer, const struct shash *symtab, > + const struct shash *macros, char **errorp) > { > struct expr_context ctx; > > ctx.lexer = lexer; > ctx.symtab = symtab; > + ctx.macros = macros; > ctx.error = NULL; > ctx.not = false; > > @@ -1040,14 +1150,15 @@ expr_parse(struct lexer *lexer, const struct shash > *symtab, char **errorp) > > /* Like expr_parse(), but the expression is taken from 's'. */ > struct expr * > -expr_parse_string(const char *s, const struct shash *symtab, char > **errorp) > +expr_parse_string(const char *s, const struct shash *symtab, > + const struct shash *macros, char **errorp) > { > struct lexer lexer; > struct expr *expr; > > lexer_init(&lexer, s); > lexer_get(&lexer); > - expr = expr_parse(&lexer, symtab, errorp); > + expr = expr_parse(&lexer, symtab, macros, errorp); > if (!*errorp && lexer.token.type != LEX_T_END) { > *errorp = xstrdup("Extra tokens at end of input."); > expr_destroy(expr); > @@ -1203,7 +1314,7 @@ expr_get_level(const struct expr *expr) > static enum expr_level > expr_parse_level(const char *s, const struct shash *symtab, char **errorp) > { > - struct expr *expr = expr_parse_string(s, symtab, errorp); > + struct expr *expr = expr_parse_string(s, symtab, NULL, errorp); > enum expr_level level = expr ? expr_get_level(expr) : EXPR_L_NOMINAL; > expr_destroy(expr); > return level; > @@ -1346,7 +1457,7 @@ parse_and_annotate(const char *s, const struct shash > *symtab, > char *error; > struct expr *expr; > > - expr = expr_parse_string(s, symtab, &error); > + expr = expr_parse_string(s, symtab, NULL, &error); > if (expr) { > expr = expr_annotate__(expr, symtab, nesting, &error); > } > diff --git a/ovn/lib/expr.h b/ovn/lib/expr.h > index 1327789..c4f8e67 100644 > --- a/ovn/lib/expr.h > +++ b/ovn/lib/expr.h > @@ -343,8 +343,10 @@ expr_from_node(const struct ovs_list *node) > void expr_format(const struct expr *, struct ds *); > void expr_print(const struct expr *); > struct expr *expr_parse(struct lexer *, const struct shash *symtab, > + const struct shash *macros, > char **errorp); > struct expr *expr_parse_string(const char *, const struct shash *symtab, > + const struct shash *macros, > char **errorp); > > struct expr *expr_clone(struct expr *); > @@ -391,4 +393,19 @@ char *expr_parse_field(struct lexer *, int n_bits, > bool rw, > const struct shash *symtab, struct mf_subfield *, > struct expr **prereqsp); > > + > +/* MACRO Variables > + * > + * Instead of referring to a set of value as: > + * {addr1, addr2, ..., addrN} > + * You can register a set of values and refer to them as: > + * $name > + * The macros should all have integer/masked-integer values. > + * The values that don't qualify are ingnored > + */ > +void expr_macros_add(struct shash *macros, const char *name, > + const char * const *values, size_t n_values); > +void expr_macros_remove(struct shash *macros, const char *name); > +void expr_macros_destroy(struct shash *macros); > + > #endif /* ovn/expr.h */ > diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c > index 556288f..bbbc7fc 100644 > --- a/ovn/lib/lex.c > +++ b/ovn/lib/lex.c > @@ -227,6 +227,10 @@ lex_token_format(const struct lex_token *token, > struct ds *s) > lex_token_format_masked_integer(token, s); > break; > > + case LEX_T_MACRO: > + ds_put_cstr(s, token->s); > + break; > + > case LEX_T_LPAREN: > ds_put_cstr(s, "("); > break; > @@ -527,6 +531,14 @@ lex_parse_id(const char *p, struct lex_token *token) > return p; > } > > +static const char * > +lex_parse_macro(const char *p, struct lex_token *token) > +{ > + p = lex_parse_id(++p, token); > + token->type = LEX_T_MACRO; > + return p; > +} > + > /* Initializes 'token' and parses the first token from the beginning of > * null-terminated string 'p' into 'token'. Stores a pointer to the > start of > * the token (after skipping white space and comments, if any) into > '*startp'. > @@ -697,6 +709,10 @@ next: > } > break; > > + case '$': > + p = lex_parse_macro(p, token); > + break; > + > case '0': case '1': case '2': case '3': case '4': > case '5': case '6': case '7': case '8': case '9': > case ':': > diff --git a/ovn/lib/lex.h b/ovn/lib/lex.h > index 578ef40..826465c 100644 > --- a/ovn/lib/lex.h > +++ b/ovn/lib/lex.h > @@ -36,6 +36,7 @@ enum lex_type { > LEX_T_STRING, /* "foo" */ > LEX_T_INTEGER, /* 12345 or 1.2.3.4 or ::1 or > 01:02:03:04:05 */ > LEX_T_MASKED_INTEGER, /* 12345/10 or 1.2.0.0/16 or ::2/127 > or... */ > + LEX_T_MACRO, /* $NAME */ > LEX_T_ERROR, /* invalid input */ > > /* Bare tokens. */ > diff --git a/tests/ovn.at b/tests/ovn.at > index a52def4..4f72107 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -434,6 +434,56 @@ AT_CHECK([expr_to_flow 'inport == "eth0" && inport == > "eth1"'], [0], [dnl > ]) > AT_CLEANUP > > +AT_SETUP([ovn -- converting expressions to flows -- address sets]) > +expr_to_flow () { > + echo "$1" | ovstest test-ovn expr-to-flows | sort > +} > +AT_CHECK([expr_to_flow 'ip4.src == {10.0.0.1, 10.0.0.2, 10.0.0.3}'], [0], > [dnl > +ip,nw_src=10.0.0.1 > +ip,nw_src=10.0.0.2 > +ip,nw_src=10.0.0.3 > +]) > +AT_CHECK([expr_to_flow 'ip4.src == $set1'], [0], [dnl > +ip,nw_src=10.0.0.1 > +ip,nw_src=10.0.0.2 > +ip,nw_src=10.0.0.3 > +]) > +AT_CHECK([expr_to_flow 'ip4.src == {1.2.3.4, $set1}'], [0], [dnl > +ip,nw_src=1.2.3.4 > +ip,nw_src=10.0.0.1 > +ip,nw_src=10.0.0.2 > +ip,nw_src=10.0.0.3 > +]) > +AT_CHECK([expr_to_flow 'ip4.src == {1.2.0.0/20, 5.5.5.0/24, $set1}'], > [0], [dnl > +ip,nw_src=1.2.0.0/20 > +ip,nw_src=10.0.0.1 > +ip,nw_src=10.0.0.2 > +ip,nw_src=10.0.0.3 > +ip,nw_src=5.5.5.0/24 > +]) > +AT_CHECK([expr_to_flow 'ip6.src == {::1, ::2, ::3}'], [0], [dnl > +ipv6,ipv6_src=::1 > +ipv6,ipv6_src=::2 > +ipv6,ipv6_src=::3 > +]) > +AT_CHECK([expr_to_flow 'ip6.src == {::1, $set2, ::4}'], [0], [dnl > +ipv6,ipv6_src=::1 > +ipv6,ipv6_src=::2 > +ipv6,ipv6_src=::3 > +ipv6,ipv6_src=::4 > +]) > +AT_CHECK([expr_to_flow 'eth.src == {00:00:00:00:00:01, 00:00:00:00:00:02, > 00:00:00:00:00:03}'], [0], [dnl > +dl_src=00:00:00:00:00:01 > +dl_src=00:00:00:00:00:02 > +dl_src=00:00:00:00:00:03 > +]) > +AT_CHECK([expr_to_flow 'eth.src == {$set3}'], [0], [dnl > +dl_src=00:00:00:00:00:01 > +dl_src=00:00:00:00:00:02 > +dl_src=00:00:00:00:00:03 > +]) > +AT_CLEANUP > + > AT_SETUP([ovn -- action parsing]) > dnl Text before => is input, text after => is expected output. > AT_DATA([test-cases.txt], [[ > diff --git a/tests/test-ovn.c b/tests/test-ovn.c > index 051f3a7..55281af 100644 > --- a/tests/test-ovn.c > +++ b/tests/test-ovn.c > @@ -239,6 +239,26 @@ create_symtab(struct shash *symtab) > expr_symtab_add_string(symtab, "big_string", MFF_XREG0, NULL); > } > > +static void > +create_macros(struct shash *macros) > +{ > + shash_init(macros); > + > + const char * const addrs1[] = { > + "10.0.0.1", "10.0.0.2", "10.0.0.3", > + }; > + const char * const addrs2[] = { > + "::1", "::2", "::3", > + }; > + const char * const addrs3[] = { > + "00:00:00:00:00:01", "00:00:00:00:00:02", "00:00:00:00:00:03", > + }; > + > + expr_macros_add(macros, "set1", addrs1, 3); > + expr_macros_add(macros, "set2", addrs2, 3); > + expr_macros_add(macros, "set3", addrs3, 3); > +} > + > static bool > lookup_port_cb(const void *ports_, const char *port_name, unsigned int > *portp) > { > @@ -255,10 +275,12 @@ static void > test_parse_expr__(int steps) > { > struct shash symtab; > + struct shash macros; > struct simap ports; > struct ds input; > > create_symtab(&symtab); > + create_macros(¯os); > > simap_init(&ports); > simap_put(&ports, "eth0", 5); > @@ -270,7 +292,8 @@ test_parse_expr__(int steps) > struct expr *expr; > char *error; > > - expr = expr_parse_string(ds_cstr(&input), &symtab, &error); > + expr = expr_parse_string(ds_cstr(&input), &symtab, ¯os, > + &error); > if (!error && steps > 0) { > expr = expr_annotate(expr, &symtab, &error); > } > @@ -307,6 +330,8 @@ test_parse_expr__(int steps) > simap_destroy(&ports); > expr_symtab_destroy(&symtab); > shash_destroy(&symtab); > + expr_macros_destroy(¯os); > + shash_destroy(¯os); > } > > static void > @@ -451,7 +476,7 @@ test_evaluate_expr(struct ovs_cmdl_context *ctx) > struct expr *expr; > char *error; > > - expr = expr_parse_string(ds_cstr(&input), &symtab, &error); > + expr = expr_parse_string(ds_cstr(&input), &symtab, NULL, &error); > if (!error) { > expr = expr_annotate(expr, &symtab, &error); > } > @@ -925,7 +950,7 @@ test_tree_shape_exhaustively(struct expr *expr, struct > shash *symtab, > expr_format(expr, &s); > > char *error; > - modified = expr_parse_string(ds_cstr(&s), symtab, &error); > + modified = expr_parse_string(ds_cstr(&s), symtab, NULL, > &error); > if (error) { > fprintf(stderr, "%s fails to parse (%s)\n", > ds_cstr(&s), error); > -- > 2.5.5 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev