On 7/1/26 4:23 PM, Xavier Simonart wrote:
> Hi Ilya
>
> On Wed, Jul 1, 2026 at 3:01 PM Ilya Maximets <[email protected]
> <mailto:[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]> <mailto:[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.
The thing I had in mind is something similar to
add_of_ports br0 1 2 3
that we have in OVS. It would be nice to just specify the numbers and
let the function/macro choose set all the names and port numbers based
on that. In which case there wouldn't be any overhead from setting the
number and all the ports would have predictable configuration.
Looking at all the OVN tests however, I see that the port names vary from
test to test, so it would be a much larger change to try and unify them.
So, it may be fine to not specify the numbers in test that don't need them.
Though it is a frequent source of test flakiness in OVS, at least, but
we do check OpenFlow and datapath flows more frequently in OVS.
There isn't a clear right or wrong solution, so I'll leave it up to you.
I also remembered that OVS is using 100 as a base for port numbers for
internal dummy bridge ports. So we may want to use 70x instead of 10x,
just to be safer.
Best regards, Ilya Maximets.
>
>
> > - 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
