On Thu, Jun 17, 2021 at 12:59 PM Mark Michelson <[email protected]> wrote: > > 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 <[email protected]> > > > > 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) >
Thanks Ben and Mark! I used wait_column in v4: https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ > > > > > >> # 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. > I changed it to hex format in v4. Thanks, Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
