[ovs-dev] [PATCH ovn 4/4] ovn.at: Fix test "virtual ports -- ovn-northd-ddlog".

[Han Zhou]

On Fri, Jun 11, 2021 at 11:24 AM Mark Michelson <mmichels at redhat.com> wrote:
>
> I noticed that there is still some other potential for this test to fail
> in slow environments. Just above your changes are some lines like this:
>
> as hv1
> ovs-vsctl set interface hv1-vif3 external-ids:iface-id=sw0-vir
>
> AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding \
> logical_port=sw0-vir) = x], [0], [])
>
> It seems like that AT_CHECK should be changed to OVS_WAIT_UNTIL since
> there could be a delay before the SB port_binding is updated.
>
> Similar patterns are used a few times in this test.
>
> If you're going to fix this test, then you probably should fix those up
too.

Thanks for spotting this. I think you are right and I added the fixes in v2
for the issue you mentioned along with one immediately below it. I quickly
looked and it seems other places in this test case when any interface is
updated there is some kind of *wait* mechanism used, so it looks ok for me
now. Let me know if you see more places that need similar fixes.

https://patchwork.ozlabs.org/project/ovn/patch/20210611224852.3267068-1-hzhou@ovn.org/

Thanks,
Han

>
> On 6/11/21 2:24 AM, Han Zhou wrote:
> > The test case fails quite often for northd-ddlog because of the tunnel
> > keys mismatch when comparing OpenFlow rules. Keys can change in
> > different runs. This patch fixes it by extracting the expected keys from
> > SB DB before comparison instead of hardcoding.
> >
> > Signed-off-by: Han Zhou <hzhou at ovn.org>
> > ---
> >   tests/ovn.at | 14 +++++++++-----
> >   1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index b440f5517..67cf75719 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -17317,14 +17317,18 @@ logical_port=sw0-vir) = x], [0], [])
> >   check_virtual_offlows_present() {
> >       hv=$1
> >
> > -    AT_CHECK([as $hv ovs-ofctl dump-flows br-int table=44 |
ofctl_strip_all | grep "priority=2000"], [0], [dnl
> > - table=44, priority=2000,ip,metadata=0x1 actions=resubmit(,45)
> > - table=44, priority=2000,ipv6,metadata=0x1 actions=resubmit(,45)
> > +    sw0_dp_key=$(fetch_column Datapath_Binding tunnel_key
external_ids:name=sw0)
> > +    lr0_dp_key=$(fetch_column Datapath_Binding tunnel_key
external_ids:name=lr0)
> > +    lr0_public_dp_key=$(fetch_column Port_Binding tunnel_key
logical_port=lr0-public)
> > +
> > +    AT_CHECK_UNQUOTED([as $hv ovs-ofctl dump-flows br-int table=44 |
ofctl_strip_all | grep "priority=2000"], [0], [dnl
> > + table=44, priority=2000,ip,metadata=0x$sw0_dp_key
actions=resubmit(,45)
> > + table=44, priority=2000,ipv6,metadata=0x$sw0_dp_key
actions=resubmit(,45)
> >   ])
> >
> > -    AT_CHECK([as $hv ovs-ofctl dump-flows br-int table=11 |
ofctl_strip_all | \
> > +    AT_CHECK_UNQUOTED([as $hv ovs-ofctl dump-flows br-int table=11 |
ofctl_strip_all | \
> >       grep "priority=92" | grep 172.168.0.50], [0], [dnl
> > - table=11,
priority=92,arp,reg14=0x3,metadata=0x3,arp_tpa=172.168.0.50,arp_op=1
actions=move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],mod_dl_src:10:54:00:00:00:10,load:0x2->NXM_OF_ARP_OP[[]],move:NXM_NX_ARP_SHA[[]]->NXM_NX_ARP_THA[[]],load:0x105400000010->NXM_NX_ARP_SHA[[]],push:NXM_OF_ARP_SPA[[]],push:NXM_OF_ARP_TPA[[]],pop:NXM_OF_ARP_SPA[[]],pop:NXM_OF_ARP_TPA[[]],move:NXM_NX_REG14[[]]->NXM_NX_REG15[[]],load:0x1->NXM_NX_REG10[[0]],resubmit(,37)
> > + table=11,
priority=92,arp,reg14=0x$lr0_public_dp_key,metadata=0x$lr0_dp_key,arp_tpa=172.168.0.50,arp_op=1
actions=move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],mod_dl_src:10:54:00:00:00:10,load:0x2->NXM_OF_ARP_OP[[]],move:NXM_NX_ARP_SHA[[]]->NXM_NX_ARP_THA[[]],load:0x105400000010->NXM_NX_ARP_SHA[[]],push:NXM_OF_ARP_SPA[[]],push:NXM_OF_ARP_TPA[[]],pop:NXM_OF_ARP_SPA[[]],pop:NXM_OF_ARP_TPA[[]],move:NXM_NX_REG14[[]]->NXM_NX_REG15[[]],load:0x1->NXM_NX_REG10[[0]],resubmit(,37)
> >   ])
> >   }
> >
> >
>