On Thu, Mar 28, 2024 at 1:29 PM Mark Michelson <mmich...@redhat.com> wrote:
>
> Thanks Han,
>
> Acked-by: Mark Michelson <mmich...@redhat.com>

Thanks Mark. Applied to main.

Han
>
> On 3/28/24 02:58, Han Zhou wrote:
> > The ovn-ofctrl-wait-before-clear setting is designed to minimize
> > downtime during the initial start-up of the ovn-controller. For this
> > purpose, the ovn-controller should wait only once upon entering the
> > S_WAIT_BEFORE_CLEAR state for the first time. Subsequent reconnections
> > to the OVS, such as those occurring during an OVS restart/upgrade,
> > should not trigger this wait. However, the current implemention always
> > waits for the configured time in the S_WAIT_BEFORE_CLEAR state, which
> > can inadvertently delay flow installations during OVS restart/upgrade,
> > potentially causing more harm than good. (The extent of the impact
> > varies based on the method used to restart OVS, including whether flow
> > save/restore tools and the flow-restore-wait feature are employed.)
> >
> > This patch avoids the unnecessary wait after the initial one.
> >
> > Fixes: 896adfd2d8b3 ("ofctrl: Support ovn-ofctrl-wait-before-clear to
reduce down time during upgrade.")
> > Signed-off-by: Han Zhou <hz...@ovn.org>
> > ---
> > v2: Addressed Mark's comments - made test case more reliable.
> >
> >   controller/ofctrl.c     | 1 -
> >   tests/ovn-controller.at | 9 +++++++--
> >   2 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> > index 6ca2ea4ce63d..6a2564604aa1 100644
> > --- a/controller/ofctrl.c
> > +++ b/controller/ofctrl.c
> > @@ -634,7 +634,6 @@ run_S_WAIT_BEFORE_CLEAR(void)
> >       if (!wait_before_clear_time ||
> >           (wait_before_clear_expire &&
> >            time_msec() >= wait_before_clear_expire)) {
> > -        wait_before_clear_expire = 0;
> >           state = S_CLEAR_FLOWS;
> >           return;
> >       }
> > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> > index fdcc5aab2bcf..3202f0beff46 100644
> > --- a/tests/ovn-controller.at
> > +++ b/tests/ovn-controller.at
> > @@ -2324,10 +2324,15 @@ lflow_run_2=$(ovn-appctl -t ovn-controller
coverage/read-counter lflow_run)
> >   AT_CHECK_UNQUOTED([echo $lflow_run_1], [0], [$lflow_run_2
> >   ])
> >
> > -# Restart OVS this time, and wait until flows are reinstalled
> > +# Restart OVS this time. Flows should be reinstalled without waiting.
> >   OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> >   start_daemon ovs-vswitchd --enable-dummy=system -vvconn
-vofproto_dpif -vunixctl
> > -OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep -F 10.1.2.4 | grep
-vF 2.2.2.2])
> > +
> > +# Sync to make sure ovn-controller is given enough time to install the
flows.
> > +check ovn-nbctl --wait=hv sync
> > +
> > +# Flow should be installed without any extra waiting.
> > +AT_CHECK([ovs-ofctl dump-flows br-int | grep -F 10.1.2.4 | grep -vF
2.2.2.2], [0], [ignore])
> >
> >   check ovn-nbctl --wait=hv lb-add lb3 3.3.3.3 10.1.2.5 \
> >   -- ls-lb-add ls1 lb3
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to