| Message ID | 20240301064214.411403-1-amusil@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] tests: Fix IPv6 prefix delegation 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 |
On 3/1/24 07:42, Ales Musil wrote: > The IPv6 prefix delegation test was failing from time to time, > because the tcpdump stderr "leaked" into the test stderr. Make sure > the stderr for every tcpdump is redirected into file and while at it > properly wait for the tcpdump to fully start. > > Signed-off-by: Ales Musil <amusil@redhat.com> > --- Hi Ales, Not a full review but aren't we missing other instances where tcpdump's stderr is not redirected. From what I can tell the following system-ovn.at tests need updates too: - 2 LSs IGMP and MLD - Load balancer health checks - IPv4 - BFD - ovn -- CoPP Would it make sense to have a tcpdump helper to wrap all calls to tcpdump and properly take care of redirection? Regards, Dumitru > tests/system-common-macros.at | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 177178067..d6c4e090e 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -438,7 +438,8 @@ chown root:dhcpd /var/lib/dhcp /var/lib/dhcp/dhcpd6.leases > chmod 775 /var/lib/dhcp > chmod 664 /var/lib/dhcp/dhcpd6.leases > > -NS_CHECK_EXEC([server], [tcpdump -nni s1 > pkt.pcap &]) > +NETNS_DAEMONIZE([server], [tcpdump -nni s1 > pkt.pcap 2>server-tcpdump.stderr], [server-tcpdump.pid]) > +OVS_WAIT_UNTIL([grep "listening" server-tcpdump.stderr]) > > NETNS_DAEMONIZE([server], [dhcpd -6 -f s1 > dhcpd.log 2>&1], [dhcpd.pid]) > ovn-nbctl --wait=hv sync > @@ -462,9 +463,11 @@ ovn-nbctl list logical_router_port rp-public > /tmp/rp-public > ovn-nbctl set logical_router_port rp-sw0 options:prefix=false > ovn-nbctl set logical_router_port rp-sw1 options:prefix=false > # Renew message > -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x05 and ip6[[113:4]]=0x${prefix} > renew.pcap &]) > +NETNS_DAEMONIZE([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x05 and ip6[[113:4]]=0x${prefix} > renew.pcap 2>renew-tcpdump.stderr], [renew-tcpdump.pid]) > +OVS_WAIT_UNTIL([grep "listening" renew-tcpdump.stderr]) > # Reply message with Status OK > -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x07 and ip6[[81:4]]=0x${prefix} > reply.pcap &]) > +NETNS_DAEMONIZE([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x07 and ip6[[81:4]]=0x${prefix} > reply.pcap 2>reply-tcpdump.stderr], [reply-tcpdump.pid]) > +OVS_WAIT_UNTIL([grep "listening" reply-tcpdump.stderr]) > > OVS_WAIT_UNTIL([ > total_pkts=$(cat renew.pcap | wc -l) > @@ -476,8 +479,6 @@ OVS_WAIT_UNTIL([ > test "${total_pkts}" = "1" > ]) > > -kill $(pidof tcpdump) > - > ovn-nbctl set logical_router_port rp-sw0 options:prefix=false > ovn-nbctl clear logical_router_port rp-sw0 ipv6_prefix > OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw0 ipv6_prefix | cut -c3-16)" = "[2001:1db8:3333]"])
On Fri, Mar 1, 2024 at 9:46 AM Dumitru Ceara <dceara@redhat.com> wrote: > On 3/1/24 07:42, Ales Musil wrote: > > The IPv6 prefix delegation test was failing from time to time, > > because the tcpdump stderr "leaked" into the test stderr. Make sure > > the stderr for every tcpdump is redirected into file and while at it > > properly wait for the tcpdump to fully start. > > > > Signed-off-by: Ales Musil <amusil@redhat.com> > > --- > > Hi Ales, > Hi Dumitru, > Not a full review but aren't we missing other instances where tcpdump's > stderr is not redirected. From what I can tell the following > system-ovn.at tests need updates too: > > - 2 LSs IGMP and MLD > - Load balancer health checks - IPv4 > - BFD > - ovn -- CoPP > > Would it make sense to have a tcpdump helper to wrap all calls to > tcpdump and properly take care of redirection? > Yeah that makes sense, there are way too many of them currently and it is a returning problem. I'll look into it and make some helper to replace all calls in v2 instead. > > Regards, > Dumitru > > > tests/system-common-macros.at | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/tests/system-common-macros.at b/tests/ > system-common-macros.at > > index 177178067..d6c4e090e 100644 > > --- a/tests/system-common-macros.at > > +++ b/tests/system-common-macros.at > > @@ -438,7 +438,8 @@ chown root:dhcpd /var/lib/dhcp > /var/lib/dhcp/dhcpd6.leases > > chmod 775 /var/lib/dhcp > > chmod 664 /var/lib/dhcp/dhcpd6.leases > > > > -NS_CHECK_EXEC([server], [tcpdump -nni s1 > pkt.pcap &]) > > +NETNS_DAEMONIZE([server], [tcpdump -nni s1 > pkt.pcap > 2>server-tcpdump.stderr], [server-tcpdump.pid]) > > +OVS_WAIT_UNTIL([grep "listening" server-tcpdump.stderr]) > > > > NETNS_DAEMONIZE([server], [dhcpd -6 -f s1 > dhcpd.log 2>&1], > [dhcpd.pid]) > > ovn-nbctl --wait=hv sync > > @@ -462,9 +463,11 @@ ovn-nbctl list logical_router_port rp-public > > /tmp/rp-public > > ovn-nbctl set logical_router_port rp-sw0 options:prefix=false > > ovn-nbctl set logical_router_port rp-sw1 options:prefix=false > > # Renew message > > -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x05 and > ip6[[113:4]]=0x${prefix} > renew.pcap &]) > > +NETNS_DAEMONIZE([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x05 and > ip6[[113:4]]=0x${prefix} > renew.pcap 2>renew-tcpdump.stderr], > [renew-tcpdump.pid]) > > +OVS_WAIT_UNTIL([grep "listening" renew-tcpdump.stderr]) > > # Reply message with Status OK > > -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x07 and > ip6[[81:4]]=0x${prefix} > reply.pcap &]) > > +NETNS_DAEMONIZE([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x07 and > ip6[[81:4]]=0x${prefix} > reply.pcap 2>reply-tcpdump.stderr], > [reply-tcpdump.pid]) > > +OVS_WAIT_UNTIL([grep "listening" reply-tcpdump.stderr]) > > > > OVS_WAIT_UNTIL([ > > total_pkts=$(cat renew.pcap | wc -l) > > @@ -476,8 +479,6 @@ OVS_WAIT_UNTIL([ > > test "${total_pkts}" = "1" > > ]) > > > > -kill $(pidof tcpdump) > > - > > ovn-nbctl set logical_router_port rp-sw0 options:prefix=false > > ovn-nbctl clear logical_router_port rp-sw0 ipv6_prefix > > OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw0 > ipv6_prefix | cut -c3-16)" = "[2001:1db8:3333]"]) > > Thanks, Ales
diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 177178067..d6c4e090e 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -438,7 +438,8 @@ chown root:dhcpd /var/lib/dhcp /var/lib/dhcp/dhcpd6.leases chmod 775 /var/lib/dhcp chmod 664 /var/lib/dhcp/dhcpd6.leases -NS_CHECK_EXEC([server], [tcpdump -nni s1 > pkt.pcap &]) +NETNS_DAEMONIZE([server], [tcpdump -nni s1 > pkt.pcap 2>server-tcpdump.stderr], [server-tcpdump.pid]) +OVS_WAIT_UNTIL([grep "listening" server-tcpdump.stderr]) NETNS_DAEMONIZE([server], [dhcpd -6 -f s1 > dhcpd.log 2>&1], [dhcpd.pid]) ovn-nbctl --wait=hv sync @@ -462,9 +463,11 @@ ovn-nbctl list logical_router_port rp-public > /tmp/rp-public ovn-nbctl set logical_router_port rp-sw0 options:prefix=false ovn-nbctl set logical_router_port rp-sw1 options:prefix=false # Renew message -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x05 and ip6[[113:4]]=0x${prefix} > renew.pcap &]) +NETNS_DAEMONIZE([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x05 and ip6[[113:4]]=0x${prefix} > renew.pcap 2>renew-tcpdump.stderr], [renew-tcpdump.pid]) +OVS_WAIT_UNTIL([grep "listening" renew-tcpdump.stderr]) # Reply message with Status OK -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x07 and ip6[[81:4]]=0x${prefix} > reply.pcap &]) +NETNS_DAEMONIZE([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x07 and ip6[[81:4]]=0x${prefix} > reply.pcap 2>reply-tcpdump.stderr], [reply-tcpdump.pid]) +OVS_WAIT_UNTIL([grep "listening" reply-tcpdump.stderr]) OVS_WAIT_UNTIL([ total_pkts=$(cat renew.pcap | wc -l) @@ -476,8 +479,6 @@ OVS_WAIT_UNTIL([ test "${total_pkts}" = "1" ]) -kill $(pidof tcpdump) - ovn-nbctl set logical_router_port rp-sw0 options:prefix=false ovn-nbctl clear logical_router_port rp-sw0 ipv6_prefix OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw0 ipv6_prefix | cut -c3-16)" = "[2001:1db8:3333]"])
The IPv6 prefix delegation test was failing from time to time, because the tcpdump stderr "leaked" into the test stderr. Make sure the stderr for every tcpdump is redirected into file and while at it properly wait for the tcpdump to fully start. Signed-off-by: Ales Musil <amusil@redhat.com> --- tests/system-common-macros.at | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)