Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 31 Jan 2023 14:56:09 +0000 | From | Andrei Gherzan <> | Subject | Re: [PATCH v2 3/4] selftests: net: udpgso_bench: Fix racing bug between the rx/tx programs |
| |
On 23/01/31 08:33AM, Willem de Bruijn wrote: > On Tue, Jan 31, 2023 at 8:06 AM Andrei Gherzan > <andrei.gherzan@canonical.com> wrote: > > > > "udpgro_bench.sh" invokes udpgso_bench_rx/udpgso_bench_tx programs > > subsequently and while doing so, there is a chance that the rx one is not > > ready to accept socket connections. This racing bug could fail the test > > with at least one of the following: > > > > ./udpgso_bench_tx: connect: Connection refused > > ./udpgso_bench_tx: sendmsg: Connection refused > > ./udpgso_bench_tx: write: Connection refused > > > > This change addresses this by making udpgro_bench.sh wait for the rx > > program to be ready before firing off the tx one - with an exponential back > > off algorithm from 1s to 10s. > > > > Signed-off-by: Andrei Gherzan <andrei.gherzan@canonical.com> > > please CC: reviewers of previous revisions on new revisions > > also for upcoming patches: please clearly mark net or net-next.
Ack. I'll do all that in next v.
> > --- > > tools/testing/selftests/net/udpgso_bench.sh | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/tools/testing/selftests/net/udpgso_bench.sh b/tools/testing/selftests/net/udpgso_bench.sh > > index dc932fd65363..20b5db8fcbde 100755 > > --- a/tools/testing/selftests/net/udpgso_bench.sh > > +++ b/tools/testing/selftests/net/udpgso_bench.sh > > @@ -7,6 +7,7 @@ readonly GREEN='\033[0;92m' > > readonly YELLOW='\033[0;33m' > > readonly RED='\033[0;31m' > > readonly NC='\033[0m' # No Color > > +readonly TESTPORT=8000 # Keep this in sync with udpgso_bench_rx/tx > > then also pass explicit -p argument to the processes to keep all three > consistent
I have no idea how I've missed it. It will be in v4.
> > > > > readonly KSFT_PASS=0 > > readonly KSFT_FAIL=1 > > @@ -56,10 +57,27 @@ trap wake_children EXIT > > > > run_one() { > > local -r args=$@ > > + local -r init_delay_s=1 > > + local -r max_delay_s=10 > > + local delay_s=0 > > + local nr_socks=0 > > > > ./udpgso_bench_rx & > > ./udpgso_bench_rx -t & > > > > + # Wait for the above test program to get ready to receive connections. > > + delay_s="${init_delay_s}" > > + while [ "$delay_s" -lt "$max_delay_s" ]; do > > + nr_socks="$(ss -lnHi | grep -c "\*:${TESTPORT}")" > > + [ "$nr_socks" -eq 2 ] && break > > + sleep "$delay_s" > > + delay="$((delay*2))" > > I don't think we need exponential back-off for something this simple
I'm happy to drop/simplify. I'll go for a simple sleep 1 in a timeout loop.
> > > + done > > + if [ "$nr_socks" -ne 2 ]; then > > + echo "timed out while waiting for udpgso_bench_rx" > > + exit 1 > > + fi > > + > > ./udpgso_bench_tx ${args} > > } > > > > -- > > 2.34.1 > >
-- Andrei Gherzan
| ![\](/images/icornerr.gif) |