| Message ID | 166759989186.515460.7366980327085014395.stgit@dceara.remote.csb |
|---|---|
| State | Changes Requested |
| Delegated to: | Mark Michelson |
| Headers | show |
| Series | Add OVN component templates. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
Hi Dumitru, I have a few comments down below On 11/4/22 18:11, Dumitru Ceara 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 Chassis_Template_Var > also track references (similar to the ones maintained for > multicast groups, address sets, port_groups, port bindings). > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > V2: > - Fix GCC build due to missing newline. > - Handle SB table rename Template_Var -> Chassis_Template_Var. > - Address Han's comments: > - Add new function to parse lflow actions. > - Move xstrdup inside lexer_parse_template_string() and execute it only > if needed. > - Match template vars only by chassis name. > - Change local_templates table to a plain smap. > - Some indentation updates. > - Use NULL template variable change handlers for chassis/Open_vSwitch > changes. > - Added more tests. > - Fix tracking of template references in lflow actions. > --- > controller/lflow.c | 136 ++++++++++++++------ > controller/lflow.h | 1 > controller/ofctrl.c | 11 +- > controller/ofctrl.h | 3 > controller/ovn-controller.c | 289 +++++++++++++++++++++++++++++++++++++++++++ > include/ovn/expr.h | 4 - > include/ovn/lex.h | 15 ++ > lib/actions.c | 9 + > lib/expr.c | 18 ++- > lib/lex.c | 57 ++++++++ > lib/objdep.c | 1 > lib/objdep.h | 1 > tests/ovn-controller.at | 50 +++++++ > tests/ovn.at | 80 ++++++++++++ > tests/test-ovn.c | 17 ++- > utilities/ovn-trace.c | 33 ++++- > 16 files changed, 658 insertions(+), 67 deletions(-) > > diff --git a/controller/lflow.c b/controller/lflow.c > index d4434bdee8..fc4371d0df 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 *, > @@ -297,6 +299,43 @@ as_info_from_expr_const(const char *as_name, const union expr_constant *c, > return true; > } > > +static bool > +lflow_parse_actions(const struct sbrec_logical_flow *lflow, > + const struct lflow_ctx_in *l_ctx_in, > + struct sset *template_vars_ref, > + struct ofpbuf *ovnacts_out, > + struct expr **prereqs_out) > +{ > + bool ingress = !strcmp(lflow->pipeline, "ingress"); > + struct ovnact_parse_params pp = { > + .symtab = &symtab, > + .dhcp_opts = l_ctx_in->dhcp_opts, > + .dhcpv6_opts = l_ctx_in->dhcpv6_opts, > + .nd_ra_opts = l_ctx_in->nd_ra_opts, > + .controller_event_opts = l_ctx_in->controller_event_opts, > + > + .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, > + .n_tables = LOG_PIPELINE_LEN, > + .cur_ltable = lflow->table_id, > + }; > + > + char *actions_expanded_s = NULL; > + const char *actions_s = > + lexer_parse_template_string(lflow->actions, l_ctx_in->template_vars, > + template_vars_ref, &actions_expanded_s); > + char *error = ovnacts_parse_string(actions_s, &pp, > + ovnacts_out, prereqs_out); > + free(actions_expanded_s); > + if (error) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", > + lflow->actions, error); > + free(error); > + return false; > + } > + return true; > +} > + > /* Parses the lflow regarding the changed address set 'as_name', and generates > * ovs flows for the newly added addresses in 'as_diff_added' only. It is > * similar to consider_logical_flow__, with the below differences: > @@ -347,27 +386,14 @@ consider_lflow_for_added_as_ips__( > > uint64_t ovnacts_stub[1024 / 8]; > struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(ovnacts_stub); > - struct ovnact_parse_params pp = { > - .symtab = &symtab, > - .dhcp_opts = l_ctx_in->dhcp_opts, > - .dhcpv6_opts = l_ctx_in->dhcpv6_opts, > - .nd_ra_opts = l_ctx_in->nd_ra_opts, > - .controller_event_opts = l_ctx_in->controller_event_opts, > - .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, > - .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); > - if (error) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", > - lflow->actions, error); > - free(error); > + if (!lflow_parse_actions(lflow, l_ctx_in, &template_vars_ref, > + &ovnacts, &prereqs)) { > ovnacts_free(ovnacts.data, ovnacts.size); > ofpbuf_uninit(&ovnacts); > + sset_destroy(&template_vars_ref); > return true; > } > > @@ -431,6 +457,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) { > @@ -502,6 +530,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); > + } The loop above seems to be an inlined implementation of store_lflow_template_refs(). I think you should just call that function instead. > + sset_destroy(&template_vars_ref); > + > return handled; > } > > @@ -913,6 +949,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) > { > @@ -920,11 +958,18 @@ 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_expanded_s = NULL; > + const char *match_s = lexer_parse_template_string(lflow->match, > + template_vars, > + template_vars_ref, > + &match_expanded_s); > + 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_expanded_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, > @@ -963,6 +1008,18 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, > return expr_simplify(e); > } > > +static void > +store_lflow_template_refs(struct objdep_mgr *lflow_deps_mgr, > + const struct sset *template_vars_ref, > + const struct sbrec_logical_flow *lflow) > +{ > + const char *tv_name; > + SSET_FOR_EACH (tv_name, template_vars_ref) { > + objdep_mgr_add(lflow_deps_mgr, OBJDEP_TYPE_TEMPLATE, tv_name, > + &lflow->header_.uuid); > + } > +} > + > static void > consider_logical_flow__(const struct sbrec_logical_flow *lflow, > const struct sbrec_datapath_binding *dp, > @@ -1015,28 +1072,16 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, > * XXX Deny changes to 'outport' in egress pipeline. */ > uint64_t ovnacts_stub[1024 / 8]; > struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(ovnacts_stub); > - struct ovnact_parse_params pp = { > - .symtab = &symtab, > - .dhcp_opts = l_ctx_in->dhcp_opts, > - .dhcpv6_opts = l_ctx_in->dhcpv6_opts, > - .nd_ra_opts = l_ctx_in->nd_ra_opts, > - .controller_event_opts = l_ctx_in->controller_event_opts, > - > - .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, > - .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); > - if (error) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", > - lflow->actions, error); > - free(error); > + if (!lflow_parse_actions(lflow, l_ctx_in, &template_vars_ref, > + &ovnacts, &prereqs)) { > ovnacts_free(ovnacts.data, ovnacts.size); > ofpbuf_uninit(&ovnacts); > + store_lflow_template_refs(l_ctx_out->lflow_deps_mgr, > + &template_vars_ref, lflow); Can you explain why we call store_lflow_template_refs() in the case where lflow_parse_actions() fails? This is different from the behavior in consider_lflow_for_added_as_ips__(), where store_lflow_template_refs() is only called when lflow_parse_actions() succeeds. > + sset_destroy(&template_vars_ref); > return; > } > > @@ -1087,6 +1132,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) { > @@ -1101,11 +1148,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); > } > > @@ -1190,6 +1239,10 @@ done: > expr_destroy(cached_expr); > expr_matches_destroy(matches); > free(matches); > + > + store_lflow_template_refs(l_ctx_out->lflow_deps_mgr, > + &template_vars_ref, lflow); > + sset_destroy(&template_vars_ref); > } > > static void > @@ -1953,8 +2006,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, Nit: this doesn't have anything to do with chassis template variables and should probably be left out of this patch. If you want to change this, it probably should be done as a follow-up patch of its own. > struct ovn_desired_flow_table *flow_table, > struct simap *ids) > { > @@ -2043,8 +2095,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); Nit: Same as my previous comment. > } > } > > diff --git a/controller/lflow.h b/controller/lflow.h > index 3f369ebfb6..9a7079f99e 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/ofctrl.c b/controller/ofctrl.c > index c779912583..f92e2017ce 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,15 @@ 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_expanded_s = NULL; > + const char *flow_exp_s = lexer_parse_template_string(flow_s, > + template_vars, > + NULL, > + &flow_expanded_s); > + char *error = expr_parse_microflow(flow_exp_s, &symtab, addr_sets, > port_groups, ofctrl_lookup_port, > br_int, &uflow); > + free(flow_expanded_s); > if (error) { > return error; > } > diff --git a/controller/ofctrl.h b/controller/ofctrl.h > index 71d3f5838a..3968245126 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 f62303e3cb..c374bd0f33 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,9 @@ 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_chassis_template_var_add_clause_chassis(&tv, OVSDB_F_EQ, > + chassis->name); > } else { > /* During initialization, we monitor all records in Chassis_Private so > * that we don't try to recreate existing ones. */ > @@ -290,6 +298,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_chassis_template_var_set_condition(ovnsb_idl, &tv), > }; > > unsigned int expected_cond_seqno = 0; > @@ -307,6 +316,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; > } > > @@ -986,7 +996,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(chassis_template_var, "chassis_template_var") > > enum sb_engine_node { > #define SB_NODE(NAME, NAME_STR) SB_##NAME, > @@ -1648,6 +1659,184 @@ runtime_data_sb_datapath_binding_handler(struct engine_node *node OVS_UNUSED, > return true; > } > > +static void > +local_templates_to_string(const struct smap *local_templates, > + struct ds *out_data) > +{ > + const struct smap_node **nodes = smap_sort(local_templates); > + > + for (size_t i = 0; i < smap_count(local_templates); i++) { > + const struct smap_node *node = nodes[i]; > + > + ds_put_format(out_data, "name: '%s' value: '%s'\n", > + node->key, node->value); > + } > + free(nodes); > +} > + > +struct ed_type_template_vars { > + struct smap local_templates; > + > + bool change_tracked; > + struct sset new; > + struct sset deleted; > + struct sset updated; > +}; > + > +static void > +template_vars_init(struct ovsdb_idl_index *tv_index_by_chassis, > + const struct sbrec_chassis *chassis, > + struct smap *local_templates) > +{ > + const struct sbrec_chassis_template_var *tv; > + struct sbrec_chassis_template_var *tv_key = > + sbrec_chassis_template_var_index_init_row(tv_index_by_chassis); > + sbrec_chassis_template_var_index_set_chassis(tv_key, chassis->name); > + > + tv = sbrec_chassis_template_var_index_find(tv_index_by_chassis, tv_key); > + if (tv) { > + smap_destroy(local_templates); > + smap_clone(local_templates, &tv->variables); > + } > + > + sbrec_chassis_template_var_index_destroy_row(tv_key); > +} > + > +static void > +template_vars_update(const struct sbrec_chassis_template_var_table *tv_table, > + const struct sbrec_chassis *chassis, > + struct smap *local_templates, struct sset *new, > + struct sset *deleted, struct sset *updated) > +{ > + const struct sbrec_chassis_template_var *tv; > + struct smap_node *node; > + SBREC_CHASSIS_TEMPLATE_VAR_TABLE_FOR_EACH_TRACKED (tv, tv_table) { > + if (strcmp(tv->chassis, chassis->name)) { > + continue; > + } > + > + if (sbrec_chassis_template_var_is_deleted(tv)) { > + SMAP_FOR_EACH (node, local_templates) { > + sset_add(deleted, node->key); > + } > + } else if (sbrec_chassis_template_var_is_new(tv)) { > + ovs_assert(smap_count(local_templates) == 0); > + SMAP_FOR_EACH (node, &tv->variables) { > + sset_add(new, node->key); > + } > + } else { > + SMAP_FOR_EACH (node, &tv->variables) { > + struct smap_node *old = smap_get_node(local_templates, > + node->key); > + if (old) { > + if (strcmp(old->value, node->value)) { > + sset_add(updated, node->key); > + } > + } else { > + sset_add(new, node->key); > + } > + } > + SMAP_FOR_EACH (node, local_templates) { > + sset_add (deleted, node->key); > + } > + } > + > + smap_destroy(local_templates); > + smap_clone(local_templates, &tv->variables); > + return; > + } > +} > + > +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); > + smap_init(&tv_data->local_templates); > + 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 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)); > + struct ovsdb_idl_index *sbrec_chassis_template_var_index_by_chassis = > + engine_ovsdb_node_get_index( > + engine_get_input("SB_chassis_template_var", node), > + "chassis"); > + > + smap_clear(&tv_data->local_templates); > + template_vars_init(sbrec_chassis_template_var_index_by_chassis, > + chassis, &tv_data->local_templates); > + engine_set_node_state(node, EN_UPDATED); > +} > + > +static bool > +template_vars_sb_chassis_template_var_handler(struct engine_node *node, > + void *data) > +{ > + struct ed_type_template_vars *tv_data = data; > + > + const struct sbrec_chassis_template_var_table *tv_table = > + EN_OVSDB_GET(engine_get_input("SB_chassis_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->local_templates, > + &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; > + > + smap_destroy(&tv_data->local_templates); > + 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; > @@ -2702,6 +2891,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 = > @@ -2734,6 +2926,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->local_templates; > > l_ctx_out->flow_table = &fo->flow_table; > l_ctx_out->group_table = &fo->group_table; > @@ -3060,6 +3253,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) > @@ -3646,6 +3897,9 @@ main(int argc, char *argv[]) > struct ovsdb_idl_index *sbrec_static_mac_binding_by_datapath > = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, > &sbrec_static_mac_binding_col_datapath); > + struct ovsdb_idl_index *sbrec_chassis_template_var_index_by_chassis > + = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, > + &sbrec_chassis_template_var_col_chassis); > > ovsdb_idl_track_add_all(ovnsb_idl_loop.idl); > ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, > @@ -3707,6 +3961,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"); > @@ -3733,6 +3988,10 @@ 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, NULL); > + engine_add_input(&en_template_vars, &en_sb_chassis, NULL); > + engine_add_input(&en_template_vars, &en_sb_chassis_template_var, > + template_vars_sb_chassis_template_var_handler); > > engine_add_input(&en_addr_sets, &en_sb_address_set, > addr_sets_sb_address_set_handler); > @@ -3800,6 +4059,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, > @@ -3908,6 +4169,8 @@ main(int argc, char *argv[]) > sbrec_mac_binding_by_datapath); > engine_ovsdb_node_add_index(&en_sb_static_mac_binding, "datapath", > sbrec_static_mac_binding_by_datapath); > + engine_ovsdb_node_add_index(&en_sb_chassis_template_var, "chassis", > + sbrec_chassis_template_var_index_by_chassis); > > struct ed_type_lflow_output *lflow_output_data = > engine_get_internal_data(&en_lflow_output); > @@ -3917,6 +4180,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, > @@ -3978,6 +4243,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->local_templates); > + > unixctl_command_register("debug/ignore-startup-delay", "", 0, 0, > debug_ignore_startup_delay, NULL); > > @@ -4354,9 +4623,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->local_templates); > if (error) { > unixctl_command_reply_error(pending_pkt.conn, error); > free(error); > @@ -4825,6 +5097,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 3b141b034c..80d95ff675 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 ecb7ace243..516bbc2bd8 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,8 @@ void lexer_syntax_error(struct lexer *, const char *message, ...) > > char *lexer_steal_error(struct lexer *); > > +const char *lexer_parse_template_string(const char *s, > + const struct smap *template_vars, > + struct sset *template_vars_ref, > + char **out_expanded); > #endif /* ovn/lex.h */ > diff --git a/lib/actions.c b/lib/actions.c > index adbb42db4f..d7d4868835 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 d1f9d28ca6..43f44771cf 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 c84d52aa8d..7b44bc3537 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,40 @@ 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'. > + */ > +const char * > +lexer_parse_template_string(const char *s, const struct smap *template_vars, > + struct sset *template_vars_ref, > + char **out_expanded) > +{ > + /* No '^' means no templates. */ > + if (!strchr(s, '^')) { > + *out_expanded = NULL; > + 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); > + *out_expanded = ds_steal_cstr(&expanded); > + return *out_expanded; It seems odd to me that lexer_parse_template_string() sets the out parameter but also gives the same pointer as a return value. I could foresee a situation where someone makes the mistake of thinking these are two distinct values and causes chaos as a result (e.g. free the output parameter and then try to use the return value after). IMO, this should follow the same pattern that other parsers in OVN use. That is, it should use the output parameter for the desired outcome and return an error string in the case something unexpected occurs. > +} > diff --git a/lib/objdep.c b/lib/objdep.c > index 092d4af261..06cf126f12 100644 > --- a/lib/objdep.c > +++ b/lib/objdep.c > @@ -245,6 +245,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 50c7b01ef1..1ea781947c 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/tests/ovn-controller.at b/tests/ovn-controller.at > index 3c3fb31c7c..5fb6c65eba 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -2337,3 +2337,53 @@ done > AT_CHECK([grep "deleted interface patch" hv1/ovs-vswitchd.log], [1], [ignore]) > OVN_CLEANUP([hv1]) > AT_CLEANUP > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn-controller - Local Chassis_Template_Var updates]) > +AT_KEYWORDS([templates]) > +ovn_start > + > +net_add n1 > +sim_add hv > +as hv > +ovs-vsctl \ > + -- add-br br-phys \ > + -- add-br br-eth0 \ > + -- add-br br-eth1 \ > + -- add-br br-eth2 > +ovn_attach n1 br-phys 192.168.0.1 > + > +m4_define([GET_LOCAL_TEMPLATE_VARS], > + [ovn-appctl debug/dump-local-template-vars | grep -v 'Local template vars']) > + > +dnl Expect no local vars initially. > +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [1], []) > + > +AT_CHECK([ovn-nbctl --wait=hv create Chassis_Template_Var chassis="hv"], [0], [ignore]) > +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [1], []) > + > +dnl Expect new vars to be handled properly. > +check ovn-nbctl --wait=hv set Chassis_Template_Var hv variables:foo=foo-hv variables:bar=bar-hv > +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [0], [dnl > +name: 'bar' value: 'bar-hv' > +name: 'foo' value: 'foo-hv' > +]) > + > +dnl Expect var updates to be handled properly. > +check ovn-nbctl --wait=hv set Chassis_Template_Var hv variables:bar=bar-new-hv > +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [0], [dnl > +name: 'bar' value: 'bar-new-hv' > +name: 'foo' value: 'foo-hv' > +]) > + > +dnl Expect var deletions to be handled properly. > +check ovn-nbctl --wait=hv remove Chassis_Template_Var hv variables bar > +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [0], [dnl > +name: 'foo' value: 'foo-hv' > +]) > + > +check ovn-nbctl --wait=hv remove Chassis_Template_Var hv variables foo > +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [1], []) > + > +AT_CLEANUP > +]) > diff --git a/tests/ovn.at b/tests/ovn.at > index f8b8db4df8..2a59235c59 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 > @@ -32975,3 +32975,81 @@ check ovn-nbctl --wait=hv sync > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([Logical flows with Chassis_Template_Var references]) > +AT_KEYWORDS([templates]) > +ovn_start > +net_add n1 > + > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > + > +check ovn-nbctl ls-add sw > + > +dnl Use --wait=sb to ensure lsp1 getting a tunnel_key before lsp2. > +check ovn-nbctl --wait=sb lsp-add sw lsp1 > +check ovn-nbctl --wait=sb lsp-add sw lsp2 > + > +AT_CHECK([ovn-nbctl create Chassis_Template_Var chassis=hv1], [0], [ignore]) > + > +check ovn-nbctl pg-add pg1 lsp1 lsp2 > +AT_CHECK([ovn-nbctl create address_set name=as1 addresses=\"1.1.1.1\",\"1.1.1.2\"], [0], [ignore]) > + > +dnl Create an ACL that uses an "uninstantiated" template. > +check ovn-nbctl acl-add sw from-lport 1 "ip4.src == 42.42.42.42 && ^CONDITION" allow > + > +check ovs-vsctl add-port br-int p1 -- set interface p1 external_ids:iface-id=lsp1 > +check ovs-vsctl add-port br-int p2 -- set interface p2 external_ids:iface-id=lsp2 > + > +wait_for_ports_up > +ovn-nbctl --wait=hv sync > + > +dnl Ensure the ACL is not translated to OpenFlow. > +as hv1 > +AT_CHECK([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42'], [1], []) > + > +dnl Create a Chassis_Template_Var mapping for CONDITION. > +check ovn-nbctl --wait=hv set Chassis_Template_Var hv1 variables:CONDITION='inport == @pg1' > + > +lsp1=0x$(fetch_column Port_Binding tunnel_key logical_port=lsp1) > +lsp2=0x$(fetch_column Port_Binding tunnel_key logical_port=lsp2) > + > +dnl Ensure the ACL is translated to OpenFlows expanding pg1. > +as hv1 > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl > + table=16, priority=1001,ip,reg14=$lsp1,metadata=0x1,nw_src=42.42.42.42 actions=resubmit(,17) > + table=16, priority=1001,ip,reg14=$lsp2,metadata=0x1,nw_src=42.42.42.42 actions=resubmit(,17) > +]) > + > +dnl Remove a port from pg1 and expect OpenFlows to be correctly updated. > +check ovn-nbctl --wait=hv pg-set-ports pg1 lsp2 > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl > + table=16, priority=1001,ip,reg14=$lsp2,metadata=0x1,nw_src=42.42.42.42 actions=resubmit(,17) > +]) > + > +dnl Change the Chassis_Template_Var mapping to use the address set. > +check ovn-nbctl --wait=hv set Chassis_Template_Var hv1 variables:CONDITION='ip4.dst == $as1' > + > +dnl Ensure the ACL is translated to OpenFlows expanding as1. > +as hv1 > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl > + table=16, priority=1001,ip,metadata=0x1,nw_src=42.42.42.42,nw_dst=1.1.1.1 actions=resubmit(,17) > + table=16, priority=1001,ip,metadata=0x1,nw_src=42.42.42.42,nw_dst=1.1.1.2 actions=resubmit(,17) > +]) > + > +dnl Remove an IP from AS1 and expect OpenFlows to be correctly updated. > +check ovn-nbctl set address_set as1 addresses=\"1.1.1.1\" > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl > + table=16, priority=1001,ip,metadata=0x1,nw_src=42.42.42.42,nw_dst=1.1.1.1 actions=resubmit(,17) > +]) > + > +dnl Remove the mapping and expect OpenFlows to be removed. > +check ovn-nbctl --wait=hv clear Chassis_Template_Var hv1 variables > +AT_CHECK([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42'], [1], []) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > diff --git a/tests/test-ovn.c b/tests/test-ovn.c > index a241f150d1..c422bafd70 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,11 @@ 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_expanded = NULL; > + const char *exp_input = > + lexer_parse_template_string(ds_cstr(&input), &template_vars, NULL, > + &exp_input_expanded); > + 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 +1418,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_expanded); > } > ds_destroy(&input); > > @@ -1419,6 +1429,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 6fa5137d95..b837884ccc 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,15 @@ 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_expanded_s = NULL; > + const char *match_s = lexer_parse_template_string(sblf->match, > + &template_vars, > + NULL, > + &match_expanded_s); > + match = expr_parse_string(match_s, &symtab, &address_sets, > &port_groups, NULL, NULL, dp->tunnel_key, > &error); > + free(match_expanded_s); > if (error) { > VLOG_WARN("%s: parsing expression failed (%s)", > sblf->match, error); > @@ -980,7 +987,12 @@ 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_expanded_s = NULL; > + const char *actions_s = > + lexer_parse_template_string(sblf->actions, &template_vars, > + NULL, &actions_expanded_s); > + error = ovnacts_parse_string(actions_s, &pp, &ovnacts, &prereqs); > + free(actions_expanded_s); > if (error) { > VLOG_WARN("%s: parsing actions failed (%s)", sblf->actions, error); > free(error); > @@ -1078,6 +1090,7 @@ read_gen_opts(void) > nd_ra_opts_init(&nd_ra_opts); > > controller_event_opts_init(&event_opts); > + smap_init(&template_vars); > } > > static void > @@ -3420,9 +3433,15 @@ 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_expanded_s = NULL; > + const char *flow_exp_s = lexer_parse_template_string(flow_s, > + &template_vars, > + NULL, > + &flow_expanded_s); > + struct expr *e = expr_parse_string(flow_exp_s, &symtab, &address_sets, > &port_groups, NULL, NULL, 0, > &error); > + free(flow_expanded_s); > if (!e) { > return trace_parse_error(error); > } > @@ -3447,9 +3466,15 @@ 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_expanded_s = NULL; > + const char *flow_exp_s = lexer_parse_template_string(flow_s, > + &template_vars, > + NULL, > + &flow_expanded_s); > + char *error = expr_parse_microflow(flow_exp_s, &symtab, &address_sets, > &port_groups, ovntrace_lookup_port, > *dpp, uflow); > + free(flow_expanded_s); > if (error) { > return trace_parse_error(error); > } >
On 11/16/22 22:33, Mark Michelson wrote: > Hi Dumitru, I have a few comments down below > > On 11/4/22 18:11, Dumitru Ceara 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 Chassis_Template_Var >> also track references (similar to the ones maintained for >> multicast groups, address sets, port_groups, port bindings). >> >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> --- >> V2: >> - Fix GCC build due to missing newline. >> - Handle SB table rename Template_Var -> Chassis_Template_Var. >> - Address Han's comments: >> - Add new function to parse lflow actions. >> - Move xstrdup inside lexer_parse_template_string() and execute it >> only >> if needed. >> - Match template vars only by chassis name. >> - Change local_templates table to a plain smap. >> - Some indentation updates. >> - Use NULL template variable change handlers for chassis/Open_vSwitch >> changes. >> - Added more tests. >> - Fix tracking of template references in lflow actions. >> --- >> controller/lflow.c | 136 ++++++++++++++------ >> controller/lflow.h | 1 >> controller/ofctrl.c | 11 +- >> controller/ofctrl.h | 3 >> controller/ovn-controller.c | 289 >> +++++++++++++++++++++++++++++++++++++++++++ >> include/ovn/expr.h | 4 - >> include/ovn/lex.h | 15 ++ >> lib/actions.c | 9 + >> lib/expr.c | 18 ++- >> lib/lex.c | 57 ++++++++ >> lib/objdep.c | 1 >> lib/objdep.h | 1 >> tests/ovn-controller.at | 50 +++++++ >> tests/ovn.at | 80 ++++++++++++ >> tests/test-ovn.c | 17 ++- >> utilities/ovn-trace.c | 33 ++++- >> 16 files changed, 658 insertions(+), 67 deletions(-) >> >> diff --git a/controller/lflow.c b/controller/lflow.c >> index d4434bdee8..fc4371d0df 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 *, >> @@ -297,6 +299,43 @@ as_info_from_expr_const(const char *as_name, >> const union expr_constant *c, >> return true; >> } >> +static bool >> +lflow_parse_actions(const struct sbrec_logical_flow *lflow, >> + const struct lflow_ctx_in *l_ctx_in, >> + struct sset *template_vars_ref, >> + struct ofpbuf *ovnacts_out, >> + struct expr **prereqs_out) >> +{ >> + bool ingress = !strcmp(lflow->pipeline, "ingress"); >> + struct ovnact_parse_params pp = { >> + .symtab = &symtab, >> + .dhcp_opts = l_ctx_in->dhcp_opts, >> + .dhcpv6_opts = l_ctx_in->dhcpv6_opts, >> + .nd_ra_opts = l_ctx_in->nd_ra_opts, >> + .controller_event_opts = l_ctx_in->controller_event_opts, >> + >> + .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, >> + .n_tables = LOG_PIPELINE_LEN, >> + .cur_ltable = lflow->table_id, >> + }; >> + >> + char *actions_expanded_s = NULL; >> + const char *actions_s = >> + lexer_parse_template_string(lflow->actions, >> l_ctx_in->template_vars, >> + template_vars_ref, >> &actions_expanded_s); >> + char *error = ovnacts_parse_string(actions_s, &pp, >> + ovnacts_out, prereqs_out); >> + free(actions_expanded_s); >> + if (error) { >> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); >> + VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", >> + lflow->actions, error); >> + free(error); >> + return false; >> + } >> + return true; >> +} >> + >> /* Parses the lflow regarding the changed address set 'as_name', and >> generates >> * ovs flows for the newly added addresses in 'as_diff_added' only. >> It is >> * similar to consider_logical_flow__, with the below differences: >> @@ -347,27 +386,14 @@ consider_lflow_for_added_as_ips__( >> uint64_t ovnacts_stub[1024 / 8]; >> struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(ovnacts_stub); >> - struct ovnact_parse_params pp = { >> - .symtab = &symtab, >> - .dhcp_opts = l_ctx_in->dhcp_opts, >> - .dhcpv6_opts = l_ctx_in->dhcpv6_opts, >> - .nd_ra_opts = l_ctx_in->nd_ra_opts, >> - .controller_event_opts = l_ctx_in->controller_event_opts, >> - .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, >> - .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); >> - if (error) { >> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); >> - VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", >> - lflow->actions, error); >> - free(error); >> + if (!lflow_parse_actions(lflow, l_ctx_in, &template_vars_ref, >> + &ovnacts, &prereqs)) { >> ovnacts_free(ovnacts.data, ovnacts.size); >> ofpbuf_uninit(&ovnacts); >> + sset_destroy(&template_vars_ref); >> return true; >> } >> @@ -431,6 +457,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) { >> @@ -502,6 +530,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); >> + } > > The loop above seems to be an inlined implementation of > store_lflow_template_refs(). I think you should just call that function > instead. > You're right. I factored out the function at some point and I failed to use it in all applicable places. Fixed in v3. >> + sset_destroy(&template_vars_ref); >> + >> return handled; >> } >> @@ -913,6 +949,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) >> { >> @@ -920,11 +958,18 @@ 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_expanded_s = NULL; >> + const char *match_s = lexer_parse_template_string(lflow->match, >> + template_vars, >> + template_vars_ref, >> + >> &match_expanded_s); >> + 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_expanded_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, >> @@ -963,6 +1008,18 @@ convert_match_to_expr(const struct >> sbrec_logical_flow *lflow, >> return expr_simplify(e); >> } >> +static void >> +store_lflow_template_refs(struct objdep_mgr *lflow_deps_mgr, >> + const struct sset *template_vars_ref, >> + const struct sbrec_logical_flow *lflow) >> +{ >> + const char *tv_name; >> + SSET_FOR_EACH (tv_name, template_vars_ref) { >> + objdep_mgr_add(lflow_deps_mgr, OBJDEP_TYPE_TEMPLATE, tv_name, >> + &lflow->header_.uuid); >> + } >> +} >> + >> static void >> consider_logical_flow__(const struct sbrec_logical_flow *lflow, >> const struct sbrec_datapath_binding *dp, >> @@ -1015,28 +1072,16 @@ consider_logical_flow__(const struct >> sbrec_logical_flow *lflow, >> * XXX Deny changes to 'outport' in egress pipeline. */ >> uint64_t ovnacts_stub[1024 / 8]; >> struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(ovnacts_stub); >> - struct ovnact_parse_params pp = { >> - .symtab = &symtab, >> - .dhcp_opts = l_ctx_in->dhcp_opts, >> - .dhcpv6_opts = l_ctx_in->dhcpv6_opts, >> - .nd_ra_opts = l_ctx_in->nd_ra_opts, >> - .controller_event_opts = l_ctx_in->controller_event_opts, >> - >> - .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, >> - .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); >> - if (error) { >> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); >> - VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", >> - lflow->actions, error); >> - free(error); >> + if (!lflow_parse_actions(lflow, l_ctx_in, &template_vars_ref, >> + &ovnacts, &prereqs)) { >> ovnacts_free(ovnacts.data, ovnacts.size); >> ofpbuf_uninit(&ovnacts); >> + store_lflow_template_refs(l_ctx_out->lflow_deps_mgr, >> + &template_vars_ref, lflow); > > Can you explain why we call store_lflow_template_refs() in the case > where lflow_parse_actions() fails? This is different from the behavior > in consider_lflow_for_added_as_ips__(), where > store_lflow_template_refs() is only called when lflow_parse_actions() > succeeds. > It should be called in case of failures too. It's for the case when an address set change is processed incrementally and the added value is a template. If that template is not yet defined we still need to record the relation between the flow using the address set and the template. I added the missing call in v3. >> + sset_destroy(&template_vars_ref); >> return; >> } >> @@ -1087,6 +1132,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) { >> @@ -1101,11 +1148,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); >> } >> @@ -1190,6 +1239,10 @@ done: >> expr_destroy(cached_expr); >> expr_matches_destroy(matches); >> free(matches); >> + >> + store_lflow_template_refs(l_ctx_out->lflow_deps_mgr, >> + &template_vars_ref, lflow); >> + sset_destroy(&template_vars_ref); >> } >> static void >> @@ -1953,8 +2006,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, > > Nit: this doesn't have anything to do with chassis template variables > and should probably be left out of this patch. If you want to change > this, it probably should be done as a follow-up patch of its own. > Ack. >> struct ovn_desired_flow_table *flow_table, >> struct simap *ids) >> { >> @@ -2043,8 +2095,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); > > Nit: Same as my previous comment. > Ack. >> } >> } >> diff --git a/controller/lflow.h b/controller/lflow.h >> index 3f369ebfb6..9a7079f99e 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/ofctrl.c b/controller/ofctrl.c >> index c779912583..f92e2017ce 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,15 @@ 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_expanded_s = NULL; >> + const char *flow_exp_s = lexer_parse_template_string(flow_s, >> + template_vars, >> + NULL, >> + >> &flow_expanded_s); >> + char *error = expr_parse_microflow(flow_exp_s, &symtab, addr_sets, >> port_groups, ofctrl_lookup_port, >> br_int, &uflow); >> + free(flow_expanded_s); >> if (error) { >> return error; >> } >> diff --git a/controller/ofctrl.h b/controller/ofctrl.h >> index 71d3f5838a..3968245126 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 f62303e3cb..c374bd0f33 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,9 @@ 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_chassis_template_var_add_clause_chassis(&tv, OVSDB_F_EQ, >> + chassis->name); >> } else { >> /* During initialization, we monitor all records in >> Chassis_Private so >> * that we don't try to recreate existing ones. */ >> @@ -290,6 +298,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_chassis_template_var_set_condition(ovnsb_idl, &tv), >> }; >> unsigned int expected_cond_seqno = 0; >> @@ -307,6 +316,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; >> } >> @@ -986,7 +996,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(chassis_template_var, "chassis_template_var") >> enum sb_engine_node { >> #define SB_NODE(NAME, NAME_STR) SB_##NAME, >> @@ -1648,6 +1659,184 @@ >> runtime_data_sb_datapath_binding_handler(struct engine_node *node >> OVS_UNUSED, >> return true; >> } >> +static void >> +local_templates_to_string(const struct smap *local_templates, >> + struct ds *out_data) >> +{ >> + const struct smap_node **nodes = smap_sort(local_templates); >> + >> + for (size_t i = 0; i < smap_count(local_templates); i++) { >> + const struct smap_node *node = nodes[i]; >> + >> + ds_put_format(out_data, "name: '%s' value: '%s'\n", >> + node->key, node->value); >> + } >> + free(nodes); >> +} >> + >> +struct ed_type_template_vars { >> + struct smap local_templates; >> + >> + bool change_tracked; >> + struct sset new; >> + struct sset deleted; >> + struct sset updated; >> +}; >> + >> +static void >> +template_vars_init(struct ovsdb_idl_index *tv_index_by_chassis, >> + const struct sbrec_chassis *chassis, >> + struct smap *local_templates) >> +{ >> + const struct sbrec_chassis_template_var *tv; >> + struct sbrec_chassis_template_var *tv_key = >> + sbrec_chassis_template_var_index_init_row(tv_index_by_chassis); >> + sbrec_chassis_template_var_index_set_chassis(tv_key, chassis->name); >> + >> + tv = sbrec_chassis_template_var_index_find(tv_index_by_chassis, >> tv_key); >> + if (tv) { >> + smap_destroy(local_templates); >> + smap_clone(local_templates, &tv->variables); >> + } >> + >> + sbrec_chassis_template_var_index_destroy_row(tv_key); >> +} >> + >> +static void >> +template_vars_update(const struct sbrec_chassis_template_var_table >> *tv_table, >> + const struct sbrec_chassis *chassis, >> + struct smap *local_templates, struct sset *new, >> + struct sset *deleted, struct sset *updated) >> +{ >> + const struct sbrec_chassis_template_var *tv; >> + struct smap_node *node; >> + SBREC_CHASSIS_TEMPLATE_VAR_TABLE_FOR_EACH_TRACKED (tv, tv_table) { >> + if (strcmp(tv->chassis, chassis->name)) { >> + continue; >> + } >> + >> + if (sbrec_chassis_template_var_is_deleted(tv)) { >> + SMAP_FOR_EACH (node, local_templates) { >> + sset_add(deleted, node->key); >> + } >> + } else if (sbrec_chassis_template_var_is_new(tv)) { >> + ovs_assert(smap_count(local_templates) == 0); >> + SMAP_FOR_EACH (node, &tv->variables) { >> + sset_add(new, node->key); >> + } >> + } else { >> + SMAP_FOR_EACH (node, &tv->variables) { >> + struct smap_node *old = smap_get_node(local_templates, >> + node->key); >> + if (old) { >> + if (strcmp(old->value, node->value)) { >> + sset_add(updated, node->key); >> + } >> + } else { >> + sset_add(new, node->key); >> + } >> + } >> + SMAP_FOR_EACH (node, local_templates) { >> + sset_add (deleted, node->key); >> + } >> + } >> + >> + smap_destroy(local_templates); >> + smap_clone(local_templates, &tv->variables); >> + return; >> + } >> +} >> + >> +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); >> + smap_init(&tv_data->local_templates); >> + 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 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)); >> + struct ovsdb_idl_index >> *sbrec_chassis_template_var_index_by_chassis = >> + engine_ovsdb_node_get_index( >> + engine_get_input("SB_chassis_template_var", node), >> + "chassis"); >> + >> + smap_clear(&tv_data->local_templates); >> + template_vars_init(sbrec_chassis_template_var_index_by_chassis, >> + chassis, &tv_data->local_templates); >> + engine_set_node_state(node, EN_UPDATED); >> +} >> + >> +static bool >> +template_vars_sb_chassis_template_var_handler(struct engine_node *node, >> + void *data) >> +{ >> + struct ed_type_template_vars *tv_data = data; >> + >> + const struct sbrec_chassis_template_var_table *tv_table = >> + EN_OVSDB_GET(engine_get_input("SB_chassis_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->local_templates, >> + &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; >> + >> + smap_destroy(&tv_data->local_templates); >> + 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; >> @@ -2702,6 +2891,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 = >> @@ -2734,6 +2926,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->local_templates; >> l_ctx_out->flow_table = &fo->flow_table; >> l_ctx_out->group_table = &fo->group_table; >> @@ -3060,6 +3253,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) >> @@ -3646,6 +3897,9 @@ main(int argc, char *argv[]) >> struct ovsdb_idl_index *sbrec_static_mac_binding_by_datapath >> = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, >> >> &sbrec_static_mac_binding_col_datapath); >> + struct ovsdb_idl_index *sbrec_chassis_template_var_index_by_chassis >> + = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, >> + >> &sbrec_chassis_template_var_col_chassis); >> ovsdb_idl_track_add_all(ovnsb_idl_loop.idl); >> ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, >> @@ -3707,6 +3961,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"); >> @@ -3733,6 +3988,10 @@ 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, NULL); >> + engine_add_input(&en_template_vars, &en_sb_chassis, NULL); >> + engine_add_input(&en_template_vars, &en_sb_chassis_template_var, >> + template_vars_sb_chassis_template_var_handler); >> engine_add_input(&en_addr_sets, &en_sb_address_set, >> addr_sets_sb_address_set_handler); >> @@ -3800,6 +4059,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, >> @@ -3908,6 +4169,8 @@ main(int argc, char *argv[]) >> sbrec_mac_binding_by_datapath); >> engine_ovsdb_node_add_index(&en_sb_static_mac_binding, "datapath", >> sbrec_static_mac_binding_by_datapath); >> + engine_ovsdb_node_add_index(&en_sb_chassis_template_var, "chassis", >> + >> sbrec_chassis_template_var_index_by_chassis); >> struct ed_type_lflow_output *lflow_output_data = >> engine_get_internal_data(&en_lflow_output); >> @@ -3917,6 +4180,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, >> @@ -3978,6 +4243,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->local_templates); >> + >> unixctl_command_register("debug/ignore-startup-delay", "", 0, 0, >> debug_ignore_startup_delay, NULL); >> @@ -4354,9 +4623,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->local_templates); >> if (error) { >> >> unixctl_command_reply_error(pending_pkt.conn, error); >> free(error); >> @@ -4825,6 +5097,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 3b141b034c..80d95ff675 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 ecb7ace243..516bbc2bd8 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,8 @@ void lexer_syntax_error(struct lexer *, const char >> *message, ...) >> char *lexer_steal_error(struct lexer *); >> +const char *lexer_parse_template_string(const char *s, >> + const struct smap >> *template_vars, >> + struct sset *template_vars_ref, >> + char **out_expanded); >> #endif /* ovn/lex.h */ >> diff --git a/lib/actions.c b/lib/actions.c >> index adbb42db4f..d7d4868835 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 d1f9d28ca6..43f44771cf 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 c84d52aa8d..7b44bc3537 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,40 @@ 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'. >> + */ >> +const char * >> +lexer_parse_template_string(const char *s, const struct smap >> *template_vars, >> + struct sset *template_vars_ref, >> + char **out_expanded) >> +{ >> + /* No '^' means no templates. */ >> + if (!strchr(s, '^')) { >> + *out_expanded = NULL; >> + 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); >> + *out_expanded = ds_steal_cstr(&expanded); >> + return *out_expanded; > > It seems odd to me that lexer_parse_template_string() sets the out > parameter but also gives the same pointer as a return value. I could > foresee a situation where someone makes the mistake of thinking these > are two distinct values and causes chaos as a result (e.g. free the > output parameter and then try to use the return value after). IMO, this > should follow the same pattern that other parsers in OVN use. That is, > it should use the output parameter for the desired outcome and return an > error string in the case something unexpected occurs. > You're right, this isn't great. I'm not sure how to best implement this however. I went with something a bit different in v3 (a new lex_str type wrapping in-heap or on-stack allocated lexer strings). I guess we should probably continue the discussion there. >> +} >> diff --git a/lib/objdep.c b/lib/objdep.c >> index 092d4af261..06cf126f12 100644 >> --- a/lib/objdep.c >> +++ b/lib/objdep.c >> @@ -245,6 +245,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 50c7b01ef1..1ea781947c 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/tests/ovn-controller.at b/tests/ovn-controller.at >> index 3c3fb31c7c..5fb6c65eba 100644 >> --- a/tests/ovn-controller.at >> +++ b/tests/ovn-controller.at >> @@ -2337,3 +2337,53 @@ done >> AT_CHECK([grep "deleted interface patch" hv1/ovs-vswitchd.log], [1], >> [ignore]) >> OVN_CLEANUP([hv1]) >> AT_CLEANUP >> + >> +OVN_FOR_EACH_NORTHD([ >> +AT_SETUP([ovn-controller - Local Chassis_Template_Var updates]) >> +AT_KEYWORDS([templates]) >> +ovn_start >> + >> +net_add n1 >> +sim_add hv >> +as hv >> +ovs-vsctl \ >> + -- add-br br-phys \ >> + -- add-br br-eth0 \ >> + -- add-br br-eth1 \ >> + -- add-br br-eth2 >> +ovn_attach n1 br-phys 192.168.0.1 >> + >> +m4_define([GET_LOCAL_TEMPLATE_VARS], >> + [ovn-appctl debug/dump-local-template-vars | grep -v 'Local >> template vars']) >> + >> +dnl Expect no local vars initially. >> +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [1], []) >> + >> +AT_CHECK([ovn-nbctl --wait=hv create Chassis_Template_Var >> chassis="hv"], [0], [ignore]) >> +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [1], []) >> + >> +dnl Expect new vars to be handled properly. >> +check ovn-nbctl --wait=hv set Chassis_Template_Var hv >> variables:foo=foo-hv variables:bar=bar-hv >> +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [0], [dnl >> +name: 'bar' value: 'bar-hv' >> +name: 'foo' value: 'foo-hv' >> +]) >> + >> +dnl Expect var updates to be handled properly. >> +check ovn-nbctl --wait=hv set Chassis_Template_Var hv >> variables:bar=bar-new-hv >> +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [0], [dnl >> +name: 'bar' value: 'bar-new-hv' >> +name: 'foo' value: 'foo-hv' >> +]) >> + >> +dnl Expect var deletions to be handled properly. >> +check ovn-nbctl --wait=hv remove Chassis_Template_Var hv variables bar >> +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [0], [dnl >> +name: 'foo' value: 'foo-hv' >> +]) >> + >> +check ovn-nbctl --wait=hv remove Chassis_Template_Var hv variables foo >> +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [1], []) >> + >> +AT_CLEANUP >> +]) >> diff --git a/tests/ovn.at b/tests/ovn.at >> index f8b8db4df8..2a59235c59 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 >> @@ -32975,3 +32975,81 @@ check ovn-nbctl --wait=hv sync >> OVN_CLEANUP([hv1]) >> AT_CLEANUP >> ]) >> + >> +OVN_FOR_EACH_NORTHD([ >> +AT_SETUP([Logical flows with Chassis_Template_Var references]) >> +AT_KEYWORDS([templates]) >> +ovn_start >> +net_add n1 >> + >> +sim_add hv1 >> +as hv1 >> +ovs-vsctl add-br br-phys >> +ovn_attach n1 br-phys 192.168.0.1 >> + >> +check ovn-nbctl ls-add sw >> + >> +dnl Use --wait=sb to ensure lsp1 getting a tunnel_key before lsp2. >> +check ovn-nbctl --wait=sb lsp-add sw lsp1 >> +check ovn-nbctl --wait=sb lsp-add sw lsp2 >> + >> +AT_CHECK([ovn-nbctl create Chassis_Template_Var chassis=hv1], [0], >> [ignore]) >> + >> +check ovn-nbctl pg-add pg1 lsp1 lsp2 >> +AT_CHECK([ovn-nbctl create address_set name=as1 >> addresses=\"1.1.1.1\",\"1.1.1.2\"], [0], [ignore]) >> + >> +dnl Create an ACL that uses an "uninstantiated" template. >> +check ovn-nbctl acl-add sw from-lport 1 "ip4.src == 42.42.42.42 && >> ^CONDITION" allow >> + >> +check ovs-vsctl add-port br-int p1 -- set interface p1 >> external_ids:iface-id=lsp1 >> +check ovs-vsctl add-port br-int p2 -- set interface p2 >> external_ids:iface-id=lsp2 >> + >> +wait_for_ports_up >> +ovn-nbctl --wait=hv sync >> + >> +dnl Ensure the ACL is not translated to OpenFlow. >> +as hv1 >> +AT_CHECK([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42'], [1], []) >> + >> +dnl Create a Chassis_Template_Var mapping for CONDITION. >> +check ovn-nbctl --wait=hv set Chassis_Template_Var hv1 >> variables:CONDITION='inport == @pg1' >> + >> +lsp1=0x$(fetch_column Port_Binding tunnel_key logical_port=lsp1) >> +lsp2=0x$(fetch_column Port_Binding tunnel_key logical_port=lsp2) >> + >> +dnl Ensure the ACL is translated to OpenFlows expanding pg1. >> +as hv1 >> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep >> '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl >> + table=16, >> priority=1001,ip,reg14=$lsp1,metadata=0x1,nw_src=42.42.42.42 >> actions=resubmit(,17) >> + table=16, >> priority=1001,ip,reg14=$lsp2,metadata=0x1,nw_src=42.42.42.42 >> actions=resubmit(,17) >> +]) >> + >> +dnl Remove a port from pg1 and expect OpenFlows to be correctly updated. >> +check ovn-nbctl --wait=hv pg-set-ports pg1 lsp2 >> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep >> '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl >> + table=16, >> priority=1001,ip,reg14=$lsp2,metadata=0x1,nw_src=42.42.42.42 >> actions=resubmit(,17) >> +]) >> + >> +dnl Change the Chassis_Template_Var mapping to use the address set. >> +check ovn-nbctl --wait=hv set Chassis_Template_Var hv1 >> variables:CONDITION='ip4.dst == $as1' >> + >> +dnl Ensure the ACL is translated to OpenFlows expanding as1. >> +as hv1 >> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep >> '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl >> + table=16, >> priority=1001,ip,metadata=0x1,nw_src=42.42.42.42,nw_dst=1.1.1.1 >> actions=resubmit(,17) >> + table=16, >> priority=1001,ip,metadata=0x1,nw_src=42.42.42.42,nw_dst=1.1.1.2 >> actions=resubmit(,17) >> +]) >> + >> +dnl Remove an IP from AS1 and expect OpenFlows to be correctly updated. >> +check ovn-nbctl set address_set as1 addresses=\"1.1.1.1\" >> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep >> '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl >> + table=16, >> priority=1001,ip,metadata=0x1,nw_src=42.42.42.42,nw_dst=1.1.1.1 >> actions=resubmit(,17) >> +]) >> + >> +dnl Remove the mapping and expect OpenFlows to be removed. >> +check ovn-nbctl --wait=hv clear Chassis_Template_Var hv1 variables >> +AT_CHECK([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42'], [1], []) >> + >> +OVN_CLEANUP([hv1]) >> +AT_CLEANUP >> +]) >> diff --git a/tests/test-ovn.c b/tests/test-ovn.c >> index a241f150d1..c422bafd70 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,11 @@ 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_expanded = NULL; >> + const char *exp_input = >> + lexer_parse_template_string(ds_cstr(&input), >> &template_vars, NULL, >> + &exp_input_expanded); >> + 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 +1418,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_expanded); >> } >> ds_destroy(&input); >> @@ -1419,6 +1429,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 6fa5137d95..b837884ccc 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,15 @@ 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_expanded_s = NULL; >> + const char *match_s = lexer_parse_template_string(sblf->match, >> + >> &template_vars, >> + NULL, >> + >> &match_expanded_s); >> + match = expr_parse_string(match_s, &symtab, &address_sets, >> &port_groups, NULL, NULL, >> dp->tunnel_key, >> &error); >> + free(match_expanded_s); >> if (error) { >> VLOG_WARN("%s: parsing expression failed (%s)", >> sblf->match, error); >> @@ -980,7 +987,12 @@ 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_expanded_s = NULL; >> + const char *actions_s = >> + lexer_parse_template_string(sblf->actions, &template_vars, >> + NULL, &actions_expanded_s); >> + error = ovnacts_parse_string(actions_s, &pp, &ovnacts, >> &prereqs); >> + free(actions_expanded_s); >> if (error) { >> VLOG_WARN("%s: parsing actions failed (%s)", >> sblf->actions, error); >> free(error); >> @@ -1078,6 +1090,7 @@ read_gen_opts(void) >> nd_ra_opts_init(&nd_ra_opts); >> controller_event_opts_init(&event_opts); >> + smap_init(&template_vars); >> } >> static void >> @@ -3420,9 +3433,15 @@ 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_expanded_s = NULL; >> + const char *flow_exp_s = lexer_parse_template_string(flow_s, >> + >> &template_vars, >> + NULL, >> + >> &flow_expanded_s); >> + struct expr *e = expr_parse_string(flow_exp_s, &symtab, >> &address_sets, >> &port_groups, NULL, NULL, 0, >> &error); >> + free(flow_expanded_s); >> if (!e) { >> return trace_parse_error(error); >> } >> @@ -3447,9 +3466,15 @@ 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_expanded_s = NULL; >> + const char *flow_exp_s = lexer_parse_template_string(flow_s, >> + &template_vars, >> + NULL, >> + >> &flow_expanded_s); >> + char *error = expr_parse_microflow(flow_exp_s, &symtab, >> &address_sets, >> &port_groups, >> ovntrace_lookup_port, >> *dpp, uflow); >> + free(flow_expanded_s); >> if (error) { >> return trace_parse_error(error); >> } >> > Thanks, Dumitru
diff --git a/controller/lflow.c b/controller/lflow.c index d4434bdee8..fc4371d0df 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 *, @@ -297,6 +299,43 @@ as_info_from_expr_const(const char *as_name, const union expr_constant *c, return true; } +static bool +lflow_parse_actions(const struct sbrec_logical_flow *lflow, + const struct lflow_ctx_in *l_ctx_in, + struct sset *template_vars_ref, + struct ofpbuf *ovnacts_out, + struct expr **prereqs_out) +{ + bool ingress = !strcmp(lflow->pipeline, "ingress"); + struct ovnact_parse_params pp = { + .symtab = &symtab, + .dhcp_opts = l_ctx_in->dhcp_opts, + .dhcpv6_opts = l_ctx_in->dhcpv6_opts, + .nd_ra_opts = l_ctx_in->nd_ra_opts, + .controller_event_opts = l_ctx_in->controller_event_opts, + + .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, + .n_tables = LOG_PIPELINE_LEN, + .cur_ltable = lflow->table_id, + }; + + char *actions_expanded_s = NULL; + const char *actions_s = + lexer_parse_template_string(lflow->actions, l_ctx_in->template_vars, + template_vars_ref, &actions_expanded_s); + char *error = ovnacts_parse_string(actions_s, &pp, + ovnacts_out, prereqs_out); + free(actions_expanded_s); + if (error) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", + lflow->actions, error); + free(error); + return false; + } + return true; +} + /* Parses the lflow regarding the changed address set 'as_name', and generates * ovs flows for the newly added addresses in 'as_diff_added' only. It is * similar to consider_logical_flow__, with the below differences: @@ -347,27 +386,14 @@ consider_lflow_for_added_as_ips__( uint64_t ovnacts_stub[1024 / 8]; struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(ovnacts_stub); - struct ovnact_parse_params pp = { - .symtab = &symtab, - .dhcp_opts = l_ctx_in->dhcp_opts, - .dhcpv6_opts = l_ctx_in->dhcpv6_opts, - .nd_ra_opts = l_ctx_in->nd_ra_opts, - .controller_event_opts = l_ctx_in->controller_event_opts, - .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, - .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); - if (error) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", - lflow->actions, error); - free(error); + if (!lflow_parse_actions(lflow, l_ctx_in, &template_vars_ref, + &ovnacts, &prereqs)) { ovnacts_free(ovnacts.data, ovnacts.size); ofpbuf_uninit(&ovnacts); + sset_destroy(&template_vars_ref); return true; } @@ -431,6 +457,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) { @@ -502,6 +530,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; } @@ -913,6 +949,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) { @@ -920,11 +958,18 @@ 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_expanded_s = NULL; + const char *match_s = lexer_parse_template_string(lflow->match, + template_vars, + template_vars_ref, + &match_expanded_s); + 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_expanded_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, @@ -963,6 +1008,18 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, return expr_simplify(e); } +static void +store_lflow_template_refs(struct objdep_mgr *lflow_deps_mgr, + const struct sset *template_vars_ref, + const struct sbrec_logical_flow *lflow) +{ + const char *tv_name; + SSET_FOR_EACH (tv_name, template_vars_ref) { + objdep_mgr_add(lflow_deps_mgr, OBJDEP_TYPE_TEMPLATE, tv_name, + &lflow->header_.uuid); + } +} + static void consider_logical_flow__(const struct sbrec_logical_flow *lflow, const struct sbrec_datapath_binding *dp, @@ -1015,28 +1072,16 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, * XXX Deny changes to 'outport' in egress pipeline. */ uint64_t ovnacts_stub[1024 / 8]; struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(ovnacts_stub); - struct ovnact_parse_params pp = { - .symtab = &symtab, - .dhcp_opts = l_ctx_in->dhcp_opts, - .dhcpv6_opts = l_ctx_in->dhcpv6_opts, - .nd_ra_opts = l_ctx_in->nd_ra_opts, - .controller_event_opts = l_ctx_in->controller_event_opts, - - .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, - .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); - if (error) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", - lflow->actions, error); - free(error); + if (!lflow_parse_actions(lflow, l_ctx_in, &template_vars_ref, + &ovnacts, &prereqs)) { ovnacts_free(ovnacts.data, ovnacts.size); ofpbuf_uninit(&ovnacts); + store_lflow_template_refs(l_ctx_out->lflow_deps_mgr, + &template_vars_ref, lflow); + sset_destroy(&template_vars_ref); return; } @@ -1087,6 +1132,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) { @@ -1101,11 +1148,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); } @@ -1190,6 +1239,10 @@ done: expr_destroy(cached_expr); expr_matches_destroy(matches); free(matches); + + store_lflow_template_refs(l_ctx_out->lflow_deps_mgr, + &template_vars_ref, lflow); + sset_destroy(&template_vars_ref); } static void @@ -1953,8 +2006,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) { @@ -2043,8 +2095,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 3f369ebfb6..9a7079f99e 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/ofctrl.c b/controller/ofctrl.c index c779912583..f92e2017ce 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,15 @@ 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_expanded_s = NULL; + const char *flow_exp_s = lexer_parse_template_string(flow_s, + template_vars, + NULL, + &flow_expanded_s); + char *error = expr_parse_microflow(flow_exp_s, &symtab, addr_sets, port_groups, ofctrl_lookup_port, br_int, &uflow); + free(flow_expanded_s); if (error) { return error; } diff --git a/controller/ofctrl.h b/controller/ofctrl.h index 71d3f5838a..3968245126 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 f62303e3cb..c374bd0f33 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,9 @@ 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_chassis_template_var_add_clause_chassis(&tv, OVSDB_F_EQ, + chassis->name); } else { /* During initialization, we monitor all records in Chassis_Private so * that we don't try to recreate existing ones. */ @@ -290,6 +298,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_chassis_template_var_set_condition(ovnsb_idl, &tv), }; unsigned int expected_cond_seqno = 0; @@ -307,6 +316,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; } @@ -986,7 +996,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(chassis_template_var, "chassis_template_var") enum sb_engine_node { #define SB_NODE(NAME, NAME_STR) SB_##NAME, @@ -1648,6 +1659,184 @@ runtime_data_sb_datapath_binding_handler(struct engine_node *node OVS_UNUSED, return true; } +static void +local_templates_to_string(const struct smap *local_templates, + struct ds *out_data) +{ + const struct smap_node **nodes = smap_sort(local_templates); + + for (size_t i = 0; i < smap_count(local_templates); i++) { + const struct smap_node *node = nodes[i]; + + ds_put_format(out_data, "name: '%s' value: '%s'\n", + node->key, node->value); + } + free(nodes); +} + +struct ed_type_template_vars { + struct smap local_templates; + + bool change_tracked; + struct sset new; + struct sset deleted; + struct sset updated; +}; + +static void +template_vars_init(struct ovsdb_idl_index *tv_index_by_chassis, + const struct sbrec_chassis *chassis, + struct smap *local_templates) +{ + const struct sbrec_chassis_template_var *tv; + struct sbrec_chassis_template_var *tv_key = + sbrec_chassis_template_var_index_init_row(tv_index_by_chassis); + sbrec_chassis_template_var_index_set_chassis(tv_key, chassis->name); + + tv = sbrec_chassis_template_var_index_find(tv_index_by_chassis, tv_key); + if (tv) { + smap_destroy(local_templates); + smap_clone(local_templates, &tv->variables); + } + + sbrec_chassis_template_var_index_destroy_row(tv_key); +} + +static void +template_vars_update(const struct sbrec_chassis_template_var_table *tv_table, + const struct sbrec_chassis *chassis, + struct smap *local_templates, struct sset *new, + struct sset *deleted, struct sset *updated) +{ + const struct sbrec_chassis_template_var *tv; + struct smap_node *node; + SBREC_CHASSIS_TEMPLATE_VAR_TABLE_FOR_EACH_TRACKED (tv, tv_table) { + if (strcmp(tv->chassis, chassis->name)) { + continue; + } + + if (sbrec_chassis_template_var_is_deleted(tv)) { + SMAP_FOR_EACH (node, local_templates) { + sset_add(deleted, node->key); + } + } else if (sbrec_chassis_template_var_is_new(tv)) { + ovs_assert(smap_count(local_templates) == 0); + SMAP_FOR_EACH (node, &tv->variables) { + sset_add(new, node->key); + } + } else { + SMAP_FOR_EACH (node, &tv->variables) { + struct smap_node *old = smap_get_node(local_templates, + node->key); + if (old) { + if (strcmp(old->value, node->value)) { + sset_add(updated, node->key); + } + } else { + sset_add(new, node->key); + } + } + SMAP_FOR_EACH (node, local_templates) { + sset_add (deleted, node->key); + } + } + + smap_destroy(local_templates); + smap_clone(local_templates, &tv->variables); + return; + } +} + +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); + smap_init(&tv_data->local_templates); + 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 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)); + struct ovsdb_idl_index *sbrec_chassis_template_var_index_by_chassis = + engine_ovsdb_node_get_index( + engine_get_input("SB_chassis_template_var", node), + "chassis"); + + smap_clear(&tv_data->local_templates); + template_vars_init(sbrec_chassis_template_var_index_by_chassis, + chassis, &tv_data->local_templates); + engine_set_node_state(node, EN_UPDATED); +} + +static bool +template_vars_sb_chassis_template_var_handler(struct engine_node *node, + void *data) +{ + struct ed_type_template_vars *tv_data = data; + + const struct sbrec_chassis_template_var_table *tv_table = + EN_OVSDB_GET(engine_get_input("SB_chassis_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->local_templates, + &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; + + smap_destroy(&tv_data->local_templates); + 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; @@ -2702,6 +2891,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 = @@ -2734,6 +2926,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->local_templates; l_ctx_out->flow_table = &fo->flow_table; l_ctx_out->group_table = &fo->group_table; @@ -3060,6 +3253,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) @@ -3646,6 +3897,9 @@ main(int argc, char *argv[]) struct ovsdb_idl_index *sbrec_static_mac_binding_by_datapath = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, &sbrec_static_mac_binding_col_datapath); + struct ovsdb_idl_index *sbrec_chassis_template_var_index_by_chassis + = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, + &sbrec_chassis_template_var_col_chassis); ovsdb_idl_track_add_all(ovnsb_idl_loop.idl); ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, @@ -3707,6 +3961,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"); @@ -3733,6 +3988,10 @@ 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, NULL); + engine_add_input(&en_template_vars, &en_sb_chassis, NULL); + engine_add_input(&en_template_vars, &en_sb_chassis_template_var, + template_vars_sb_chassis_template_var_handler); engine_add_input(&en_addr_sets, &en_sb_address_set, addr_sets_sb_address_set_handler); @@ -3800,6 +4059,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, @@ -3908,6 +4169,8 @@ main(int argc, char *argv[]) sbrec_mac_binding_by_datapath); engine_ovsdb_node_add_index(&en_sb_static_mac_binding, "datapath", sbrec_static_mac_binding_by_datapath); + engine_ovsdb_node_add_index(&en_sb_chassis_template_var, "chassis", + sbrec_chassis_template_var_index_by_chassis); struct ed_type_lflow_output *lflow_output_data = engine_get_internal_data(&en_lflow_output); @@ -3917,6 +4180,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, @@ -3978,6 +4243,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->local_templates); + unixctl_command_register("debug/ignore-startup-delay", "", 0, 0, debug_ignore_startup_delay, NULL); @@ -4354,9 +4623,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->local_templates); if (error) { unixctl_command_reply_error(pending_pkt.conn, error); free(error); @@ -4825,6 +5097,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 3b141b034c..80d95ff675 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 ecb7ace243..516bbc2bd8 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,8 @@ void lexer_syntax_error(struct lexer *, const char *message, ...) char *lexer_steal_error(struct lexer *); +const char *lexer_parse_template_string(const char *s, + const struct smap *template_vars, + struct sset *template_vars_ref, + char **out_expanded); #endif /* ovn/lex.h */ diff --git a/lib/actions.c b/lib/actions.c index adbb42db4f..d7d4868835 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 d1f9d28ca6..43f44771cf 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 c84d52aa8d..7b44bc3537 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,40 @@ 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'. + */ +const char * +lexer_parse_template_string(const char *s, const struct smap *template_vars, + struct sset *template_vars_ref, + char **out_expanded) +{ + /* No '^' means no templates. */ + if (!strchr(s, '^')) { + *out_expanded = NULL; + 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); + *out_expanded = ds_steal_cstr(&expanded); + return *out_expanded; +} diff --git a/lib/objdep.c b/lib/objdep.c index 092d4af261..06cf126f12 100644 --- a/lib/objdep.c +++ b/lib/objdep.c @@ -245,6 +245,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 50c7b01ef1..1ea781947c 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/tests/ovn-controller.at b/tests/ovn-controller.at index 3c3fb31c7c..5fb6c65eba 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -2337,3 +2337,53 @@ done AT_CHECK([grep "deleted interface patch" hv1/ovs-vswitchd.log], [1], [ignore]) OVN_CLEANUP([hv1]) AT_CLEANUP + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn-controller - Local Chassis_Template_Var updates]) +AT_KEYWORDS([templates]) +ovn_start + +net_add n1 +sim_add hv +as hv +ovs-vsctl \ + -- add-br br-phys \ + -- add-br br-eth0 \ + -- add-br br-eth1 \ + -- add-br br-eth2 +ovn_attach n1 br-phys 192.168.0.1 + +m4_define([GET_LOCAL_TEMPLATE_VARS], + [ovn-appctl debug/dump-local-template-vars | grep -v 'Local template vars']) + +dnl Expect no local vars initially. +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [1], []) + +AT_CHECK([ovn-nbctl --wait=hv create Chassis_Template_Var chassis="hv"], [0], [ignore]) +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [1], []) + +dnl Expect new vars to be handled properly. +check ovn-nbctl --wait=hv set Chassis_Template_Var hv variables:foo=foo-hv variables:bar=bar-hv +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [0], [dnl +name: 'bar' value: 'bar-hv' +name: 'foo' value: 'foo-hv' +]) + +dnl Expect var updates to be handled properly. +check ovn-nbctl --wait=hv set Chassis_Template_Var hv variables:bar=bar-new-hv +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [0], [dnl +name: 'bar' value: 'bar-new-hv' +name: 'foo' value: 'foo-hv' +]) + +dnl Expect var deletions to be handled properly. +check ovn-nbctl --wait=hv remove Chassis_Template_Var hv variables bar +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [0], [dnl +name: 'foo' value: 'foo-hv' +]) + +check ovn-nbctl --wait=hv remove Chassis_Template_Var hv variables foo +AT_CHECK([GET_LOCAL_TEMPLATE_VARS], [1], []) + +AT_CLEANUP +]) diff --git a/tests/ovn.at b/tests/ovn.at index f8b8db4df8..2a59235c59 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 @@ -32975,3 +32975,81 @@ check ovn-nbctl --wait=hv sync OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([Logical flows with Chassis_Template_Var references]) +AT_KEYWORDS([templates]) +ovn_start +net_add n1 + +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + +check ovn-nbctl ls-add sw + +dnl Use --wait=sb to ensure lsp1 getting a tunnel_key before lsp2. +check ovn-nbctl --wait=sb lsp-add sw lsp1 +check ovn-nbctl --wait=sb lsp-add sw lsp2 + +AT_CHECK([ovn-nbctl create Chassis_Template_Var chassis=hv1], [0], [ignore]) + +check ovn-nbctl pg-add pg1 lsp1 lsp2 +AT_CHECK([ovn-nbctl create address_set name=as1 addresses=\"1.1.1.1\",\"1.1.1.2\"], [0], [ignore]) + +dnl Create an ACL that uses an "uninstantiated" template. +check ovn-nbctl acl-add sw from-lport 1 "ip4.src == 42.42.42.42 && ^CONDITION" allow + +check ovs-vsctl add-port br-int p1 -- set interface p1 external_ids:iface-id=lsp1 +check ovs-vsctl add-port br-int p2 -- set interface p2 external_ids:iface-id=lsp2 + +wait_for_ports_up +ovn-nbctl --wait=hv sync + +dnl Ensure the ACL is not translated to OpenFlow. +as hv1 +AT_CHECK([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42'], [1], []) + +dnl Create a Chassis_Template_Var mapping for CONDITION. +check ovn-nbctl --wait=hv set Chassis_Template_Var hv1 variables:CONDITION='inport == @pg1' + +lsp1=0x$(fetch_column Port_Binding tunnel_key logical_port=lsp1) +lsp2=0x$(fetch_column Port_Binding tunnel_key logical_port=lsp2) + +dnl Ensure the ACL is translated to OpenFlows expanding pg1. +as hv1 +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl + table=16, priority=1001,ip,reg14=$lsp1,metadata=0x1,nw_src=42.42.42.42 actions=resubmit(,17) + table=16, priority=1001,ip,reg14=$lsp2,metadata=0x1,nw_src=42.42.42.42 actions=resubmit(,17) +]) + +dnl Remove a port from pg1 and expect OpenFlows to be correctly updated. +check ovn-nbctl --wait=hv pg-set-ports pg1 lsp2 +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl + table=16, priority=1001,ip,reg14=$lsp2,metadata=0x1,nw_src=42.42.42.42 actions=resubmit(,17) +]) + +dnl Change the Chassis_Template_Var mapping to use the address set. +check ovn-nbctl --wait=hv set Chassis_Template_Var hv1 variables:CONDITION='ip4.dst == $as1' + +dnl Ensure the ACL is translated to OpenFlows expanding as1. +as hv1 +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl + table=16, priority=1001,ip,metadata=0x1,nw_src=42.42.42.42,nw_dst=1.1.1.1 actions=resubmit(,17) + table=16, priority=1001,ip,metadata=0x1,nw_src=42.42.42.42,nw_dst=1.1.1.2 actions=resubmit(,17) +]) + +dnl Remove an IP from AS1 and expect OpenFlows to be correctly updated. +check ovn-nbctl set address_set as1 addresses=\"1.1.1.1\" +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42' | ofctl_strip_all], [0], [dnl + table=16, priority=1001,ip,metadata=0x1,nw_src=42.42.42.42,nw_dst=1.1.1.1 actions=resubmit(,17) +]) + +dnl Remove the mapping and expect OpenFlows to be removed. +check ovn-nbctl --wait=hv clear Chassis_Template_Var hv1 variables +AT_CHECK([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42'], [1], []) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) diff --git a/tests/test-ovn.c b/tests/test-ovn.c index a241f150d1..c422bafd70 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,11 @@ 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_expanded = NULL; + const char *exp_input = + lexer_parse_template_string(ds_cstr(&input), &template_vars, NULL, + &exp_input_expanded); + 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 +1418,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_expanded); } ds_destroy(&input); @@ -1419,6 +1429,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 6fa5137d95..b837884ccc 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,15 @@ 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_expanded_s = NULL; + const char *match_s = lexer_parse_template_string(sblf->match, + &template_vars, + NULL, + &match_expanded_s); + match = expr_parse_string(match_s, &symtab, &address_sets, &port_groups, NULL, NULL, dp->tunnel_key, &error); + free(match_expanded_s); if (error) { VLOG_WARN("%s: parsing expression failed (%s)", sblf->match, error); @@ -980,7 +987,12 @@ 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_expanded_s = NULL; + const char *actions_s = + lexer_parse_template_string(sblf->actions, &template_vars, + NULL, &actions_expanded_s); + error = ovnacts_parse_string(actions_s, &pp, &ovnacts, &prereqs); + free(actions_expanded_s); if (error) { VLOG_WARN("%s: parsing actions failed (%s)", sblf->actions, error); free(error); @@ -1078,6 +1090,7 @@ read_gen_opts(void) nd_ra_opts_init(&nd_ra_opts); controller_event_opts_init(&event_opts); + smap_init(&template_vars); } static void @@ -3420,9 +3433,15 @@ 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_expanded_s = NULL; + const char *flow_exp_s = lexer_parse_template_string(flow_s, + &template_vars, + NULL, + &flow_expanded_s); + struct expr *e = expr_parse_string(flow_exp_s, &symtab, &address_sets, &port_groups, NULL, NULL, 0, &error); + free(flow_expanded_s); if (!e) { return trace_parse_error(error); } @@ -3447,9 +3466,15 @@ 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_expanded_s = NULL; + const char *flow_exp_s = lexer_parse_template_string(flow_s, + &template_vars, + NULL, + &flow_expanded_s); + char *error = expr_parse_microflow(flow_exp_s, &symtab, &address_sets, &port_groups, ovntrace_lookup_port, *dpp, uflow); + free(flow_expanded_s); if (error) { return trace_parse_error(error); }
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 Chassis_Template_Var also track references (similar to the ones maintained for multicast groups, address sets, port_groups, port bindings). Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- V2: - Fix GCC build due to missing newline. - Handle SB table rename Template_Var -> Chassis_Template_Var. - Address Han's comments: - Add new function to parse lflow actions. - Move xstrdup inside lexer_parse_template_string() and execute it only if needed. - Match template vars only by chassis name. - Change local_templates table to a plain smap. - Some indentation updates. - Use NULL template variable change handlers for chassis/Open_vSwitch changes. - Added more tests. - Fix tracking of template references in lflow actions. --- controller/lflow.c | 136 ++++++++++++++------ controller/lflow.h | 1 controller/ofctrl.c | 11 +- controller/ofctrl.h | 3 controller/ovn-controller.c | 289 +++++++++++++++++++++++++++++++++++++++++++ include/ovn/expr.h | 4 - include/ovn/lex.h | 15 ++ lib/actions.c | 9 + lib/expr.c | 18 ++- lib/lex.c | 57 ++++++++ lib/objdep.c | 1 lib/objdep.h | 1 tests/ovn-controller.at | 50 +++++++ tests/ovn.at | 80 ++++++++++++ tests/test-ovn.c | 17 ++- utilities/ovn-trace.c | 33 ++++- 16 files changed, 658 insertions(+), 67 deletions(-)