Hi Ilya On Wed, Jul 1, 2026 at 3:01 PM Ilya Maximets <[email protected]> wrote:
> On 7/1/26 2:45 PM, Xavier Simonart wrote: > > Hi Ilya > > > > Thanks for the review and the comments > > > > > > On Mon, Jun 29, 2026 at 7:08 PM Ilya Maximets <[email protected] > <mailto:[email protected]>> wrote: > > > > On 6/29/26 2:40 PM, Xavier Simonart via dev wrote: > > > Using ofport_request can make the test flaky, as OVS might have > > > already used the requested ofport (typically when ovn created > > > ovn-hv[0-9]-0 ports). In that case, it takes some time (between > > > a few msec to multiple seconds) until the requested ofport is > > > obtained. OVN handles this properly (flows are recalculated), but > > > tests do not handle this properly (e.g. if it happens when > comparing > > > I-P and recompute flows). > > > > > > This patch removes all ofport-request, querying the obtained ofport > > > when needed. > > > > Hmm. I'm not sure this is the right approach here. Maybe we can add > > some sort of prefix to the numbers instead? e.g. > ofport-request=10$i$j > > > > WDYT? > > > > I think we should > > - Remove the ofport-request when not needed (99% of the time). > > But why? Is there any harm in having them in case we use the 10x numbering > scheme? There should be no conflicts in that case. > Having 10x numbering would be fine too; there's no harm. We need the change regardless (whether deleting or using 10x numbering). But when they are useless (i.e. in most cases), I think having them is more error-prone—e.g., if you need to add a port in the middle of the test, you would need to check which ofport were already requested. That being said I will not fight for this - so if you feel it is wrong to delete them, I am fine with using the 10x schema everywhere. > > > - Use the schema you propose (ofport-request=10$i$j) as it will be > easier/safer to handle than a potentially 'complex' sort, > > or guesses on the ofport which OVS will assign. > > > > > > Normally, the ofport-request is used exactly to avoid the > flakyness... > > > > > @@ -14338,26 +14249,29 @@ for i in 1 2; do > > > done > > > done > > > > > > -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int |awk > '/table=OFTABLE_LOG_TO_PHY/ && !/actions=drop/{print substr($8, 16, > length($8))}' |sort -n], [0], [dnl > > > -10 > > > -11 > > > +ofport01=$(as hv1 wait_and_get_ofport name=vif01) > > > +ofport11=$(as hv1 wait_and_get_ofport name=vif11) > > > +AT_CHECK_UNQUOTED([as hv1 ovs-ofctl dump-flows br-int |awk > '/table=OFTABLE_LOG_TO_PHY/ && !/actions=drop/{print substr($8, 16, > length($8))}' |sort -n], [0], [dnl > > > +$ofport01 > > > +$ofport11 > > > ]) > > > > Here, for example, you're sorting the command output, but there is no > > guarantee that $ofport01 is less than $ofport11. So this test may > > become or still be flaky. > > > > Best regards, Ilya Maximets. > > > > Thanks > > Xavier > Thanks Xavier _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
