* [PATCH v3 1/3] selftests: net: .gitignore the scratch directory of bpf
@ 2023-01-27 14:09 Andrei Gherzan
2023-01-27 14:09 ` [PATCH v3 2/3] selftests: net: Unify references to nat6to4.o when running udpgro_frglist.sh Andrei Gherzan
2023-01-27 14:09 ` [PATCH v3 3/3] selftests: net: Fix udpgro_frglist.sh shellcheck warnings and errors Andrei Gherzan
0 siblings, 2 replies; 5+ messages in thread
From: Andrei Gherzan @ 2023-01-27 14:09 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shuah Khan
Cc: Andrei Gherzan, netdev, linux-kselftest, linux-kernel, bpf
The net/bpf Makefile uses a similar build infrastructure to BPF[1] while
building libbpf as a dependency of nat6to4. This change adds a .gitignore
entry for SCRATCH_DIR where libbpf and its headers end up built/installed.
[1] Introduced in commit 837a3d66d698 ("selftests: net: Add
cross-compilation support for BPF programs")
Signed-off-by: Andrei Gherzan <andrei.gherzan@canonical.com>
---
tools/testing/selftests/net/.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index a6911cae368c..0d07dd13c973 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -40,6 +40,7 @@ test_unix_oob
timestamping
tls
toeplitz
+/tools
tun
txring_overwrite
txtimestamp
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v3 2/3] selftests: net: Unify references to nat6to4.o when running udpgro_frglist.sh 2023-01-27 14:09 [PATCH v3 1/3] selftests: net: .gitignore the scratch directory of bpf Andrei Gherzan @ 2023-01-27 14:09 ` Andrei Gherzan 2023-01-27 14:09 ` [PATCH v3 3/3] selftests: net: Fix udpgro_frglist.sh shellcheck warnings and errors Andrei Gherzan 1 sibling, 0 replies; 5+ messages in thread From: Andrei Gherzan @ 2023-01-27 14:09 UTC (permalink / raw) To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan Cc: Andrei Gherzan, netdev, linux-kselftest, linux-kernel, bpf This change refactors the nat6to4.o references to use a variable for consistency and also reformats two long lines. Signed-off-by: Andrei Gherzan <andrei.gherzan@canonical.com> --- tools/testing/selftests/net/udpgro_frglist.sh | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/net/udpgro_frglist.sh b/tools/testing/selftests/net/udpgro_frglist.sh index 0a6359bed0b9..e1ca49de2491 100755 --- a/tools/testing/selftests/net/udpgro_frglist.sh +++ b/tools/testing/selftests/net/udpgro_frglist.sh @@ -6,6 +6,7 @@ readonly PEER_NS="ns-peer-$(mktemp -u XXXXXX)" BPF_FILE="../bpf/xdp_dummy.bpf.o" +BPF_NAT6TO4_FILE="nat6to4.o" cleanup() { local -r jobs="$(jobs -p)" @@ -40,9 +41,13 @@ run_one() { ip -n "${PEER_NS}" link set veth1 xdp object ${BPF_FILE} section xdp tc -n "${PEER_NS}" qdisc add dev veth1 clsact - tc -n "${PEER_NS}" filter add dev veth1 ingress prio 4 protocol ipv6 bpf object-file nat6to4.o section schedcls/ingress6/nat_6 direct-action - tc -n "${PEER_NS}" filter add dev veth1 egress prio 4 protocol ip bpf object-file nat6to4.o section schedcls/egress4/snat4 direct-action - echo ${rx_args} + tc -n "${PEER_NS}" filter add dev veth1 ingress prio 4 protocol \ + ipv6 bpf object-file "$BPF_NAT6TO4_FILE" section \ + schedcls/ingress6/nat_6 direct-action + tc -n "${PEER_NS}" filter add dev veth1 egress prio 4 protocol \ + ip bpf object-file "$BPF_NAT6TO4_FILE" section \ + schedcls/egress4/snat4 direct-action + echo ${rx_args} ip netns exec "${PEER_NS}" ./udpgso_bench_rx ${rx_args} -r & # Hack: let bg programs complete the startup @@ -88,7 +93,7 @@ if [ ! -f ${BPF_FILE} ]; then exit -1 fi -if [ ! -f nat6to4.o ]; then +if [ ! -f "$BPF_NAT6TO4_FILE" ]; then echo "Missing nat6to4 helper. Build bpf nat6to4.o selftest first" exit -1 fi -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 3/3] selftests: net: Fix udpgro_frglist.sh shellcheck warnings and errors 2023-01-27 14:09 [PATCH v3 1/3] selftests: net: .gitignore the scratch directory of bpf Andrei Gherzan 2023-01-27 14:09 ` [PATCH v3 2/3] selftests: net: Unify references to nat6to4.o when running udpgro_frglist.sh Andrei Gherzan @ 2023-01-27 14:09 ` Andrei Gherzan 2023-01-27 14:32 ` Maciej Żenczykowski 1 sibling, 1 reply; 5+ messages in thread From: Andrei Gherzan @ 2023-01-27 14:09 UTC (permalink / raw) To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan, Lina Wang, Maciej enczykowski Cc: Andrei Gherzan, netdev, linux-kselftest, linux-kernel This change fixes the following shellcheck warnings and errors: * SC2155 (warning): Declare and assign separately to avoid masking return values. * SC2124 (warning): Assigning an array to a string! Assign as array, or use instead of @ to concatenate. * SC2034 (warning): ipv4_args appears unused. Verify use (or export if used externally). * SC2242 (error): Can only exit with status 0-255. Other data should be written to stdout/stderr. * SC2068 (error): Double quote array expansions to avoid re-splitting elements. Fixes: edae34a3ed92 ("selftests net: add UDP GRO fraglist + bpf self-tests") Signed-off-by: Andrei Gherzan <andrei.gherzan@canonical.com> --- tools/testing/selftests/net/udpgro_frglist.sh | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/net/udpgro_frglist.sh b/tools/testing/selftests/net/udpgro_frglist.sh index e1ca49de2491..97bf20e9afd8 100755 --- a/tools/testing/selftests/net/udpgro_frglist.sh +++ b/tools/testing/selftests/net/udpgro_frglist.sh @@ -3,7 +3,8 @@ # # Run a series of udpgro benchmarks -readonly PEER_NS="ns-peer-$(mktemp -u XXXXXX)" +PEER_NS="ns-peer-$(mktemp -u XXXXXX)" +readonly PEER_NS BPF_FILE="../bpf/xdp_dummy.bpf.o" BPF_NAT6TO4_FILE="nat6to4.o" @@ -19,7 +20,7 @@ trap cleanup EXIT run_one() { # use 'rx' as separator between sender args and receiver args - local -r all="$@" + local -r all="$*" local -r tx_args=${all%rx*} local rx_args=${all#*rx} @@ -56,13 +57,13 @@ run_one() { } run_in_netns() { - local -r args=$@ + local -r args="$*" echo ${args} ./in_netns.sh $0 __subprocess ${args} } run_udp() { - local -r args=$@ + local -r args="$*" echo "udp gso - over veth touching data" run_in_netns ${args} -u -S 0 rx -4 -v @@ -72,7 +73,7 @@ run_udp() { } run_tcp() { - local -r args=$@ + local -r args="$*" echo "tcp - over veth touching data" run_in_netns ${args} -t rx -4 -t @@ -80,7 +81,6 @@ run_tcp() { run_all() { local -r core_args="-l 4" - local -r ipv4_args="${core_args} -4 -D 192.168.1.1" local -r ipv6_args="${core_args} -6 -D 2001:db8::1" echo "ipv6" @@ -90,19 +90,19 @@ run_all() { if [ ! -f ${BPF_FILE} ]; then echo "Missing ${BPF_FILE}. Build bpf selftest first" - exit -1 + exit 1 fi if [ ! -f "$BPF_NAT6TO4_FILE" ]; then echo "Missing nat6to4 helper. Build bpf nat6to4.o selftest first" - exit -1 + exit 1 fi if [[ $# -eq 0 ]]; then run_all elif [[ $1 == "__subprocess" ]]; then shift - run_one $@ + run_one "$@" else - run_in_netns $@ + run_in_netns "$@" fi -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 3/3] selftests: net: Fix udpgro_frglist.sh shellcheck warnings and errors 2023-01-27 14:09 ` [PATCH v3 3/3] selftests: net: Fix udpgro_frglist.sh shellcheck warnings and errors Andrei Gherzan @ 2023-01-27 14:32 ` Maciej Żenczykowski 2023-01-27 18:43 ` Andrei Gherzan 0 siblings, 1 reply; 5+ messages in thread From: Maciej Żenczykowski @ 2023-01-27 14:32 UTC (permalink / raw) To: Andrei Gherzan Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan, Lina Wang, netdev, linux-kselftest, linux-kernel On Fri, Jan 27, 2023 at 6:09 AM Andrei Gherzan <andrei.gherzan@canonical.com> wrote: > > This change fixes the following shellcheck warnings and errors: > > * SC2155 (warning): Declare and assign separately to avoid masking return > values. > * SC2124 (warning): Assigning an array to a string! Assign as array, or use > instead of @ to concatenate. > * SC2034 (warning): ipv4_args appears unused. Verify use (or export if used > externally). > * SC2242 (error): Can only exit with status 0-255. Other data should be > written to stdout/stderr. > * SC2068 (error): Double quote array expansions to avoid re-splitting > elements. > > Fixes: edae34a3ed92 ("selftests net: add UDP GRO fraglist + bpf self-tests") > Signed-off-by: Andrei Gherzan <andrei.gherzan@canonical.com> > --- > tools/testing/selftests/net/udpgro_frglist.sh | 20 +++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/tools/testing/selftests/net/udpgro_frglist.sh b/tools/testing/selftests/net/udpgro_frglist.sh > index e1ca49de2491..97bf20e9afd8 100755 > --- a/tools/testing/selftests/net/udpgro_frglist.sh > +++ b/tools/testing/selftests/net/udpgro_frglist.sh > @@ -3,7 +3,8 @@ > # > # Run a series of udpgro benchmarks > > -readonly PEER_NS="ns-peer-$(mktemp -u XXXXXX)" > +PEER_NS="ns-peer-$(mktemp -u XXXXXX)" > +readonly PEER_NS > > BPF_FILE="../bpf/xdp_dummy.bpf.o" > BPF_NAT6TO4_FILE="nat6to4.o" > @@ -19,7 +20,7 @@ trap cleanup EXIT > > run_one() { > # use 'rx' as separator between sender args and receiver args > - local -r all="$@" > + local -r all="$*" this should technically use arrays, something like local -a -r args=("$@") but perhaps just get rid of args and just use "$@" directly below > local -r tx_args=${all%rx*} > local rx_args=${all#*rx} > > @@ -56,13 +57,13 @@ run_one() { > } > > run_in_netns() { > - local -r args=$@ > + local -r args="$*" > echo ${args} > ./in_netns.sh $0 __subprocess ${args} ie. here could just use "$@" directly twice instead of defining args. $0 should be doublequoted - though I guess it'll never be empty, and is unlikely to include spaces. > } > > run_udp() { > - local -r args=$@ > + local -r args="$*" > > echo "udp gso - over veth touching data" > run_in_netns ${args} -u -S 0 rx -4 -v > @@ -72,7 +73,7 @@ run_udp() { > } > > run_tcp() { > - local -r args=$@ > + local -r args="$*" > > echo "tcp - over veth touching data" > run_in_netns ${args} -t rx -4 -t > @@ -80,7 +81,6 @@ run_tcp() { > > run_all() { > local -r core_args="-l 4" is this still useful? embed directly in ipv6_args > - local -r ipv4_args="${core_args} -4 -D 192.168.1.1" perhaps this should stay as a comment?? > local -r ipv6_args="${core_args} -6 -D 2001:db8::1" > > echo "ipv6" > @@ -90,19 +90,19 @@ run_all() { > > if [ ! -f ${BPF_FILE} ]; then double quote "${BPF_FILE}" in case space in file name > echo "Missing ${BPF_FILE}. Build bpf selftest first" > - exit -1 > + exit 1 > fi > > if [ ! -f "$BPF_NAT6TO4_FILE" ]; then there seems to be inconsistency around [ vs [[, use [[ if relying on bash anyway > echo "Missing nat6to4 helper. Build bpf nat6to4.o selftest first" > - exit -1 > + exit 1 > fi > > if [[ $# -eq 0 ]]; then > run_all > elif [[ $1 == "__subprocess" ]]; then while this does indeed work, imho $1 should be "$1" to be less confusing > shift > - run_one $@ > + run_one "$@" > else > - run_in_netns $@ > + run_in_netns "$@" > fi > -- > 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 3/3] selftests: net: Fix udpgro_frglist.sh shellcheck warnings and errors 2023-01-27 14:32 ` Maciej Żenczykowski @ 2023-01-27 18:43 ` Andrei Gherzan 0 siblings, 0 replies; 5+ messages in thread From: Andrei Gherzan @ 2023-01-27 18:43 UTC (permalink / raw) To: Maciej Żenczykowski Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan, Lina Wang, netdev, linux-kselftest, linux-kernel On 23/01/27 06:32AM, Maciej Żenczykowski wrote: > On Fri, Jan 27, 2023 at 6:09 AM Andrei Gherzan > <andrei.gherzan@canonical.com> wrote: > > > > This change fixes the following shellcheck warnings and errors: > > > > * SC2155 (warning): Declare and assign separately to avoid masking return > > values. > > * SC2124 (warning): Assigning an array to a string! Assign as array, or use > > instead of @ to concatenate. > > * SC2034 (warning): ipv4_args appears unused. Verify use (or export if used > > externally). > > * SC2242 (error): Can only exit with status 0-255. Other data should be > > written to stdout/stderr. > > * SC2068 (error): Double quote array expansions to avoid re-splitting > > elements. > > > > Fixes: edae34a3ed92 ("selftests net: add UDP GRO fraglist + bpf self-tests") > > Signed-off-by: Andrei Gherzan <andrei.gherzan@canonical.com> > > --- > > tools/testing/selftests/net/udpgro_frglist.sh | 20 +++++++++---------- > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/tools/testing/selftests/net/udpgro_frglist.sh b/tools/testing/selftests/net/udpgro_frglist.sh > > index e1ca49de2491..97bf20e9afd8 100755 > > --- a/tools/testing/selftests/net/udpgro_frglist.sh > > +++ b/tools/testing/selftests/net/udpgro_frglist.sh > > @@ -3,7 +3,8 @@ > > # > > # Run a series of udpgro benchmarks > > > > -readonly PEER_NS="ns-peer-$(mktemp -u XXXXXX)" > > +PEER_NS="ns-peer-$(mktemp -u XXXXXX)" > > +readonly PEER_NS > > > > BPF_FILE="../bpf/xdp_dummy.bpf.o" > > BPF_NAT6TO4_FILE="nat6to4.o" > > @@ -19,7 +20,7 @@ trap cleanup EXIT > > > > run_one() { > > # use 'rx' as separator between sender args and receiver args > > - local -r all="$@" > > + local -r all="$*" > > this should technically use arrays, something like > > local -a -r args=("$@") > > but perhaps just get rid of args and just use "$@" directly below Using it directly would not work with the substitutions later because it would try to apply it to each positional parameter in turn. Same when using an intermediate array. > > > local -r tx_args=${all%rx*} > > local rx_args=${all#*rx} > > > > @@ -56,13 +57,13 @@ run_one() { > > } > > > > run_in_netns() { > > - local -r args=$@ > > + local -r args="$*" > > echo ${args} > > ./in_netns.sh $0 __subprocess ${args} > > ie. here could just use "$@" directly twice instead of defining args. > $0 should be doublequoted - though I guess it'll never be empty, and > is unlikely to include spaces. That sounds fine to me. I'll include it in v4. > > } > > > > run_udp() { > > - local -r args=$@ > > + local -r args="$*" > > > > echo "udp gso - over veth touching data" > > run_in_netns ${args} -u -S 0 rx -4 -v > > @@ -72,7 +73,7 @@ run_udp() { > > } > > > > run_tcp() { > > - local -r args=$@ > > + local -r args="$*" > > > > echo "tcp - over veth touching data" > > run_in_netns ${args} -t rx -4 -t > > @@ -80,7 +81,6 @@ run_tcp() { > > > > run_all() { > > local -r core_args="-l 4" > > is this still useful? embed directly in ipv6_args > > > - local -r ipv4_args="${core_args} -4 -D 192.168.1.1" > > perhaps this should stay as a comment?? Well the way I see it is one or the other. We either embed and drop ipv4_args or keep ipv4_args as a comment but also core_args. I'll do the former in v4 if no other objections. > > > local -r ipv6_args="${core_args} -6 -D 2001:db8::1" > > > > echo "ipv6" > > @@ -90,19 +90,19 @@ run_all() { > > > > if [ ! -f ${BPF_FILE} ]; then > > double quote > "${BPF_FILE}" > in case space in file name This change only targets warning/error issues. There are way more "info" ones, but I didn't want to splash a big patch for all those. > > > echo "Missing ${BPF_FILE}. Build bpf selftest first" > > - exit -1 > > + exit 1 > > fi > > > > if [ ! -f "$BPF_NAT6TO4_FILE" ]; then > > there seems to be inconsistency around [ vs [[, use [[ if relying on bash anyway > > > echo "Missing nat6to4 helper. Build bpf nat6to4.o selftest first" > > - exit -1 > > + exit 1 > > fi > > > > if [[ $# -eq 0 ]]; then > > run_all > > elif [[ $1 == "__subprocess" ]]; then > > while this does indeed work, imho $1 should be "$1" to be less confusing Agreed and again, there are a good set of other places where this is needed. Should I just address them all in an extra patch? Again, this one only scoped warnings/errors to avoid impact. Thanks for the review. -- Andrei Gherzan ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-01-27 18:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-27 14:09 [PATCH v3 1/3] selftests: net: .gitignore the scratch directory of bpf Andrei Gherzan 2023-01-27 14:09 ` [PATCH v3 2/3] selftests: net: Unify references to nat6to4.o when running udpgro_frglist.sh Andrei Gherzan 2023-01-27 14:09 ` [PATCH v3 3/3] selftests: net: Fix udpgro_frglist.sh shellcheck warnings and errors Andrei Gherzan 2023-01-27 14:32 ` Maciej Żenczykowski 2023-01-27 18:43 ` Andrei Gherzan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox