On 6/14/21 2:44 PM, Ben Pfaff wrote:
On Fri, Jun 11, 2021 at 03:48:52PM -0700, 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.

There are some other potential timing issues in this test and this
patch fixes them as well by replacing AT_CHECK with OVS_WAIT_UNTIL.

Signed-off-by: Han Zhou <hz...@ovn.org>

Awesome!  Thank you.

-AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding \
+OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns chassis find port_binding \
  logical_port=sw0-vir) = x], [0], [])

I think the above can be better written:
     wait_row_count Port_Binding 0 logical_port=sw0-vir

I don't think this is correct. The test is not attempting to wait for the Port_Binding record to be deleted. It's waiting for the chassis column in the Port_Binding to contain an empty string. I think wait_column() could work:

wait_column "" Port_Binding chassis logical_port=sw0-vir

(assuming that testing for an empty string works)



  # Cleanup hv1-vif3.
  as hv1
  ovs-vsctl del-port hv1-vif3
-AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding \
+OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns chassis find port_binding \
  logical_port=sw0-vir) = x], [0], [])

Ditto?

+    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)

I think that the above will retrieve tunnel keys in decimal...

+    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)
  ])

...therefore I think that the above 0x should not be there.  (I guess
the test passes because the numbers in the test are all under 10.)

Yeah, it should probably be fine for this test to not worry about this.


Thanks,

Ben.
_______________________________________________
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

Reply via email to