[ovs-dev] [PATCH ovn] ovn.at: Fix flaky test "controller I-P handling with monitoring disabled".

Numan Siddique numans at ovn.org
Wed Oct 6 17:09:16 UTC 2021


On Wed, Sep 15, 2021 at 4:53 AM Xavier Simonart <xsimonar at redhat.com> wrote:
>
> Test was waiting for port to be up in SBDB before checking number of flows
> in OVS. However, there is no guarantee that all flows are installed
> in OVS when port is up. Test was randomly failing as some flows were
> installed, but not all.
> To fix this, we wait until the last flow (with actions=output) is
> installed.
> Also fixed small typo in logging (for the same test).
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2004390
> Fixes: f8a81693b0 ("ovn-controller: Fix the missing flows with monitor-all set to True")
> Signed-off-by: Xavier Simonart <xsimonar at redhat.com>

Thanks for fixing this issue.  I applied this patch to the main branch.

I did one change.  Please see below

Thanks
Numan

> ---
>  tests/ovn.at | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 30625ec37..18aeacd02 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -23448,6 +23448,12 @@ ovs-vsctl -- add-port br-int hv1-vif1 -- \
>
>  wait_for_ports_up sw0-p1
>
> +# Wait for last flow to be installed
> +OVS_WAIT_UNTIL([
> +    test $(as hv1 ovs-ofctl dump-flows br-int | \
> +    grep "actions=output" -c) -eq 1
> +])
> +
>  # Get the number of OF flows in hv1 and hv2
>  hv1_offlows=$(as hv1 ovs-ofctl dump-flows br-int | wc -l)
>  echo "hv1 flows : $hv1_offlows"
> @@ -23462,6 +23468,12 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \
>
>  wait_for_ports_up sw0-p2
>
> +# Wait for last flow to be installed
> +OVS_WAIT_UNTIL([
> +    test $(as hv2 ovs-ofctl dump-flows br-int | \
> +    grep "actions=output" -c) -eq 1
> +])
> +
>  hv2_offlows=$(as hv2 ovs-ofctl dump-flows br-int | wc -l)
>  echo "hv2 flows : $hv2_offlows"
>  AT_CHECK([test $hv2_offlows -gt 0])
> @@ -23500,9 +23512,15 @@ ovs-vsctl -- add-port br-int hv1-vif1 -- \
>
>  wait_for_ports_up sw0-p1
>
> +# Wait for last flow to be installed

With  the commit 2cc42bdef("northd: Change the default value of
ignore_lsp_down to true.")
merged,  this arp flow may not be the last flow.  So I replaced "last
flow" with "arp flow".

Numan

> +OVS_WAIT_UNTIL([
> +    test $(as hv1 ovs-ofctl dump-flows br-int | \
> +    grep "actions=output" -c) -eq 1
> +])
> +
>  # Get the number of OF flows in hv1 and hv2
>  hv1_offlows_mon=$(as hv1 ovs-ofctl dump-flows br-int | wc -l)
> -echo "hv1 flows after monitor-all=true : $hv1_offlows"
> +echo "hv1 flows after monitor-all=true : $hv1_offlows_mon"
>  AT_CHECK([test "$hv1_offlows" = "$hv1_offlows_mon"])
>
>  as hv2
> @@ -23514,8 +23532,14 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \
>
>  wait_for_ports_up sw0-p2
>
> +# Wait for last flow to be installed
> +OVS_WAIT_UNTIL([
> +    test $(as hv2 ovs-ofctl dump-flows br-int | \
> +    grep "actions=output" -c) -eq 1
> +])
> +
>  hv2_offlows_mon=$(as hv2 ovs-ofctl dump-flows br-int | wc -l)
> -echo "hv2 flows after monitor-all=true : $hv2_offlows"
> +echo "hv2 flows after monitor-all=true : $hv2_offlows_mon"
>  AT_CHECK([test "$hv2_offlows" = "$hv2_offlows_mon"])
>
>  OVN_CLEANUP([hv1], [hv2])
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list