| Message ID | 20240110124045.778257-2-amusil@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/2] tests: Reduce flakiness of daemon ssl files change test | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Thanks Ales, Acked-by: Mark Michelson <mmichels@redhat.com> On 1/10/24 07:40, Ales Musil wrote: > The CoPP test modifies a meter from drop=1 to drop=10, there are two > issues with this change: > > 1) It takes some time for this change to propagate into OvS. > 2) Depending on the timing the 10 packet limit might not fit into > single pktps bucket. > > To address those issues lower the pktps to 5, this has lower chance > for the bucket to overflow into the next second. Also wait for OvS > to receive the meter update and the meter stats reset. > > One thing to note is that even with this change the test can still > fail with more packets being allowed through than expected, however > the chance is highly reduced. For comparison, the test was failing > on ARM in ~1/5 of runs. After this change there wasn't single failure > in a loop with 100 runs. > > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > tests/system-ovn.at | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 3a692e341..7f240fef0 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -7355,7 +7355,8 @@ rm -f reject.pcap > > # Let's update the meter > NS_EXEC([sw01], [tcpdump -l -n -i sw01 icmp -Q in > reject.pcap &]) > -check ovn-nbctl --may-exist --wait=hv meter-add acl-meter drop 10 pktps 0 > +check ovn-nbctl --may-exist --wait=hv meter-add acl-meter drop 5 pktps 0 > +OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow15 meter-stats br-int | grep -q packet_count:0]) > ip netns exec sw01 scapy -H <<-EOF > p = IP(src="192.168.1.2", dst="192.168.1.1") / UDP(dport = 12345) / Raw(b"X"*64) > send (p, iface='sw01', loop = 0, verbose = 0, count = 40) > @@ -7364,7 +7365,7 @@ EOF > # 10pps > OVS_WAIT_UNTIL([ > n_reject=$(grep unreachable reject.pcap | wc -l) > - test "${n_reject}" = "10" > + test "${n_reject}" = "5" > ]) > > kill $(pidof tcpdump)
On Tue, Jan 16, 2024 at 4:42 PM Mark Michelson <mmichels@redhat.com> wrote: > > Thanks Ales, > > Acked-by: Mark Michelson <mmichels@redhat.com> Thanks Ales. I applied this patch to the main. Numan > > On 1/10/24 07:40, Ales Musil wrote: > > The CoPP test modifies a meter from drop=1 to drop=10, there are two > > issues with this change: > > > > 1) It takes some time for this change to propagate into OvS. > > 2) Depending on the timing the 10 packet limit might not fit into > > single pktps bucket. > > > > To address those issues lower the pktps to 5, this has lower chance > > for the bucket to overflow into the next second. Also wait for OvS > > to receive the meter update and the meter stats reset. > > > > One thing to note is that even with this change the test can still > > fail with more packets being allowed through than expected, however > > the chance is highly reduced. For comparison, the test was failing > > on ARM in ~1/5 of runs. After this change there wasn't single failure > > in a loop with 100 runs. > > > > Signed-off-by: Ales Musil <amusil@redhat.com> > > --- > > tests/system-ovn.at | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > index 3a692e341..7f240fef0 100644 > > --- a/tests/system-ovn.at > > +++ b/tests/system-ovn.at > > @@ -7355,7 +7355,8 @@ rm -f reject.pcap > > > > # Let's update the meter > > NS_EXEC([sw01], [tcpdump -l -n -i sw01 icmp -Q in > reject.pcap &]) > > -check ovn-nbctl --may-exist --wait=hv meter-add acl-meter drop 10 pktps 0 > > +check ovn-nbctl --may-exist --wait=hv meter-add acl-meter drop 5 pktps 0 > > +OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow15 meter-stats br-int | grep -q packet_count:0]) > > ip netns exec sw01 scapy -H <<-EOF > > p = IP(src="192.168.1.2", dst="192.168.1.1") / UDP(dport = 12345) / Raw(b"X"*64) > > send (p, iface='sw01', loop = 0, verbose = 0, count = 40) > > @@ -7364,7 +7365,7 @@ EOF > > # 10pps > > OVS_WAIT_UNTIL([ > > n_reject=$(grep unreachable reject.pcap | wc -l) > > - test "${n_reject}" = "10" > > + test "${n_reject}" = "5" > > ]) > > > > kill $(pidof tcpdump) > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 3a692e341..7f240fef0 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -7355,7 +7355,8 @@ rm -f reject.pcap # Let's update the meter NS_EXEC([sw01], [tcpdump -l -n -i sw01 icmp -Q in > reject.pcap &]) -check ovn-nbctl --may-exist --wait=hv meter-add acl-meter drop 10 pktps 0 +check ovn-nbctl --may-exist --wait=hv meter-add acl-meter drop 5 pktps 0 +OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow15 meter-stats br-int | grep -q packet_count:0]) ip netns exec sw01 scapy -H <<-EOF p = IP(src="192.168.1.2", dst="192.168.1.1") / UDP(dport = 12345) / Raw(b"X"*64) send (p, iface='sw01', loop = 0, verbose = 0, count = 40) @@ -7364,7 +7365,7 @@ EOF # 10pps OVS_WAIT_UNTIL([ n_reject=$(grep unreachable reject.pcap | wc -l) - test "${n_reject}" = "10" + test "${n_reject}" = "5" ]) kill $(pidof tcpdump)
The CoPP test modifies a meter from drop=1 to drop=10, there are two issues with this change: 1) It takes some time for this change to propagate into OvS. 2) Depending on the timing the 10 packet limit might not fit into single pktps bucket. To address those issues lower the pktps to 5, this has lower chance for the bucket to overflow into the next second. Also wait for OvS to receive the meter update and the meter stats reset. One thing to note is that even with this change the test can still fail with more packets being allowed through than expected, however the chance is highly reduced. For comparison, the test was failing on ARM in ~1/5 of runs. After this change there wasn't single failure in a loop with 100 runs. Signed-off-by: Ales Musil <amusil@redhat.com> --- tests/system-ovn.at | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)