On Mon, Aug 31, 2020 at 5:45 PM Dumitru Ceara <[email protected]> wrote:
> If a Port_Binding is deleted from the Southbound DB and the > corresponding OVS interface is also deleted from the OVS DB, and > if both updates are received and processed by ovn-controller in > the same iteration, ovn-controller should process port_binding > delete operations first. > > This commit also adds three new unixctl debug commands for > ovn-controller: > - debug/pause: pause ovn-controller processing, except unixctl commands. > - debug/resume: resume ovn-controller processing. > - debug/status: return the status of the ovn-controller processing. > > These new commands are needed by the test for this scenario as without > them we have no way of ensuring predictable results. Users should not > use these commands in production. This is also why the commands are not > documented. > > CC: Numan Siddique <[email protected]> > Fixes: 6b0f01116bab ("ovn-controller: Handle runtime data changes in flow > output engine") > Reported-by: Tim Rozet <[email protected]> > Reported-at: https://bugzilla.redhat.com/1871961 > Signed-off-by: Dumitru Ceara <[email protected]> > --- > v2: Rework the fix per Numan's suggestion and run the port_binding > handler before the ovs iface handler. > --- > Thanks Dumitru for v2. I applied this patch to master and branch-20.06. Numan > controller/ovn-controller.c | 71 > ++++++++++++++++++++++++++++++++++++++++++--- > tests/ovn.at | 38 ++++++++++++++++++++++++ > 2 files changed, 105 insertions(+), 4 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index d299c61..874087c 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -73,6 +73,9 @@ static unixctl_cb_func extend_table_list; > static unixctl_cb_func inject_pkt; > static unixctl_cb_func engine_recompute_cmd; > static unixctl_cb_func cluster_state_reset_cmd; > +static unixctl_cb_func debug_pause_execution; > +static unixctl_cb_func debug_resume_execution; > +static unixctl_cb_func debug_status_execution; > > #define DEFAULT_BRIDGE_NAME "br-int" > #define DEFAULT_PROBE_INTERVAL_MSEC 5000 > @@ -2276,10 +2279,6 @@ main(int argc, char *argv[]) > > engine_add_input(&en_runtime_data, &en_ovs_open_vswitch, NULL); > engine_add_input(&en_runtime_data, &en_ovs_bridge, NULL); > - engine_add_input(&en_runtime_data, &en_ovs_port, > - engine_noop_handler); > - engine_add_input(&en_runtime_data, &en_ovs_interface, > - runtime_data_ovs_interface_handler); > engine_add_input(&en_runtime_data, &en_ovs_qos, NULL); > > engine_add_input(&en_runtime_data, &en_sb_chassis, NULL); > @@ -2288,6 +2287,15 @@ main(int argc, char *argv[]) > engine_add_input(&en_runtime_data, &en_sb_port_binding, > runtime_data_sb_port_binding_handler); > > + /* The OVS interface handler for runtime_data changes MUST be executed > + * after the sb_port_binding_handler as port_binding deletes must be > + * processed first. > + */ > + engine_add_input(&en_runtime_data, &en_ovs_port, > + engine_noop_handler); > + engine_add_input(&en_runtime_data, &en_ovs_interface, > + runtime_data_ovs_interface_handler); > + > struct engine_arg engine_arg = { > .sb_idl = ovnsb_idl_loop.idl, > .ovs_idl = ovs_idl_loop.idl, > @@ -2342,6 +2350,14 @@ main(int argc, char *argv[]) > cluster_state_reset_cmd, > &reset_ovnsb_idl_min_index); > > + bool paused = false; > + unixctl_command_register("debug/pause", "", 0, 0, > debug_pause_execution, > + &paused); > + unixctl_command_register("debug/resume", "", 0, 0, > debug_resume_execution, > + &paused); > + unixctl_command_register("debug/status", "", 0, 0, > debug_status_execution, > + &paused); > + > unsigned int ovs_cond_seqno = UINT_MAX; > unsigned int ovnsb_cond_seqno = UINT_MAX; > > @@ -2350,6 +2366,15 @@ main(int argc, char *argv[]) > restart = false; > bool sb_monitor_all = false; > while (!exiting) { > + /* If we're paused just run the unixctl server and skip most of > the > + * processing loop. > + */ > + if (paused) { > + unixctl_server_run(unixctl); > + unixctl_server_wait(unixctl); > + goto loop_done; > + } > + > engine_init_run(); > > struct ovsdb_idl_txn *ovs_idl_txn = > ovsdb_idl_loop_run(&ovs_idl_loop); > @@ -2604,6 +2629,8 @@ main(int argc, char *argv[]) > > ovsdb_idl_track_clear(ovnsb_idl_loop.idl); > ovsdb_idl_track_clear(ovs_idl_loop.idl); > + > +loop_done: > poll_block(); > if (should_service_stop()) { > exiting = true; > @@ -2857,3 +2884,39 @@ cluster_state_reset_cmd(struct unixctl_conn *conn, > int argc OVS_UNUSED, > poll_immediate_wake(); > unixctl_command_reply(conn, NULL); > } > + > +static void > +debug_pause_execution(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *paused_) > +{ > + bool *paused = paused_; > + > + VLOG_INFO("User triggered execution pause."); > + *paused = true; > + unixctl_command_reply(conn, NULL); > +} > + > +static void > +debug_resume_execution(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *paused_) > +{ > + bool *paused = paused_; > + > + VLOG_INFO("User triggered execution resume."); > + *paused = false; > + poll_immediate_wake(); > + unixctl_command_reply(conn, NULL); > +} > + > +static void > +debug_status_execution(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *paused_) > +{ > + bool *paused = paused_; > + > + if (*paused) { > + unixctl_command_reply(conn, "paused"); > + } else { > + unixctl_command_reply(conn, "running"); > + } > +} > diff --git a/tests/ovn.at b/tests/ovn.at > index c4edbd9..5ad51c0 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -21371,3 +21371,41 @@ AT_CHECK([ovn-sbctl find mac ip=10.0.0.2 > mac='"00:00:00:00:03:02"' logical_port= > OVN_CLEANUP([hv1],[hv2]) > > AT_CLEANUP > + > +AT_SETUP([ovn -- Delete Port_Binding and OVS port Incremental Processing]) > +ovn_start > + > +net_add n1 > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.10 > + > +ovn-nbctl ls-add ls > +ovn-nbctl lsp-add ls lsp > + > +as hv1 ovs-vsctl \ > + -- add-port br-int vif1 \ > + -- set Interface vif1 external_ids:iface-id=lsp > + > +# Wait for port to be bound. > +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns _uuid --bare list chassis hv1 > | wc -l) -eq 1]) > +ch=$(ovn-sbctl --columns _uuid --bare list chassis hv1) > +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns chassis --bare list > port_binding lsp | grep $ch -c) -eq 1]) > + > +# Pause ovn-controller. > +as hv1 ovn-appctl -t ovn-controller debug/pause > + > +# Delete port binding and OVS port. The updates will be processed in the > same > +# loop in ovn-controller when it resumes. > +ovn-nbctl --wait=sb lsp-del lsp > +as hv1 ovs-vsctl del-port vif1 > + > +# Resume ovn-controller. > +as hv1 ovn-appctl -t ovn-controller debug/resume > + > +# Make sure ovn-controller runs fine. > +OVS_WAIT_UNTIL([test x$(as hv1 ovn-appctl -t ovn-controller debug/status) > = "xrunning"]) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > -- > 1.8.3.1 > > _______________________________________________ > 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
