[PATCH 01/08] NTB: ntb_test: Safely use paths with whitespace

11 views
Skip to first unread message

Serge Semin

unread,
Nov 30, 2017, 4:43:11 PM11/30/17
to jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, log...@deltatee.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org, Serge Semin
If some of variables like LOC/REM or LOCAL_*/REMOTE_* got
whitespaces, the script may fail with syntax error.

Signed-off-by: Serge Semin <fancer...@gmail.com>
---
tools/testing/selftests/ntb/ntb_test.sh | 62 ++++++++++++++++-----------------
1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
index 5fc7ad359e21..a8647ad891eb 100755
--- a/tools/testing/selftests/ntb/ntb_test.sh
+++ b/tools/testing/selftests/ntb/ntb_test.sh
@@ -87,7 +87,7 @@ set -e

function _modprobe()
{
- modprobe "$@"
+ modprobe "$@"

if [[ "$REMOTE_HOST" != "" ]]; then
ssh "$REMOTE_HOST" modprobe "$@"
@@ -274,13 +274,13 @@ function pingpong_test()

echo "Running ping pong tests on: $(basename $LOC) / $(basename $REM)"

- LOC_START=$(read_file $LOC/count)
- REM_START=$(read_file $REM/count)
+ LOC_START=$(read_file "$LOC/count")
+ REM_START=$(read_file "$REM/count")

sleep 7

- LOC_END=$(read_file $LOC/count)
- REM_END=$(read_file $REM/count)
+ LOC_END=$(read_file "$LOC/count")
+ REM_END=$(read_file "$REM/count")

if [[ $LOC_START == $LOC_END ]] || [[ $REM_START == $REM_END ]]; then
echo "Ping pong counter not incrementing!" >&2
@@ -304,15 +304,15 @@ function perf_test()
max_mw_size=$MAX_MW_SIZE use_dma=$USE_DMA

echo "Running local perf test $WITH DMA"
- write_file "" $LOCAL_PERF/run
+ write_file "" "$LOCAL_PERF/run"
echo -n " "
- read_file $LOCAL_PERF/run
+ read_file "$LOCAL_PERF/run"
echo " Passed"

echo "Running remote perf test $WITH DMA"
- write_file "" $REMOTE_PERF/run
+ write_file "" "$REMOTE_PERF/run"
echo -n " "
- read_file $REMOTE_PERF/run
+ read_file "$REMOTE_PERF/run"
echo " Passed"

_modprobe -r ntb_perf
@@ -320,39 +320,39 @@ function perf_test()

function ntb_tool_tests()
{
- LOCAL_TOOL=$DEBUGFS/ntb_tool/$LOCAL_DEV
- REMOTE_TOOL=$REMOTE_HOST:$DEBUGFS/ntb_tool/$REMOTE_DEV
+ LOCAL_TOOL="$DEBUGFS/ntb_tool/$LOCAL_DEV"
+ REMOTE_TOOL="$REMOTE_HOST:$DEBUGFS/ntb_tool/$REMOTE_DEV"

echo "Starting ntb_tool tests..."

_modprobe ntb_tool

- write_file Y $LOCAL_TOOL/link_event
- write_file Y $REMOTE_TOOL/link_event
+ write_file "Y" "$LOCAL_TOOL/link_event"
+ write_file "Y" "$REMOTE_TOOL/link_event"

- link_test $LOCAL_TOOL $REMOTE_TOOL
- link_test $REMOTE_TOOL $LOCAL_TOOL
+ link_test "$LOCAL_TOOL" "$REMOTE_TOOL"
+ link_test "$REMOTE_TOOL" "$LOCAL_TOOL"

#Ensure the link is up on both sides before continuing
- write_file Y $LOCAL_TOOL/link_event
- write_file Y $REMOTE_TOOL/link_event
+ write_file "Y" "$LOCAL_TOOL/link_event"
+ write_file "Y" "$REMOTE_TOOL/link_event"

- for PEER_TRANS in $(ls $LOCAL_TOOL/peer_trans*); do
+ for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
PT=$(basename $PEER_TRANS)
- write_file $MW_SIZE $LOCAL_TOOL/$PT
- write_file $MW_SIZE $REMOTE_TOOL/$PT
+ write_file $MW_SIZE "$LOCAL_TOOL/$PT"
+ write_file $MW_SIZE "$REMOTE_TOOL/$PT"
done

- doorbell_test $LOCAL_TOOL $REMOTE_TOOL
- doorbell_test $REMOTE_TOOL $LOCAL_TOOL
- scratchpad_test $LOCAL_TOOL $REMOTE_TOOL
- scratchpad_test $REMOTE_TOOL $LOCAL_TOOL
+ doorbell_test "$LOCAL_TOOL" "$REMOTE_TOOL"
+ doorbell_test "$REMOTE_TOOL" "$LOCAL_TOOL"
+ scratchpad_test "$LOCAL_TOOL" "$REMOTE_TOOL"
+ scratchpad_test "$REMOTE_TOOL" "$LOCAL_TOOL"

- for MW in $(ls $LOCAL_TOOL/mw*); do
+ for MW in $(ls "$LOCAL_TOOL"/mw*); do
MW=$(basename $MW)

- mw_test $MW $LOCAL_TOOL $REMOTE_TOOL
- mw_test $MW $REMOTE_TOOL $LOCAL_TOOL
+ mw_test $MW "$LOCAL_TOOL" "$REMOTE_TOOL"
+ mw_test $MW "$REMOTE_TOOL" "$LOCAL_TOOL"
done

_modprobe -r ntb_tool
@@ -360,8 +360,8 @@ function ntb_tool_tests()

function ntb_pingpong_tests()
{
- LOCAL_PP=$DEBUGFS/ntb_pingpong/$LOCAL_DEV
- REMOTE_PP=$REMOTE_HOST:$DEBUGFS/ntb_pingpong/$REMOTE_DEV
+ LOCAL_PP="$DEBUGFS/ntb_pingpong/$LOCAL_DEV"
+ REMOTE_PP="$REMOTE_HOST:$DEBUGFS/ntb_pingpong/$REMOTE_DEV"

echo "Starting ntb_pingpong tests..."

@@ -374,8 +374,8 @@ function ntb_pingpong_tests()

function ntb_perf_tests()
{
- LOCAL_PERF=$DEBUGFS/ntb_perf/$LOCAL_DEV
- REMOTE_PERF=$REMOTE_HOST:$DEBUGFS/ntb_perf/$REMOTE_DEV
+ LOCAL_PERF="$DEBUGFS/ntb_perf/$LOCAL_DEV"
+ REMOTE_PERF="$REMOTE_HOST:$DEBUGFS/ntb_perf/$REMOTE_DEV"

echo "Starting ntb_perf tests..."

--
2.12.0

Serge Semin

unread,
Nov 30, 2017, 4:43:14 PM11/30/17
to jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, log...@deltatee.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org, Serge Semin
Multi-port interface is now available in ntb_tool driver. According
to the new NTB API, there might be more than two devices connected over
NTB. It means each device can have multiple freely enumerated ports.
Each port got index assigned by NTB hardware driver. This test is
performed to determine the local and peer ports as well as their indexes.

Signed-off-by: Serge Semin <fancer...@gmail.com>
---
tools/testing/selftests/ntb/ntb_test.sh | 52 +++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)

diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
index a8647ad891eb..541ba70ad640 100755
--- a/tools/testing/selftests/ntb/ntb_test.sh
+++ b/tools/testing/selftests/ntb/ntb_test.sh
@@ -127,6 +127,56 @@ function write_file()
fi
}

+function check_file()
+{
+ split_remote $1
+
+ if [[ "$REMOTE" != "" ]]; then
+ ssh "$REMOTE" "[[ -e ${VPATH} ]]"
+ else
+ [[ -e ${VPATH} ]]
+ fi
+}
+
+function find_pidx()
+{
+ PORT=$1
+ PPATH=$2
+
+ for ((i = 0; i < 64; i++)); do
+ PEER_DIR="$PPATH/peer$i"
+
+ check_file ${PEER_DIR} || break
+
+ PEER_PORT=$(read_file "${PEER_DIR}/port")
+ if [[ ${PORT} -eq $PEER_PORT ]]; then
+ echo $i
+ return 0
+ fi
+ done
+
+ return 1
+}
+
+function port_test()
+{
+ LOC=$1
+ REM=$2
+
+ echo "Running port tests on: $(basename $LOC) / $(basename $REM)"
+
+ LOCAL_PORT=$(read_file "$LOC/port")
+ REMOTE_PORT=$(read_file "$REM/port")
+
+ LOCAL_PIDX=$(find_pidx ${REMOTE_PORT} "$LOC")
+ REMOTE_PIDX=$(find_pidx ${LOCAL_PORT} "$REM")
+
+ echo "Local port ${LOCAL_PORT} with index ${REMOTE_PIDX} on remote host"
+ echo "Peer port ${REMOTE_PORT} with index ${LOCAL_PIDX} on local host"
+
+ echo " Passed"
+}
+
function link_test()
{
LOC=$1
@@ -327,6 +377,8 @@ function ntb_tool_tests()

_modprobe ntb_tool

+ port_test "$LOCAL_TOOL" "$REMOTE_TOOL"
+
write_file "Y" "$LOCAL_TOOL/link_event"
write_file "Y" "$REMOTE_TOOL/link_event"

--
2.12.0

Serge Semin

unread,
Nov 30, 2017, 4:43:16 PM11/30/17
to jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, log...@deltatee.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org, Serge Semin
Link Up and Down methods are used to change NTB link settings on
local side only for multi-port devices. Link is considered up
only if both sides local and peer set it up. Intel/AMD hardware
acts a bit different by assigning the Primary and Secondary roles,
so Primary device only is able to change the link state. Such behaviour
should be reflected in the test code.

Signed-off-by: Serge Semin <fancer...@gmail.com>
---
tools/testing/selftests/ntb/ntb_test.sh | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
index 541ba70ad640..247458c6d8dc 100755
--- a/tools/testing/selftests/ntb/ntb_test.sh
+++ b/tools/testing/selftests/ntb/ntb_test.sh
@@ -138,6 +138,11 @@ function check_file()
fi
}

+function subdirname()
+{
+ echo $(basename $(dirname $1)) 2> /dev/nulle
+}
+
function find_pidx()
{
PORT=$1
@@ -183,9 +188,9 @@ function link_test()
REM=$2
EXP=0

- echo "Running link tests on: $(basename $LOC) / $(basename $REM)"
+ echo "Running link tests on: $(subdirname $LOC) / $(subdirname $REM)"

- if ! write_file "N" "$LOC/link" 2> /dev/null; then
+ if ! write_file "N" "$LOC/../link" 2> /dev/null; then
echo " Unsupported"
return
fi
@@ -193,12 +198,11 @@ function link_test()
write_file "N" "$LOC/link_event"

if [[ $(read_file "$REM/link") != "N" ]]; then
- echo "Expected remote link to be down in $REM/link" >&2
+ echo "Expected link to be down in $REM/link" >&2
exit -1
fi

- write_file "Y" "$LOC/link"
- write_file "Y" "$LOC/link_event"
+ write_file "Y" "$LOC/../link"

echo " Passed"
}
@@ -379,15 +383,15 @@ function ntb_tool_tests()

port_test "$LOCAL_TOOL" "$REMOTE_TOOL"

- write_file "Y" "$LOCAL_TOOL/link_event"
- write_file "Y" "$REMOTE_TOOL/link_event"
+ LOCAL_PEER_TOOL="$LOCAL_TOOL/peer$LOCAL_PIDX"
+ REMOTE_PEER_TOOL="$REMOTE_TOOL/peer$REMOTE_PIDX"

- link_test "$LOCAL_TOOL" "$REMOTE_TOOL"
- link_test "$REMOTE_TOOL" "$LOCAL_TOOL"
+ link_test "$LOCAL_PEER_TOOL" "$REMOTE_PEER_TOOL"
+ link_test "$REMOTE_PEER_TOOL" "$LOCAL_PEER_TOOL"

#Ensure the link is up on both sides before continuing
- write_file "Y" "$LOCAL_TOOL/link_event"
- write_file "Y" "$REMOTE_TOOL/link_event"
+ write_file "Y" "$LOCAL_PEER_TOOL/link_event"
+ write_file "Y" "$REMOTE_PEER_TOOL/link_event"

for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
PT=$(basename $PEER_TRANS)
--
2.12.0

Serge Semin

unread,
Nov 30, 2017, 4:43:19 PM11/30/17
to jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, log...@deltatee.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org, Serge Semin
DB interface of ntb_tool driver hasn't been changed much, but
db_valid_mask DebugFS file has still been added. In this case
it's much better to test all valid DB bits instead of using
the predefined mask, which may be incorrect in general.

Signed-off-by: Serge Semin <fancer...@gmail.com>
---
tools/testing/selftests/ntb/ntb_test.sh | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
index 247458c6d8dc..3d43885ea4b5 100755
--- a/tools/testing/selftests/ntb/ntb_test.sh
+++ b/tools/testing/selftests/ntb/ntb_test.sh
@@ -18,7 +18,6 @@ LIST_DEVS=FALSE

DEBUGFS=${DEBUGFS-/sys/kernel/debug}

-DB_BITMASK=0x7FFF
PERF_RUN_ORDER=32
MAX_MW_SIZE=0
RUN_DMA_TESTS=
@@ -39,7 +38,6 @@ function show_help()
echo "be highly recommended."
echo
echo "Options:"
- echo " -b BITMASK doorbell clear bitmask for ntb_tool"
echo " -C don't cleanup ntb modules on exit"
echo " -d run dma tests"
echo " -h show this help message"
@@ -56,7 +54,6 @@ function parse_args()
OPTIND=0
while getopts "b:Cdhlm:r:p:w:" opt; do
case "$opt" in
- b) DB_BITMASK=${OPTARG} ;;
C) DONT_CLEANUP=1 ;;
d) RUN_DMA_TESTS=1 ;;
h) show_help; exit 0 ;;
@@ -215,21 +212,30 @@ function doorbell_test()

echo "Running db tests on: $(basename $LOC) / $(basename $REM)"

- write_file "c $DB_BITMASK" "$REM/db"
+ DB_VALID_MASK=$(read_file "$LOC/db_valid_mask")

- for ((i=1; i <= 8; i++)); do
- let DB=$(read_file "$REM/db") || true
- if [[ "$DB" != "$EXP" ]]; then
+ write_file "c $DB_VALID_MASK" "$REM/db"
+
+ for ((i = 0; i < 64; i++)); do
+ DB=$(read_file "$REM/db")
+ if [[ "$DB" -ne "$EXP" ]]; then
echo "Doorbell doesn't match expected value $EXP " \
"in $REM/db" >&2
exit -1
fi

- let "MASK=1 << ($i-1)" || true
- let "EXP=$EXP | $MASK" || true
+ let "MASK = (1 << $i) & $DB_VALID_MASK" || true
+ let "EXP = $EXP | $MASK" || true
+
write_file "s $MASK" "$LOC/peer_db"
done

+ write_file "c $DB_VALID_MASK" "$REM/db_mask"
+ write_file $DB_VALID_MASK "$REM/db_event"
+ write_file "s $DB_VALID_MASK" "$REM/db_mask"
+
+ write_file "c $DB_VALID_MASK" "$REM/db"
+
echo " Passed"
}

@@ -393,14 +399,15 @@ function ntb_tool_tests()
write_file "Y" "$LOCAL_PEER_TOOL/link_event"
write_file "Y" "$REMOTE_PEER_TOOL/link_event"

+ doorbell_test "$LOCAL_TOOL" "$REMOTE_TOOL"
+ doorbell_test "$REMOTE_TOOL" "$LOCAL_TOOL"
+
for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
PT=$(basename $PEER_TRANS)
write_file $MW_SIZE "$LOCAL_TOOL/$PT"
write_file $MW_SIZE "$REMOTE_TOOL/$PT"
done

- doorbell_test "$LOCAL_TOOL" "$REMOTE_TOOL"
- doorbell_test "$REMOTE_TOOL" "$LOCAL_TOOL"
scratchpad_test "$LOCAL_TOOL" "$REMOTE_TOOL"
scratchpad_test "$REMOTE_TOOL" "$LOCAL_TOOL"

--
2.12.0

Serge Semin

unread,
Nov 30, 2017, 4:43:21 PM11/30/17
to jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, log...@deltatee.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org, Serge Semin
Scratchpad NTB API has changed so has the ntb_tool driver. Outbound
Scratchpad DebugFS files have been moved to peer specific directories.
Each scratchpad is now available via separate file. The test code
has been accordingly altered.

Signed-off-by: Serge Semin <fancer...@gmail.com>
---
tools/testing/selftests/ntb/ntb_test.sh | 43 ++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
index 3d43885ea4b5..d37c974902e5 100755
--- a/tools/testing/selftests/ntb/ntb_test.sh
+++ b/tools/testing/selftests/ntb/ntb_test.sh
@@ -239,35 +239,44 @@ function doorbell_test()
echo " Passed"
}

-function read_spad()
+function get_files_count()
{
- VPATH=$1
- IDX=$2
+ NAME=$1
+ LOC=$2
+
+ split_remote $LOC

- ROW=($(read_file "$VPATH" | grep -e "^$IDX"))
- let VAL=${ROW[1]} || true
- echo $VAL
+ if [[ "$REMOTE" == "" ]]; then
+ echo $(ls -1 "$LOC"/${NAME}* 2>/dev/null | wc -l)
+ else
+ echo $(ssh "$REMOTE" "ls -1 \"$VPATH\"/${NAME}* | \
+ wc -l" 2> /dev/null)
+ fi
}

function scratchpad_test()
{
LOC=$1
REM=$2
- CNT=$(read_file "$LOC/spad" | wc -l)

- echo "Running spad tests on: $(basename $LOC) / $(basename $REM)"
+ echo "Running spad tests on: $(subdirname $LOC) / $(subdirname $REM)"
+
+ CNT=$(get_files_count "spad" "$LOC")
+
+ if [[ $CNT -eq 0 ]]; then
+ echo " Unsupported"
+ return
+ fi

for ((i = 0; i < $CNT; i++)); do
VAL=$RANDOM
- write_file "$i $VAL" "$LOC/peer_spad"
- RVAL=$(read_spad "$REM/spad" $i)
+ write_file "$VAL" "$LOC/spad$i"
+ RVAL=$(read_file "$REM/../spad$i")

- if [[ "$VAL" != "$RVAL" ]]; then
- echo "Scratchpad doesn't match expected value $VAL " \
- "in $REM/spad, got $RVAL" >&2
+ if [[ "$VAL" -ne "$RVAL" ]]; then
+ echo "Scratchpad $i value $RVAL doesn't match $VAL" >&2
exit -1
fi
-
done

echo " Passed"
@@ -402,15 +411,15 @@ function ntb_tool_tests()
doorbell_test "$LOCAL_TOOL" "$REMOTE_TOOL"
doorbell_test "$REMOTE_TOOL" "$LOCAL_TOOL"

+ scratchpad_test "$LOCAL_PEER_TOOL" "$REMOTE_PEER_TOOL"
+ scratchpad_test "$REMOTE_PEER_TOOL" "$LOCAL_PEER_TOOL"
+
for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
PT=$(basename $PEER_TRANS)
write_file $MW_SIZE "$LOCAL_TOOL/$PT"
write_file $MW_SIZE "$REMOTE_TOOL/$PT"
done

- scratchpad_test "$LOCAL_TOOL" "$REMOTE_TOOL"
- scratchpad_test "$REMOTE_TOOL" "$LOCAL_TOOL"
-
for MW in $(ls "$LOCAL_TOOL"/mw*); do
MW=$(basename $MW)

--
2.12.0

Serge Semin

unread,
Nov 30, 2017, 4:43:22 PM11/30/17
to jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, log...@deltatee.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org, Serge Semin
Messages NTB API is now available. ntb_tool driver has been altered
to perform messages send and receive operation. The test of messages
read/write to/from peer device has been added to the script.

Signed-off-by: Serge Semin <fancer...@gmail.com>
---
tools/testing/selftests/ntb/ntb_test.sh | 37 +++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)

diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
index d37c974902e5..36f6a444c0d9 100755
--- a/tools/testing/selftests/ntb/ntb_test.sh
+++ b/tools/testing/selftests/ntb/ntb_test.sh
@@ -282,6 +282,40 @@ function scratchpad_test()
echo " Passed"
}

+function message_test()
+{
+ LOC=$1
+ REM=$2
+
+ echo "Running msg tests on: $(subdirname $LOC) / $(subdirname $REM)"
+
+ CNT=$(get_files_count "msg" "$LOC")
+
+ if [[ $CNT -eq 0 ]]; then
+ echo " Unsupported"
+ return
+ fi
+
+ MSG_OUTBITS_MASK=$(read_file "$LOC/../msg_inbits")
+ MSG_INBITS_MASK=$(read_file "$REM/../msg_inbits")
+
+ write_file "c $MSG_OUTBITS_MASK" "$LOC/../msg_sts"
+ write_file "c $MSG_INBITS_MASK" "$REM/../msg_sts"
+
+ for ((i = 0; i < $CNT; i++)); do
+ VAL=$RANDOM
+ write_file "$VAL" "$LOC/msg$i"
+ RVAL=$(read_file "$REM/../msg$i")
+
+ if [[ "$VAL" -ne "${RVAL%%<-*}" ]]; then
+ echo "Message $i value $RVAL doesn't match $VAL" >&2
+ exit -1
+ fi
+ done
+
+ echo " Passed"
+}
+
function write_mw()
{
split_remote $2
@@ -414,6 +448,9 @@ function ntb_tool_tests()
scratchpad_test "$LOCAL_PEER_TOOL" "$REMOTE_PEER_TOOL"
scratchpad_test "$REMOTE_PEER_TOOL" "$LOCAL_PEER_TOOL"

+ message_test "$LOCAL_PEER_TOOL" "$REMOTE_PEER_TOOL"
+ message_test "$REMOTE_PEER_TOOL" "$LOCAL_PEER_TOOL"
+
for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
PT=$(basename $PEER_TRANS)
write_file $MW_SIZE "$LOCAL_TOOL/$PT"
--
2.12.0

Serge Semin

unread,
Nov 30, 2017, 4:43:24 PM11/30/17
to jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, log...@deltatee.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org, Serge Semin
There are devices (like IDT PCIe switches), which outbound MWs xlat address
is setup on peer side. In this case local side is supposed to allocate
a memory buffer and somehow deliver the xlat DMA address to peer so one
could set the outbound MW up. The MW test is altered so to support both
previous Intel/AMD and new IDT-like devices.

Signed-off-by: Serge Semin <fancer...@gmail.com>
---
tools/testing/selftests/ntb/ntb_test.sh | 89 +++++++++++++++++++++++++--------
1 file changed, 68 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
index 36f6a444c0d9..88c8ceb20607 100755
--- a/tools/testing/selftests/ntb/ntb_test.sh
+++ b/tools/testing/selftests/ntb/ntb_test.sh
@@ -43,7 +43,9 @@ function show_help()
echo " -h show this help message"
echo " -l list available local and remote PCI ids"
echo " -r REMOTE_HOST specify the remote's hostname to connect"
- echo " to for the test (using ssh)"
+ echo " to for the test (using ssh)"
+ echo " -m MW_SIZE memory window size for ntb_tool"
+ echo " (default: $MW_SIZE)"
echo " -p NUM ntb_perf run order (default: $PERF_RUN_ORDER)"
echo " -w max_mw_size maxmium memory window size"
echo
@@ -316,6 +318,32 @@ function message_test()
echo " Passed"
}

+function get_number()
+{
+ KEY=$1
+
+ sed -n "s/^\(${KEY}\)[ \t]*\(0x[0-9a-fA-F]*\)\(\[p\]\)\?$/\2/p"
+}
+
+function mw_alloc()
+{
+ IDX=$1
+ LOC=$2
+ REM=$3
+
+ write_file $MW_SIZE "$LOC/mw_trans$IDX"
+
+ INB_MW=$(read_file "$LOC/mw_trans$IDX")
+ MW_ALIGNED_SIZE=$(echo "$INB_MW" | get_number "Window Size")
+ MW_DMA_ADDR=$(echo "$INB_MW" | get_number "DMA Address")
+
+ write_file "$MW_DMA_ADDR:$(($MW_ALIGNED_SIZE))" "$REM/peer_mw_trans$IDX"
+
+ if [[ $MW_SIZE -ne $MW_ALIGNED_SIZE ]]; then
+ echo "MW $IDX size aligned to $MW_ALIGNED_SIZE"
+ fi
+}
+
function write_mw()
{
split_remote $2
@@ -328,17 +356,15 @@ function write_mw()
fi
}

-function mw_test()
+function mw_check()
{
IDX=$1
LOC=$2
REM=$3

- echo "Running $IDX tests on: $(basename $LOC) / $(basename $REM)"
-
- write_mw "$LOC/$IDX"
+ write_mw "$LOC/mw$IDX"

- split_remote "$LOC/$IDX"
+ split_remote "$LOC/mw$IDX"
if [[ "$REMOTE" == "" ]]; then
A=$VPATH
else
@@ -346,7 +372,7 @@ function mw_test()
ssh "$REMOTE" cat "$VPATH" > "$A"
fi

- split_remote "$REM/peer_$IDX"
+ split_remote "$REM/peer_mw$IDX"
if [[ "$REMOTE" == "" ]]; then
B=$VPATH
else
@@ -354,7 +380,7 @@ function mw_test()
ssh "$REMOTE" cat "$VPATH" > "$B"
fi

- cmp -n $MW_SIZE "$A" "$B"
+ cmp -n $MW_ALIGNED_SIZE "$A" "$B"
if [[ $? != 0 ]]; then
echo "Memory window $MW did not match!" >&2
fi
@@ -366,8 +392,39 @@ function mw_test()
if [[ "$B" == "/tmp/*" ]]; then
rm "$B"
fi
+}
+
+function mw_free()
+{
+ IDX=$1
+ LOC=$2
+ REM=$3
+
+ write_file "$MW_DMA_ADDR:0" "$REM/peer_mw_trans$IDX"
+
+ write_file 0 "$LOC/mw_trans$IDX"
+}
+
+function mw_test()
+{
+ LOC=$1
+ REM=$2
+
+ CNT=$(get_files_count "mw_trans" "$LOC")
+
+ for ((i = 0; i < $CNT; i++)); do
+ echo "Running mw$i tests on: $(subdirname $LOC) / " \
+ "$(subdirname $REM)"
+
+ mw_alloc $i $LOC $REM
+
+ mw_check $i $LOC $REM
+
+ mw_free $i $LOC $REM
+
+ echo " Passed"
+ done

- echo " Passed"
}

function pingpong_test()
@@ -451,18 +508,8 @@ function ntb_tool_tests()
message_test "$LOCAL_PEER_TOOL" "$REMOTE_PEER_TOOL"
message_test "$REMOTE_PEER_TOOL" "$LOCAL_PEER_TOOL"

- for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
- PT=$(basename $PEER_TRANS)
- write_file $MW_SIZE "$LOCAL_TOOL/$PT"
- write_file $MW_SIZE "$REMOTE_TOOL/$PT"
- done
-
- for MW in $(ls "$LOCAL_TOOL"/mw*); do
- MW=$(basename $MW)
-
- mw_test $MW "$LOCAL_TOOL" "$REMOTE_TOOL"
- mw_test $MW "$REMOTE_TOOL" "$LOCAL_TOOL"
- done
+ mw_test "$LOCAL_PEER_TOOL" "$REMOTE_PEER_TOOL"
+ mw_test "$REMOTE_PEER_TOOL" "$LOCAL_PEER_TOOL"

_modprobe -r ntb_tool
}
--
2.12.0

Serge Semin

unread,
Nov 30, 2017, 4:43:29 PM11/30/17
to jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, log...@deltatee.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org, Serge Semin
ntb_perf driver has been also updated so to have the multi-port
interface support. User now must specify what peer port is going
to be used to perform the test.

Signed-off-by: Serge Semin <fancer...@gmail.com>
---
tools/testing/selftests/ntb/ntb_test.sh | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
index 88c8ceb20607..b91497d1291f 100755
--- a/tools/testing/selftests/ntb/ntb_test.sh
+++ b/tools/testing/selftests/ntb/ntb_test.sh
@@ -39,15 +39,16 @@ function show_help()
echo
echo "Options:"
echo " -C don't cleanup ntb modules on exit"
- echo " -d run dma tests"
echo " -h show this help message"
echo " -l list available local and remote PCI ids"
echo " -r REMOTE_HOST specify the remote's hostname to connect"
echo " to for the test (using ssh)"
echo " -m MW_SIZE memory window size for ntb_tool"
echo " (default: $MW_SIZE)"
- echo " -p NUM ntb_perf run order (default: $PERF_RUN_ORDER)"
- echo " -w max_mw_size maxmium memory window size"
+ echo " -d run dma tests for ntb_perf"
+ echo " -p ORDER total data order for ntb_perf"
+ echo " (default: $PERF_RUN_ORDER)"
+ echo " -w MAX_MW_SIZE maxmium memory window size for ntb_perf"
echo
}

@@ -460,17 +461,17 @@ function perf_test()
WITH="without"
fi

- _modprobe ntb_perf run_order=$PERF_RUN_ORDER \
+ _modprobe ntb_perf total_order=$PERF_RUN_ORDER \
max_mw_size=$MAX_MW_SIZE use_dma=$USE_DMA

echo "Running local perf test $WITH DMA"
- write_file "" "$LOCAL_PERF/run"
+ write_file "$LOCAL_PIDX" "$LOCAL_PERF/run"
echo -n " "
read_file "$LOCAL_PERF/run"
echo " Passed"

echo "Running remote perf test $WITH DMA"
- write_file "" "$REMOTE_PERF/run"
+ write_file "$REMOTE_PIDX" "$REMOTE_PERF/run"
echo -n " "
read_file "$REMOTE_PERF/run"
echo " Passed"
--
2.12.0

Logan Gunthorpe

unread,
Nov 30, 2017, 5:04:39 PM11/30/17
to Serge Semin, jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org
Hey,

I took a cursory look at this series and it largely looks good to me.
Nice work. If I have time in the next couple days I'll do a more
thorough review. However, does the series not depend on changes to
ntb_tool? It would have been good to include a cover letter describing
the intention/plan.


On 30/11/17 02:42 PM, Serge Semin wrote:
> If some of variables like LOC/REM or LOCAL_*/REMOTE_* got
> whitespaces, the script may fail with syntax error.
>
> Signed-off-by: Serge Semin <fancer...@gmail.com>

Seeing this is a bug fix I Ack Patch 1 and suggest it get merged ahead
of the series.

Acked-by: Logan Gunthorpe <log...@deltatee.com>
Fixes: a9c59ef77458 ("ntb_test: Add a selftest script for the NTB
subsystem")

Thanks for the fix.

Logan

Serge Semin

unread,
Nov 30, 2017, 5:12:58 PM11/30/17
to Logan Gunthorpe, jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org
On Thu, Nov 30, 2017 at 03:04:29PM -0700, Logan Gunthorpe <log...@deltatee.com> wrote:

Hello

> Hey,
>
> I took a cursory look at this series and it largely looks good to me. Nice
> work. If I have time in the next couple days I'll do a more thorough review.
> However, does the series not depend on changes to ntb_tool? It would have
> been good to include a cover letter describing the intention/plan.
>

In general the intention of the patchset was to update the ntb_test.sh script
so one would be compatible with new NTB test drivers I've also sent. It was
asked of me by the NTB core maintainers. Sorry for not mentioning it in the
cover letter.

-Sergey

>
> On 30/11/17 02:42 PM, Serge Semin wrote:
> >If some of variables like LOC/REM or LOCAL_*/REMOTE_* got
> >whitespaces, the script may fail with syntax error.
> >
> >Signed-off-by: Serge Semin <fancer...@gmail.com>
>
> Seeing this is a bug fix I Ack Patch 1 and suggest it get merged ahead of
> the series.
>
> Acked-by: Logan Gunthorpe <log...@deltatee.com>
> Fixes: a9c59ef77458 ("ntb_test: Add a selftest script for the NTB
> subsystem")
>
> Thanks for the fix.
>
> Logan
>
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+...@googlegroups.com.
> To post to this group, send email to linu...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/923625e8-f071-c9f6-a3e2-c16480d4aea6%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

Logan Gunthorpe

unread,
Nov 30, 2017, 5:26:42 PM11/30/17
to Serge Semin, jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org


On 30/11/17 03:12 PM, Serge Semin wrote:
> In general the intention of the patchset was to update the ntb_test.sh script
> so one would be compatible with new NTB test drivers I've also sent. It was
> asked of me by the NTB core maintainers. Sorry for not mentioning it in the
> cover letter.

I understand that. But most importantly a series needs to say what it's
based off of and given than it clearly depends on the ntb_tool changes
(which I saw later) it was a bit confusing.

I think this should be in the same series as the ntb_tool changes as the
ntb_test changes depend on ntb_tool and we shouldn't merge the ntb_tool
changes without the ntb_test changes as it would break them.

Logan

Logan Gunthorpe

unread,
Nov 30, 2017, 5:37:16 PM11/30/17
to Serge Semin, jdm...@kudzu.us, dave....@intel.com, Allen...@emc.com, Shyam-su...@amd.com, Xiangl...@amd.com, Sergey...@t-platforms.ru, linu...@googlegroups.com, linux-...@vger.kernel.org

On 30/11/17 02:42 PM, Serge Semin wrote:
> +function find_pidx()
> +{
> + PORT=$1
> + PPATH=$2
> +
> + for ((i = 0; i < 64; i++)); do
> + PEER_DIR="$PPATH/peer$i"
> +
> + check_file ${PEER_DIR} || break
> +
> + PEER_PORT=$(read_file "${PEER_DIR}/port")
> + if [[ ${PORT} -eq $PEER_PORT ]]; then
> + echo $i
> + return 0
> + fi
> + done
> +
> + return 1
> +}

Actually, per my earlier comments on other messages. I think it would be
best if each of the patches in this series also included the relevant
changes to ntb_tool. Then just ditch the ntb_tool patch. For example,
this patch would include adding the "port" file to ntb_tool and the
relevant test to ntb_test.

Otherwise, when the ntb_tool patch is committed, the ntb_test breaks and
then is fixed in subsequent patches. In an ideal world, this would be
avoided in case we ever want to do a bisect involving ntb_test.

Logan
Reply all
Reply to author
Forward
0 new messages