On Mon, Mar 28, 2022 at 1:48 AM Han Zhou <[email protected]> wrote: > > Add an engine node en_northd_internal_version as an input to > en_lflow_output. When this node is updated, it triggers a recompute for > en_lflow_output. This node adds SB_Global as its only input, and it is > updated only when SB_Global's options:northd_internal_version is updated. > > In a later patch the northd_internal_version will be used in > en_lflow_output and impact flow generation. > > Signed-off-by: Han Zhou <[email protected]> > --- > controller/ovn-controller.c | 66 +++++++++++++++++++++++++++++++++++++ > tests/ovn-controller.at | 46 ++++++++++++++++++++++++++ > 2 files changed, 112 insertions(+) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index ea5e9df41..26ca97825 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -954,6 +954,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl) > } > > #define SB_NODES \ > + SB_NODE(sb_global, "sb_global") \ > SB_NODE(chassis, "chassis") \ > SB_NODE(encap, "encap") \ > SB_NODE(address_set, "address_set") \ > @@ -2194,6 +2195,65 @@ non_vif_data_ovs_iface_handler(struct engine_node > *node, void *data OVS_UNUSED) > return local_nonvif_data_handle_ovs_iface_changes(iface_table); > } > > +struct ed_type_northd_internal_version { > + char *ver; > +}; > + > + > +static void * > +en_northd_internal_version_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED) > +{ > + struct ed_type_northd_internal_version *n_ver = xzalloc(sizeof *n_ver); > + n_ver->ver = xstrdup(""); > + return n_ver; > +} > + > +static void > +en_northd_internal_version_cleanup(void *data) > +{ > + struct ed_type_northd_internal_version *n_ver = data; > + free(n_ver->ver); > +} > + > +static void > +en_northd_internal_version_run(struct engine_node *node, void *data) > +{ > + struct ed_type_northd_internal_version *n_ver = data; > + struct sbrec_sb_global_table *sb_global_table = > + (struct sbrec_sb_global_table *)EN_OVSDB_GET( > + engine_get_input("SB_sb_global", node)); > + const struct sbrec_sb_global *sb_global = > + sbrec_sb_global_table_first(sb_global_table); > + free(n_ver->ver); > + n_ver->ver = > + xstrdup(sb_global ? smap_get_def(&sb_global->options, > + "northd_internal_version", "") : > ""); > + engine_set_node_state(node, EN_UPDATED); > +} > + > +static bool > +en_northd_internal_version_sb_sb_global_handler(struct engine_node *node, > + void *data) > +{ > + struct ed_type_northd_internal_version *n_ver = data; > + struct sbrec_sb_global_table *sb_global_table = > + (struct sbrec_sb_global_table *)EN_OVSDB_GET( > + engine_get_input("SB_sb_global", node)); > + const struct sbrec_sb_global *sb_global = > + sbrec_sb_global_table_first(sb_global_table); > + > + const char *new_ver = > + sb_global ? smap_get_def(&sb_global->options, > + "northd_internal_version", "") : ""; > + if (strcmp(new_ver, n_ver->ver)) { > + free(n_ver->ver); > + n_ver->ver = xstrdup(new_ver); > + engine_set_node_state(node, EN_UPDATED); > + } > + return true; > +} > + > struct lflow_output_persistent_data { > struct lflow_cache *lflow_cache; > }; > @@ -3243,6 +3303,7 @@ main(int argc, char *argv[]) > ENGINE_NODE(flow_output, "flow_output"); > ENGINE_NODE_WITH_CLEAR_TRACK_DATA(addr_sets, "addr_sets"); > ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_groups, "port_groups"); > + ENGINE_NODE(northd_internal_version, "northd_internal_version"); > > #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR); > SB_NODES > @@ -3291,6 +3352,11 @@ main(int argc, char *argv[]) > engine_add_input(&en_pflow_output, &en_ovs_open_vswitch, NULL); > engine_add_input(&en_pflow_output, &en_ovs_bridge, NULL); > > + engine_add_input(&en_northd_internal_version, &en_sb_sb_global, > + en_northd_internal_version_sb_sb_global_handler); > + > + engine_add_input(&en_lflow_output, &en_northd_internal_version, NULL); > + > /* Keep en_addr_sets before en_runtime_data because > * lflow_output_runtime_data_handler may *partially* reprocess a lflow > when > * the lflow is attached to a DP group and a new DP in that DP group is > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index 6e4c24f0f..236f1c015 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -2057,3 +2057,49 @@ AT_CHECK([echo $(($reprocess_count_new - > $reprocess_count_old))], [0], [2 > > OVN_CLEANUP([hv1]) > AT_CLEANUP > + > +AT_SETUP([ovn-controller - I-P handle northd_internal_version change]) > + > +ovn_start --backup-northd=none > + > +net_add n1 > +sim_add hv1 > +as hv1 > +check ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ > + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 > + > +check ovn-nbctl ls-add ls1 > + > +check ovn-nbctl lsp-add ls1 ls1-lp1 \ > +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" > + > +wait_for_ports_up > +ovn-appctl -t ovn-controller vlog/set file:dbg > + > +read_counter() { > + ovn-appctl -t ovn-controller coverage/read-counter $1 > +} > + > +# nb_cfg update in sb_global shouldn't trigger lflow_run. > +lflow_run_old=$(read_counter lflow_run) > +ovn-nbctl --wait=hv sync > +lflow_run_new=$(read_counter lflow_run) > +AT_CHECK([echo $(($lflow_run_new - $lflow_run_old))], [0], [0 > +]) > + > +# northd_internal_version update in sb_global:options should trigger > lflow_run. > +as northd > +OVS_APP_EXIT_AND_WAIT(ovn-northd) > +as hv1 > +lflow_run_old=$(read_counter lflow_run) > +check ovn-sbctl set SB_Global . options:northd_internal_version=foo > +sleep 0.1
Thanks for the patch series. I've just one minor comment here. Instead of sleep, I'd suggest using OVS_WAIT_UNTIL() to avoid flaky test failure in case CI is slow. I've acked the entire series in patch 0. Thanks Numan > +lflow_run_new=$(read_counter lflow_run) > +AT_CHECK([echo $(($lflow_run_new - $lflow_run_old))], [0], [1 > +]) > + > +as northd start_daemon ovn-northd > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > -- > 2.30.2 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
