Logical fields are defined as either being writeable or read-only. There is no way to make fields writeable only in specific scenarios.
This commit changes the boolean writeability field to a field of flags indicating contexts where a field is writeable. Any time that nested actions are used (i.e. actions enclosed in curly braces), a new scope may be set for the nested action. For this particular commit, no functionality is changed, and only a "default" scope is added that mirrors the current setup. A future commit will make use of this feature. Signed-off-by: Mark Michelson <mmich...@redhat.com> --- include/ovn/expr.h | 52 ++++++++++++++++++++++++++++++++++------------ lib/actions.c | 44 +++++++++++++++++++++------------------ lib/expr.c | 35 ++++++++++++++++++------------- 3 files changed, 83 insertions(+), 48 deletions(-) diff --git a/include/ovn/expr.h b/include/ovn/expr.h index 9838251c1..11bfdad5b 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -83,6 +83,10 @@ enum expr_level { EXPR_L_ORDINAL }; +enum expr_write_scope { + WR_DEFAULT = (1 << 0), /* Writeable at "global" level */ +}; + const char *expr_level_to_string(enum expr_level); /* A symbol. @@ -255,7 +259,8 @@ struct expr_symbol { char *prereqs; bool must_crossproduct; - bool rw; + enum expr_write_scope rw; /* Bit map indicating in which nested contexts + * the symbol is writeable */ }; void expr_symbol_format(const struct expr_symbol *, struct ds *); @@ -273,20 +278,40 @@ bool expr_field_parse(struct lexer *, const struct shash *symtab, struct expr_field *, struct expr **prereqsp); void expr_field_format(const struct expr_field *, struct ds *); -struct expr_symbol *expr_symtab_add_field(struct shash *symtab, - const char *name, enum mf_field_id, - const char *prereqs, - bool must_crossproduct); -struct expr_symbol *expr_symtab_add_subfield(struct shash *symtab, - const char *name, - const char *prereqs, - const char *subfield); -struct expr_symbol *expr_symtab_add_string(struct shash *symtab, - const char *name, enum mf_field_id, - const char *prereqs); +struct expr_symbol *expr_symtab_add_field_scoped(struct shash *symtab, + const char *name, + enum mf_field_id, + const char *prereqs, + bool must_crossproduct, + enum expr_write_scope scope); + +#define expr_symtab_add_field(SYMTAB, NAME, MF_FIELD_ID, PREREQS, \ + MUST_CROSSPRODUCT) \ + expr_symtab_add_field_scoped((SYMTAB), (NAME), (MF_FIELD_ID), (PREREQS), \ + (MUST_CROSSPRODUCT), WR_DEFAULT) + +struct expr_symbol *expr_symtab_add_subfield_scoped(struct shash *symtab, + const char *name, const char *prereqs, const char *subfield, + enum expr_write_scope scope); + +#define expr_symtab_add_subfield(SYMTAB, NAME, PREREQS, SUBFIELD) \ + expr_symtab_add_subfield_scoped((SYMTAB), (NAME), (PREREQS), \ + (SUBFIELD), WR_DEFAULT) + +struct expr_symbol *expr_symtab_add_string_scoped(struct shash *symtab, + const char *name, + enum mf_field_id, + const char *prereqs, + enum expr_write_scope scope); + +#define expr_symtab_add_string(SYMTAB, NAME, MF_FIELD_ID, PREREQS) \ + expr_symtab_add_string_scoped((SYMTAB), (NAME), (MF_FIELD_ID), (PREREQS), \ + WR_DEFAULT) + struct expr_symbol *expr_symtab_add_predicate(struct shash *symtab, const char *name, const char *expansion); + struct expr_symbol *expr_symtab_add_ovn_field(struct shash *symtab, const char *name, enum ovn_field_id id); @@ -452,7 +477,8 @@ void expr_matches_print(const struct hmap *matches, FILE *); /* Action parsing helper. */ -char *expr_type_check(const struct expr_field *, int n_bits, bool rw) +char *expr_type_check(const struct expr_field *, int n_bits, bool rw, + enum expr_write_scope scope) OVS_WARN_UNUSED_RESULT; struct mf_subfield expr_resolve_field(const struct expr_field *); diff --git a/lib/actions.c b/lib/actions.c index b423995c3..afd1c20bf 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -195,6 +195,7 @@ struct action_context { struct ofpbuf *ovnacts; /* Actions. */ struct expr *prereqs; /* Prerequisites to apply to match. */ int depth; /* Current nested action depth. */ + enum expr_write_scope scope; /* Current writeability scope */ }; static void parse_actions(struct action_context *, enum lex_type sentinel); @@ -207,7 +208,7 @@ action_parse_field(struct action_context *ctx, return false; } - char *error = expr_type_check(f, n_bits, rw); + char *error = expr_type_check(f, n_bits, rw, ctx->scope); if (error) { lexer_error(ctx->lexer, "%s", error); free(error); @@ -374,7 +375,7 @@ parse_LOAD(struct action_context *ctx, const struct expr_field *lhs) load->dst = *lhs; - char *error = expr_type_check(lhs, lhs->n_bits, true); + char *error = expr_type_check(lhs, lhs->n_bits, true, ctx->scope); if (error) { ctx->ovnacts->size = ofs; lexer_error(ctx->lexer, "%s", error); @@ -513,9 +514,9 @@ parse_assignment_action(struct action_context *ctx, bool exchange, return; } - char *error = expr_type_check(lhs, lhs->n_bits, true); + char *error = expr_type_check(lhs, lhs->n_bits, true, ctx->scope); if (!error) { - error = expr_type_check(&rhs, rhs.n_bits, exchange); + error = expr_type_check(&rhs, rhs.n_bits, exchange, ctx->scope); } if (error) { lexer_error(ctx->lexer, "%s", error); @@ -1186,7 +1187,8 @@ static void parse_select_action(struct action_context *ctx, struct expr_field *res_field) { /* Check if the result field is modifiable. */ - char *error = expr_type_check(res_field, res_field->n_bits, true); + char *error = expr_type_check(res_field, res_field->n_bits, true, + ctx->scope); if (error) { lexer_error(ctx->lexer, "%s", error); free(error); @@ -1337,7 +1339,7 @@ encode_CT_CLEAR(const struct ovnact_null *null OVS_UNUSED, * actions on a packet derived from the one being processed. */ static void parse_nested_action(struct action_context *ctx, enum ovnact_type type, - const char *prereq) + const char *prereq, enum expr_write_scope scope) { if (!lexer_force_match(ctx->lexer, LEX_T_LCURLY)) { return; @@ -1357,6 +1359,7 @@ parse_nested_action(struct action_context *ctx, enum ovnact_type type, .ovnacts = &nested, .prereqs = NULL, .depth = ctx->depth + 1, + .scope = scope, }; parse_actions(&inner_ctx, LEX_T_RCURLY); @@ -1387,61 +1390,61 @@ parse_nested_action(struct action_context *ctx, enum ovnact_type type, static void parse_ARP(struct action_context *ctx) { - parse_nested_action(ctx, OVNACT_ARP, "ip4"); + parse_nested_action(ctx, OVNACT_ARP, "ip4", ctx->scope); } static void parse_ICMP4(struct action_context *ctx) { - parse_nested_action(ctx, OVNACT_ICMP4, "ip4"); + parse_nested_action(ctx, OVNACT_ICMP4, "ip4", ctx->scope); } static void parse_ICMP4_ERROR(struct action_context *ctx) { - parse_nested_action(ctx, OVNACT_ICMP4_ERROR, "ip4"); + parse_nested_action(ctx, OVNACT_ICMP4_ERROR, "ip4", ctx->scope); } static void parse_ICMP6(struct action_context *ctx) { - parse_nested_action(ctx, OVNACT_ICMP6, "ip6"); + parse_nested_action(ctx, OVNACT_ICMP6, "ip6", ctx->scope); } static void parse_ICMP6_ERROR(struct action_context *ctx) { - parse_nested_action(ctx, OVNACT_ICMP6_ERROR, "ip6"); + parse_nested_action(ctx, OVNACT_ICMP6_ERROR, "ip6", ctx->scope); } static void parse_TCP_RESET(struct action_context *ctx) { - parse_nested_action(ctx, OVNACT_TCP_RESET, "tcp"); + parse_nested_action(ctx, OVNACT_TCP_RESET, "tcp", ctx->scope); } static void parse_ND_NA(struct action_context *ctx) { - parse_nested_action(ctx, OVNACT_ND_NA, "nd_ns"); + parse_nested_action(ctx, OVNACT_ND_NA, "nd_ns", ctx->scope); } static void parse_ND_NA_ROUTER(struct action_context *ctx) { - parse_nested_action(ctx, OVNACT_ND_NA_ROUTER, "nd_ns"); + parse_nested_action(ctx, OVNACT_ND_NA_ROUTER, "nd_ns", ctx->scope); } static void parse_ND_NS(struct action_context *ctx) { - parse_nested_action(ctx, OVNACT_ND_NS, "ip6"); + parse_nested_action(ctx, OVNACT_ND_NS, "ip6", ctx->scope); } static void parse_CLONE(struct action_context *ctx) { - parse_nested_action(ctx, OVNACT_CLONE, NULL); + parse_nested_action(ctx, OVNACT_CLONE, NULL, WR_DEFAULT); } static void @@ -1946,7 +1949,7 @@ parse_lookup_mac_bind(struct action_context *ctx, struct ovnact_lookup_mac_bind *lookup_mac) { /* Validate that the destination is a 1-bit, modifiable field. */ - char *error = expr_type_check(dst, 1, true); + char *error = expr_type_check(dst, 1, true, ctx->scope); if (error) { lexer_error(ctx->lexer, "%s", error); free(error); @@ -2178,7 +2181,7 @@ parse_put_opts(struct action_context *ctx, const struct expr_field *dst, lexer_get(ctx->lexer); /* Skip '('. */ /* Validate that the destination is a 1-bit, modifiable field. */ - char *error = expr_type_check(dst, 1, true); + char *error = expr_type_check(dst, 1, true, ctx->scope); if (error) { lexer_error(ctx->lexer, "%s", error); free(error); @@ -2577,7 +2580,7 @@ parse_dns_lookup(struct action_context *ctx, const struct expr_field *dst, return; } /* Validate that the destination is a 1-bit, modifiable field. */ - char *error = expr_type_check(dst, 1, true); + char *error = expr_type_check(dst, 1, true, ctx->scope); if (error) { lexer_error(ctx->lexer, "%s", error); free(error); @@ -3102,7 +3105,7 @@ parse_check_pkt_larger(struct action_context *ctx, struct ovnact_check_pkt_larger *cipl) { /* Validate that the destination is a 1-bit, modifiable field. */ - char *error = expr_type_check(dst, 1, true); + char *error = expr_type_check(dst, 1, true, ctx->scope); if (error) { lexer_error(ctx->lexer, "%s", error); free(error); @@ -3566,6 +3569,7 @@ ovnacts_parse(struct lexer *lexer, const struct ovnact_parse_params *pp, .lexer = lexer, .ovnacts = ovnacts, .prereqs = NULL, + .scope = WR_DEFAULT, }; if (!lexer->error) { parse_actions(&ctx, LEX_T_END); diff --git a/lib/expr.c b/lib/expr.c index 497b2accc..c07e7dd4d 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -1447,7 +1447,7 @@ expr_symbol_format(const struct expr_symbol *symbol, struct ds *s) static struct expr_symbol * add_symbol(struct shash *symtab, const char *name, int width, const char *prereqs, enum expr_level level, - bool must_crossproduct, bool rw) + bool must_crossproduct, enum expr_write_scope rw) { struct expr_symbol *symbol = xzalloc(sizeof *symbol); symbol->name = xstrdup(name); @@ -1471,9 +1471,10 @@ add_symbol(struct shash *symtab, const char *name, int width, * Use subfields to duplicate or subset a field (you can even make a subfield * include all the bits of the "parent" field if you like). */ struct expr_symbol * -expr_symtab_add_field(struct shash *symtab, const char *name, - enum mf_field_id id, const char *prereqs, - bool must_crossproduct) +expr_symtab_add_field_scoped(struct shash *symtab, const char *name, + enum mf_field_id id, const char *prereqs, + bool must_crossproduct, + enum expr_write_scope scope) { const struct mf_field *field = mf_from_id(id); struct expr_symbol *symbol; @@ -1482,7 +1483,8 @@ expr_symtab_add_field(struct shash *symtab, const char *name, (field->maskable == MFM_FULLY ? EXPR_L_ORDINAL : EXPR_L_NOMINAL), - must_crossproduct, field->writable); + must_crossproduct, + field->writable ? scope : 0); symbol->field = field; return symbol; } @@ -1511,8 +1513,9 @@ parse_field_from_string(const char *s, const struct shash *symtab, * 'subfield' must describe the subfield as a string, e.g. "vlan.tci[0..11]" * for the low 12 bits of a larger field named "vlan.tci". */ struct expr_symbol * -expr_symtab_add_subfield(struct shash *symtab, const char *name, - const char *prereqs, const char *subfield) +expr_symtab_add_subfield_scoped(struct shash *symtab, const char *name, + const char *prereqs, const char *subfield, + enum expr_write_scope scope) { struct expr_symbol *symbol; struct expr_field f; @@ -1531,7 +1534,7 @@ expr_symtab_add_subfield(struct shash *symtab, const char *name, } symbol = add_symbol(symtab, name, f.n_bits, prereqs, level, false, - f.symbol->rw); + f.symbol->rw ? scope : 0); symbol->parent = f.symbol; symbol->parent_ofs = f.ofs; return symbol; @@ -1540,14 +1543,15 @@ expr_symtab_add_subfield(struct shash *symtab, const char *name, /* Adds a string-valued symbol named 'name' to 'symtab' with the specified * 'prereqs'. */ struct expr_symbol * -expr_symtab_add_string(struct shash *symtab, const char *name, - enum mf_field_id id, const char *prereqs) +expr_symtab_add_string_scoped(struct shash *symtab, const char *name, + enum mf_field_id id, const char *prereqs, + enum expr_write_scope scope) { const struct mf_field *field = mf_from_id(id); struct expr_symbol *symbol; symbol = add_symbol(symtab, name, 0, prereqs, EXPR_L_NOMINAL, false, - field->writable); + field->writable ? scope : 0); symbol->field = field; return symbol; } @@ -1610,7 +1614,7 @@ expr_symtab_add_predicate(struct shash *symtab, const char *name, return NULL; } - symbol = add_symbol(symtab, name, 1, NULL, level, false, false); + symbol = add_symbol(symtab, name, 1, NULL, level, false, 0); symbol->predicate = xstrdup(expansion); return symbol; } @@ -1623,7 +1627,7 @@ expr_symtab_add_ovn_field(struct shash *symtab, const char *name, struct expr_symbol *symbol; symbol = add_symbol(symtab, name, ovn_field->n_bits, NULL, - EXPR_L_NOMINAL, false, true); + EXPR_L_NOMINAL, false, UINT32_MAX); symbol->ovn_field = ovn_field; return symbol; } @@ -3322,7 +3326,8 @@ expr_evaluate(const struct expr *e, const struct flow *uflow, * if 'f' is acceptable, otherwise a malloc()'d error message that the caller * must free(). */ char * OVS_WARN_UNUSED_RESULT -expr_type_check(const struct expr_field *f, int n_bits, bool rw) +expr_type_check(const struct expr_field *f, int n_bits, bool rw, + uint32_t write_scope) { if (n_bits != f->n_bits) { if (n_bits && f->n_bits) { @@ -3340,7 +3345,7 @@ expr_type_check(const struct expr_field *f, int n_bits, bool rw) } } - if (rw && !f->symbol->rw) { + if (rw && !(f->symbol->rw & write_scope)) { return xasprintf("Field %s is not modifiable.", f->symbol->name); } -- 2.25.4 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev