On Fri, Apr 16, 2021 at 12:05:55PM -0400, Mark Michelson wrote: > On 4/1/21 7:20 PM, Ben Pfaff wrote: > > Signed-off-by: Ben Pfaff <[email protected]> > > --- > > tests/ovn.at | 67 ++++++++++++++++++++++++++++++++++++++-------------- > > 1 file changed, 49 insertions(+), 18 deletions(-) > > > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 391a8bcd9323..7b6789125ffc 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -9467,9 +9467,10 @@ wait_for_ports_up > > check ovn-nbctl --wait=hv sync > > as hv1 ovs-vsctl show > > -echo "*************************" > > -ovn-sbctl list DNS > > -echo "*************************" > > +ovn-sbctl list DNS > dns > > +AT_CAPTURE_FILE([dns]) > > +ovn-sbctl dump-flows > sbflows > > +AT_CAPTURE_FILE([sbflows]) > > reset_pcap_file() { > > local iface=$1 > > @@ -9582,7 +9583,13 @@ test_dns() { > > echo $request >> $outport.expected > > done > > fi > > - as hv1 ovs-appctl netdev-dummy/receive hv1-vif$inport $request > > + if true; then > > + as hv1 ovs-appctl ofproto/trace br-int in_port=hv1-vif$inport > > $request > trace$trace > > + trace=$(expr $trace + 1) > > + else > > + as hv1 ovs-appctl dpctl/del-flows > > + as hv1 ovs-appctl netdev-dummy/receive hv1-vif$inport $request > > + fi > > I don't understand this. > 1) Why "if true"? Doesn't that mean the else will never run? Why have it, > then?
It's just a way of commenting out the false case, so that it's easy for a developer to change it back if there's some reason to do so. > 2) The "trace$trace" files don't appear to be used by the test and aren't > captured for debugging purposes. It would be nice to capture them. > 3) Why use ofproto/trace over netdev-dummy/receive? Does ofproto/trace > populate the VIF pcap files the same way? Wouldn't netdev-dummy/receive > indicate a more "real" traversal of the datapath than what ofproto/trace > claims? ofproto/trace happens to have the same effect in this case and the traces make it easier for the developer to find out what happened. > 4) $trace is not initialized to 0 anywhere that I can see. That was a mistake, but it happens to work because "expr + 1" has value 1. I'll drop this for the next version. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
