On 4/16/20 9:21 AM, Anton Ivanov wrote: > > On 15/04/2020 16:28, Numan Siddique wrote: >> On Wed, Apr 15, 2020 at 5:05 PM Ilya Maximets <[email protected]> wrote: >>> On 4/15/20 1:22 PM, Ilya Maximets wrote: >>>> On 4/15/20 11:10 AM, [email protected] wrote: >>>>> From: Anton Ivanov <[email protected]> >>>>> >>>>> Test 121 was configuring the interface with test traffic >>>>> before the LSP was configured. As a result, test traffic >>>>> could be processed before the LSP was set resulting in a >>>>> test failure >>>>> >>>>> Signed-off-by: Anton Ivanov <[email protected]> >>>>> --- >>>> Hi. Thanks for improving the tests! >>>> I didn't check the main logic here, just a few general notes: >>>> >>>> 1. Please, add "ovn" to the subject prefix while sending ovn patches, >>>> i.e. --subject-prefix="PATCH ovn", so the robot (and people) will >>>> know where to apply this patch. >>>> >>>> 2. See inline. >>> 3. It also might make sense to have more meaningful patch name as >>> test numbers change over time. Something like: >>> "ovn.at: Fix race condition in RARP test." >>> >>>> Best regards, Ilya Maximets. >>>> >>>>> tests/ovn.at | 12 +++++++----- >>>>> 1 file changed, 7 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/tests/ovn.at b/tests/ovn.at >>>>> index 9a44f0a6f..cf6956cdd 100644 >>>>> --- a/tests/ovn.at >>>>> +++ b/tests/ovn.at >>>>> @@ -9318,6 +9318,7 @@ OVN_CHECK_PACKETS([hv3/vif1-tx.pcap], >>>>> [hv3-vif1.expected]) >>>>> >>>>> # Now add bridge-mappings on hv2, which should make everything work >>>>> as hv2 ovs-vsctl set open . >>>>> external-ids:ovn-bridge-mappings=phys:br-phys >>>>> +sleep 1 >>>> Do we need this sleep here? Looks like unrelated change. >>>> >>>>> # Wait until the patch ports are created in hv2 to connect br-int >>>>> to br-phys >>>>> OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \ >>>>> @@ -17356,11 +17357,6 @@ ovs-vsctl add-br br-phys >>>>> ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys >>>>> ovn_attach n1 br-phys 192.168.0.1 >>>>> >>>>> -ovs-vsctl add-port br-int vif11 -- \ >>>>> - set Interface vif11 external-ids:iface-id=lp11 \ >>>>> - options:tx_pcap=hv1/vif11-tx.pcap \ >>>>> - options:rxq_pcap=hv1/vif11-rx.pcap \ >>>>> - ofport-request=11 >>>>> >>>>> lsp_name=lp11 >>>>> >>>>> @@ -17368,6 +17364,12 @@ ovn-nbctl lsp-add ls1 lp11 >>>>> ovn-nbctl lsp-set-addresses lp11 "f0:00:00:00:00:11" >>>>> ovn-nbctl lsp-set-port-security lp11 f0:00:00:00:00:11 >>>>> >>>>> +ovs-vsctl add-port br-int vif11 -- \ >>>>> + set Interface vif11 external-ids:iface-id=lp11 \ >>>>> + options:tx_pcap=hv1/vif11-tx.pcap \ >>>>> + options:rxq_pcap=hv1/vif11-rx.pcap \ >>>>> + ofport-request=11 >>>>> + >>>>> OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp11` = xup]) >> Hi Anton, >> >> Thanks for the patch. >> >> I don't think this can be the reason for the failure. >> >> A user can create an OVS interface first (with external_ids:iface-id >> set) or >> can create the logical port first. Both are valid use cases. >> ovn-controller >> will bind the logical port (and maps the ovs interface to the logical >> port) >> and program the flows when an ovs interface has >> external_ids:iface-id configured and the corresponding logical port >> present. >> >> The test traffic can not be processed until the logical port is mapped to >> the ovs interface and flows programmed. >> >> I think the issue is somewhere else. > > Statement of the fact - the as is test rarely (like once in a 50+ > times), flakes out on my machine. It is not easy to trigger and I can > trigger it only on the fastest test system I have at present and only > once in a blue moon. > > After reordering the statements as per the suggested patch, I ran this > test in a loop for 3 hours - no flakes. > > A. >
Hi Anton, It might be related to the fact that the RARP packet is sent out before the patch port to br-phys is up. Could you please try with the following patch to confirm if that's the case? Thanks, Dumitru diff --git a/tests/ovn.at b/tests/ovn.at index f83d3f5..07689e3 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -17439,6 +17439,9 @@ ovs-vsctl add-port br-int vif11 -- \ lsp_name=lp11 ovn-nbctl lsp-add ls1 lp11 +OVS_WAIT_UNTIL([test $(as hv1 ovs-vsctl --bare --columns _uuid list \ + interface patch-br-int-to-ln1 | wc -l)] = 1) + ovn-nbctl lsp-set-addresses lp11 "f0:00:00:00:00:11" ovn-nbctl lsp-set-port-security lp11 f0:00:00:00:00:11 >> >> Thanks >> Numan >> >> >> >>>>> ovn-nbctl --wait=sb sync >>>>> >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
