diff mbox series

[ovs-dev] tests/system-traffic: Ensure no name resolution for tcpdump.

Message ID 20231020232208.1184598-1-frode.nordahl@canonical.com
State Accepted
Commit 6cfb3d1ff5137c9bc3e361bf76f412b7f2a9f13a
Headers show
Series [ovs-dev] tests/system-traffic: Ensure no name resolution for tcpdump. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Frode Nordahl Oct. 20, 2023, 11:22 p.m. UTC 
Depending on system configuration, executing tcpdump without the
-n parameter, may prolong the execution time for tcpdump while it
attempts name resolution.

This delay may in turn lead to test failures due to contents of
tables to check being evicted.

We recently started to see this problem with the
"conntrack -IPv6 ICMP6 Related with SNAT" test.

For consistency, this patch adds the -n parameter to all tcpdump
calls in system-traffic.at.

Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
---
 tests/system-traffic.at | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Simon Horman Oct. 25, 2023, 9:43 a.m. UTC | #1 
On Sat, Oct 21, 2023 at 01:22:08AM +0200, Frode Nordahl wrote:
> Depending on system configuration, executing tcpdump without the
> -n parameter, may prolong the execution time for tcpdump while it
> attempts name resolution.
> 
> This delay may in turn lead to test failures due to contents of
> tables to check being evicted.
> 
> We recently started to see this problem with the
> "conntrack -IPv6 ICMP6 Related with SNAT" test.
> 
> For consistency, this patch adds the -n parameter to all tcpdump
> calls in system-traffic.at.
> 
> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>

Thanks Frode,

I think that in general tcpdump -n is good practice unless lookup
is really needed.

Acked-by: Simon Horman <horms@ovn.org>
Ilya Maximets Oct. 25, 2023, 10:37 p.m. UTC | #2 
On 10/25/23 11:43, Simon Horman wrote:
> On Sat, Oct 21, 2023 at 01:22:08AM +0200, Frode Nordahl wrote:
>> Depending on system configuration, executing tcpdump without the
>> -n parameter, may prolong the execution time for tcpdump while it
>> attempts name resolution.
>>
>> This delay may in turn lead to test failures due to contents of
>> tables to check being evicted.
>>
>> We recently started to see this problem with the
>> "conntrack -IPv6 ICMP6 Related with SNAT" test.
>>
>> For consistency, this patch adds the -n parameter to all tcpdump
>> calls in system-traffic.at.
>>
>> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
> 
> Thanks Frode,
> 
> I think that in general tcpdump -n is good practice unless lookup
> is really needed.
> 
> Acked-by: Simon Horman <horms@ovn.org>

Thanks, Frode and Simon!

Applied and backported down to 2.17, since it's a test fix.

Best regards, Ilya Maximets.
Eelco Chaudron Oct. 26, 2023, 12:18 p.m. UTC | #3 
On 21 Oct 2023, at 1:22, Frode Nordahl wrote:

> Depending on system configuration, executing tcpdump without the
> -n parameter, may prolong the execution time for tcpdump while it
> attempts name resolution.
>
> This delay may in turn lead to test failures due to contents of
> tables to check being evicted.
>
> We recently started to see this problem with the
> "conntrack -IPv6 ICMP6 Related with SNAT" test.
>
> For consistency, this patch adds the -n parameter to all tcpdump
> calls in system-traffic.at.
>
> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>

This change looks good to me.

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Eelco Chaudron Oct. 26, 2023, 12:18 p.m. UTC | #4 
On 26 Oct 2023, at 14:18, Eelco Chaudron wrote:

> On 21 Oct 2023, at 1:22, Frode Nordahl wrote:
>
>> Depending on system configuration, executing tcpdump without the
>> -n parameter, may prolong the execution time for tcpdump while it
>> attempts name resolution.
>>
>> This delay may in turn lead to test failures due to contents of
>> tables to check being evicted.
>>
>> We recently started to see this problem with the
>> "conntrack -IPv6 ICMP6 Related with SNAT" test.
>>
>> For consistency, this patch adds the -n parameter to all tcpdump
>> calls in system-traffic.at.
>>
>> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
>
> This change looks good to me.
>
> Acked-by: Eelco Chaudron <echaudro@redhat.com>

Looks like I’m too late it was already applied ;)
diff mbox series

Patch

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 418cd32fe..1df2a5419 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -888,7 +888,7 @@  NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00::100 | FORMAT_PING], [0]
 ])
 
 dnl Start tcpdump to capture the encapsulated packets.
-NETNS_DAEMONIZE([at_ns0], [tcpdump -U -i p0 -w p0.pcap], [tcpdump.pid])
+NETNS_DAEMONIZE([at_ns0], [tcpdump -n -U -i p0 -w p0.pcap], [tcpdump.pid])
 sleep 1
 
 dnl Generate a single packet trough the controler that needs an ARP modification
@@ -3814,7 +3814,7 @@  table=0,in_port=ovs-p1,ct_state=+trk+rel+rpl,icmp,actions=ovs-p0
 AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
 
 rm p0.pcap
-NETNS_DAEMONIZE([at_ns0], [tcpdump -l -U -i p0 -w p0.pcap 2> tcpdump0_err], [tcpdump0.pid])
+NETNS_DAEMONIZE([at_ns0], [tcpdump -n -l -U -i p0 -w p0.pcap 2> tcpdump0_err], [tcpdump0.pid])
 OVS_WAIT_UNTIL([grep "listening" tcpdump0_err])
 
 dnl Send UDP packet from 10.1.1.1:1234 to 10.1.1.240:80
@@ -6158,7 +6158,7 @@  table=10 priority=0 action=drop
 AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
 
 rm p0.pcap
-OVS_DAEMONIZE([tcpdump -U -i ovs-p0 -w p0.pcap], [tcpdump.pid])
+OVS_DAEMONIZE([tcpdump -n -U -i ovs-p0 -w p0.pcap], [tcpdump.pid])
 sleep 1
 
 dnl UDP packets from ns0->ns1 should solicit "destination unreachable" response.
@@ -6182,7 +6182,7 @@  AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sed -e 's/dst=
 udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.2XX,sport=<cleared>,dport=<cleared>),mark=1
 ])
 
-AT_CHECK([tcpdump -v "icmp" -r p0.pcap 2>/dev/null | grep -E 'wrong|bad'], [1], [ignore-nolog])
+AT_CHECK([tcpdump -n -v "icmp" -r p0.pcap 2>/dev/null | grep -E 'wrong|bad'], [1], [ignore-nolog])
 
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
@@ -6927,13 +6927,13 @@  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2])
 AT_CHECK([ovs-appctl dpctl/flush-conntrack])
 
 rm p0.pcap
-OVS_DAEMONIZE([tcpdump -U -i ovs-p0 -w p0.pcap], [tcpdump.pid])
+OVS_DAEMONIZE([tcpdump -n -U -i ovs-p0 -w p0.pcap], [tcpdump.pid])
 sleep 1
 
 dnl UDP packets from ns0->ns1 should solicit "destination unreachable" response.
 NS_CHECK_EXEC([at_ns0], [bash -c "echo a | nc -6 $NC_EOF_OPT -u fc00::2 1"])
 
-AT_CHECK([tcpdump -v "icmp6" -r p0.pcap 2>/dev/null | grep -E 'wrong|bad'], [1], [ignore-nolog])
+AT_CHECK([tcpdump -n -v "icmp6" -r p0.pcap 2>/dev/null | grep -E 'wrong|bad'], [1], [ignore-nolog])
 
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl
 udp,orig=(src=fc00::1,dst=fc00::2,sport=<cleared>,dport=<cleared>),reply=(src=fc00::2,dst=fc00::240,sport=<cleared>,dport=<cleared>)
@@ -6962,7 +6962,7 @@  table=0,in_port=ovs-p1,ct_state=+trk+rel+rpl,icmp6,actions=ovs-p0
 AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
 
 rm p0.pcap
-NETNS_DAEMONIZE([at_ns0], [tcpdump -l -U -i p0 -w p0.pcap 2> tcpdump0_err], [tcpdump0.pid])
+NETNS_DAEMONIZE([at_ns0], [tcpdump -n -l -U -i p0 -w p0.pcap 2> tcpdump0_err], [tcpdump0.pid])
 OVS_WAIT_UNTIL([grep "listening" tcpdump0_err])
 
 dnl Send UDP packet from [[fc00::1]]:1234 to [[fc00::240]]:80
@@ -7653,7 +7653,7 @@  table=2,in_port=ovs-server,ip,ct_state=+trk+rpl,actions=output:ovs-client
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 
 rm server.pcap
-NETNS_DAEMONIZE([server], [tcpdump -l -U -i server -w server.pcap 2>tcpdump0_err], [tcpdump0.pid])
+NETNS_DAEMONIZE([server], [tcpdump -n -l -U -i server -w server.pcap 2>tcpdump0_err], [tcpdump0.pid])
 OVS_WAIT_UNTIL([grep "listening" tcpdump0_err])
 
 dnl Send UDP client->server
@@ -7695,7 +7695,7 @@  dnl Check the ICMP error in reply direction
 AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=42])
 
 rm client.pcap
-NETNS_DAEMONIZE([client], [tcpdump -l -U -i client -w client.pcap 2>tcpdump1_err], [tcpdump1.pid])
+NETNS_DAEMONIZE([client], [tcpdump -n -l -U -i client -w client.pcap 2>tcpdump1_err], [tcpdump1.pid])
 OVS_WAIT_UNTIL([grep "listening" tcpdump1_err])
 
 dnl Send UDP client->server