On 10/7/22 04:03, Han Zhou wrote:
> On Fri, Sep 30, 2022 at 7:02 AM Dumitru Ceara <[email protected]> wrote:
>>
>> Expand SB.Template_Var records in two stages:
>> 1. first expand them to local values in match/action strings
>> 2. then reparse the expanded strings
>>
>> For the case when a lflow references a Template_Var also track
>> references (similar to the ones maintained for multicast groups, address
>> sets, port_groups, port bindings).
>>
>> Signed-off-by: Dumitru Ceara <[email protected]>
> 
> Hi Dumitru,
> 

Hi Han,

> In addition to the two-stage parsing concerns we are discussing in the

I'm doing some more targetted performance testing.  I'll update the
other thread when I have more data to share.

> other thread, I have some minor comments below. The major one is whether we
> should allow matching hostname or not.
> 
>> ---
>>  controller/lflow.c          |   59 +++++++-
>>  controller/lflow.h          |    1
>>  controller/lport.c          |    3
>>  controller/ofctrl.c         |    9 +
>>  controller/ofctrl.h         |    3
>>  controller/ovn-controller.c |  317
> +++++++++++++++++++++++++++++++++++++++++++
>>  include/ovn/expr.h          |    4 -
>>  include/ovn/lex.h           |   14 +-
>>  lib/actions.c               |    9 +
>>  lib/expr.c                  |   18 ++
>>  lib/lex.c                   |   55 +++++++
>>  lib/objdep.c                |    1
>>  lib/objdep.h                |    1
>>  lib/ovn-util.c              |    7 +
>>  lib/ovn-util.h              |    3
>>  tests/ovn.at                |    2
>>  tests/test-ovn.c            |   16 ++
>>  utilities/ovn-trace.c       |   26 +++-
>>  18 files changed, 512 insertions(+), 36 deletions(-)
>>
>> diff --git a/controller/lflow.c b/controller/lflow.c
>> index f9f2e6286..ad58fcd3c 100644
>> --- a/controller/lflow.c
>> +++ b/controller/lflow.c
>> @@ -81,6 +81,8 @@ convert_match_to_expr(const struct sbrec_logical_flow *,
>>                        const struct local_datapath *ldp,
>>                        struct expr **prereqs, const struct shash
> *addr_sets,
>>                        const struct shash *port_groups,
>> +                      const struct smap *template_vars,
>> +                      struct sset *template_vars_ref,
>>                        struct objdep_mgr *, bool *pg_addr_set_ref);
>>  static void
>>  add_matches_to_flow_table(const struct sbrec_logical_flow *,
>> @@ -356,10 +358,15 @@ consider_lflow_for_added_as_ips__(
>>          .n_tables = LOG_PIPELINE_LEN,
>>          .cur_ltable = lflow->table_id,
>>      };
>> +    struct sset template_vars_ref = SSET_INITIALIZER(&template_vars_ref);
>>      struct expr *prereqs = NULL;
>>      char *error;
>>
>> -    error = ovnacts_parse_string(lflow->actions, &pp, &ovnacts,
> &prereqs);
>> +    char *actions_s =
> lexer_parse_template_string(xstrdup(lflow->actions),
> 
> It may be better to xstrdup inside the function when necessary, and use
> const char* as parameter.
> 

Definitely, I'll change it in v2.

>> +
>  l_ctx_in->template_vars,
>> +                                                  &template_vars_ref);
>> +    error = ovnacts_parse_string(actions_s, &pp, &ovnacts, &prereqs);
> 
> The two functions can be wrapped to a single function, because they will be
> called together again in consider_logical_flow__().
> 

I can do that.  I didn't do it initially because there's already quite
some overlap between consider_lflow_for_added_as_ips__() and
consider_logical_flow__().  But that's something for a different patch I
guess.

>> +    free(actions_s);
>>      if (error) {
>>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
>>          VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s",
>> @@ -367,6 +374,7 @@ consider_lflow_for_added_as_ips__(
>>          free(error);
>>          ovnacts_free(ovnacts.data, ovnacts.size);
>>          ofpbuf_uninit(&ovnacts);
>> +        sset_destroy(&template_vars_ref);
>>          return true;
>>      }
>>
>> @@ -430,6 +438,8 @@ consider_lflow_for_added_as_ips__(
>>      struct expr *expr = convert_match_to_expr(lflow, ldp, &prereqs,
>>                                                l_ctx_in->addr_sets,
>>                                                l_ctx_in->port_groups,
>> +                                              l_ctx_in->template_vars,
>> +                                              &template_vars_ref,
>>                                                l_ctx_out->lflow_deps_mgr,
> NULL);
>>      shash_replace((struct shash *)l_ctx_in->addr_sets, as_name, real_as);
>>      if (new_fake_as) {
>> @@ -501,6 +511,14 @@ done:
>>      ofpbuf_uninit(&ovnacts);
>>      expr_destroy(expr);
>>      expr_matches_destroy(&matches);
>> +
>> +    const char *tv_name;
>> +    SSET_FOR_EACH (tv_name, &template_vars_ref) {
>> +        objdep_mgr_add(l_ctx_out->lflow_deps_mgr, OBJDEP_TYPE_TEMPLATE,
>> +                       tv_name, &lflow->header_.uuid);
>> +    }
>> +    sset_destroy(&template_vars_ref);
>> +
>>      return handled;
>>  }
>>
>> @@ -911,6 +929,8 @@ convert_match_to_expr(const struct sbrec_logical_flow
> *lflow,
>>                        struct expr **prereqs,
>>                        const struct shash *addr_sets,
>>                        const struct shash *port_groups,
>> +                      const struct smap *template_vars,
>> +                      struct sset *template_vars_ref,
>>                        struct objdep_mgr *mgr,
>>                        bool *pg_addr_set_ref)
>>  {
>> @@ -918,11 +938,16 @@ convert_match_to_expr(const struct
> sbrec_logical_flow *lflow,
>>      struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref);
>>      char *error = NULL;
>>
>> -    struct expr *e = expr_parse_string(lflow->match, &symtab, addr_sets,
>> +    char *match_s = lexer_parse_template_string(xstrdup(lflow->match),
>> +                                                template_vars,
>> +                                                template_vars_ref);
>> +    struct expr *e = expr_parse_string(match_s, &symtab, addr_sets,
>>                                         port_groups, &addr_sets_ref,
>>                                         &port_groups_ref,
>>                                         ldp->datapath->tunnel_key,
>>                                         &error);
>> +    free(match_s);
>> +
>>      struct shash_node *addr_sets_ref_node;
>>      SHASH_FOR_EACH (addr_sets_ref_node, &addr_sets_ref) {
>>          objdep_mgr_add_with_refcount(mgr, OBJDEP_TYPE_ADDRSET,
>> @@ -1027,7 +1052,12 @@ consider_logical_flow__(const struct
> sbrec_logical_flow *lflow,
>>      struct expr *prereqs = NULL;
>>      char *error;
>>
>> -    error = ovnacts_parse_string(lflow->actions, &pp, &ovnacts,
> &prereqs);
>> +    struct sset template_vars_ref = SSET_INITIALIZER(&template_vars_ref);
>> +    char *actions_s =
> lexer_parse_template_string(xstrdup(lflow->actions),
>> +
>  l_ctx_in->template_vars,
>> +                                                  &template_vars_ref);
>> +    error = ovnacts_parse_string(actions_s, &pp, &ovnacts, &prereqs);
>> +    free(actions_s);
>>      if (error) {
>>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
>>          VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s",
>> @@ -1035,6 +1065,7 @@ consider_logical_flow__(const struct
> sbrec_logical_flow *lflow,
>>          free(error);
>>          ovnacts_free(ovnacts.data, ovnacts.size);
>>          ofpbuf_uninit(&ovnacts);
>> +        sset_destroy(&template_vars_ref);
>>          return;
>>      }
>>
>> @@ -1085,6 +1116,8 @@ consider_logical_flow__(const struct
> sbrec_logical_flow *lflow,
>>      case LCACHE_T_NONE:
>>          expr = convert_match_to_expr(lflow, ldp, &prereqs,
> l_ctx_in->addr_sets,
>>                                       l_ctx_in->port_groups,
>> +                                     l_ctx_in->template_vars,
>> +                                     &template_vars_ref,
>>                                       l_ctx_out->lflow_deps_mgr,
>>                                       &pg_addr_set_ref);
>>          if (!expr) {
>> @@ -1099,11 +1132,13 @@ consider_logical_flow__(const struct
> sbrec_logical_flow *lflow,
>>      }
>>
>>      /* If caching is enabled and this is a not cached expr that doesn't
> refer
>> -     * to address sets or port groups, save it to potentially cache it
> later.
>> +     * to address sets, port groups, or template variables, save it to
>> +     * potentially cache it later.
>>       */
>>      if (lcv_type == LCACHE_T_NONE
>>              && lflow_cache_is_enabled(l_ctx_out->lflow_cache)
>> -            && !pg_addr_set_ref) {
>> +            && !pg_addr_set_ref
>> +            && sset_is_empty(&template_vars_ref)) {
>>          cached_expr = expr_clone(expr);
>>      }
>>
>> @@ -1188,6 +1223,13 @@ done:
>>      expr_destroy(cached_expr);
>>      expr_matches_destroy(matches);
>>      free(matches);
>> +
>> +    const char *tv_name;
>> +    SSET_FOR_EACH (tv_name, &template_vars_ref) {
>> +        objdep_mgr_add(l_ctx_out->lflow_deps_mgr, OBJDEP_TYPE_TEMPLATE,
>> +                       tv_name, &lflow->header_.uuid);
>> +    }
>> +    sset_destroy(&template_vars_ref);
>>  }
>>
>>  static void
>> @@ -1951,8 +1993,7 @@ add_lb_ct_snat_hairpin_flows(struct
> ovn_controller_lb *lb,
>>
>>  static void
>>  consider_lb_hairpin_flows(const struct sbrec_load_balancer *sbrec_lb,
>> -                          const struct hmap *local_datapaths,
>> -                          bool use_ct_mark,
>> +                          const struct hmap *local_datapaths, bool
> use_ct_mark,
>>                            struct ovn_desired_flow_table *flow_table,
>>                            struct simap *ids)
>>  {
>> @@ -2041,8 +2082,8 @@ add_lb_hairpin_flows(const struct
> sbrec_load_balancer_table *lb_table,
>>              ovs_assert(id_pool_alloc_id(pool, &id));
>>              simap_put(ids, lb->name, id);
>>          }
>> -        consider_lb_hairpin_flows(lb, local_datapaths, use_ct_mark,
>> -                                  flow_table, ids);
>> +        consider_lb_hairpin_flows(lb, local_datapaths, use_ct_mark,
> flow_table,
>> +                                  ids);
>>      }
>>  }
>>
>> diff --git a/controller/lflow.h b/controller/lflow.h
>> index a9a0f7506..8175bdbea 100644
>> --- a/controller/lflow.h
>> +++ b/controller/lflow.h
>> @@ -112,6 +112,7 @@ struct lflow_ctx_in {
>>      const struct hmap *dhcp_opts;
>>      const struct hmap *dhcpv6_opts;
>>      const struct controller_event_options *controller_event_opts;
>> +    const struct smap *template_vars;
>>      bool lb_hairpin_use_ct_mark;
>>  };
>>
>> diff --git a/controller/lport.c b/controller/lport.c
>> index add7e91aa..f12ce4e63 100644
>> --- a/controller/lport.c
>> +++ b/controller/lport.c
>> @@ -134,8 +134,7 @@ lport_can_bind_on_this_chassis(const struct
> sbrec_chassis *chassis_rec,
>>      enum can_bind can_bind = CAN_BIND_AS_MAIN;
>>      for (chassis = strtok_r(tokstr, ",", &save_ptr); chassis != NULL;
>>           chassis = strtok_r(NULL, ",", &save_ptr)) {
>> -        if (!strcmp(chassis, chassis_rec->name)
>> -                || !strcmp(chassis, chassis_rec->hostname)) {
>> +        if (chassis_name_equals(chassis, chassis_rec)) {
>>              free(tokstr);
>>              return can_bind;
>>          }
>> diff --git a/controller/ofctrl.c b/controller/ofctrl.c
>> index c77991258..bf161dfde 100644
>> --- a/controller/ofctrl.c
>> +++ b/controller/ofctrl.c
>> @@ -2873,7 +2873,8 @@ ofctrl_lookup_port(const void *br_int_, const char
> *port_name,
>>  char *
>>  ofctrl_inject_pkt(const struct ovsrec_bridge *br_int, const char *flow_s,
>>                    const struct shash *addr_sets,
>> -                  const struct shash *port_groups)
>> +                  const struct shash *port_groups,
>> +                  const struct smap *template_vars)
>>  {
>>      int version = rconn_get_version(swconn);
>>      if (version < 0) {
>> @@ -2881,9 +2882,13 @@ ofctrl_inject_pkt(const struct ovsrec_bridge
> *br_int, const char *flow_s,
>>      }
>>
>>      struct flow uflow;
>> -    char *error = expr_parse_microflow(flow_s, &symtab, addr_sets,
>> +    char *flow_exp_s = lexer_parse_template_string(xstrdup(flow_s),
>> +                                                   template_vars,
>> +                                                   NULL);
>> +    char *error = expr_parse_microflow(flow_exp_s, &symtab, addr_sets,
>>                                         port_groups, ofctrl_lookup_port,
>>                                         br_int, &uflow);
>> +    free(flow_exp_s);
>>      if (error) {
>>          return error;
>>      }
>> diff --git a/controller/ofctrl.h b/controller/ofctrl.h
>> index 71d3f5838..396824512 100644
>> --- a/controller/ofctrl.h
>> +++ b/controller/ofctrl.h
>> @@ -71,7 +71,8 @@ void ofctrl_ct_flush_zone(uint16_t zone_id);
>>
>>  char *ofctrl_inject_pkt(const struct ovsrec_bridge *br_int,
>>                          const char *flow_s, const struct shash
> *addr_sets,
>> -                        const struct shash *port_groups);
>> +                        const struct shash *port_groups,
>> +                        const struct smap *template_vars);
>>
>>  /* Flow table interfaces to the rest of ovn-controller. */
>>
>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> index 32a24a076..40afebf4e 100644
>> --- a/controller/ovn-controller.c
>> +++ b/controller/ovn-controller.c
>> @@ -91,6 +91,7 @@ static unixctl_cb_func debug_pause_execution;
>>  static unixctl_cb_func debug_resume_execution;
>>  static unixctl_cb_func debug_status_execution;
>>  static unixctl_cb_func debug_dump_local_bindings;
>> +static unixctl_cb_func debug_dump_local_template_vars;
>>  static unixctl_cb_func debug_dump_lflow_conj_ids;
>>  static unixctl_cb_func lflow_cache_flush_cmd;
>>  static unixctl_cb_func lflow_cache_show_stats_cmd;
>> @@ -170,6 +171,8 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
>>       *
>>       * Monitor IGMP_Groups for local chassis.
>>       *
>> +     * Monitor Template_Var for local chassis.
>> +     *
>>       * We always monitor patch ports because they allow us to see the
> linkages
>>       * between related logical datapaths.  That way, when we know that
> we have
>>       * a VIF on a particular logical switch, we immediately know to
> monitor all
>> @@ -184,6 +187,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
>>      struct ovsdb_idl_condition ip_mcast =
> OVSDB_IDL_CONDITION_INIT(&ip_mcast);
>>      struct ovsdb_idl_condition igmp = OVSDB_IDL_CONDITION_INIT(&igmp);
>>      struct ovsdb_idl_condition chprv = OVSDB_IDL_CONDITION_INIT(&chprv);
>> +    struct ovsdb_idl_condition tv = OVSDB_IDL_CONDITION_INIT(&tv);
>>
>>      /* Always monitor all logical datapath groups. Otherwise, DPG
> updates may
>>       * be received *after* the lflows using it are seen by
> ovn-controller.
>> @@ -201,6 +205,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
>>          ovsdb_idl_condition_add_clause_true(&ip_mcast);
>>          ovsdb_idl_condition_add_clause_true(&igmp);
>>          ovsdb_idl_condition_add_clause_true(&chprv);
>> +        ovsdb_idl_condition_add_clause_true(&tv);
>>          goto out;
>>      }
>>
>> @@ -239,6 +244,11 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
>>          /* Monitors Chassis_Private record for current chassis only. */
>>          sbrec_chassis_private_add_clause_name(&chprv, OVSDB_F_EQ,
>>                                                chassis->name);
>> +
>> +        sbrec_template_var_add_clause_chassis_name(&tv, OVSDB_F_EQ,
>> +                                                   chassis->name);
>> +        sbrec_template_var_add_clause_chassis_name(&tv, OVSDB_F_EQ,
>> +                                                   chassis->hostname);
>>      } else {
>>          /* During initialization, we monitor all records in
> Chassis_Private so
>>           * that we don't try to recreate existing ones. */
>> @@ -290,6 +300,7 @@ out:;
>>          sbrec_ip_multicast_set_condition(ovnsb_idl, &ip_mcast),
>>          sbrec_igmp_group_set_condition(ovnsb_idl, &igmp),
>>          sbrec_chassis_private_set_condition(ovnsb_idl, &chprv),
>> +        sbrec_template_var_set_condition(ovnsb_idl, &tv),
>>      };
>>
>>      unsigned int expected_cond_seqno = 0;
>> @@ -307,6 +318,7 @@ out:;
>>      ovsdb_idl_condition_destroy(&ip_mcast);
>>      ovsdb_idl_condition_destroy(&igmp);
>>      ovsdb_idl_condition_destroy(&chprv);
>> +    ovsdb_idl_condition_destroy(&tv);
>>      return expected_cond_seqno;
>>  }
>>
>> @@ -990,7 +1002,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>>      SB_NODE(load_balancer, "load_balancer") \
>>      SB_NODE(fdb, "fdb") \
>>      SB_NODE(meter, "meter") \
>> -    SB_NODE(static_mac_binding, "static_mac_binding")
>> +    SB_NODE(static_mac_binding, "static_mac_binding") \
>> +    SB_NODE(template_var, "template_var")
>>
>>  enum sb_engine_node {
>>  #define SB_NODE(NAME, NAME_STR) SB_##NAME,
>> @@ -1652,6 +1665,214 @@ runtime_data_sb_datapath_binding_handler(struct
> engine_node *node OVS_UNUSED,
>>      return true;
>>  }
>>
>> +struct local_templates {
>> +    struct uuidset uuids;
>> +    struct smap vars;
>> +};
>> +
>> +static void
>> +local_templates_init(struct local_templates *table)
>> +{
>> +    uuidset_init(&table->uuids);
>> +    smap_init(&table->vars);
>> +}
>> +
>> +static void
>> +local_templates_clear(struct local_templates *table)
>> +{
>> +    uuidset_clear(&table->uuids);
>> +    smap_clear(&table->vars);
>> +}
>> +
>> +static void
>> +local_templates_destroy(struct local_templates *table)
>> +{
>> +    uuidset_destroy(&table->uuids);
>> +    smap_destroy(&table->vars);
>> +}
>> +
>> +static void
>> +local_templates_insert(struct local_templates *table,
>> +                       const char *name, const struct uuid *uuid,
>> +                       const char *value)
>> +{
>> +    uuidset_insert(&table->uuids, uuid);
>> +    smap_replace(&table->vars, name, value);
>> +}
>> +
>> +static void
>> +local_templates_remove(struct local_templates *table,
>> +                       const char *name, const struct uuid *uuid)
>> +{
>> +    uuidset_find_and_delete(&table->uuids, uuid);
>> +    smap_remove(&table->vars, name);
>> +}
>> +
>> +static bool
>> +local_templates_contains_uuid(const struct local_templates *table,
>> +                              const struct uuid *uuid)
>> +{
>> +    return !!uuidset_find(CONST_CAST(struct uuidset *, &table->uuids),
> uuid);
>> +}
>> +
>> +static void
>> +local_templates_to_string(const struct local_templates *table,
>> +                             struct ds *out_data)
>> +{
>> +    const struct smap_node *node;
>> +
>> +    SMAP_FOR_EACH (node, &table->vars) {
>> +        ds_put_format(out_data, "name: '%s' value: '%s'\n",
>> +                      node->key, node->value);
>> +    }
>> +}
>> +
>> +struct ed_type_template_vars {
>> +    struct local_templates var_table;
>> +
>> +    bool change_tracked;
>> +    struct sset new;
>> +    struct sset deleted;
>> +    struct sset updated;
>> +};
>> +
>> +static void
>> +template_vars_init(const struct sbrec_template_var_table *tv_table,
>> +                   const struct sbrec_chassis *chassis,
>> +                   struct local_templates *var_table)
>> +{
>> +    const struct sbrec_template_var *tv;
>> +    SBREC_TEMPLATE_VAR_TABLE_FOR_EACH (tv, tv_table) {
>> +        if (chassis_name_equals(tv->chassis_name, chassis)) {
> 
> I am not sure if it is a good idea to allow using hostname to match the
> template var name. It provides flexibility to CMS, but we will need more
> complexity to protect against corner cases.
> For example, if there are two records:
> r1: name="abc", value="v1"
> r2: hostname="abc", value="v2"
> 
> Now with the current logic, whichever is handled later will take precedence
> (in the local_templates.vars) and the value will be used (assume r2 "v2" is
> used). This may be fine, because the user should be responsible for the
> inconsistent configurations.
> 
> Later, when the user detects the problem and wants to correct the
> configuration. He/she deletes the r2 and expects the var "abc" to be
> expanded as "v1". But the logic in template_vars_update() would call
> local_templates_remove() which simply deletes the var ("abc" -> "v2")
> instead of replacing it with ("abc" -> "v1"). The uuid of "abc" -> "v1"
> will still be left in the uuidset, which is useless. This is an unexpected
> behavior.
> 
> Similar behavior would happen if there are duplicate hostnames, e.g.:
> r1: hostname="abc", value="v1"
> r2: hostname="abc", value="v2"
> 

Very good point, nice catch!

> So, if we really want to support the flexibility, we will need to track the
> duplicated keys, and handle the delete/replace properly, which I feel is a
> little bit overkill for this. I think CMS should be able to always use
> chassis_name. This would simplify the code because chassis_name is part of
> the table key and no need to check redundancy.
> 
> In addition, in either case, I don't see a benefit of maintaining the
> uuidset. Did I miss anything?
> 

The uuidset is a left over from my try to make template variables use a
generic predicate.  I was using it exactly to determine duplicates but
I'll remove it in v2.

I think we should stick to chassis->name as you suggested.  That will
make the code less complex and avoid ambiguities like you described above.

>> +            local_templates_insert(var_table, tv->name,
>> +                                      &tv->header_.uuid, tv->value);
> 
> nit: indent alignment problem.
> 

Ack.

>> +        }
>> +    }
>> +}
>> +
>> +static void
>> +template_vars_update(const struct sbrec_template_var_table *tv_table,
>> +                     const struct sbrec_chassis *chassis,
>> +                     struct local_templates *var_table,
>> +                     struct sset *new, struct sset *deleted,
>> +                     struct sset *updated)
>> +{
>> +    const struct sbrec_template_var *tv;
>> +    SBREC_TEMPLATE_VAR_TABLE_FOR_EACH_TRACKED (tv, tv_table) {
>> +        if (!sbrec_template_var_is_deleted(tv)) {
>> +            continue;
>> +        }
>> +
>> +        if (!local_templates_contains_uuid(var_table,
> &tv->header_.uuid)) {
>> +            continue;
>> +        }
>> +
>> +        local_templates_remove(var_table, tv->name, &tv->header_.uuid);
>> +        sset_add(deleted, tv->name);
>> +    }
>> +
>> +    SBREC_TEMPLATE_VAR_TABLE_FOR_EACH_TRACKED (tv, tv_table) {
>> +        if (sbrec_template_var_is_deleted(tv)) {
>> +            continue;
>> +        }
>> +
>> +        struct sset *tv_set = sbrec_template_var_is_new(tv)
>> +                              ? new : updated;
>> +        if (chassis_name_equals(tv->chassis_name, chassis)) {
>> +            local_templates_insert(var_table, tv->name,
>> +                                      &tv->header_.uuid, tv->value);
> 
> nit: indent alignment problem.
> 

Ack.

> Thanks,
> Han
> 

Thanks for the review!

Regards,
Dumitru

>> +            sset_add(tv_set, tv->name);
>> +        }
>> +    }
>> +}
>> +
>> +static void *
>> +en_template_vars_init(struct engine_node *node OVS_UNUSED,
>> +                      struct engine_arg *arg OVS_UNUSED)
>> +{
>> +    struct ed_type_template_vars *tv_data = xzalloc(sizeof *tv_data);
>> +    local_templates_init(&tv_data->var_table);
>> +    tv_data->change_tracked = false;
>> +    sset_init(&tv_data->new);
>> +    sset_init(&tv_data->deleted);
>> +    sset_init(&tv_data->updated);
>> +    return tv_data;
>> +}
>> +
>> +static void
>> +en_template_vars_run(struct engine_node *node, void *data)
>> +{
>> +    struct ed_type_template_vars *tv_data = data;
>> +
>> +    const struct sbrec_template_var_table *tv_table =
>> +        EN_OVSDB_GET(engine_get_input("SB_template_var", node));
>> +    const struct ovsrec_open_vswitch_table *ovs_table =
>> +        EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
>> +    struct ovsdb_idl_index *sbrec_chassis_by_name =
>> +        engine_ovsdb_node_get_index(engine_get_input("SB_chassis", node),
>> +                                    "name");
>> +    const struct sbrec_chassis *chassis =
>> +        chassis_lookup_by_name(sbrec_chassis_by_name,
>> +                               get_ovs_chassis_id(ovs_table));
>> +
>> +    local_templates_clear(&tv_data->var_table);
>> +
>> +    template_vars_init(tv_table, chassis, &tv_data->var_table);
>> +    engine_set_node_state(node, EN_UPDATED);
>> +}
>> +
>> +static bool
>> +template_vars_sb_template_var_handler(struct engine_node *node, void
> *data)
>> +{
>> +    struct ed_type_template_vars *tv_data = data;
>> +
>> +    const struct sbrec_template_var_table *tv_table =
>> +        EN_OVSDB_GET(engine_get_input("SB_template_var", node));
>> +    const struct ovsrec_open_vswitch_table *ovs_table =
>> +        EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
>> +    struct ovsdb_idl_index *sbrec_chassis_by_name =
>> +        engine_ovsdb_node_get_index(engine_get_input("SB_chassis", node),
>> +                                    "name");
>> +    const struct sbrec_chassis *chassis =
>> +        chassis_lookup_by_name(sbrec_chassis_by_name,
>> +                               get_ovs_chassis_id(ovs_table));
>> +
>> +    template_vars_update(tv_table, chassis, &tv_data->var_table,
>> +                         &tv_data->new, &tv_data->deleted,
> &tv_data->updated);
>> +
>> +    if (!sset_is_empty(&tv_data->new) ||
> !sset_is_empty(&tv_data->deleted) ||
>> +            !sset_is_empty(&tv_data->updated)) {
>> +        engine_set_node_state(node, EN_UPDATED);
>> +    } else {
>> +        engine_set_node_state(node, EN_UNCHANGED);
>> +    }
>> +
>> +    tv_data->change_tracked = true;
>> +    return true;
>> +}
>> +
>> +static void
>> +en_template_vars_clear_tracked_data(void *data)
>> +{
>> +    struct ed_type_template_vars *tv_data = data;
>> +
>> +    sset_clear(&tv_data->new);
>> +    sset_clear(&tv_data->deleted);
>> +    sset_clear(&tv_data->updated);
>> +    tv_data->change_tracked = false;
>> +}
>> +
>> +static void
>> +en_template_vars_cleanup(void *data)
>> +{
>> +    struct ed_type_template_vars *tv_data = data;
>> +    local_templates_destroy(&tv_data->var_table);
>> +
>> +    sset_destroy(&tv_data->new);
>> +    sset_destroy(&tv_data->deleted);
>> +    sset_destroy(&tv_data->updated);
>> +}
>> +
>>  struct ed_type_addr_sets {
>>      struct shash addr_sets;
>>      bool change_tracked;
>> @@ -2706,6 +2927,9 @@ init_lflow_ctx(struct engine_node *node,
>>      struct ed_type_dhcp_options *dhcp_opts =
>>          engine_get_input_data("dhcp_options", node);
>>
>> +    struct ed_type_template_vars *template_vars =
>> +        engine_get_input_data("template_vars", node);
>> +
>>      l_ctx_in->sbrec_multicast_group_by_name_datapath =
>>          sbrec_mc_group_by_name_dp;
>>      l_ctx_in->sbrec_logical_flow_by_logical_datapath =
>> @@ -2738,6 +2962,7 @@ init_lflow_ctx(struct engine_node *node,
>>      l_ctx_in->dhcp_opts = &dhcp_opts->v4_opts;
>>      l_ctx_in->dhcpv6_opts = &dhcp_opts->v6_opts;
>>      l_ctx_in->controller_event_opts = &fo->controller_event_opts;
>> +    l_ctx_in->template_vars = &template_vars->var_table.vars;
>>
>>      l_ctx_out->flow_table = &fo->flow_table;
>>      l_ctx_out->group_table = &fo->group_table;
>> @@ -3064,6 +3289,64 @@ lflow_output_port_groups_handler(struct
> engine_node *node, void *data)
>>      return true;
>>  }
>>
>> +static bool
>> +lflow_output_template_vars_handler(struct engine_node *node, void *data)
>> +{
>> +    struct ed_type_template_vars *tv_data =
>> +        engine_get_input_data("template_vars", node);
>> +
>> +    struct ed_type_lflow_output *fo = data;
>> +    struct lflow_ctx_out l_ctx_out;
>> +    struct lflow_ctx_in l_ctx_in;
>> +    init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out);
>> +
>> +    const char *res_name;
>> +    bool changed;
>> +
>> +    if (!tv_data->change_tracked) {
>> +        return false;
>> +    }
>> +
>> +    SSET_FOR_EACH (res_name, &tv_data->deleted) {
>> +        if (!objdep_mgr_handle_change(l_ctx_out.lflow_deps_mgr,
>> +                                      OBJDEP_TYPE_TEMPLATE,
>> +                                      res_name, lflow_handle_changed_ref,
>> +                                      l_ctx_out.lflows_processed,
>> +                                      &l_ctx_in, &l_ctx_out, &changed)) {
>> +            return false;
>> +        }
>> +        if (changed) {
>> +            engine_set_node_state(node, EN_UPDATED);
>> +        }
>> +    }
>> +    SSET_FOR_EACH (res_name, &tv_data->updated) {
>> +        if (!objdep_mgr_handle_change(l_ctx_out.lflow_deps_mgr,
>> +                                      OBJDEP_TYPE_TEMPLATE,
>> +                                      res_name, lflow_handle_changed_ref,
>> +                                      l_ctx_out.lflows_processed,
>> +                                      &l_ctx_in, &l_ctx_out, &changed)) {
>> +            return false;
>> +        }
>> +        if (changed) {
>> +            engine_set_node_state(node, EN_UPDATED);
>> +        }
>> +    }
>> +    SSET_FOR_EACH (res_name, &tv_data->new) {
>> +        if (!objdep_mgr_handle_change(l_ctx_out.lflow_deps_mgr,
>> +                                      OBJDEP_TYPE_TEMPLATE,
>> +                                      res_name, lflow_handle_changed_ref,
>> +                                      l_ctx_out.lflows_processed,
>> +                                      &l_ctx_in, &l_ctx_out, &changed)) {
>> +            return false;
>> +        }
>> +        if (changed) {
>> +            engine_set_node_state(node, EN_UPDATED);
>> +        }
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>  static bool
>>  lflow_output_runtime_data_handler(struct engine_node *node,
>>                                    void *data OVS_UNUSED)
>> @@ -3711,6 +3994,7 @@ main(int argc, char *argv[])
>>
>>      /* Define inc-proc-engine nodes. */
>>      ENGINE_NODE(sb_ro, "sb_ro");
>> +    ENGINE_NODE_WITH_CLEAR_TRACK_DATA(template_vars, "template_vars");
>>      ENGINE_NODE_WITH_CLEAR_TRACK_DATA_IS_VALID(ct_zones, "ct_zones");
>>      ENGINE_NODE_WITH_CLEAR_TRACK_DATA(ovs_interface_shadow,
>>                                        "ovs_interface_shadow");
>> @@ -3737,6 +4021,11 @@ main(int argc, char *argv[])
>>  #undef OVS_NODE
>>
>>      /* Add dependencies between inc-proc-engine nodes. */
>> +    engine_add_input(&en_template_vars, &en_ovs_open_vswitch,
>> +                     engine_noop_handler);
>> +    engine_add_input(&en_template_vars, &en_sb_chassis,
> engine_noop_handler);
>> +    engine_add_input(&en_template_vars, &en_sb_template_var,
>> +                     template_vars_sb_template_var_handler);
>>
>>      engine_add_input(&en_addr_sets, &en_sb_address_set,
>>                       addr_sets_sb_address_set_handler);
>> @@ -3804,6 +4093,8 @@ main(int argc, char *argv[])
>>                       lflow_output_addr_sets_handler);
>>      engine_add_input(&en_lflow_output, &en_port_groups,
>>                       lflow_output_port_groups_handler);
>> +    engine_add_input(&en_lflow_output, &en_template_vars,
>> +                     lflow_output_template_vars_handler);
>>      engine_add_input(&en_lflow_output, &en_runtime_data,
>>                       lflow_output_runtime_data_handler);
>>      engine_add_input(&en_lflow_output, &en_non_vif_data,
>> @@ -3921,6 +4212,8 @@ main(int argc, char *argv[])
>>          engine_get_internal_data(&en_ct_zones);
>>      struct ed_type_runtime_data *runtime_data =
>>          engine_get_internal_data(&en_runtime_data);
>> +    struct ed_type_template_vars *template_vars_data =
>> +        engine_get_internal_data(&en_template_vars);
>>
>>      ofctrl_init(&lflow_output_data->group_table,
>>                  &lflow_output_data->meter_table,
>> @@ -3982,6 +4275,10 @@ main(int argc, char *argv[])
>>                               debug_dump_lflow_conj_ids,
>>                               &lflow_output_data->conj_ids);
>>
>> +    unixctl_command_register("debug/dump-local-template-vars", "", 0, 0,
>> +                             debug_dump_local_template_vars,
>> +                             &template_vars_data->var_table);
>> +
>>      unixctl_command_register("debug/ignore-startup-delay", "", 0, 0,
>>                               debug_ignore_startup_delay, NULL);
>>
>> @@ -4358,9 +4655,12 @@ main(int argc, char *argv[])
>>                      engine_get_data(&en_addr_sets);
>>                  struct ed_type_port_groups *pg_data =
>>                      engine_get_data(&en_port_groups);
>> -                if (br_int && chassis && as_data && pg_data) {
>> +                struct ed_type_template_vars *tv_data =
>> +                    engine_get_data(&en_template_vars);
>> +                if (br_int && chassis && as_data && pg_data && tv_data) {
>>                      char *error = ofctrl_inject_pkt(br_int,
> pending_pkt.flow_s,
>> -                        &as_data->addr_sets,
> &pg_data->port_groups_cs_local);
>> +                        &as_data->addr_sets,
> &pg_data->port_groups_cs_local,
>> +                        &tv_data->var_table.vars);
>>                      if (error) {
>>                          unixctl_command_reply_error(pending_pkt.conn,
> error);
>>                          free(error);
>> @@ -4829,6 +5129,17 @@ debug_dump_lflow_conj_ids(struct unixctl_conn
> *conn, int argc OVS_UNUSED,
>>      ds_destroy(&conj_ids_dump);
>>  }
>>
>> +static void
>> +debug_dump_local_template_vars(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
>> +                               const char *argv[] OVS_UNUSED, void
> *local_vars)
>> +{
>> +    struct ds tv_str = DS_EMPTY_INITIALIZER;
>> +    ds_put_cstr(&tv_str, "Local template vars:\n");
>> +    local_templates_to_string(local_vars, &tv_str);
>> +    unixctl_command_reply(conn, ds_cstr(&tv_str));
>> +    ds_destroy(&tv_str);
>> +}
>> +
>>  static void
>>  debug_ignore_startup_delay(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
>>                             const char *argv[] OVS_UNUSED, void *arg
> OVS_UNUSED)
>> diff --git a/include/ovn/expr.h b/include/ovn/expr.h
>> index 3b141b034..80d95ff67 100644
>> --- a/include/ovn/expr.h
>> +++ b/include/ovn/expr.h
>> @@ -59,6 +59,7 @@
>>  #include "openvswitch/match.h"
>>  #include "openvswitch/meta-flow.h"
>>  #include "logical-fields.h"
>> +#include "smap.h"
>>
>>  struct ds;
>>  struct expr;
>> @@ -521,7 +522,8 @@ union expr_constant {
>>      char *string;
>>  };
>>
>> -bool expr_constant_parse(struct lexer *, const struct expr_field *,
>> +bool expr_constant_parse(struct lexer *,
>> +                         const struct expr_field *,
>>                           union expr_constant *);
>>  void expr_constant_format(const union expr_constant *,
>>                            enum expr_constant_type, struct ds *);
>> diff --git a/include/ovn/lex.h b/include/ovn/lex.h
>> index ecb7ace24..38c4aa9f3 100644
>> --- a/include/ovn/lex.h
>> +++ b/include/ovn/lex.h
>> @@ -23,6 +23,8 @@
>>   * This is a simple lexical analyzer (or tokenizer) for OVN match
> expressions
>>   * and ACLs. */
>>
>> +#include "smap.h"
>> +#include "sset.h"
>>  #include "openvswitch/meta-flow.h"
>>
>>  struct ds;
>> @@ -37,7 +39,8 @@ enum lex_type {
>>      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_PORT_GROUP,            /* @NAME */
>> +    LEX_T_PORT_GROUP,           /* @NAME */
>> +    LEX_T_TEMPLATE,             /* ^NAME */
>>      LEX_T_ERROR,                /* invalid input */
>>
>>      /* Bare tokens. */
>> @@ -86,9 +89,9 @@ struct lex_token {
>>      /* One of LEX_*. */
>>      enum lex_type type;
>>
>> -    /* Meaningful for LEX_T_ID, LEX_T_STRING, LEX_T_ERROR, LEX_T_MACRO
> only.
>> -     * For these token types, 's' may point to 'buffer'; otherwise, it
> points
>> -     * to malloc()ed memory owned by the token.
>> +    /* Meaningful for LEX_T_ID, LEX_T_STRING, LEX_T_ERROR, LEX_T_MACRO,
>> +     * LEX_T_TEMPLATE only.  For these token types, 's' may point to
> 'buffer';
>> +     * otherwise, it points to malloc()ed memory owned by the token.
>>       *
>>       * Must be NULL for other token types.
>>       *
>> @@ -151,4 +154,7 @@ void lexer_syntax_error(struct lexer *, const char
> *message, ...)
>>
>>  char *lexer_steal_error(struct lexer *);
>>
>> +char *lexer_parse_template_string(char *s, const struct smap
> *template_vars,
>> +                                  struct sset *template_vars_ref);
>> +
>>  #endif /* ovn/lex.h */
>> diff --git a/lib/actions.c b/lib/actions.c
>> index adbb42db4..d7d486883 100644
>> --- a/lib/actions.c
>> +++ b/lib/actions.c
>> @@ -258,8 +258,8 @@ add_prerequisite(struct action_context *ctx, const
> char *prerequisite)
>>      struct expr *expr;
>>      char *error;
>>
>> -    expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
>> -                             NULL, NULL, 0, &error);
>> +    expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
> NULL,
>> +                             NULL, 0, &error);
>>      ovs_assert(!error);
>>      ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
>>  }
>> @@ -4695,6 +4695,11 @@ parse_set_action(struct action_context *ctx)
>>  static bool
>>  parse_action(struct action_context *ctx)
>>  {
>> +    if (ctx->lexer->token.type == LEX_T_TEMPLATE) {
>> +        lexer_error(ctx->lexer, "Unexpanded template.");
>> +        return false;
>> +    }
>> +
>>      if (ctx->lexer->token.type != LEX_T_ID) {
>>          lexer_syntax_error(ctx->lexer, NULL);
>>          return false;
>> diff --git a/lib/expr.c b/lib/expr.c
>> index d1f9d28ca..43f44771c 100644
>> --- a/lib/expr.c
>> +++ b/lib/expr.c
>> @@ -695,7 +695,9 @@ parse_field(struct expr_context *ctx, struct
> expr_field *f)
>>          return false;
>>      }
>>
>> -    symbol = shash_find_data(ctx->symtab, ctx->lexer->token.s);
>> +    symbol = ctx->symtab
>> +             ? shash_find_data(ctx->symtab, ctx->lexer->token.s)
>> +             : NULL;
>>      if (!symbol) {
>>          lexer_syntax_error(ctx->lexer, "expecting field name");
>>          return false;
>> @@ -894,7 +896,10 @@ parse_constant(struct expr_context *ctx, struct
> expr_constant_set *cs,
>>          cs->as_name = NULL;
>>      }
>>
>> -    if (ctx->lexer->token.type == LEX_T_STRING) {
>> +    if (ctx->lexer->token.type == LEX_T_TEMPLATE) {
>> +        lexer_error(ctx->lexer, "Unexpanded template.");
>> +        return false;
>> +    } else if (ctx->lexer->token.type == LEX_T_STRING) {
>>          if (!assign_constant_set_type(ctx, cs, EXPR_C_STRING)) {
>>              return false;
>>          }
>> @@ -978,7 +983,9 @@ expr_constant_parse(struct lexer *lexer, const struct
> expr_field *f,
>>          return false;
>>      }
>>
>> -    struct expr_context ctx = { .lexer = lexer };
>> +    struct expr_context ctx = {
>> +        .lexer = lexer,
>> +    };
>>
>>      struct expr_constant_set cs;
>>      memset(&cs, 0, sizeof cs);
>> @@ -1332,7 +1339,10 @@ expr_parse_primary(struct expr_context *ctx, bool
> *atomic)
>>          return e;
>>      }
>>
>> -    if (ctx->lexer->token.type == LEX_T_ID) {
>> +    if (ctx->lexer->token.type == LEX_T_TEMPLATE) {
>> +        lexer_error(ctx->lexer, "Unexpanded template.");
>> +        return NULL;
>> +    } else if (ctx->lexer->token.type == LEX_T_ID) {
>>          struct expr_field f;
>>          enum expr_relop r;
>>          struct expr_constant_set c;
>> diff --git a/lib/lex.c b/lib/lex.c
>> index c84d52aa8..b01a7d75c 100644
>> --- a/lib/lex.c
>> +++ b/lib/lex.c
>> @@ -235,6 +235,10 @@ lex_token_format(const struct lex_token *token,
> struct ds *s)
>>          ds_put_format(s, "@%s", token->s);
>>          break;
>>
>> +    case LEX_T_TEMPLATE:
>> +        ds_put_format(s, "^%s", token->s);
>> +        break;
>> +
>>      case LEX_T_LPAREN:
>>          ds_put_cstr(s, "(");
>>          break;
>> @@ -588,6 +592,18 @@ lex_parse_port_group(const char *p, struct lex_token
> *token)
>>      return lex_parse_id(p, LEX_T_PORT_GROUP, token);
>>  }
>>
>> +static const char *
>> +lex_parse_template(const char *p, struct lex_token *token)
>> +{
>> +    p++;
>> +    if (!lex_is_id1(*p)) {
>> +        lex_error(token, "`^' must be followed by a valid identifier.");
>> +        return p;
>> +    }
>> +
>> +    return lex_parse_id(p, LEX_T_TEMPLATE, token);
>> +}
>> +
>>  /* 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'.
>> @@ -766,6 +782,10 @@ next:
>>          p = lex_parse_port_group(p, token);
>>          break;
>>
>> +    case '^':
>> +        p = lex_parse_template(p, token);
>> +        break;
>> +
>>      case ':':
>>          if (p[1] != ':') {
>>              token->type = LEX_T_COLON;
>> @@ -1031,3 +1051,38 @@ lexer_steal_error(struct lexer *lexer)
>>      lexer->error = NULL;
>>      return error;
>>  }
>> +
>> +/* Takes ownership of 's' and expands all templates that are encountered
>> + * in the contents of 's', if possible.  Adds the encountered template
> names
>> + * to 'template_vars_ref'.
>> + */
>> +char *
>> +lexer_parse_template_string(char *s, const struct smap *template_vars,
>> +                            struct sset *template_vars_ref)
>> +{
>> +    /* No '^' means no templates. */
>> +    if (!strchr(s, '^')) {
>> +        return s;
>> +    }
>> +
>> +    struct ds expanded = DS_EMPTY_INITIALIZER;
>> +
>> +    struct lexer lexer;
>> +    lexer_init(&lexer, s);
>> +
>> +    while (lexer_get(&lexer) != LEX_T_END) {
>> +        if (lexer.token.type == LEX_T_TEMPLATE) {
>> +            ds_put_cstr(&expanded, smap_get_def(template_vars,
> lexer.token.s,
>> +                                                lexer.token.s));
>> +            if (template_vars_ref) {
>> +                sset_add(template_vars_ref, lexer.token.s);
>> +            }
>> +        } else {
>> +            lex_token_format(&lexer.token, &expanded);
>> +        }
>> +    }
>> +
>> +    lexer_destroy(&lexer);
>> +    free(s);
>> +    return ds_steal_cstr(&expanded);
>> +}
>> \ No newline at end of file
>> diff --git a/lib/objdep.c b/lib/objdep.c
>> index 0224a5d06..0e62b88a9 100644
>> --- a/lib/objdep.c
>> +++ b/lib/objdep.c
>> @@ -246,6 +246,7 @@ objdep_type_name(enum objdep_type type)
>>          [OBJDEP_TYPE_PORTGROUP] = "Port_Group",
>>          [OBJDEP_TYPE_PORTBINDING] = "Port_Binding",
>>          [OBJDEP_TYPE_MC_GROUP] = "Multicast_Group",
>> +        [OBJDEP_TYPE_TEMPLATE] = "Template",
>>      };
>>
>>      ovs_assert(type < OBJDEP_TYPE_MAX);
>> diff --git a/lib/objdep.h b/lib/objdep.h
>> index 059a11a62..b96c98163 100644
>> --- a/lib/objdep.h
>> +++ b/lib/objdep.h
>> @@ -26,6 +26,7 @@ enum objdep_type {
>>      OBJDEP_TYPE_PORTGROUP,
>>      OBJDEP_TYPE_PORTBINDING,
>>      OBJDEP_TYPE_MC_GROUP,
>> +    OBJDEP_TYPE_TEMPLATE,
>>      OBJDEP_TYPE_MAX,
>>  };
>>
>> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
>> index d80db179a..596e93129 100644
>> --- a/lib/ovn-util.c
>> +++ b/lib/ovn-util.c
>> @@ -615,6 +615,13 @@ ovn_logical_flow_hash_datapath(const struct uuid
> *logical_datapath,
>>      return hash_add(hash, uuid_hash(logical_datapath));
>>  }
>>
>> +bool
>> +chassis_name_equals(const char *chassis_name,
>> +                    const struct sbrec_chassis *target)
>> +{
>> +    return !strcmp(chassis_name, target->name) ||
>> +        !strcmp(chassis_name, target->hostname);
>> +}
>>
>>  struct tnlid_node {
>>      struct hmap_node hmap_node;
>> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
>> index 145f974ed..c53d5a19a 100644
>> --- a/lib/ovn-util.h
>> +++ b/lib/ovn-util.h
>> @@ -29,6 +29,7 @@
>>  #define ROUTE_ORIGIN_STATIC "static"
>>
>>  struct nbrec_logical_router_port;
>> +struct sbrec_chassis;
>>  struct sbrec_logical_flow;
>>  struct svec;
>>  struct uuid;
>> @@ -119,6 +120,8 @@ uint32_t ovn_logical_flow_hash(uint8_t table_id, enum
> ovn_pipeline pipeline,
>>                                 const char *match, const char *actions);
>>  uint32_t ovn_logical_flow_hash_datapath(const struct uuid
> *logical_datapath,
>>                                          uint32_t hash);
>> +bool chassis_name_equals(const char *chassis_name,
>> +                         const struct sbrec_chassis *target);
>>  void ovn_conn_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
>>                     const char *argv[] OVS_UNUSED, void *idl_);
>>
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index 0dc23dbd2..ee9c7f55f 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -259,7 +259,7 @@ fe:x => error("Invalid numeric constant.")
>>  & => error("`&' is only valid as part of `&&'.")
>>  | => error("`|' is only valid as part of `||'.")
>>
>> -^ => error("Invalid character `^' in input.")
>> +^ => error("`^' must be followed by a valid identifier.")
>>  ])
>>  AT_CAPTURE_FILE([input.txt])
>>  sed 's/ =>.*//' test-cases.txt > input.txt
>> diff --git a/tests/test-ovn.c b/tests/test-ovn.c
>> index a241f150d..7842cd915 100644
>> --- a/tests/test-ovn.c
>> +++ b/tests/test-ovn.c
>> @@ -291,12 +291,14 @@ test_parse_expr__(int steps)
>>      struct shash symtab;
>>      struct shash addr_sets;
>>      struct shash port_groups;
>> +    struct smap template_vars;
>>      struct simap ports;
>>      struct ds input;
>>
>>      create_symtab(&symtab);
>>      create_addr_sets(&addr_sets);
>>      create_port_groups(&port_groups);
>> +    smap_init(&template_vars);
>>
>>      simap_init(&ports);
>>      simap_put(&ports, "eth0", 5);
>> @@ -355,6 +357,7 @@ test_parse_expr__(int steps)
>>      shash_destroy(&addr_sets);
>>      expr_const_sets_destroy(&port_groups);
>>      shash_destroy(&port_groups);
>> +    smap_destroy(&template_vars);
>>  }
>>
>>  static void
>> @@ -913,8 +916,8 @@ test_tree_shape_exhaustively(struct expr *expr,
> struct shash *symtab,
>>              expr_format(expr, &s);
>>
>>              char *error;
>> -            modified = expr_parse_string(ds_cstr(&s), symtab, NULL,
>> -                                         NULL, NULL, NULL, 0, &error);
>> +            modified = expr_parse_string(ds_cstr(&s), symtab, NULL,
> NULL, NULL,
>> +                                         NULL, 0, &error);
>>              if (error) {
>>                  fprintf(stderr, "%s fails to parse (%s)\n",
>>                          ds_cstr(&s), error);
>> @@ -1286,6 +1289,8 @@ test_parse_actions(struct ovs_cmdl_context *ctx
> OVS_UNUSED)
>>      struct ds input;
>>      bool ok = true;
>>
>> +    struct smap template_vars = SMAP_INITIALIZER(&template_vars);
>> +
>>      create_symtab(&symtab);
>>      create_gen_opts(&dhcp_opts, &dhcpv6_opts, &nd_ra_opts, &event_opts);
>>
>> @@ -1322,7 +1327,10 @@ test_parse_actions(struct ovs_cmdl_context *ctx
> OVS_UNUSED)
>>              .n_tables = 24,
>>              .cur_ltable = 10,
>>          };
>> -        error = ovnacts_parse_string(ds_cstr(&input), &pp, &ovnacts,
> &prereqs);
>> +        char *exp_input =
> lexer_parse_template_string(xstrdup(ds_cstr(&input)),
>> +                                                      &template_vars,
>> +                                                      NULL);
>> +        error = ovnacts_parse_string(exp_input, &pp, &ovnacts, &prereqs);
>>          if (!error) {
>>              /* Convert the parsed representation back to a string and
> print it,
>>               * if it's different from the input. */
>> @@ -1409,6 +1417,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx
> OVS_UNUSED)
>>          expr_destroy(prereqs);
>>          ovnacts_free(ovnacts.data, ovnacts.size);
>>          ofpbuf_uninit(&ovnacts);
>> +        free(exp_input);
>>      }
>>      ds_destroy(&input);
>>
>> @@ -1419,6 +1428,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx
> OVS_UNUSED)
>>      dhcp_opts_destroy(&dhcpv6_opts);
>>      nd_ra_opts_destroy(&nd_ra_opts);
>>      controller_event_opts_destroy(&event_opts);
>> +    smap_destroy(&template_vars);
>>      ovn_extend_table_destroy(&group_table);
>>      ovn_extend_table_destroy(&meter_table);
>>      exit(ok ? EXIT_SUCCESS : EXIT_FAILURE);
>> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
>> index d9e7129d9..112dec2de 100644
>> --- a/utilities/ovn-trace.c
>> +++ b/utilities/ovn-trace.c
>> @@ -505,6 +505,7 @@ static struct hmap dhcp_opts;   /* Contains "struct
> gen_opts_map"s. */
>>  static struct hmap dhcpv6_opts; /* Contains "struct gen_opts_map"s. */
>>  static struct hmap nd_ra_opts; /* Contains "struct gen_opts_map"s. */
>>  static struct controller_event_options event_opts;
>> +static struct smap template_vars;
>>
>>  static struct ovntrace_datapath *
>>  ovntrace_datapath_find_by_sb_uuid(const struct uuid *sb_uuid)
>> @@ -955,9 +956,13 @@ parse_lflow_for_datapath(const struct
> sbrec_logical_flow *sblf,
>>
>>          char *error;
>>          struct expr *match;
>> -        match = expr_parse_string(sblf->match, &symtab, &address_sets,
>> +        char *match_s = lexer_parse_template_string(xstrdup(sblf->match),
>> +                                                    &template_vars,
>> +                                                    NULL);
>> +        match = expr_parse_string(match_s, &symtab, &address_sets,
>>                                    &port_groups, NULL, NULL,
> dp->tunnel_key,
>>                                    &error);
>> +        free(match_s);
>>          if (error) {
>>              VLOG_WARN("%s: parsing expression failed (%s)",
>>                        sblf->match, error);
>> @@ -980,7 +985,11 @@ parse_lflow_for_datapath(const struct
> sbrec_logical_flow *sblf,
>>          uint64_t stub[1024 / 8];
>>          struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(stub);
>>          struct expr *prereqs;
>> -        error = ovnacts_parse_string(sblf->actions, &pp, &ovnacts,
> &prereqs);
>> +        char *actions_s =
> lexer_parse_template_string(xstrdup(sblf->actions),
>> +                                                      &template_vars,
>> +                                                      NULL);
>> +        error = ovnacts_parse_string(actions_s, &pp, &ovnacts, &prereqs);
>> +        free(actions_s);
>>          if (error) {
>>              VLOG_WARN("%s: parsing actions failed (%s)", sblf->actions,
> error);
>>              free(error);
>> @@ -1078,6 +1087,7 @@ read_gen_opts(void)
>>      nd_ra_opts_init(&nd_ra_opts);
>>
>>      controller_event_opts_init(&event_opts);
>> +    smap_init(&template_vars);
>>  }
>>
>>  static void
>> @@ -3428,9 +3438,13 @@ trace_parse(const char *dp_s, const char *flow_s,
>>           *
>>           * First make sure that the expression parses. */
>>          char *error;
>> -        struct expr *e = expr_parse_string(flow_s, &symtab,
> &address_sets,
>> +        char *flow_exp_s = lexer_parse_template_string(xstrdup(flow_s),
>> +                                                       &template_vars,
>> +                                                       NULL);
>> +        struct expr *e = expr_parse_string(flow_exp_s, &symtab,
> &address_sets,
>>                                             &port_groups, NULL, NULL, 0,
>>                                             &error);
>> +        free(flow_exp_s);
>>          if (!e) {
>>              return trace_parse_error(error);
>>          }
>> @@ -3455,9 +3469,13 @@ trace_parse(const char *dp_s, const char *flow_s,
>>          free(port_name);
>>      }
>>
>> -    char *error = expr_parse_microflow(flow_s, &symtab, &address_sets,
>> +    char *flow_exp_s = lexer_parse_template_string(xstrdup(flow_s),
>> +                                                   &template_vars,
>> +                                                   NULL);
>> +    char *error = expr_parse_microflow(flow_exp_s, &symtab,
> &address_sets,
>>                                         &port_groups,
> ovntrace_lookup_port,
>>                                         *dpp, uflow);
>> +    free(flow_exp_s);
>>      if (error) {
>>          return trace_parse_error(error);
>>      }
>>
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to