[ovs-dev] [PATCH v3 1/2] Add address set support.
Babu Shanmugam
bschanmu at redhat.com
Fri Jun 24 10:13:01 UTC 2016
Hi Flavio,
I agree with your suggestions and will include them in the next version.
Thank you,
Babu
On Thursday 23 June 2016 10:46 PM, Flaviof wrote:
>
>
> On Thu, Jun 23, 2016 at 1:05 AM, <bschanmu at redhat.com
> <mailto:bschanmu at redhat.com>> wrote:
>
> From: Russel Bryant <russell at ovn.org <mailto:russell at 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 <russell at ovn.org
> <mailto:russell at ovn.org>>
> Co-authored-by: Babu Shanmugam <bschanmu at redhat.com
> <mailto:bschanmu at redhat.com>>
> Signed-off-by: Babu Shanmugam <bschanmu at redhat.com
> <mailto:bschanmu at 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 <http://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
> <http://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 <http://ovn.at> b/tests/ovn.at
> <http://ovn.at>
> index a52def4..4f72107 100644
> --- a/tests/ovn.at <http://ovn.at>
> +++ b/tests/ovn.at <http://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
> <http://1.2.0.0/20>, 5.5.5.0/24 <http://5.5.5.0/24>, $set1}'],
> [0], [dnl
> +ip,nw_src=1.2.0.0/20 <http://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 <http://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 at openvswitch.org <mailto:dev at openvswitch.org>
> http://openvswitch.org/mailman/listinfo/dev
>
>
More information about the dev
mailing list