@@ -106,20 +106,23 @@ const char *expr_level_to_string(enum expr_level);
* Fields:
*
* One might, for example, define a field named "vlan.tci" to refer to
- * MFF_VLAN_TCI. For integer fields, 'field' specifies the referent; for
- * string fields, 'field' is NULL.
+ * MFF_VLAN_TCI. 'field' specifies the field.
*
- * 'expansion' is NULL.
+ * 'parent' and 'predicate' are NULL, and 'parent_ofs' is 0.
*
* Integer fields can be nominal or ordinal (see below). String fields are
* always nominal.
*
* Subfields:
*
- * 'expansion' is a string that specifies a subfield of some larger field,
- * e.g. "vlan.tci[0..11]" for a field that represents a VLAN VID.
+ * 'parent' specifies the field (which may itself be a subfield,
+ * recursively) in which the subfield is embedded, and 'parent_ofs' a
+ * bitwise offset from the least-significant bit of the parent. The
+ * subfield can contain a subset of the bits of the parent or all of them
+ * (in the latter case the subfield is really just a synonym for the
+ * parent).
*
- * 'field' is NULL.
+ * 'field' and 'predicate' are NULL.
*
* Only ordinal fields (see below) may have subfields, and subfields are
* always ordinal.
@@ -127,16 +130,15 @@ const char *expr_level_to_string(enum expr_level);
* Predicates:
*
* A predicate is an arbitrary Boolean expression that can be used in an
- * expression much like a 1-bit field. 'expansion' specifies the Boolean
+ * expression much like a 1-bit field. 'predicate' specifies the Boolean
* expression, e.g. "ip4" might expand to "eth.type == 0x800". The
- * expansion of a predicate might refer to other predicates, e.g. "icmp4"
- * might expand to "ip4 && ip4.proto == 1".
+ * epxression might refer to other predicates, e.g. "icmp4" might expand to
+ * "ip4 && ip4.proto == 1".
*
- * 'field' is NULL.
+ * 'field' and 'parent' are NULL, and 'parent_ofs' is 0.
*
- * A predicate whose expansion refers to any nominal field or predicate
- * (see below) is nominal; other predicates have Boolean level of
- * measurement.
+ * A predicate that refers to any nominal field or predicate (see below) is
+ * nominal; other predicates have Boolean level of measurement.
*
*
* Level of Measurement
@@ -239,8 +241,10 @@ struct expr_symbol {
char *name;
int width;
- const struct mf_field *field;
- char *expansion;
+ const struct mf_field *field; /* Fields only, otherwise NULL. */
+ const struct expr_symbol *parent; /* Subfields only, otherwise NULL. */
+ int parent_ofs; /* Subfields only, otherwise 0. */
+ char *predicate; /* Predicates only, otherwise NULL. */
enum expr_level level;
@@ -592,7 +592,7 @@ make_cmp(struct expr_context *ctx,
}
if (f->symbol->level == EXPR_L_NOMINAL) {
- if (f->symbol->expansion) {
+ if (f->symbol->predicate) {
ovs_assert(f->symbol->width > 0);
for (size_t i = 0; i < cs->n_values; i++) {
const union mf_subvalue *value = &cs->values[i].value;
@@ -1209,7 +1209,8 @@ expr_symtab_add_subfield(struct shash *symtab, const char *name,
symbol = add_symbol(symtab, name, f.n_bits, prereqs, level, false,
f.symbol->rw);
- symbol->expansion = xstrdup(subfield);
+ symbol->parent = f.symbol;
+ symbol->parent_ofs = f.ofs;
return symbol;
}
@@ -1285,7 +1286,7 @@ expr_symtab_add_predicate(struct shash *symtab, const char *name,
}
symbol = add_symbol(symtab, name, 1, NULL, level, false, false);
- symbol->expansion = xstrdup(expansion);
+ symbol->predicate = xstrdup(expansion);
return symbol;
}
@@ -1301,7 +1302,7 @@ expr_symtab_destroy(struct shash *symtab)
shash_delete(symtab, node);
free(symbol->name);
free(symbol->prereqs);
- free(symbol->expansion);
+ free(symbol->predicate);
free(symbol);
}
}
@@ -1444,36 +1445,27 @@ expr_annotate_cmp(struct expr *expr, const struct shash *symtab,
}
}
- if (symbol->expansion) {
- if (symbol->level == EXPR_L_ORDINAL) {
- struct expr_field field;
+ if (symbol->parent) {
+ expr->cmp.symbol = symbol->parent;
+ mf_subvalue_shift(&expr->cmp.value, symbol->parent_ofs);
+ mf_subvalue_shift(&expr->cmp.mask, symbol->parent_ofs);
+ } else if (symbol->predicate) {
+ struct expr *predicate;
- if (!parse_field_from_string(symbol->expansion, symtab,
- &field, errorp)) {
- goto error;
- }
-
- expr->cmp.symbol = field.symbol;
- mf_subvalue_shift(&expr->cmp.value, field.ofs);
- mf_subvalue_shift(&expr->cmp.mask, field.ofs);
- } else {
- struct expr *expansion;
-
- expansion = parse_and_annotate(symbol->expansion, symtab,
- nesting, errorp);
- if (!expansion) {
- goto error;
- }
-
- bool positive = (expr->cmp.value.integer & htonll(1)) != 0;
- positive ^= expr->cmp.relop == EXPR_R_NE;
- if (!positive) {
- expr_not(expansion);
- }
+ predicate = parse_and_annotate(symbol->predicate, symtab,
+ nesting, errorp);
+ if (!predicate) {
+ goto error;
+ }
- expr_destroy(expr);
- expr = expansion;
+ bool positive = (expr->cmp.value.integer & htonll(1)) != 0;
+ positive ^= expr->cmp.relop == EXPR_R_NE;
+ if (!positive) {
+ expr_not(predicate);
}
+
+ expr_destroy(expr);
+ expr = predicate;
}
*errorp = NULL;
@@ -2707,7 +2699,7 @@ expand_symbol(struct expr_context *ctx, bool rw,
{
const struct expr_symbol *orig_symbol = f->symbol;
- if (f->symbol->expansion && f->symbol->level != EXPR_L_ORDINAL) {
+ if (f->symbol->predicate) {
expr_error(ctx, "Predicate symbol %s used where lvalue required.",
f->symbol->name);
return false;
@@ -2730,21 +2722,13 @@ expand_symbol(struct expr_context *ctx, bool rw,
}
/* If there's no expansion, we're done. */
- if (!f->symbol->expansion) {
+ if (!f->symbol->parent) {
break;
}
/* Expand. */
- struct expr_field expansion;
- char *error;
- if (!parse_field_from_string(f->symbol->expansion, ctx->symtab,
- &expansion, &error)) {
- expr_error(ctx, "%s", error);
- free(error);
- return false;
- }
- f->symbol = expansion.symbol;
- f->ofs += expansion.ofs;
+ f->ofs += f->symbol->parent_ofs;
+ f->symbol = f->symbol->parent;
}
if (rw && !f->symbol->field->writable) {
Until now, symbols that represent subfields and predicates were both implemented as the same string member, named 'expansion', inside struct expr. This makes it a little inconvenient to find the parent of a subfield for two reasons. First, one must actually parse the string, e.g. to convert "vlan.tci[13..15]" into a pointer to a struct. Second, and more importantly, to parse the string it's necessary to have access to the symbol table, which isn't always convenient to pass around. This commit avoids the problem by breaking apart subfields and predicates and giving the former a direct pointer to the parent symbol. We could do the same thing for predicates by storing a pointer to a pre-built struct expr, but so far it's not necessary. Signed-off-by: Ben Pfaff <blp@ovn.org> --- include/ovn/expr.h | 34 ++++++++++++++------------ ovn/lib/expr.c | 70 +++++++++++++++++++++--------------------------------- 2 files changed, 46 insertions(+), 58 deletions(-)