diff mbox series

[ovs-dev,ovn,v3,3/5] Add expression writeability scopes.

Message ID 20200727211920.3697694-4-mmichels@redhat.com
State Superseded
Headers show
Series Add ECMP symmetric replies | expand

Commit Message

Mark Michelson July 27, 2020, 9:19 p.m. UTC
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 <mmichels@redhat.com>
---
 include/ovn/expr.h | 52 ++++++++++++++++++++++++++++++++++------------
 lib/actions.c      | 44 +++++++++++++++++++++------------------
 lib/expr.c         | 35 ++++++++++++++++++-------------
 3 files changed, 83 insertions(+), 48 deletions(-)
diff mbox series

Patch

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);
     }