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

[David Ahern <dsahern@gmail.com>]

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>