diff mbox series

[bpf-next,v2,11/11] selftests/xsk: introduce XSKTEST_ETH environment variable

Message ID 20230824122853.3494-12-magnus.karlsson@gmail.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series seltests/xsk: various improvements to xskxceiver | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-28 success Logs for veristat
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs_no_alu32 on s390x with gcc
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/cc_maintainers warning 5 maintainers not CCed: shuah@kernel.org yonghong.song@linux.dev mykolal@fb.com linux-kselftest@vger.kernel.org jonathan.lemon@gmail.com
netdev/build_clang success Errors and warnings before: 9 this patch: 9
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success net selftest script(s) already in Makefile
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 9 this patch: 9
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 59 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-0 success Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain_full }}
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 fail Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for veristat
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for x86_64 with gcc

Commit Message

Magnus Karlsson Aug. 24, 2023, 12:28 p.m. UTC 
From: Magnus Karlsson <magnus.karlsson@intel.com>

Introduce the XSKTEST_ETH environment variable to be able to set the
network interface that should be used for testing.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

Comments

Maciej Fijalkowski Aug. 25, 2023, 12:57 p.m. UTC | #1 
On Thu, Aug 24, 2023 at 02:28:53PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Introduce the XSKTEST_ETH environment variable to be able to set the
> network interface that should be used for testing.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/test_xsk.sh | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> index 9ec718043c1a..3e0a2302a185 100755
> --- a/tools/testing/selftests/bpf/test_xsk.sh
> +++ b/tools/testing/selftests/bpf/test_xsk.sh
> @@ -88,14 +88,12 @@
>  
>  . xsk_prereqs.sh
>  
> -ETH=""
> -
>  while getopts "vi:dm:lt:h" flag
>  do
>  	case "${flag}" in
>  		v) verbose=1;;
>  		d) debug=1;;
> -		i) ETH=${OPTARG};;
> +		i) XSKTEST_ETH=${OPTARG};;
>  		m) XSKTEST_MODE=${OPTARG};;
>  		l) list=1;;
>  		t) XSKTEST_TEST=${OPTARG};;
> @@ -157,9 +155,9 @@ if [[ $help -eq 1 ]]; then
>          exit
>  fi
>  
> -if [ ! -z $ETH ]; then
> -	VETH0=${ETH}
> -	VETH1=${ETH}
> +if [ -n "$XSKTEST_ETH" ]; then

Sorry - is point of this patch is just to invert the logic and rename the
env var?

> +	VETH0=${XSKTEST_ETH}
> +	VETH1=${XSKTEST_ETH}
>  else
>  	validate_root_exec
>  	validate_veth_support ${VETH0}
> @@ -203,10 +201,10 @@ fi
>  
>  exec_xskxceiver
>  
> -if [ -z $ETH ]; then
> +if [ -z $XSKTEST_ETH ]; then
>  	cleanup_exit ${VETH0} ${VETH1}
>  else
> -	cleanup_iface ${ETH} ${MTU}
> +	cleanup_iface ${XSKTEST_ETH} ${MTU}
>  fi
>  
>  if [[ $list -eq 1 ]]; then
> @@ -216,17 +214,17 @@ fi
>  TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
>  busy_poll=1
>  
> -if [ -z $ETH ]; then
> +if [ -z $XSKTEST_ETH ]; then
>  	setup_vethPairs
>  fi
>  exec_xskxceiver
>  
>  ## END TESTS
>  
> -if [ -z $ETH ]; then
> +if [ -z $XSKTEST_ETH ]; then
>  	cleanup_exit ${VETH0} ${VETH1}
>  else
> -	cleanup_iface ${ETH} ${MTU}
> +	cleanup_iface ${XSKTEST_ETH} ${MTU}
>  fi
>  
>  failures=0
> -- 
> 2.34.1
>
Magnus Karlsson Aug. 25, 2023, 1:03 p.m. UTC | #2 
On Fri, 25 Aug 2023 at 14:57, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Thu, Aug 24, 2023 at 02:28:53PM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Introduce the XSKTEST_ETH environment variable to be able to set the
> > network interface that should be used for testing.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >  tools/testing/selftests/bpf/test_xsk.sh | 20 +++++++++-----------
> >  1 file changed, 9 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > index 9ec718043c1a..3e0a2302a185 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -88,14 +88,12 @@
> >
> >  . xsk_prereqs.sh
> >
> > -ETH=""
> > -
> >  while getopts "vi:dm:lt:h" flag
> >  do
> >       case "${flag}" in
> >               v) verbose=1;;
> >               d) debug=1;;
> > -             i) ETH=${OPTARG};;
> > +             i) XSKTEST_ETH=${OPTARG};;
> >               m) XSKTEST_MODE=${OPTARG};;
> >               l) list=1;;
> >               t) XSKTEST_TEST=${OPTARG};;
> > @@ -157,9 +155,9 @@ if [[ $help -eq 1 ]]; then
> >          exit
> >  fi
> >
> > -if [ ! -z $ETH ]; then
> > -     VETH0=${ETH}
> > -     VETH1=${ETH}
> > +if [ -n "$XSKTEST_ETH" ]; then
>
> Sorry - is point of this patch is just to invert the logic and rename the
> env var?

The purpose was to make it setable from the outside and give it a name
that is more descriptive and targeted only to xskxceiver.

> > +     VETH0=${XSKTEST_ETH}
> > +     VETH1=${XSKTEST_ETH}
> >  else
> >       validate_root_exec
> >       validate_veth_support ${VETH0}
> > @@ -203,10 +201,10 @@ fi
> >
> >  exec_xskxceiver
> >
> > -if [ -z $ETH ]; then
> > +if [ -z $XSKTEST_ETH ]; then
> >       cleanup_exit ${VETH0} ${VETH1}
> >  else
> > -     cleanup_iface ${ETH} ${MTU}
> > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> >  fi
> >
> >  if [[ $list -eq 1 ]]; then
> > @@ -216,17 +214,17 @@ fi
> >  TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
> >  busy_poll=1
> >
> > -if [ -z $ETH ]; then
> > +if [ -z $XSKTEST_ETH ]; then
> >       setup_vethPairs
> >  fi
> >  exec_xskxceiver
> >
> >  ## END TESTS
> >
> > -if [ -z $ETH ]; then
> > +if [ -z $XSKTEST_ETH ]; then
> >       cleanup_exit ${VETH0} ${VETH1}
> >  else
> > -     cleanup_iface ${ETH} ${MTU}
> > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> >  fi
> >
> >  failures=0
> > --
> > 2.34.1
> >
Maciej Fijalkowski Aug. 25, 2023, 1:19 p.m. UTC | #3 
On Fri, Aug 25, 2023 at 03:03:58PM +0200, Magnus Karlsson wrote:
> On Fri, 25 Aug 2023 at 14:57, Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > On Thu, Aug 24, 2023 at 02:28:53PM +0200, Magnus Karlsson wrote:
> > > From: Magnus Karlsson <magnus.karlsson@intel.com>
> > >
> > > Introduce the XSKTEST_ETH environment variable to be able to set the
> > > network interface that should be used for testing.
> > >
> > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > > ---
> > >  tools/testing/selftests/bpf/test_xsk.sh | 20 +++++++++-----------
> > >  1 file changed, 9 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > > index 9ec718043c1a..3e0a2302a185 100755
> > > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > > @@ -88,14 +88,12 @@
> > >
> > >  . xsk_prereqs.sh
> > >
> > > -ETH=""
> > > -
> > >  while getopts "vi:dm:lt:h" flag
> > >  do
> > >       case "${flag}" in
> > >               v) verbose=1;;
> > >               d) debug=1;;
> > > -             i) ETH=${OPTARG};;
> > > +             i) XSKTEST_ETH=${OPTARG};;
> > >               m) XSKTEST_MODE=${OPTARG};;
> > >               l) list=1;;
> > >               t) XSKTEST_TEST=${OPTARG};;
> > > @@ -157,9 +155,9 @@ if [[ $help -eq 1 ]]; then
> > >          exit
> > >  fi
> > >
> > > -if [ ! -z $ETH ]; then
> > > -     VETH0=${ETH}
> > > -     VETH1=${ETH}
> > > +if [ -n "$XSKTEST_ETH" ]; then
> >
> > Sorry - is point of this patch is just to invert the logic and rename the
> > env var?
> 
> The purpose was to make it setable from the outside and give it a name
> that is more descriptive and targeted only to xskxceiver.

and this is accomplished by not having ETH initialized here? What will be
'the outside' ?

Currently I don't see much value within this patch, unless you explain the
need for setting this from outside of this script. Maybe I missed some
discussion from v1. I can live with this variable being ETH, what's more
concerning/confusing to me is that for ZC we have to set VETH0 and VETH1
to ETH and then use that later on.


> 
> > > +     VETH0=${XSKTEST_ETH}
> > > +     VETH1=${XSKTEST_ETH}
> > >  else
> > >       validate_root_exec
> > >       validate_veth_support ${VETH0}
> > > @@ -203,10 +201,10 @@ fi
> > >
> > >  exec_xskxceiver
> > >
> > > -if [ -z $ETH ]; then
> > > +if [ -z $XSKTEST_ETH ]; then
> > >       cleanup_exit ${VETH0} ${VETH1}
> > >  else
> > > -     cleanup_iface ${ETH} ${MTU}
> > > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> > >  fi
> > >
> > >  if [[ $list -eq 1 ]]; then
> > > @@ -216,17 +214,17 @@ fi
> > >  TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
> > >  busy_poll=1
> > >
> > > -if [ -z $ETH ]; then
> > > +if [ -z $XSKTEST_ETH ]; then
> > >       setup_vethPairs
> > >  fi
> > >  exec_xskxceiver
> > >
> > >  ## END TESTS
> > >
> > > -if [ -z $ETH ]; then
> > > +if [ -z $XSKTEST_ETH ]; then
> > >       cleanup_exit ${VETH0} ${VETH1}
> > >  else
> > > -     cleanup_iface ${ETH} ${MTU}
> > > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> > >  fi
> > >
> > >  failures=0
> > > --
> > > 2.34.1
> > >
Magnus Karlsson Aug. 25, 2023, 1:34 p.m. UTC | #4 
On Fri, 25 Aug 2023 at 15:20, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Fri, Aug 25, 2023 at 03:03:58PM +0200, Magnus Karlsson wrote:
> > On Fri, 25 Aug 2023 at 14:57, Maciej Fijalkowski
> > <maciej.fijalkowski@intel.com> wrote:
> > >
> > > On Thu, Aug 24, 2023 at 02:28:53PM +0200, Magnus Karlsson wrote:
> > > > From: Magnus Karlsson <magnus.karlsson@intel.com>
> > > >
> > > > Introduce the XSKTEST_ETH environment variable to be able to set the
> > > > network interface that should be used for testing.
> > > >
> > > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > > > ---
> > > >  tools/testing/selftests/bpf/test_xsk.sh | 20 +++++++++-----------
> > > >  1 file changed, 9 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > > > index 9ec718043c1a..3e0a2302a185 100755
> > > > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > > > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > > > @@ -88,14 +88,12 @@
> > > >
> > > >  . xsk_prereqs.sh
> > > >
> > > > -ETH=""
> > > > -
> > > >  while getopts "vi:dm:lt:h" flag
> > > >  do
> > > >       case "${flag}" in
> > > >               v) verbose=1;;
> > > >               d) debug=1;;
> > > > -             i) ETH=${OPTARG};;
> > > > +             i) XSKTEST_ETH=${OPTARG};;
> > > >               m) XSKTEST_MODE=${OPTARG};;
> > > >               l) list=1;;
> > > >               t) XSKTEST_TEST=${OPTARG};;
> > > > @@ -157,9 +155,9 @@ if [[ $help -eq 1 ]]; then
> > > >          exit
> > > >  fi
> > > >
> > > > -if [ ! -z $ETH ]; then
> > > > -     VETH0=${ETH}
> > > > -     VETH1=${ETH}
> > > > +if [ -n "$XSKTEST_ETH" ]; then
> > >
> > > Sorry - is point of this patch is just to invert the logic and rename the
> > > env var?
> >
> > The purpose was to make it setable from the outside and give it a name
> > that is more descriptive and targeted only to xskxceiver.
>
> and this is accomplished by not having ETH initialized here? What will be
> 'the outside' ?
>
> Currently I don't see much value within this patch, unless you explain the
> need for setting this from outside of this script. Maybe I missed some

Outside = from the command line or your environment variables. ETH is
not a good name to have set in your environment variable. Who knows
what it would be used for. with XSKTEST_* at least you know it is used
for the xsk testing.

> discussion from v1. I can live with this variable being ETH, what's more
> concerning/confusing to me is that for ZC we have to set VETH0 and VETH1
> to ETH and then use that later on.

That is in the old code. Nothing I am trying to address here.

If this patch is useful or not, I do not know to be honest. Just added
it because Przemyslaw suggested that we make the mode setable from
your env variables in your shell. Weird to have the mode setable but
not the ethernet interface. So either we drop both and make it
impossible to use env variables, or support setting both from the env
variables. Do not have a strong opinion either way. Maybe this is just
an unnecessary "feature" that will not be used.

>
> >
> > > > +     VETH0=${XSKTEST_ETH}
> > > > +     VETH1=${XSKTEST_ETH}
> > > >  else
> > > >       validate_root_exec
> > > >       validate_veth_support ${VETH0}
> > > > @@ -203,10 +201,10 @@ fi
> > > >
> > > >  exec_xskxceiver
> > > >
> > > > -if [ -z $ETH ]; then
> > > > +if [ -z $XSKTEST_ETH ]; then
> > > >       cleanup_exit ${VETH0} ${VETH1}
> > > >  else
> > > > -     cleanup_iface ${ETH} ${MTU}
> > > > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> > > >  fi
> > > >
> > > >  if [[ $list -eq 1 ]]; then
> > > > @@ -216,17 +214,17 @@ fi
> > > >  TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
> > > >  busy_poll=1
> > > >
> > > > -if [ -z $ETH ]; then
> > > > +if [ -z $XSKTEST_ETH ]; then
> > > >       setup_vethPairs
> > > >  fi
> > > >  exec_xskxceiver
> > > >
> > > >  ## END TESTS
> > > >
> > > > -if [ -z $ETH ]; then
> > > > +if [ -z $XSKTEST_ETH ]; then
> > > >       cleanup_exit ${VETH0} ${VETH1}
> > > >  else
> > > > -     cleanup_iface ${ETH} ${MTU}
> > > > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> > > >  fi
> > > >
> > > >  failures=0
> > > > --
> > > > 2.34.1
> > > >
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 9ec718043c1a..3e0a2302a185 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -88,14 +88,12 @@ 
 
 . xsk_prereqs.sh
 
-ETH=""
-
 while getopts "vi:dm:lt:h" flag
 do
 	case "${flag}" in
 		v) verbose=1;;
 		d) debug=1;;
-		i) ETH=${OPTARG};;
+		i) XSKTEST_ETH=${OPTARG};;
 		m) XSKTEST_MODE=${OPTARG};;
 		l) list=1;;
 		t) XSKTEST_TEST=${OPTARG};;
@@ -157,9 +155,9 @@  if [[ $help -eq 1 ]]; then
         exit
 fi
 
-if [ ! -z $ETH ]; then
-	VETH0=${ETH}
-	VETH1=${ETH}
+if [ -n "$XSKTEST_ETH" ]; then
+	VETH0=${XSKTEST_ETH}
+	VETH1=${XSKTEST_ETH}
 else
 	validate_root_exec
 	validate_veth_support ${VETH0}
@@ -203,10 +201,10 @@  fi
 
 exec_xskxceiver
 
-if [ -z $ETH ]; then
+if [ -z $XSKTEST_ETH ]; then
 	cleanup_exit ${VETH0} ${VETH1}
 else
-	cleanup_iface ${ETH} ${MTU}
+	cleanup_iface ${XSKTEST_ETH} ${MTU}
 fi
 
 if [[ $list -eq 1 ]]; then
@@ -216,17 +214,17 @@  fi
 TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
 busy_poll=1
 
-if [ -z $ETH ]; then
+if [ -z $XSKTEST_ETH ]; then
 	setup_vethPairs
 fi
 exec_xskxceiver
 
 ## END TESTS
 
-if [ -z $ETH ]; then
+if [ -z $XSKTEST_ETH ]; then
 	cleanup_exit ${VETH0} ${VETH1}
 else
-	cleanup_iface ${ETH} ${MTU}
+	cleanup_iface ${XSKTEST_ETH} ${MTU}
 fi
 
 failures=0