| Message ID | 20180329233134.24151-1-pvorel@suse.cz |
|---|---|
| Headers | show |
| Series | Rewritting network tests into new shell API | expand |
On 03/30/2018 02:31 AM, Petr Vorel wrote: > Hi, > > changes v1->v2: > * Make new API default for test_net.sh > * Rename variable TST_USE_NEW_API => TST_USE_LEGACY_API > * Set TST_USE_LEGACY_API=1 in majority of network scripts. > Hi Petr, A few questions about the changes to the new API: I think, you wrote about renaming test_net.sh to tst_net.sh, and the corresponded test_net_stress.sh, so why we can't keep the copy to the old API during migration and make the new one with the new name? What is actually changing in the new API that require the changes in test_net.sh except tst_resm/tst_brkm renamed to tst_res/tst_brk? The new variables TST_NEEDS_ROOT and TST_SETUP, TST_TESTFUNC? Can we run a test without them temporary, without tst_run(), e.g. define some stubs and old API with the new one in tst_net.sh? tst_exit() { tst_do_exit() } etc. I remember that ROD function not using the script but tst_rod binary so may affect some scripts, like "ROD cd" for example... but may be it is already fixed, I need to check. And we need to remove tst_rmdir() because it will be handled in the library as we already have TST_TMPDIR_RHOST there. Best regards, Alexey > NOTE: still kept the old script names. > > I'm not happy with the state of testscripts/network.sh. I kept it using > the legacy API. I tried to migrate it into new API, but that would > require too much changes (and I don't like to have tst_run call in > testscripts/network.sh). > > Kind regards, > Petr > > Petr Vorel (3): > test_net.sh: Support both old and new shell APIs > network/interface: Cleanup if4-addr-change > net: Migrate test_net_stress.sh and it's dependencies to new shell API > > testcases/lib/test_net.sh | 133 +++++++++++++-------- > testcases/lib/tst_test.sh | 1 + > testcases/network/busy_poll/busy_poll01.sh | 1 + > testcases/network/busy_poll/busy_poll02.sh | 1 + > testcases/network/busy_poll/busy_poll03.sh | 1 + > testcases/network/dccp/dccp01.sh | 1 + > testcases/network/dctcp/dctcp01.sh | 1 + > testcases/network/dhcp/dhcpd_tests.sh | 1 + > testcases/network/dhcp/dnsmasq_tests.sh | 1 + > testcases/network/iproute/ip_tests.sh | 1 + > testcases/network/multicast/mc_cmds/mc_cmds | 1 + > testcases/network/multicast/mc_commo/mc_commo | 1 + > testcases/network/multicast/mc_member/mc_member | 1 + > testcases/network/multicast/mc_opts/mc_opts | 1 + > testcases/network/nfs/fsx-linux/fsx.sh | 1 + > testcases/network/nfs/nfs_stress/nfs01 | 1 + > testcases/network/nfs/nfs_stress/nfs02 | 1 + > testcases/network/nfs/nfs_stress/nfs03 | 1 + > testcases/network/nfs/nfs_stress/nfs04 | 1 + > testcases/network/nfs/nfs_stress/nfs05 | 1 + > testcases/network/nfs/nfs_stress/nfs06 | 1 + > testcases/network/nfs/nfslock01/nfslock01 | 1 + > testcases/network/nfs/nfsstat01/nfsstat01 | 1 + > testcases/network/rpc/basic_tests/rpc01/rpc01 | 1 + > .../network/rpc/basic_tests/rpcinfo/rpcinfo01 | 1 + > testcases/network/rpc/basic_tests/rup/rup01 | 1 + > testcases/network/rpc/basic_tests/rusers/rusers01 | 1 + > testcases/network/rpc/rpc-tirpc/rpc_test.sh | 1 + > testcases/network/sctp/sctp01.sh | 1 + > testcases/network/sockets/bind_noport01.sh | 1 + > .../network/stress/broken_ip/broken_ip4-checksum | 1 + > .../network/stress/broken_ip/broken_ip4-dstaddr | 1 + > .../network/stress/broken_ip/broken_ip4-fragment | 1 + > testcases/network/stress/broken_ip/broken_ip4-ihl | 1 + > .../network/stress/broken_ip/broken_ip4-protcol | 1 + > .../network/stress/broken_ip/broken_ip4-totlen | 1 + > .../network/stress/broken_ip/broken_ip4-version | 1 + > .../network/stress/broken_ip/broken_ip6-dstaddr | 1 + > .../network/stress/broken_ip/broken_ip6-nexthdr | 1 + > testcases/network/stress/broken_ip/broken_ip6-plen | 1 + > .../network/stress/broken_ip/broken_ip6-version | 1 + > testcases/network/stress/dns/dns-stress | 1 + > testcases/network/stress/ftp/ftp-download-stress | 1 + > testcases/network/stress/ftp/ftp-upload-stress | 1 + > testcases/network/stress/http/http-stress | 1 + > testcases/network/stress/interface/if-addr-adddel | 31 ++--- > .../network/stress/interface/if-addr-addlarge | 37 +++--- > testcases/network/stress/interface/if-mtu-change | 24 ++-- > testcases/network/stress/interface/if-route-adddel | 26 ++-- > .../network/stress/interface/if-route-addlarge | 28 ++--- > testcases/network/stress/interface/if-updown | 27 ++--- > testcases/network/stress/interface/if4-addr-change | 74 +++++++----- > testcases/network/stress/ipsec/ipsec_lib.sh | 1 + > .../grp-operation/mcast-group-multiple-socket | 16 +-- > .../multicast/grp-operation/mcast-group-same-group | 17 ++- > .../grp-operation/mcast-group-single-socket | 17 ++- > .../grp-operation/mcast-group-source-filter | 17 ++- > .../stress/multicast/grp-operation/mcast-lib.sh | 15 +-- > .../network/stress/ns-tools/test_net_stress.sh | 24 +++- > testcases/network/stress/ssh/ssh-stress | 1 + > testcases/network/tcp_cmds/arping/arping01.sh | 1 + > .../network/tcp_cmds/clockdiff/clockdiff01.sh | 1 + > testcases/network/tcp_cmds/ipneigh/ipneigh01.sh | 1 + > testcases/network/tcp_cmds/ping/ping01.sh | 1 + > testcases/network/tcp_cmds/ping/ping02.sh | 1 + > testcases/network/tcp_cmds/rlogin/rlogin01 | 1 + > testcases/network/tcp_cmds/sendfile/sendfile01 | 1 + > testcases/network/tcp_cmds/tcpdump/tcpdump01 | 1 + > testcases/network/tcp_cmds/telnet/telnet01 | 1 + > .../network/tcp_cmds/tracepath/tracepath01.sh | 1 + > testcases/network/tcp_fastopen/tcp_fastopen_run.sh | 1 + > testcases/network/traceroute/traceroute01.sh | 1 + > testcases/network/virt/geneve01.sh | 1 + > testcases/network/virt/gre01.sh | 1 + > testcases/network/virt/ipvlan01.sh | 1 + > testcases/network/virt/macvlan01.sh | 1 + > testcases/network/virt/macvtap01.sh | 1 + > testcases/network/virt/vlan01.sh | 1 + > testcases/network/virt/vlan02.sh | 1 + > testcases/network/virt/vlan03.sh | 1 + > testcases/network/virt/vxlan01.sh | 1 + > testcases/network/virt/vxlan02.sh | 1 + > testcases/network/virt/vxlan03.sh | 1 + > testcases/network/xinetd/xinetd_tests.sh | 1 + > testscripts/network.sh | 4 +- > 85 files changed, 340 insertions(+), 220 deletions(-) >
Hi Alexey, thanks for your comments! > Hi Petr, > A few questions about the changes to the new API: > I think, you wrote about renaming test_net.sh to tst_net.sh, and the > corresponded test_net_stress.sh, so why we can't keep the copy to the > old API during migration and make the new one with the new name? That's another option, but I thought, it'd be easier to have just one version of test_net.sh which supports both APIs, than maintain two files (test_net.sh and tst_net.sh). But I might be wrong. > What is actually changing in the new API that require the changes > in test_net.sh except tst_resm/tst_brkm renamed to tst_res/tst_brk? > The new variables TST_NEEDS_ROOT and TST_SETUP, TST_TESTFUNC? Yes (you list all or most of them in your reply below) + the need to call tst_run(). I did most of the changes with simple script using sed (manual fixes are still needed). tst_run() is the biggest change in the behavior, which complicates migrating of testscripts/network.sh. I haven't figured out, what would be an elegant way to move it and I'd prefer to migrate it into new version. Not sure whether we'd like to add part of "network_settings" execution to tst_run() for tst_net.sh and thus to be run twice (once by testscripts/network.sh and then by test which is called by testscripts/network.sh). Now I see the best would be move this "network_settings" stuff into new called function tst_net_run() which would set it and at the end call tst_run() + remove sourcing test_net.sh in testscripts/network.sh (only testing script would load it and call tst_net_run() at the end instead of tst_run()). > Can we run a test without them temporary, without tst_run(), > e.g. define some stubs and old API with the new one in > tst_net.sh? Do you mean to move all network scripts to new API, using these "shim" in tst_net.sh? I'd prefer tests were really using new API (to benefit from it), I just don't want to migrate all tests at once. This can be dangerous trying to implement new API functions in legacy one. That's why I implemented in tst_net.sh functions starting with underscore which choose the implementation via $TST_USE_LEGACY_API. I wanted to ony tst_net.sh itself using it. > tst_exit() > { > tst_do_exit() > } > etc. > I remember that ROD function not using the script but tst_rod binary > so may affect some scripts, like "ROD cd" for example... but may be it > is already fixed, I need to check. Git log shows it's not fixed. Good point. (s/ROD cd /cd /g) > And we need to remove tst_rmdir() because it will be handled in the > library as we already have TST_TMPDIR_RHOST there. Yes, I handled it in test_net.sh and during rewrite. > Best regards, > Alexey Kind regards, Petr
On 31.03.2018 02:59, Petr Vorel wrote: > Hi Alexey, > > thanks for your comments! > >> Hi Petr, > >> A few questions about the changes to the new API: > >> I think, you wrote about renaming test_net.sh to tst_net.sh, and the >> corresponded test_net_stress.sh, so why we can't keep the copy to the >> old API during migration and make the new one with the new name? > That's another option, but I thought, it'd be easier to have just one version of > test_net.sh which supports both APIs, than maintain two files (test_net.sh and > tst_net.sh). But I might be wrong. > >> What is actually changing in the new API that require the changes >> in test_net.sh except tst_resm/tst_brkm renamed to tst_res/tst_brk? > >> The new variables TST_NEEDS_ROOT and TST_SETUP, TST_TESTFUNC? > Yes (you list all or most of them in your reply below) + the need to call tst_run(). > > I did most of the changes with simple script using sed (manual fixes are still needed). > > tst_run() is the biggest change in the behavior, which complicates migrating of > testscripts/network.sh. I haven't figured out, what would be an elegant way to move it and > I'd prefer to migrate it into new version. Not sure whether we'd like to add part of > "network_settings" execution to tst_run() for tst_net.sh and thus to be run twice (once > by testscripts/network.sh and then by test which is called by testscripts/network.sh). > > Now I see the best would be move this "network_settings" stuff into new called function > tst_net_run() which would set it and at the end call tst_run() + remove sourcing > test_net.sh in testscripts/network.sh (only testing script would load it and call > tst_net_run() at the end instead of tst_run()). Hi Petr, I think we need to add a new flag, similar to LTP C library that can define TST_NO_DEFAULT_MAIN, i.e. we could add TST_NO_DEFAULT_RUN in the tst_test.sh. > >> Can we run a test without them temporary, without tst_run(), >> e.g. define some stubs and old API with the new one in >> tst_net.sh? > Do you mean to move all network scripts to new API, using these "shim" in tst_net.sh? > I'd prefer tests were really using new API (to benefit from it), I just don't want to > migrate all tests at once. This can be dangerous trying to implement new API functions in > legacy one.> That's why I implemented in tst_net.sh functions starting with underscore which choose the It's better to place underscore in the end of a function. > implementation via $TST_USE_LEGACY_API. I wanted to ony tst_net.sh itself using it. > Thanks, Alexey
Hi Alexey, again, thanks for your comments. ... > > tst_run() is the biggest change in the behavior, which complicates migrating of > > testscripts/network.sh. I haven't figured out, what would be an elegant way to move it and > > I'd prefer to migrate it into new version. Not sure whether we'd like to add part of > > "network_settings" execution to tst_run() for tst_net.sh and thus to be run twice (once > > by testscripts/network.sh and then by test which is called by testscripts/network.sh). > > Now I see the best would be move this "network_settings" stuff into new called function > > tst_net_run() which would set it and at the end call tst_run() + remove sourcing > > test_net.sh in testscripts/network.sh (only testing script would load it and call > > tst_net_run() at the end instead of tst_run()). > Hi Petr, > I think we need to add a new flag, similar to LTP C library that can > define TST_NO_DEFAULT_MAIN, i.e. we could add TST_NO_DEFAULT_RUN in > the tst_test.sh. I was thinking about it as well, while trying to implement it. ATM network.sh is for handling getopts and setting environment variables. I was even thinking whether 1) get rid of it at all 2) have network settings as "first test". This is the concept of tst_net_run(), I described previously. 1) is probably not a good idea, as separation of functions in test_net.sh and using it in network.sh is probably useful), but sourcing test_net.sh twice and handling correctly getopts for tst_test.sh make things a bit complicated. I'll try to come up with some solution in v3 (+ rename tests, as I wrote). > >> Can we run a test without them temporary, without tst_run(), > >> e.g. define some stubs and old API with the new one in > >> tst_net.sh? > > Do you mean to move all network scripts to new API, using these "shim" in tst_net.sh? > > I'd prefer tests were really using new API (to benefit from it), I just don't want to > > migrate all tests at once. This can be dangerous trying to implement new API functions in > > legacy one.> That's why I implemented in tst_net.sh functions starting with underscore which choose the > It's better to place underscore in the end of a function. OK, I changed them. I used them at the beginning as the same is for kernel / glibc internal (well, they have it doubled). I hope the migration of all network scripts will not take months, so it should be something temporary. > > implementation via $TST_USE_LEGACY_API. I wanted to ony tst_net.sh itself using it. > Thanks, > Alexey Kind regards, Petr