On Tue, Mar 18, 2025 at 6:08 AM Aleksandr Smirnov <alekssmirnov@k2.cloud> wrote:
>
> Remove wait-before-clear option because lflow upload
> is delayed with that fix
> 'controller: Delay initial flow upload to OVS.'
>
> Signed-off-by: Aleksandr Smirnov <alekssmirnov@k2.cloud>
Please update the ovn-controller.8.xml since you removed the option.
Also please add a NEWS item entry that the option is no longer needed
and will not have any effect if configured.
With those addressed:
Acked-by: Numan Siddique <num...@ovn.org>
Numan
> ---
> controller/ofctrl.c | 34 +------------
> tests/ovn-controller.at | 106 ----------------------------------------
> 2 files changed, 1 insertion(+), 139 deletions(-)
>
> diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> index 304b9bbc8..b0b051e77 100644
> --- a/controller/ofctrl.c
> +++ b/controller/ofctrl.c
> @@ -352,14 +352,6 @@ static enum ofctrl_state state;
> /* Release wait before clear stage. */
> static bool wait_before_clear_proceed = false;
>
> -/* The time (ms) to stay in the state S_WAIT_BEFORE_CLEAR. Read from
> - * external_ids: ovn-ofctrl-wait-before-clear. */
> -static unsigned int wait_before_clear_time = 0;
> -
> -/* The time when the state S_WAIT_BEFORE_CLEAR should complete.
> - * If the timer is not started yet, it is set to 0. */
> -static long long int wait_before_clear_expire = 0;
> -
> /* Transaction IDs for messages in flight to the switch. */
> static ovs_be32 xid, xid2;
>
> @@ -642,26 +634,9 @@ error:
> static void
> run_S_WAIT_BEFORE_CLEAR(void)
> {
> - if (wait_before_clear_time == 0) {
> - if (wait_before_clear_proceed) {
> - state = S_CLEAR_FLOWS;
> - }
> -
> - return;
> - }
> -
> - if (!wait_before_clear_time ||
> - (wait_before_clear_expire &&
> - time_msec() >= wait_before_clear_expire)) {
> + if (wait_before_clear_proceed) {
> state = S_CLEAR_FLOWS;
> - return;
> }
> -
> - if (!wait_before_clear_expire) {
> - /* Start the timer. */
> - wait_before_clear_expire = time_msec() + wait_before_clear_time;
> - }
> - poll_timer_wait_until(wait_before_clear_expire);
> }
>
> static void
> @@ -837,13 +812,6 @@ ofctrl_run(const char *conn_target, int probe_interval,
> const struct ovsrec_open_vswitch *cfg =
> ovsrec_open_vswitch_table_first(ovs_table);
> ovs_assert(cfg);
> - unsigned int _wait_before_clear_time =
> - smap_get_uint(&cfg->external_ids, "ovn-ofctrl-wait-before-clear", 0);
> - if (_wait_before_clear_time != wait_before_clear_time) {
> - VLOG_INFO("ofctrl-wait-before-clear is now %u ms (was %u ms)",
> - _wait_before_clear_time, wait_before_clear_time);
> - wait_before_clear_time = _wait_before_clear_time;
> - }
>
> bool progress = true;
> for (int i = 0; progress && i < 50; i++) {
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 812616711..0988a9108 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -2357,112 +2357,6 @@ OVS_WAIT_UNTIL([
> OVN_CLEANUP([hv1])
> AT_CLEANUP
>
> -AT_SETUP([ovn-controller - ofctrl wait before clearing flows])
> -
> -ovn_start
> -
> -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 10.1.2.3"
> -
> -check ovn-nbctl lb-add lb1 1.1.1.1 10.1.2.3 \
> --- ls-lb-add ls1 lb1
> -
> -check ovn-nbctl lb-add lb2 2.2.2.2 10.1.2.4 \
> --- ls-lb-add ls1 lb2
> -
> -check ovn-nbctl --wait=hv sync
> -# There should be 2 group IDs allocated
> -AT_CHECK([ovn-appctl -t ovn-controller group-table-list | awk '{print $2}' |
> sort | uniq | wc -l], [0], [2
> -])
> -
> -# Stop ovn-controller
> -OVS_APP_EXIT_AND_WAIT([ovn-controller])
> -
> -# Set 5 seconds wait time before clearing OVS flows.
> -check ovs-vsctl set open . external_ids:ovn-ofctrl-wait-before-clear=5000
> -
> -# The old OVS flows should remain (this is regardless of the configuration)
> -AT_CHECK([ovs-ofctl dump-flows br-int | grep -F 10.1.2.3], [0], [ignore])
> -
> -# We should have 2 flows with groups.
> -AT_CHECK([ovs-ofctl dump-flows br-int | grep group -c], [0], [2
> -])
> -
> -# Make a change to the ls1-lp1's IP
> -check ovn-nbctl --wait=sb lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01
> 10.1.2.4"
> -
> -# Start ovn-controller, which should compute new flows but not apply them
> -# until the wait time is completed.
> -start_daemon ovn-controller
> -
> -# Wait for octrl to run - it will handle the wait-before-clear
> -OVS_WAIT_UNTIL([grep -q 'wait-before-clear' hv1/ovn-controller.log])
> -
> -# Check that there is no flow using 10.1.2.4 except the lb one (using
> 2.2.2.2)
> -OVS_WAIT_UNTIL([test 0 = $(ovs-ofctl dump-flows br-int | grep -F 10.1.2.4 |
> grep -cvF 2.2.2.2)])
> -
> -# Check in the middle of the wait.
> -# Wait for ovn-controller to claim the port as it might cause some lflow_run.
> -OVS_WAIT_UNTIL([test 2 = $(grep -c 'Claiming lport ls1-lp1 for this chassis'
> hv1/ovn-controller.log)])
> -
> -lflow_run_1=$(ovn-appctl -t ovn-controller coverage/read-counter lflow_run)
> -AT_CHECK([ovs-ofctl dump-flows br-int | grep -F 10.1.2.3], [0], [ignore])
> -
> -# We should have 2 flows with groups.
> -AT_CHECK([ovs-ofctl dump-flows br-int | grep group -c], [0], [2
> -])
> -
> -sleep 5
> -
> -# Check after the wait
> -OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep -F 10.1.2.4 | grep -vF
> 2.2.2.2])
> -
> -# We should have 2 flows with groups.
> -AT_CHECK([ovs-ofctl dump-flows br-int | grep group -c], [0], [2
> -])
> -lflow_run_2=$(ovn-appctl -t ovn-controller coverage/read-counter lflow_run)
> -
> -# Verify that the flow compute completed during the wait (after the wait it
> -# merely installs the new flows).
> -AT_CHECK_UNQUOTED([echo $lflow_run_1], [0], [$lflow_run_2
> -])
> -
> -# Restart OVS this time. Flows should be reinstalled without waiting.
> -# Set the wait-before-clear to a large value (60s) to make the test more
> reliable.
> -check ovs-vsctl set open . external_ids:ovn-ofctrl-wait-before-clear=60000
> -check ovn-nbctl --wait=hv sync
> -
> -OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> -start_daemon ovs-vswitchd --enable-dummy=system -vvconn -vofproto_dpif
> -vunixctl
> -
> -# Flow should be installed without waiting for another 60s.
> -OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep -F 10.1.2.4 | grep -vF
> 2.2.2.2])
> -
> -check ovn-nbctl --wait=hv lb-add lb3 3.3.3.3 10.1.2.5 \
> --- ls-lb-add ls1 lb3
> -
> -# There should be 3 group IDs allocated (this is to ensure the group ID
> -# allocation is correct after ofctrl state reset.
> -AT_CHECK([ovn-appctl -t ovn-controller group-table-list | awk '{print $2}' |
> sort | uniq | wc -l], [0], [3
> -])
> -
> -# We should have 3 flows with groups.
> -AT_CHECK([ovs-ofctl dump-flows br-int | grep group -c], [0], [3
> -])
> -
> -OVN_CLEANUP([hv1])
> -AT_CLEANUP
> -
> OVN_FOR_EACH_NORTHD([
> AT_SETUP([ovn-controller - ofctrl delay until all monitored updates come])
>
> --
> 2.47.0
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
