| Message ID | 60919291657a9ee89c708d8aababc28ebe1420be.1573821780.git.jbenc@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf] selftests: bpf: fix test_tc_tunnel hanging | expand |
On Fri, Nov 15, 2019 at 7:43 AM Jiri Benc <jbenc@redhat.com> wrote: > > When run_kselftests.sh is run, it hangs after test_tc_tunnel.sh. The reason > is test_tc_tunnel.sh ensures the server ('nc -l') is run all the time, > starting it again every time it is expected to terminate. The exception is > the final client_connect: the server is not started anymore, which ensures > no process is kept running after the test is finished. > > For a sit test, though, the script is terminated prematurely without the > final client_connect and the 'nc' process keeps running. This in turn causes > the run_one function in kselftest/runner.sh to hang forever, waiting for the > runaway process to finish. > > Ensure a remaining server is terminated on cleanup. > > Fixes: f6ad6accaa9d ("selftests/bpf: expand test_tc_tunnel with SIT encap") > Cc: Willem de Bruijn <willemb@google.com> > Signed-off-by: Jiri Benc <jbenc@redhat.com> Acked-by: Willem de Bruijn <willemb@google.com> Yes, I had missed that the server gets restarted even though the SIT test has to bail instead of run the last, BPF decap, test. Thanks Jiri.
On 11/15/19 1:43 PM, Jiri Benc wrote: > When run_kselftests.sh is run, it hangs after test_tc_tunnel.sh. The reason > is test_tc_tunnel.sh ensures the server ('nc -l') is run all the time, > starting it again every time it is expected to terminate. The exception is > the final client_connect: the server is not started anymore, which ensures > no process is kept running after the test is finished. > > For a sit test, though, the script is terminated prematurely without the > final client_connect and the 'nc' process keeps running. This in turn causes > the run_one function in kselftest/runner.sh to hang forever, waiting for the > runaway process to finish. > > Ensure a remaining server is terminated on cleanup. > > Fixes: f6ad6accaa9d ("selftests/bpf: expand test_tc_tunnel with SIT encap") > Cc: Willem de Bruijn <willemb@google.com> > Signed-off-by: Jiri Benc <jbenc@redhat.com> Looks like your Fixes tag is wrong: [...] Applying: selftests: bpf: Fix test_tc_tunnel hanging Switched to branch 'master' Your branch is up to date with 'origin/master'. Updating 808c9f7ebfff..e2090d0451c5 Fast-forward tools/testing/selftests/bpf/test_tc_tunnel.sh | 5 +++++ 1 file changed, 5 insertions(+) Deleted branch mbox (was e2090d0451c5). Commit: e2090d0451c5 ("selftests: bpf: Fix test_tc_tunnel hanging") Fixes tag: Fixes: f6ad6accaa9d ("selftests/bpf: expand test_tc_tunnel with SIT encap") Has these problem(s): - Target SHA1 does not exist [...] Thanks, Daniel
On Fri, Nov 15, 2019 at 5:02 PM Daniel Borkmann <daniel@iogearbox.net> wrote: > > On 11/15/19 1:43 PM, Jiri Benc wrote: > > When run_kselftests.sh is run, it hangs after test_tc_tunnel.sh. The reason > > is test_tc_tunnel.sh ensures the server ('nc -l') is run all the time, > > starting it again every time it is expected to terminate. The exception is > > the final client_connect: the server is not started anymore, which ensures > > no process is kept running after the test is finished. > > > > For a sit test, though, the script is terminated prematurely without the > > final client_connect and the 'nc' process keeps running. This in turn causes > > the run_one function in kselftest/runner.sh to hang forever, waiting for the > > runaway process to finish. > > > > Ensure a remaining server is terminated on cleanup. > > > > Fixes: f6ad6accaa9d ("selftests/bpf: expand test_tc_tunnel with SIT encap") > > Cc: Willem de Bruijn <willemb@google.com> > > Signed-off-by: Jiri Benc <jbenc@redhat.com> > > Looks like your Fixes tag is wrong: > > [...] > Applying: selftests: bpf: Fix test_tc_tunnel hanging > Switched to branch 'master' > Your branch is up to date with 'origin/master'. > Updating 808c9f7ebfff..e2090d0451c5 > Fast-forward > tools/testing/selftests/bpf/test_tc_tunnel.sh | 5 +++++ > 1 file changed, 5 insertions(+) > Deleted branch mbox (was e2090d0451c5). > Commit: e2090d0451c5 ("selftests: bpf: Fix test_tc_tunnel hanging") > Fixes tag: Fixes: f6ad6accaa9d ("selftests/bpf: expand test_tc_tunnel with SIT encap") > Has these problem(s): > - Target SHA1 does not exist > [...] Ah, a typo. This is the SHA1 in my tree, note the aa9d --> aa99d $ git fetch davem-net-next $ git log -1 --oneline -- tools/testing/selftests/bpf/test_tc_tunnel.sh f6ad6accaa99d selftests/bpf: expand test_tc_tunnel with SIT encap
On Fri, 15 Nov 2019 17:05:42 -0500, Willem de Bruijn wrote: > Ah, a typo. This is the SHA1 in my tree, note the aa9d --> aa99d > > $ git fetch davem-net-next > $ git log -1 --oneline -- tools/testing/selftests/bpf/test_tc_tunnel.sh > f6ad6accaa99d selftests/bpf: expand test_tc_tunnel with SIT encap Indeed, it should have been: Fixes: f6ad6accaa99 ("selftests/bpf: expand test_tc_tunnel with SIT encap") Not sure how that happened, I'm sorry for that. Thanks for catching it. Should I resend with the fixed commit message? Sorry again, Jiri
On 11/18/19 10:51 AM, Jiri Benc wrote: > On Fri, 15 Nov 2019 17:05:42 -0500, Willem de Bruijn wrote: >> Ah, a typo. This is the SHA1 in my tree, note the aa9d --> aa99d >> >> $ git fetch davem-net-next >> $ git log -1 --oneline -- tools/testing/selftests/bpf/test_tc_tunnel.sh >> f6ad6accaa99d selftests/bpf: expand test_tc_tunnel with SIT encap > > Indeed, it should have been: > > Fixes: f6ad6accaa99 ("selftests/bpf: expand test_tc_tunnel with SIT encap") > > Not sure how that happened, I'm sorry for that. Thanks for catching it. > Should I resend with the fixed commit message? > > Sorry again, Ok, fixed up and applied both of your patches to bpf-next, thanks!
diff --git a/tools/testing/selftests/bpf/test_tc_tunnel.sh b/tools/testing/selftests/bpf/test_tc_tunnel.sh index ff0d31d38061..7c76b841b17b 100755 --- a/tools/testing/selftests/bpf/test_tc_tunnel.sh +++ b/tools/testing/selftests/bpf/test_tc_tunnel.sh @@ -62,6 +62,10 @@ cleanup() { if [[ -f "${infile}" ]]; then rm "${infile}" fi + + if [[ -n $server_pid ]]; then + kill $server_pid 2> /dev/null + fi } server_listen() { @@ -77,6 +81,7 @@ client_connect() { verify_data() { wait "${server_pid}" + server_pid= # sha1sum returns two fields [sha1] [filepath] # convert to bash array and access first elem insum=($(sha1sum ${infile}))
When run_kselftests.sh is run, it hangs after test_tc_tunnel.sh. The reason is test_tc_tunnel.sh ensures the server ('nc -l') is run all the time, starting it again every time it is expected to terminate. The exception is the final client_connect: the server is not started anymore, which ensures no process is kept running after the test is finished. For a sit test, though, the script is terminated prematurely without the final client_connect and the 'nc' process keeps running. This in turn causes the run_one function in kselftest/runner.sh to hang forever, waiting for the runaway process to finish. Ensure a remaining server is terminated on cleanup. Fixes: f6ad6accaa9d ("selftests/bpf: expand test_tc_tunnel with SIT encap") Cc: Willem de Bruijn <willemb@google.com> Signed-off-by: Jiri Benc <jbenc@redhat.com> --- tools/testing/selftests/bpf/test_tc_tunnel.sh | 5 +++++ 1 file changed, 5 insertions(+)