[LTP] [PATCH 1/6] ftp/ftp01: Use tst_net.sh

[LTP] [PATCH 1/6] ftp/ftp01: Use tst_net.sh

[Akihiko Odaki]

This allows to use SSH rather than RSH.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 testcases/network/tcp_cmds/ftp/ftp01.sh | 64 +++++++------------------
 1 file changed, 17 insertions(+), 47 deletions(-)

diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
index 8d23abc62..a78d8adc0 100755
--- a/testcases/network/tcp_cmds/ftp/ftp01.sh
+++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
@@ -40,6 +40,11 @@
 #
 #----------------------------------------------------------------------
 
+TST_TESTFUNC=do_test
+TST_SETUP=do_setup
+TST_NEEDS_CMDS='awk ftp'
+TST_NEEDS_TMPDIR=1
+
 #-----------------------------------------------------------------------
 #
 # FUNCTION:  do_setup
@@ -50,25 +55,11 @@ do_setup()
 {
 
     TC=ftp
-    TCtmp=${TCtmp:-$LTPROOT/$TC${EXEC_SUFFIX}$$}
-    TCdat=${TCdat:-$LTPROOT/datafiles}
     SLEEPTIME=${SLEEPTIME:-0}
     ASCII_FILES=${ASCII_FILES:-"ascii.sm ascii.med ascii.lg ascii.jmb"}
     BIN_FILES=${BIN_FILES:-"bin.sm bin.med bin.lg bin.jmb"}
 
-    RHOST=${RHOST:-`hostname`}
     RUSER=${RUSER:-root}
-    PASSWD=${PASSWD:-.pasroot}
-
-    tst_setup
-
-    exists awk ftp rsh
-
-    cd "$TCtmp"
-
-    rsh -n -l root $RHOST mkdir -p "$TCtmp"
-    rsh -n -l root $RHOST chown -R ${RUSER} "$TCtmp"
-    [ $? = 0 ] || end_testcase "Check .rhosts file on remote machine."
 
 }
 
@@ -95,53 +86,35 @@ do_test()
                 if [ $a = "get" ]; then
                     {
                         echo user $RUSER $PASSWD
-                        echo lcd $TCtmp
                         echo $i
-                        echo cd $TCdat
+                        echo cd $TST_NET_DATAROOT
                         echo $a $j
                         echo quit
                     } | ftp -nv $RHOST
-                    SUM1=`ls -l $TCtmp/$j  | awk '{print $5}'`
-                    SUM2=`ls -l $TCdat/$j | awk '{print $5}'`
-                    rm -f $TCtmp/$j
+                    SUM1=`ls -l $j  | awk '{print $5}'`
+                    SUM2=`ls -l $TST_NET_DATAROOT/$j | awk '{print $5}'`
+                    rm -f $j
                 else
                     {
                         echo user $RUSER $PASSWD
-                        echo lcd $TCdat
+                        echo lcd $TST_NET_DATAROOT
                         echo $i
-                        echo cd $TCtmp
+                        echo cd $TST_TMPDIR
                         echo $a $j
                         echo quit
                     } | ftp -nv $RHOST
-                    SUM1=`rsh -n -l root $RHOST sum $TCtmp/$j | awk '{print $1}'`
-                    SUM2=`sum $TCdat/$j | awk '{print $1}'`
-                    rsh -n -l root $RHOST rm -f $TCtmp/$j
+                    SUM1=`tst_rhost_run -c "sum $TST_TMPDIR/$j" -s | awk '{print $1}'`
+                    SUM2=`sum $TST_NET_DATAROOT/$j | awk '{print $1}'`
+                    tst_rhost_run -c "rm -f $TST_TMPDIR/$j"
                 fi
 
-                if [ $SUM1 = $SUM2 ]; then
-                    tst_resm TINFO "Test Successful doing ftp $a $j $i"
-                else
-                    end_testcase "Test Fail: Wrong sum while performing ftp $a $j $i"
-                fi
+                EXPECT_PASS "[ '$SUM1' = '$SUM2' ]"
                 sleep $SLEEPTIME
             done
         done
     done
 }
 
-
-#-----------------------------------------------------------------------
-#
-# FUNCTION:  do_cleanup
-#
-#-----------------------------------------------------------------------
-
-do_cleanup()
-{
-    rsh -n -l root $RHOST rmdir "$TCtmp"
-    tst_cleanup
-}
-
 #----------------------------------------------------------------------
 # FUNCTION: MAIN
 # PURPOSE:  To invoke the functions to perform the tasks described in
@@ -150,9 +123,6 @@ do_cleanup()
 # OUTPUT:   A testcase run log with the results of the execution of this
 #           test.
 #----------------------------------------------------------------------
-. net_cmdlib.sh
+. tst_net.sh
 
-read_opts $*
-do_setup
-do_test
-end_testcase
+tst_run
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/6] ftp/ftp01: Remove verbose comments

[Akihiko Odaki]

Suggested-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 testcases/network/tcp_cmds/ftp/ftp01.sh | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
index a78d8adc0..2d61377b3 100755
--- a/testcases/network/tcp_cmds/ftp/ftp01.sh
+++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
@@ -45,12 +45,6 @@ TST_SETUP=do_setup
 TST_NEEDS_CMDS='awk ftp'
 TST_NEEDS_TMPDIR=1
 
-#-----------------------------------------------------------------------
-#
-# FUNCTION:  do_setup
-#
-#-----------------------------------------------------------------------
-
 do_setup()
 {
 
@@ -63,12 +57,6 @@ do_setup()
 
 }
 
-#-----------------------------------------------------------------------
-#
-# FUNCTION:  do_test
-#
-#-----------------------------------------------------------------------
-
 do_test()
 {
 
@@ -115,14 +103,6 @@ do_test()
     done
 }
 
-#----------------------------------------------------------------------
-# FUNCTION: MAIN
-# PURPOSE:  To invoke the functions to perform the tasks described in
-#           the prologue.
-# INPUT:    None.
-# OUTPUT:   A testcase run log with the results of the execution of this
-#           test.
-#----------------------------------------------------------------------
 . tst_net.sh
 
 tst_run
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/6] ftp/ftp01: Remove old-style command substitution

[Akihiko Odaki]

Suggested-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 testcases/network/tcp_cmds/ftp/ftp01.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
index 2d61377b3..f8a3c98e8 100755
--- a/testcases/network/tcp_cmds/ftp/ftp01.sh
+++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
@@ -79,8 +79,8 @@ do_test()
                         echo $a $j
                         echo quit
                     } | ftp -nv $RHOST
-                    SUM1=`ls -l $j  | awk '{print $5}'`
-                    SUM2=`ls -l $TST_NET_DATAROOT/$j | awk '{print $5}'`
+                    SUM1="$(ls -l $j  | awk '{print $5}')"
+                    SUM2="$(ls -l $TST_NET_DATAROOT/$j | awk '{print $5}')"
                     rm -f $j
                 else
                     {
@@ -91,8 +91,8 @@ do_test()
                         echo $a $j
                         echo quit
                     } | ftp -nv $RHOST
-                    SUM1=`tst_rhost_run -c "sum $TST_TMPDIR/$j" -s | awk '{print $1}'`
-                    SUM2=`sum $TST_NET_DATAROOT/$j | awk '{print $1}'`
+                    SUM1="$(tst_rhost_run -c "sum $TST_TMPDIR/$j" -s | awk '{print $1}')"
+                    SUM2="$(sum $TST_NET_DATAROOT/$j | awk '{print $1}')"
                     tst_rhost_run -c "rm -f $TST_TMPDIR/$j"
                 fi
 
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 4/6] ftp/ftp01: Remove sleep option

[Akihiko Odaki]

Suggested-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 testcases/network/tcp_cmds/ftp/ftp01.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
index f8a3c98e8..10b35c384 100755
--- a/testcases/network/tcp_cmds/ftp/ftp01.sh
+++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
@@ -49,7 +49,6 @@ do_setup()
 {
 
     TC=ftp
-    SLEEPTIME=${SLEEPTIME:-0}
     ASCII_FILES=${ASCII_FILES:-"ascii.sm ascii.med ascii.lg ascii.jmb"}
     BIN_FILES=${BIN_FILES:-"bin.sm bin.med bin.lg bin.jmb"}
 
@@ -97,7 +96,6 @@ do_test()
                 fi
 
                 EXPECT_PASS "[ '$SUM1' = '$SUM2' ]"
-                sleep $SLEEPTIME
             done
         done
     done
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 5/6] ftp/ftp01: Make variables local

[Akihiko Odaki]

Suggested-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 testcases/network/tcp_cmds/ftp/ftp01.sh | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
index 10b35c384..8c0cffdea 100755
--- a/testcases/network/tcp_cmds/ftp/ftp01.sh
+++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
@@ -58,6 +58,7 @@ do_setup()
 
 do_test()
 {
+    local sum1 sum2
 
     for i in binary ascii; do
 
@@ -78,8 +79,8 @@ do_test()
                         echo $a $j
                         echo quit
                     } | ftp -nv $RHOST
-                    SUM1="$(ls -l $j  | awk '{print $5}')"
-                    SUM2="$(ls -l $TST_NET_DATAROOT/$j | awk '{print $5}')"
+                    sum1="$(ls -l $j  | awk '{print $5}')"
+                    sum2="$(ls -l $TST_NET_DATAROOT/$j | awk '{print $5}')"
                     rm -f $j
                 else
                     {
@@ -90,12 +91,12 @@ do_test()
                         echo $a $j
                         echo quit
                     } | ftp -nv $RHOST
-                    SUM1="$(tst_rhost_run -c "sum $TST_TMPDIR/$j" -s | awk '{print $1}')"
-                    SUM2="$(sum $TST_NET_DATAROOT/$j | awk '{print $1}')"
+                    sum1="$(tst_rhost_run -c "sum $TST_TMPDIR/$j" -s | awk '{print $1}')"
+                    sum2="$(sum $TST_NET_DATAROOT/$j | awk '{print $1}')"
                     tst_rhost_run -c "rm -f $TST_TMPDIR/$j"
                 fi
 
-                EXPECT_PASS "[ '$SUM1' = '$SUM2' ]"
+                EXPECT_PASS "[ '$sum1' = '$sum2' ]"
             done
         done
     done
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 6/6] ftp/ftp01: Split the test function

[Akihiko Odaki]

Suggested-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 testcases/network/tcp_cmds/ftp/ftp01.sh | 88 ++++++++++++++-----------
 1 file changed, 49 insertions(+), 39 deletions(-)

diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
index 8c0cffdea..8bb659d54 100755
--- a/testcases/network/tcp_cmds/ftp/ftp01.sh
+++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
@@ -49,56 +49,66 @@ do_setup()
 {
 
     TC=ftp
-    ASCII_FILES=${ASCII_FILES:-"ascii.sm ascii.med ascii.lg ascii.jmb"}
-    BIN_FILES=${BIN_FILES:-"bin.sm bin.med bin.lg bin.jmb"}
-
     RUSER=${RUSER:-root}
 
 }
 
 do_test()
+{
+    case $1 in
+    1) test_get binary;;
+    2) test_get ascii;;
+    3) test_put binary;;
+    4) test_put ascii;;
+    esac
+}
+
+list_files()
+{
+    case $1 in
+    binary) echo 'ascii.sm ascii.med ascii.lg ascii.jmb';;
+    ascii) echo 'bin.sm bin.med bin.lg bin.jmb';;
+    esac
+}
+
+test_get()
 {
     local sum1 sum2
 
-    for i in binary ascii; do
+    for file in $(list_files $1); do
+        {
+            echo user $RUSER $PASSWD
+            echo $1
+            echo cd $TST_NET_DATAROOT
+            echo get $file
+            echo quit
+        } | ftp -nv $RHOST
+
+        sum1="$(ls -l $file  | awk '{print $5}')"
+        sum2="$(ls -l $TST_NET_DATAROOT/$file | awk '{print $5}')"
+        rm -f $file
+        EXPECT_PASS "[ '$sum1' = '$sum2' ]"
+    done
+}
 
-        if [ $i = "binary" ]; then
-            FILES=$BIN_FILES
-        fi
-        if [ $i = "ascii" ]; then
-            FILES=$ASCII_FILES
-        fi
-        for j in $FILES; do
+test_put()
+{
+    local sum1 sum2
 
-            for a in get put; do
-                if [ $a = "get" ]; then
-                    {
-                        echo user $RUSER $PASSWD
-                        echo $i
-                        echo cd $TST_NET_DATAROOT
-                        echo $a $j
-                        echo quit
-                    } | ftp -nv $RHOST
-                    sum1="$(ls -l $j  | awk '{print $5}')"
-                    sum2="$(ls -l $TST_NET_DATAROOT/$j | awk '{print $5}')"
-                    rm -f $j
-                else
-                    {
-                        echo user $RUSER $PASSWD
-                        echo lcd $TST_NET_DATAROOT
-                        echo $i
-                        echo cd $TST_TMPDIR
-                        echo $a $j
-                        echo quit
-                    } | ftp -nv $RHOST
-                    sum1="$(tst_rhost_run -c "sum $TST_TMPDIR/$j" -s | awk '{print $1}')"
-                    sum2="$(sum $TST_NET_DATAROOT/$j | awk '{print $1}')"
-                    tst_rhost_run -c "rm -f $TST_TMPDIR/$j"
-                fi
+    for file in $(list_files $1); do
+        {
+            echo user $RUSER $PASSWD
+            echo lcd $TST_NET_DATAROOT
+            echo $1
+            echo cd $TST_TMPDIR
+            echo post $file
+            echo quit
+        } | ftp -nv $RHOST
 
-                EXPECT_PASS "[ '$sum1' = '$sum2' ]"
-            done
-        done
+        sum1="$(tst_rhost_run -c "sum $TST_TMPDIR/$file" -s | awk '{print $1}')"
+        sum2="$(sum $TST_NET_DATAROOT/$file | awk '{print $1}')"
+        tst_rhost_run -c "rm -f $TST_TMPDIR/$file"
+        EXPECT_PASS "[ '$sum1' = '$sum2' ]"
     done
 }
 
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/6] ftp/ftp01: Use tst_net.sh

[Petr Vorel]

Hi Akihiko,

we do this rewrite in a single commit. It does not make much sense to split it.
I'd squash it, if it were ready, but we're not there yet.
Also, you're supposed to replace GPL verbose text at the top, history etc.
with just:

#!/bin/sh
# SPDX-License-Identifier: GPL-2.0-or-later
# Copyright (c) 2022 Akihiko Odaki <akihiko.odaki@daynix.com>
# Copyright (c) 2003 Manoj Iyer <manjo@mail.utexas.edu>
# Copyright (c) 2001 Robbie Williamson <robbiew@us.ibm.com>

TST_TESTFUNC=do_test
TST_CNT=4
...
You forget TST_CNT=4, thus only single test is being run.

i.e. all the useless and now even outdated comments (even mentioning .rhosts, history, setup)
must go away.

The main problem is, that with improperly installed data files test happily
passes, because compares empty strings:

ftp01 1 TINFO: timeout per run is 0h 5m 0s
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.sm': No such file or directory
ls: cannot access '/opt/ltp/testcases/bin/datafiles/ascii.sm': No such file or directory
ftp01 1 TPASS: [ '' = '' ] passed as expected
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.med': No such file or directory
ls: cannot access '/opt/ltp/testcases/bin/datafiles/ascii.med': No such file or directory
ftp01 1 TPASS: [ '' = '' ] passed as expected
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.lg': No such file or directory
ls: cannot access '/opt/ltp/testcases/bin/datafiles/ascii.lg': No such file or directory
ftp01 1 TPASS: [ '' = '' ] passed as expected
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.jmb': No such file or directory
ls: cannot access '/opt/ltp/testcases/bin/datafiles/ascii.jmb': No such file or directory
ftp01 1 TPASS: [ '' = '' ] passed as expected

Summary:
passed   4
failed   0
broken   0
skipped  0
warnings 0

When installing it properly, it does not work:

ftp01 1 TINFO: timeout per run is 0h 5m 0s
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.sm': No such file or directory
ftp01 1 TFAIL: [ '' = '220' ] failed unexpectedly
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.med': No such file or directory
ftp01 1 TFAIL: [ '' = '4020' ] failed unexpectedly
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.lg': No such file or directory
ftp01 1 TFAIL: [ '' = '80020' ] failed unexpectedly
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.jmb': No such file or directory
ftp01 1 TFAIL: [ '' = '1600020' ] failed unexpectedly
ftp01 2 TINFO: AppArmor enabled, this may affect test results
ftp01 2 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
ftp01 2 TINFO: loaded AppArmor profiles: none

Summary:
passed   0
failed   4
broken   0
skipped  0
warnings 0

First, there should be some check for $sum1 or $sum2 not being empty,
and probably if the file exists (probably in the setup).

The reason is in your version of setup function:
do_setup()
{

    TC=ftp
    RUSER=${RUSER:-root}

}

Here you have
* empty lines (remove it)
* TC=ftp is useless (remove it)
* RUSER could be defined above the setup function
(more readable, no need to be in the setup function)

But somehow cd into $TST_NET_DATAROOT in ftp code does not work.
I've tried to copy it instead:

do_setup()
{
    local file

    for file in $ASCII_FILES $BINARY_FILES; do
        ROD cp -v $TST_NET_DATAROOT/$file .
    done
}

But that does not work. I need some time to figure out what's wrong
(not an FTP expert).

Other things to fix:

Also file variable in test_get() and test_put() should be also declared with
local.

I don't like list_files() at all, why not just define
at the top of the test and then use them as I suggested before?

ASCII_FILES='bin.sm bin.med bin.lg bin.jmb'
BINARY_FILES='ascii.sm ascii.med ascii.lg ascii.jmb'

do_test()
{
    case $1 in
    1) test_get $BIN_FILES;;
    2) test_get $ASCII_FILES;;
    3) test_put $BIN_FILES;;
    4) test_put $ASCII_FILES;;
    esac
}

Also, thinking about the tool which gets the size. Looking at
https://stackoverflow.com/questions/1815329/portable-way-to-get-file-size-in-bytes-in-the-shell
https://unix.stackexchange.com/questions/16640/how-can-i-get-the-size-of-a-file-in-a-bash-script
'ls -l | awk '{print $5}' might be really the best tool to get file size as both
ls and awk are probably everywhere. And although we wrote some C code to avoid
external dependencies, here I'd keep it, because other alternatives has
drawbacks:

'du -b' is fast, but might not be everywhere.

'stat -c %s' is reported not to be portable:
https://stackoverflow.com/questions/1815329/portable-way-to-get-file-size-in-bytes-in-the-shell#comment21604978_5258707

'wc -c' is reported to be slow and also might not be everywhere:
https://stackoverflow.com/questions/1815329/portable-way-to-get-file-size-in-bytes-in-the-shell#comment21604978_5258707

'find "$file" -printf "%s"' would also work, but might not be everywhere.

I'll try to fix the problem, but not sure when I get time to fix it.
I've added all fixes into my fork, feel free to use it if you have time to post
new version. But please, test it before whether it works.
https://github.com/pevik/ltp/commits/akihiko_odaki/ftp01.v2.fixes

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/6] ftp/ftp01: Use tst_net.sh

[Petr Vorel]

Hi Akihiko,

I'm sorry I overlooked your message at v1
https://lore.kernel.org/ltp/9894db50-6319-a818-c995-3ba9ab102c4b@daynix.com/

I guess script expects $PASSWD to be set.
Also trying to follow the instruction, none of them (allowing root in
/etc/ftpusers or filling /root/.netrc with machine localhost) work for me:
https://github.com/linux-test-project/ltp/tree/master/testcases/network#ftp-and-telnet-setup

I suppose any ftpd implementation must be run on the remote server (localhost,
if tst_net.sh uses netns - the default or on remote host, if ssh is used).
Otherwise ftp command "user $RUSER $PASSWD" cannot work, right?

Although it should behave the same regardless netns or ssh is used,
how do you test v3, with ssh or with netns (the default)? Or both?

It'd be nice if there was connection detection in the setup and tst_brk TCONF if
login does not work. The best would be to have configuration in the setup() +
cleanup in the cleanup()).

There are tests, which do vsftpd configuration: ftp-download-stress.sh,
ftp-upload-stress.sh.

BTW in the past we seriously considered to delete these highlevel smoke tests.
LTP is concentrated to test kernel API and internals, thus we didn't see much
value with smoke tests like this (that's why they haven't been rewritten to use
new API), specially if they require complex setup and get false positives when
SUT not configured properly. That's why it'd be nice to at least TCONF (if not
doing whole setup).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/6] ftp/ftp01: Use tst_net.sh

[Akihiko Odaki]

Hi Petr,

On 2022/10/21 15:22, Petr Vorel wrote:
> Hi Akihiko,
> 
> I'm sorry I overlooked your message at v1
> https://lore.kernel.org/ltp/9894db50-6319-a818-c995-3ba9ab102c4b@daynix.com/
> 
> I guess script expects $PASSWD to be set.
> Also trying to follow the instruction, none of them (allowing root in
> /etc/ftpusers or filling /root/.netrc with machine localhost) work for me:
> https://github.com/linux-test-project/ltp/tree/master/testcases/network#ftp-and-telnet-setup
> 
> I suppose any ftpd implementation must be run on the remote server (localhost,
> if tst_net.sh uses netns - the default or on remote host, if ssh is used).
> Otherwise ftp command "user $RUSER $PASSWD" cannot work, right?
> 
> Although it should behave the same regardless netns or ssh is used,
> how do you test v3, with ssh or with netns (the default)? Or both?

Thanks for sharing your fork. I tried it with SSH, and I had to make the 
following modifications to run it correctly:
- I had to restore commented out "cd $TST_NET_DATAROOT".
- I had to change the arguments of test_get and test_put back to 
"binary" and "ascii".
   It is used to set the transfer mode (search for "echo $1").

> 
> It'd be nice if there was connection detection in the setup and tst_brk TCONF if
> login does not work. The best would be to have configuration in the setup() +
> cleanup in the cleanup()).
> 
> There are tests, which do vsftpd configuration: ftp-download-stress.sh,
> ftp-upload-stress.sh.
> 
> BTW in the past we seriously considered to delete these highlevel smoke tests.
> LTP is concentrated to test kernel API and internals, thus we didn't see much
> value with smoke tests like this (that's why they haven't been rewritten to use
> new API), specially if they require complex setup and get false positives when
> SUT not configured properly. That's why it'd be nice to at least TCONF (if not
> doing whole setup).

I just modified this test because it is annoying to set up rsh just to 
fix this test so I would rather not put more effort for further 
improvement. Personally I don't object to remove this test either.

Regards,
Akihiko Odaki

> 
> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/6] ftp/ftp01: Use tst_net.sh

[Petr Vorel]

Hi Akihiko,

> Hi Petr,

> On 2022/10/21 15:22, Petr Vorel wrote:
> > Hi Akihiko,

> > I'm sorry I overlooked your message at v1
> > https://lore.kernel.org/ltp/9894db50-6319-a818-c995-3ba9ab102c4b@daynix.com/

> > I guess script expects $PASSWD to be set.
> > Also trying to follow the instruction, none of them (allowing root in
> > /etc/ftpusers or filling /root/.netrc with machine localhost) work for me:
> > https://github.com/linux-test-project/ltp/tree/master/testcases/network#ftp-and-telnet-setup

> > I suppose any ftpd implementation must be run on the remote server (localhost,
> > if tst_net.sh uses netns - the default or on remote host, if ssh is used).
> > Otherwise ftp command "user $RUSER $PASSWD" cannot work, right?

> > Although it should behave the same regardless netns or ssh is used,
> > how do you test v3, with ssh or with netns (the default)? Or both?

> Thanks for sharing your fork. I tried it with SSH, and I had to make the
> following modifications to run it correctly:
> - I had to restore commented out "cd $TST_NET_DATAROOT".
Ack, this is correct.
> - I had to change the arguments of test_get and test_put back to "binary"
> and "ascii".
>   It is used to set the transfer mode (search for "echo $1").
Ah, I'm sorry I overlooked this. This could be solved with passing it as a first
parameter and shift, but I'll stop to be picky on an implementation detail.

> > It'd be nice if there was connection detection in the setup and tst_brk TCONF if
> > login does not work. The best would be to have configuration in the setup() +
> > cleanup in the cleanup()).

> > There are tests, which do vsftpd configuration: ftp-download-stress.sh,
> > ftp-upload-stress.sh.

> > BTW in the past we seriously considered to delete these highlevel smoke tests.
> > LTP is concentrated to test kernel API and internals, thus we didn't see much
> > value with smoke tests like this (that's why they haven't been rewritten to use
> > new API), specially if they require complex setup and get false positives when
> > SUT not configured properly. That's why it'd be nice to at least TCONF (if not
> > doing whole setup).

> I just modified this test because it is annoying to set up rsh just to fix
> this test so I would rather not put more effort for further improvement.
Understand, ack. Thanks for your work!

> Personally I don't object to remove this test either.
The fastest solution is to merge your fixed version.
But there should be even more modifications:
RHOST is obsolete, instead $(tst_ipaddr rhost) should be used.

Therefore we should decide if this smoke test (and other FTP tests in LTP) is
worth effort. If yes, I should force myself to create ftp_lib.sh and migrate
everything to work similar way as ftp-upload-stress.sh.

Kind regards,
Petr

> Regards,
> Akihiko Odaki


> > Kind regards,
> > Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/6] ftp/ftp01: Use tst_net.sh

[Petr Vorel]

Hi Akihiko,

...
> > I just modified this test because it is annoying to set up rsh just to fix
> > this test so I would rather not put more effort for further improvement.
> Understand, ack. Thanks for your work!

> > Personally I don't object to remove this test either.
> The fastest solution is to merge your fixed version.
> But there should be even more modifications:
> RHOST is obsolete, instead $(tst_ipaddr rhost) should be used.

> Therefore we should decide if this smoke test (and other FTP tests in LTP) is
> worth effort. If yes, I should force myself to create ftp_lib.sh and migrate
> everything to work similar way as ftp-upload-stress.sh.

Merged as it's some improvement. I'm not sure if I invest time to FTP in the
future, maybe we should really delete it.

The only significant change I did was to force running over SSH:
with RHOST="${RHOST:-localhost}"

In my case only first half of the tests is working (suppose just wrong setup),
but on netns everything si broken and you also tested it on SSH I dared to do
this change. I documented the proper fix above in case anybody cares.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/6] ftp/ftp01: Use tst_net.sh

[Akihiko Odaki]

Hi,

Thanks for suggestions, improvements, and merging.

This test assumes FTP is set up and running and that defeats the purpose 
of netns. It is certainly possible to say the FTP test functionality is 
covered with testcases/network/stress/ftp and this test can be removed, 
but for now, the fixes allows me to execute net.tcp_cmds tests without 
explicitly excluding this.

Regards,
Akihiko Odaki

On 2022/10/24 19:18, Petr Vorel wrote:
> Hi Akihiko,
> 
> ...
>>> I just modified this test because it is annoying to set up rsh just to fix
>>> this test so I would rather not put more effort for further improvement.
>> Understand, ack. Thanks for your work!
> 
>>> Personally I don't object to remove this test either.
>> The fastest solution is to merge your fixed version.
>> But there should be even more modifications:
>> RHOST is obsolete, instead $(tst_ipaddr rhost) should be used.
> 
>> Therefore we should decide if this smoke test (and other FTP tests in LTP) is
>> worth effort. If yes, I should force myself to create ftp_lib.sh and migrate
>> everything to work similar way as ftp-upload-stress.sh.
> 
> Merged as it's some improvement. I'm not sure if I invest time to FTP in the
> future, maybe we should really delete it.
> 
> The only significant change I did was to force running over SSH:
> with RHOST="${RHOST:-localhost}"
> 
> In my case only first half of the tests is working (suppose just wrong setup),
> but on netns everything si broken and you also tested it on SSH I dared to do
> this change. I documented the proper fix above in case anybody cares.
> 
> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/6] ftp/ftp01: Use tst_net.sh

[Petr Vorel]

Hi Akihiko,

> Hi,

> Thanks for suggestions, improvements, and merging.

> This test assumes FTP is set up and running and that defeats the purpose of
> netns. It is certainly possible to say the FTP test functionality is covered
> with testcases/network/stress/ftp and this test can be removed, but for now,
> the fixes allows me to execute net.tcp_cmds tests without explicitly
> excluding this.

Thanks a lot for confirming that it works with my additional changes.

IMHO ftp can be tested via, that's actually done in ftp-upload-stress.sh.

Kind regards,
Petr

> Regards,
> Akihiko Odaki

> On 2022/10/24 19:18, Petr Vorel wrote:
> > Hi Akihiko,

> > ...
> > > > I just modified this test because it is annoying to set up rsh just to fix
> > > > this test so I would rather not put more effort for further improvement.
> > > Understand, ack. Thanks for your work!

> > > > Personally I don't object to remove this test either.
> > > The fastest solution is to merge your fixed version.
> > > But there should be even more modifications:
> > > RHOST is obsolete, instead $(tst_ipaddr rhost) should be used.

> > > Therefore we should decide if this smoke test (and other FTP tests in LTP) is
> > > worth effort. If yes, I should force myself to create ftp_lib.sh and migrate
> > > everything to work similar way as ftp-upload-stress.sh.

> > Merged as it's some improvement. I'm not sure if I invest time to FTP in the
> > future, maybe we should really delete it.

> > The only significant change I did was to force running over SSH:
> > with RHOST="${RHOST:-localhost}"

> > In my case only first half of the tests is working (suppose just wrong setup),
> > but on netns everything si broken and you also tested it on SSH I dared to do
> > this change. I documented the proper fix above in case anybody cares.

> > Kind regards,
> > Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp