diff mbox series

[ovs-dev] tests: Fix learned flows causing flaky tests.

Message ID 20260825125351.2275725-1-xsimonar@redhat.com
State Superseded
Delegated to: Ales Musil
Headers show
Series [ovs-dev] tests: Fix learned flows causing flaky tests. | 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 success github build: passed

Commit Message

Xavier Simonart Aug. 25, 2026, 12:53 p.m. UTC
CHECK_AFTER_RECOMPUTE and OVN CHECK_FLOWS_AFTER_RECOMPUTE macros compare
flows before and after recompute, and reports test failure if those flows
are different.
However learned flows can happen at any time e.g. due to IPv6 Router
Solicitation. If the test is long they might also be deleted.
So, ignore learned flows when comparing flows before and after recompute.

See for instance [0] for a test failing for this reason.

[0] https://github.com/ovsrobot/ovn/actions/runs/32723237137/job/97422492101#step:12:5869

Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
---
 tests/ovn-macros.at | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Ales Musil Aug. 31, 2026, 8:08 a.m. UTC | #1
On Tue, Aug 25, 2026 at 2:54 PM Xavier Simonart via dev <
ovs-dev@openvswitch.org> wrote:

> CHECK_AFTER_RECOMPUTE and OVN CHECK_FLOWS_AFTER_RECOMPUTE macros compare
> flows before and after recompute, and reports test failure if those flows
> are different.
> However learned flows can happen at any time e.g. due to IPv6 Router
> Solicitation. If the test is long they might also be deleted.
> So, ignore learned flows when comparing flows before and after recompute.
>
> See for instance [0] for a test failing for this reason.
>
> [0]
> https://github.com/ovsrobot/ovn/actions/runs/32723237137/job/97422492101#step:12:5869
>
> Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
> ---
>

Hi Xaiver,

thank you for the patch. Should we also exclude flows with
hard_timeout set? We have an example where we set
hard_timeout and not idle_timeout.

 tests/ovn-macros.at | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
> index 2a1faf677..a1aa67a38 100644
> --- a/tests/ovn-macros.at
> +++ b/tests/ovn-macros.at
> @@ -274,7 +274,9 @@ m4_define([CHECK_FLOWS_AFTER_RECOMPUTE], [
>            check ovn-nbctl --wait=hv sync
>        fi
>        DUMP_FLOWS([$sbox], [flows-$hv-after])
> -      diff flows-$hv-before flows-$hv-after > flow-diff
> +      # Avoid learned flows i.e. flows w/ idle_timeout and no learn.
> +      diff flows-$hv-before flows-$hv-after | grep "^[<>]" | awk
> '!/idle_timeout=/ || /learn/' > flow-diff
> +
>        AT_CHECK([wc -l < flow-diff], [0], [0
>  ])
>      fi
> @@ -354,7 +356,8 @@ m4_define([CHECK_AFTER_RECOMPUTE], [
>  ])
>
>        # Compare and store flow differences before and after recompute
> -      diff -u flows-$hv-before flows-$hv-after | grep "^+ \|^- " >
> flow-diff-$hv
> +      # Avoid learned flows i.e. flows w/ idle_timeout and no learn.
> +      diff -u flows-$hv-before flows-$hv-after | grep "^+ \|^- " | awk
> '!/idle_timeout=/ || /learn/' > flow-diff-$hv
>        if [[ -n "$related_ports" ]]; then
>          tag=$(ovn-sbctl --bare --columns tag list port_binding
> $related_ports)
>          if [[ -n "$tag" ]]; then
> --
> 2.47.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Regards,
Ales
Xavier Simonart Sept. 9, 2026, 1:15 p.m. UTC | #2
Hi Ales

Thanks for the review.

On Mon, Aug 31, 2026 at 10:08 AM Ales Musil <amusil@redhat.com> wrote:

>
>
> On Tue, Aug 25, 2026 at 2:54 PM Xavier Simonart via dev <
> ovs-dev@openvswitch.org> wrote:
>
>> CHECK_AFTER_RECOMPUTE and OVN CHECK_FLOWS_AFTER_RECOMPUTE macros compare
>> flows before and after recompute, and reports test failure if those flows
>> are different.
>> However learned flows can happen at any time e.g. due to IPv6 Router
>> Solicitation. If the test is long they might also be deleted.
>> So, ignore learned flows when comparing flows before and after recompute.
>>
>> See for instance [0] for a test failing for this reason.
>>
>> [0]
>> https://github.com/ovsrobot/ovn/actions/runs/32723237137/job/97422492101#step:12:5869
>>
>> Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
>> ---
>>
>
> Hi Xaiver,
>
> thank you for the patch. Should we also exclude flows with
> hard_timeout set? We have an example where we set
> hard_timeout and not idle_timeout.
>
I have not seen any tests failing due to the hard_timeout, but probably
because those timeout are quite long.
But I think skipping those flows as well is a good idea.
I'll update this in v2.

>
>  tests/ovn-macros.at | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
>> index 2a1faf677..a1aa67a38 100644
>> --- a/tests/ovn-macros.at
>> +++ b/tests/ovn-macros.at
>> @@ -274,7 +274,9 @@ m4_define([CHECK_FLOWS_AFTER_RECOMPUTE], [
>>            check ovn-nbctl --wait=hv sync
>>        fi
>>        DUMP_FLOWS([$sbox], [flows-$hv-after])
>> -      diff flows-$hv-before flows-$hv-after > flow-diff
>> +      # Avoid learned flows i.e. flows w/ idle_timeout and no learn.
>> +      diff flows-$hv-before flows-$hv-after | grep "^[<>]" | awk
>> '!/idle_timeout=/ || /learn/' > flow-diff
>> +
>>        AT_CHECK([wc -l < flow-diff], [0], [0
>>  ])
>>      fi
>> @@ -354,7 +356,8 @@ m4_define([CHECK_AFTER_RECOMPUTE], [
>>  ])
>>
>>        # Compare and store flow differences before and after recompute
>> -      diff -u flows-$hv-before flows-$hv-after | grep "^+ \|^- " >
>> flow-diff-$hv
>> +      # Avoid learned flows i.e. flows w/ idle_timeout and no learn.
>> +      diff -u flows-$hv-before flows-$hv-after | grep "^+ \|^- " | awk
>> '!/idle_timeout=/ || /learn/' > flow-diff-$hv
>>        if [[ -n "$related_ports" ]]; then
>>          tag=$(ovn-sbctl --bare --columns tag list port_binding
>> $related_ports)
>>          if [[ -n "$tag" ]]; then
>> --
>> 2.47.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
> Regards,
> Ales
>
Thanks
Xavier
diff mbox series

Patch

diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 2a1faf677..a1aa67a38 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -274,7 +274,9 @@  m4_define([CHECK_FLOWS_AFTER_RECOMPUTE], [
           check ovn-nbctl --wait=hv sync
       fi
       DUMP_FLOWS([$sbox], [flows-$hv-after])
-      diff flows-$hv-before flows-$hv-after > flow-diff
+      # Avoid learned flows i.e. flows w/ idle_timeout and no learn.
+      diff flows-$hv-before flows-$hv-after | grep "^[<>]" | awk '!/idle_timeout=/ || /learn/' > flow-diff
+
       AT_CHECK([wc -l < flow-diff], [0], [0
 ])
     fi
@@ -354,7 +356,8 @@  m4_define([CHECK_AFTER_RECOMPUTE], [
 ])
 
       # Compare and store flow differences before and after recompute
-      diff -u flows-$hv-before flows-$hv-after | grep "^+ \|^- " > flow-diff-$hv
+      # Avoid learned flows i.e. flows w/ idle_timeout and no learn.
+      diff -u flows-$hv-before flows-$hv-after | grep "^+ \|^- " | awk '!/idle_timeout=/ || /learn/' > flow-diff-$hv
       if [[ -n "$related_ports" ]]; then
         tag=$(ovn-sbctl --bare --columns tag list port_binding $related_ports)
         if [[ -n "$tag" ]]; then