On Wed, Sep 15, 2021 at 4:53 AM Xavier Simonart <xsimo...@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 <xsimo...@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
> 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
