diff mbox series

[ovs-dev,v2,2/2] controller: Remove wait-before-clear option.

Message ID 20250507110555.41641-3-alekssmirnov@k2.cloud
State Deferred
Headers show
Series controller: Replace wait-before-clear with automated detectiohn of final lflow update. | expand

Checks

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

Commit Message

Aleksandr Smirnov May 7, 2025, 11:05 a.m. UTC
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>
Acked-by: Numan Siddique <numans@ovn.org>
---
 NEWS                            |   3 +
 controller/ofctrl.c             |  34 +---------
 controller/ovn-controller.8.xml |  34 ----------
 tests/ovn-controller.at         | 106 --------------------------------
 4 files changed, 4 insertions(+), 173 deletions(-)
---
  v2: Addressed Numan's comments.
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index f82f25754..fda8807f2 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,9 @@  Post v25.03.0
    - Introduce exclude-router-ips-from-garp in logical_switch_port column
      so that the router port IPs are not advertised in the GARPs.
    - Added support for port mirroring in OVN overlay.
+   - An ovn-controller option wait-before-clear is no longer supported.
+     It will be ignored if used. An ovn-controller will automatically care
+     about proper delay before clearing lflow.
 
 OVN v25.03.0 - 07 Mar 2025
 --------------------------
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/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index 5007f5f80..3f2cf5bae 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -294,40 +294,6 @@ 
         The default value is considered false if this option is not defined.
       </dd>
 
-      <dt><code>external_ids:ovn-ofctrl-wait-before-clear</code></dt>
-      <dd>
-        The time, in milliseconds, to wait before clearing flows in OVS after
-        OpenFlow connection/reconnection during <code>ovn-controller</code>
-        initialization. The purpose of this wait is to give time for
-        <code>ovn-controller</code> to compute the new flows before clearing
-        existing ones, to avoid data plane down time during
-        <code>ovn-controller</code> restart/upgrade at large scale
-        environments where recomputing the flows takes more than a few seconds
-        or even longer. It is difficult for <code>ovn-controller</code>
-        to determine when the new flows computing is completed, because of the
-        dynamics in the cloud environments, which is why this configuration is
-        provided for users to adjust based on the scale of the environment. By
-        default, it is 0, which means clearing existing flows without waiting.
-        Not setting the value, or setting it too small, may result in data
-        plane down time during upgrade/restart, while setting it too big may
-        result in unnecessary extra control plane latency of applying new
-        changes of CMS during upgrade/restart. In most cases, a slightly bigger
-        value is not harmful, because the extra control plane latency happens
-        only once during the OpenFlow connection. To get a reasonable range of
-        the value setting, it is recommended to run the below commands on a
-        node in the target environment and then set this configuration to twice
-        the value of <code>Maximum</code> shown in the output of the second
-        command.
-        <ul>
-          <li>
-            <code>ovn-appctl -t ovn-controller inc-engine/recompute</code>
-          </li>
-          <li>
-            <code>ovn-appctl -t ovn-controller stopwatch/show flow-generation</code>
-          </li>
-        </ul>
-      </dd>
-
       <dt><code>external_ids:ovn-enable-lflow-cache</code></dt>
       <dd>
         The boolean flag indicates if <code>ovn-controller</code> should
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index a646381b7..3fcb4bdbc 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -2349,112 +2349,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])