On Fri, Oct 7, 2022 at 1:26 AM Dumitru Ceara <[email protected]> wrote:
>
> 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);
Hi Dumitru, sorry that I forgot to add one more comment here. For the above
two noop handler usages, I think they should use NULL - to trigger
recompute. I don't see any obvious reason of using noop handler here.
noop handler is used only if we know there is dependency for an input but
for some reason we can safely ignore the changes of that input. If that's
the case, let's add some documentation to clarify. If it is because we know
that these changes will trigger recompute at other nodes, it shouldn't be a
reason to use noop handler, and we should simply use NULL.
Thanks,
Han
> >> + 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
