| Message ID | 20200617083832.526221-1-numans@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,ovn] Fix the test case "80. ovn -- 4 HV, 1 LS, 1 LR, packet test with HA distributed router gateway port" | expand |
Bleep bloop. Greetings Numan Siddique, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: error: sha1 information is lacking or useless (testsuite.dir/at-groups/80/stdout). error: could not build fake ancestor hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0001 Fix the test case "80. ovn -- 4 HV, 1 LS, 1 LR, packet test with HA distributed router gateway port" When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
Acked-by: Mark Michelson <mmichels@redhat.com> This is a reluctant ack. I think all this does is narrow the possible window during which the additional ARP can be received. In theory, it may still be possible on a busy/slow enough system to still see this problem intermittently. However, it's also possible that the window is so small that it would require extraordinary (read: impossible) circumstances to occur. Thus why I'm fine with acking this. I think a better long-term solution in a case like this one is to introduce some sort of looser OVN_CHECK_PACKETS macro that expects certain packets to be present but will not fail if there are additional packets (it may even have a variation to pass only if the extra packets are identical to any expected packets). This way, timing inconsistencies are eliminated completely. On 6/17/20 4:38 AM, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > This test case is failing intermittently in my local setup. > > *********************** > rcv_n=3 exp_n=2 > ovn.at:12: wait succeeded immediately > ../../tests/ovn.at:9473: sort $rcv_text > --- expout 2020-06-17 10:57:23.467337249 +0530 > +++ tests/testsuite.dir/at-groups/80/stdout 2020-06-17 10:57:23.469337182 +0530 > @@ -1,2 +1,3 @@ > f0000001020400000201020308004500001c000000003f110100c0a80102ac1001030035111100080000 > ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 > +ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 > ******************* > > Before calling 'OVN_CHECK_PACKETS', there is a sleep of 1 second. This is not required, > and looks like this sleep is causing an extra ARP packet to be received. > > This patch deletes this sleep. The macro OVN_CHECK_PACKETS will anyway wait for the expected > packets to be received. > > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > tests/ovn.at | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/tests/ovn.at b/tests/ovn.at > index 7e1ace556..1987b8062 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -9468,8 +9468,6 @@ grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` -eq 1 > # Resend packet from foo1 to outside1 > as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > > - sleep 1 > - > OVN_CHECK_PACKETS([ext1/vif1-tx.pcap], [ext1-vif1.expected]) > $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" $active_gw/br-phys_n1-tx.pcap > packets > cat packets | grep $expected > exp >
On Thu, Jun 18, 2020 at 6:17 PM Mark Michelson <mmichels@redhat.com> wrote: > Acked-by: Mark Michelson <mmichels@redhat.com> > > This is a reluctant ack. I think all this does is narrow the possible > window during which the additional ARP can be received. In theory, it > may still be possible on a busy/slow enough system to still see this > problem intermittently. However, it's also possible that the window is > so small that it would require extraordinary (read: impossible) > circumstances to occur. Thus why I'm fine with acking this. > > I think a better long-term solution in a case like this one is to > introduce some sort of looser OVN_CHECK_PACKETS macro that expects > certain packets to be present but will not fail if there are additional > packets (it may even have a variation to pass only if the extra packets > are identical to any expected packets). This way, timing inconsistencies > are eliminated completely. > > Thanks. Sleep is not required at all. But as you said the issue could still occur. Sleep makes the occurrence of this issue very frequently. I think we are seeing this issue after the first 3 patches are merged, which means ovn-controller became a bit faster in handling the updates. I applied this patch to master. Thanks Numan On 6/17/20 4:38 AM, numans@ovn.org wrote: > > From: Numan Siddique <numans@ovn.org> > > > > This test case is failing intermittently in my local setup. > > > > *********************** > > rcv_n=3 exp_n=2 > > ovn.at:12: wait succeeded immediately > > ../../tests/ovn.at:9473: sort $rcv_text > > --- expout 2020-06-17 10:57:23.467337249 +0530 > > +++ tests/testsuite.dir/at-groups/80/stdout 2020-06-17 > 10:57:23.469337182 +0530 > > @@ -1,2 +1,3 @@ > > > f0000001020400000201020308004500001c000000003f110100c0a80102ac1001030035111100080000 > > > ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 > > > +ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 > > ******************* > > > > Before calling 'OVN_CHECK_PACKETS', there is a sleep of 1 second. This > is not required, > > and looks like this sleep is causing an extra ARP packet to be received. > > > > This patch deletes this sleep. The macro OVN_CHECK_PACKETS will anyway > wait for the expected > > packets to be received. > > > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > tests/ovn.at | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 7e1ace556..1987b8062 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -9468,8 +9468,6 @@ grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` > -eq 1 > > # Resend packet from foo1 to outside1 > > as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > > > > - sleep 1 > > - > > OVN_CHECK_PACKETS([ext1/vif1-tx.pcap], [ext1-vif1.expected]) > > $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" > $active_gw/br-phys_n1-tx.pcap > packets > > cat packets | grep $expected > exp > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
--- expout 2020-06-17 10:57:23.467337249 +0530 +++ tests/testsuite.dir/at-groups/80/stdout 2020-06-17 10:57:23.469337182 +0530 @@ -1,2 +1,3 @@ f0000001020400000201020308004500001c000000003f110100c0a80102ac1001030035111100080000 ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 +ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 ******************* Before calling 'OVN_CHECK_PACKETS', there is a sleep of 1 second. This is not required, and looks like this sleep is causing an extra ARP packet to be received. This patch deletes this sleep. The macro OVN_CHECK_PACKETS will anyway wait for the expected packets to be received. Signed-off-by: Numan Siddique <numans@ovn.org> --- tests/ovn.at | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/ovn.at b/tests/ovn.at index 7e1ace556..1987b8062 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -9468,8 +9468,6 @@ grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` -eq 1 # Resend packet from foo1 to outside1 as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet - sleep 1 - OVN_CHECK_PACKETS([ext1/vif1-tx.pcap], [ext1-vif1.expected]) $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" $active_gw/br-phys_n1-tx.pcap > packets cat packets | grep $expected > exp
From: Numan Siddique <numans@ovn.org> This test case is failing intermittently in my local setup. *********************** rcv_n=3 exp_n=2 ovn.at:12: wait succeeded immediately ../../tests/ovn.at:9473: sort $rcv_text