On 8/26/22 17:16, Dumitru Ceara wrote: > On 8/22/22 12:27, Xavier Simonart wrote: >> Testsuite used ofport-request for most of its tests. >> However, this causes random tess failures as the Openflow port >> number assigned to patch ports can change after its initial assignment. >> For instance: >> - OVN adds patch port ovn-hv1-0 => get ofport 1. >> - tests adds vif1 (w/ ofport-request=1) and vif2 (w/ ofport-request=2). >> Now, vif1 will have ofport 1, vif2 ofport 2 and ovn-hv1-0 will be 3. >> OVN controller handles properly the behavior i.e. it updates the flows >> when ofport changes. >> However, this takes some time (roundtrip) and flows might only be installed >> after ports are reported up/ovn-installed. This causes some tests >> to be flaky, as they might send packets or check flows before OVN completed >> ofport related changes. >> >> If CMS wants to use ofport-request, OVN should have an option to use it >> internally as well for its patch ports (e.g. through an option providing the >> range of port numbers to use). >> >> Signed-off-by: Xavier Simonart <[email protected]> >> --- > > Looks good to me, thanks for taking the time to clean this up! > > Acked-by: Dumitru Ceara <[email protected]> > > One note though, your other patch here is adding an ofport-request, we > should probably drop that too: > > https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ >
On a second look I noticed we're introducing a race in some of the tests with this patch, e.g.: ovs-vsctl -- add-port br-int hv1-vif0 ofport=$(ovs-vsctl --bare --columns ofport find Interface name=hv1-vif0) There's no guarantee that ovs-vswitchd gets a chance to run and allocate the ofport. Unfortunately we probably need to use an OVS_WAIT_UNTIL or similar here too. I'll mark the patch as "Changes Requested" in patchwork. Could you please send an amended v2? Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
