Re: [PATCH net] selftests: pmtu.sh: Kill tcpdump processes launched by subshell.

[Guillaume Nault <gnault@redhat.com>]

On Tue, Mar 08, 2022 at 11:21:58AM -0700, David Ahern wrote:
> On 3/7/22 3:38 PM, Guillaume Nault wrote:
> > The cleanup() function takes care of killing processes launched by the
> > test functions. It relies on variables like ${tcpdump_pids} to get the
> > relevant PIDs. But tests are run in their own subshell, so updated
> > *_pids values are invisible to other shells. Therefore cleanup() never
> > sees any process to kill:
> > 
> > $ ./tools/testing/selftests/net/pmtu.sh -t pmtu_ipv4_exception
> > TEST: ipv4: PMTU exceptions                                         [ OK ]
> > TEST: ipv4: PMTU exceptions - nexthop objects                       [ OK ]
> > 
> > $ pgrep -af tcpdump
> > 6084 tcpdump -s 0 -i veth_A-R1 -w pmtu_ipv4_exception_veth_A-R1.pcap
> > 6085 tcpdump -s 0 -i veth_R1-A -w pmtu_ipv4_exception_veth_R1-A.pcap
> > 6086 tcpdump -s 0 -i veth_R1-B -w pmtu_ipv4_exception_veth_R1-B.pcap
> > 6087 tcpdump -s 0 -i veth_B-R1 -w pmtu_ipv4_exception_veth_B-R1.pcap
> > 6088 tcpdump -s 0 -i veth_A-R2 -w pmtu_ipv4_exception_veth_A-R2.pcap
> > 6089 tcpdump -s 0 -i veth_R2-A -w pmtu_ipv4_exception_veth_R2-A.pcap
> > 6090 tcpdump -s 0 -i veth_R2-B -w pmtu_ipv4_exception_veth_R2-B.pcap
> > 6091 tcpdump -s 0 -i veth_B-R2 -w pmtu_ipv4_exception_veth_B-R2.pcap
> > 6228 tcpdump -s 0 -i veth_A-R1 -w pmtu_ipv4_exception_veth_A-R1.pcap
> > 6229 tcpdump -s 0 -i veth_R1-A -w pmtu_ipv4_exception_veth_R1-A.pcap
> > 6230 tcpdump -s 0 -i veth_R1-B -w pmtu_ipv4_exception_veth_R1-B.pcap
> > 6231 tcpdump -s 0 -i veth_B-R1 -w pmtu_ipv4_exception_veth_B-R1.pcap
> > 6232 tcpdump -s 0 -i veth_A-R2 -w pmtu_ipv4_exception_veth_A-R2.pcap
> > 6233 tcpdump -s 0 -i veth_R2-A -w pmtu_ipv4_exception_veth_R2-A.pcap
> > 6234 tcpdump -s 0 -i veth_R2-B -w pmtu_ipv4_exception_veth_R2-B.pcap
> > 6235 tcpdump -s 0 -i veth_B-R2 -w pmtu_ipv4_exception_veth_B-R2.pcap
> > 
> > Fix this by running cleanup() in the context of the test subshell.
> > Now that each test cleans the environment after completion, there's no
> > need for calling cleanup() again when the next test starts. So let's
> > drop it from the setup() function. This is okay because cleanup() is
> > also called when pmtu.sh starts, so even the first test starts in a
> > clean environment.
> > 
> > Note: PAUSE_ON_FAIL is still evaluated before cleanup(), so one can
> > still inspect the test environment upon failure when using -p.
> > 
> > Fixes: a92a0a7b8e7c ("selftests: pmtu: Simplify cleanup and namespace names")
> > Signed-off-by: Guillaume Nault <gnault@redhat.com>
> > ---
> >  tools/testing/selftests/net/pmtu.sh | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> 
> Reviewed-by: David Ahern <dsahern@kernel.org>

Hum, looks like for such short lived tcpdump sessions, we also need to
disable buffered mode. Otherwise tcpdump can be killed before it
actually processed the packets.

Also there's another problem, with the xfrmudp tests not recording the
correct PID in ${nettest_pids}.

I'll send v2 to also fix all these problems.