Mark the summary result as FAIL to prevent from confusing the selftest framework if some of them are failed.
Previously, the selftest framework always treats it as *ok* even though some of them are failed actually. That's because the script tdc.sh always return 0.
# All test results: # # 1..97 # ok 1 83be - Create FQ-PIE with invalid number of flows # ok 2 8b6e - Create RED with no flags [...snip] # ok 6 5f15 - Create RED with flags ECN, harddrop # ok 7 53e8 - Create RED with flags ECN, nodrop # ok 8 d091 - Fail to create RED with only nodrop flag # ok 9 af8e - Create RED with flags ECN, nodrop, harddrop # not ok 10 ce7d - Add mq Qdisc to multi-queue device (4 queues) # Could not match regex pattern. Verify command output: # qdisc mq 1: root # qdisc fq_codel 0: parent 1:4 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64 # qdisc fq_codel 0: parent 1:3 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64 [...snip] # ok 96 6979 - Change quantum of a strict ETS band # ok 97 9a7d - Change ETS strict band without quantum # # # # ok 1 selftests: tc-testing: tdc.sh <<< summary result
CC: Philip Li philip.li@intel.com Reported-by: kernel test robot lkp@intel.com Signed-off-by: Li Zhijian zhijianx.li@intel.com --- V2: Fix missing ':' --- tools/testing/selftests/tc-testing/tdc.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/tc-testing/tdc.py b/tools/testing/selftests/tc-testing/tdc.py index a3e43189d940..ee22e3447ec7 100755 --- a/tools/testing/selftests/tc-testing/tdc.py +++ b/tools/testing/selftests/tc-testing/tdc.py @@ -716,6 +716,7 @@ def set_operation_mode(pm, parser, args, remaining): list_test_cases(alltests) exit(0)
+ exit_code = 0 # KSFT_PASS if len(alltests): req_plugins = pm.get_required_plugins(alltests) try: @@ -724,6 +725,8 @@ def set_operation_mode(pm, parser, args, remaining): print('The following plugins were not found:') print('{}'.format(pde.missing_pg)) catresults = test_runner(pm, args, alltests) + if catresults.count_failures() != 0: + exit_code = 1 # KSFT_FAIL if args.format == 'none': print('Test results output suppression requested\n') else: @@ -748,6 +751,8 @@ def set_operation_mode(pm, parser, args, remaining): gid=int(os.getenv('SUDO_GID'))) else: print('No tests found\n') + exit_code = 4 # KSFT_SKIP + exit(exit_code)
def main(): """ @@ -767,8 +772,5 @@ def main():
set_operation_mode(pm, parser, args, remaining)
- exit(0) - - if __name__ == "__main__": main()
qdiscs/fq_pie requires CONFIG_NET_SCH_FQ_PIE, otherwise tc will fail to create a fq_pie qdisc.
It fixes following issue: # not ok 57 83be - Create FQ-PIE with invalid number of flows # Command exited with 2, expected 0 # Error: Specified qdisc not found.
Signed-off-by: Li Zhijian zhijianx.li@intel.com --- tools/testing/selftests/tc-testing/config | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config index b71828df5a6d..b1cd7efa4512 100644 --- a/tools/testing/selftests/tc-testing/config +++ b/tools/testing/selftests/tc-testing/config @@ -60,6 +60,7 @@ CONFIG_NET_IFE_SKBTCINDEX=m CONFIG_NET_SCH_FIFO=y CONFIG_NET_SCH_ETS=m CONFIG_NET_SCH_RED=m +CONFIG_NET_SCH_FQ_PIE=m
# ## Network testing
Install netdevsim to provide /sys/bus/netdevsim/new_device interface.
It helps to fix: # ok 97 9a7d - Change ETS strict band without quantum # skipped - skipped - previous setup failed 11 ce7d # # # -----> prepare stage *** Could not execute: "echo "1 1 4" > /sys/bus/netdevsim/new_device" # # -----> prepare stage *** Error message: "/bin/sh: 1: cannot create /sys/bus/netdevsim/new_device: Directory nonexistent # " # # -----> prepare stage *** Aborting test run. # # # <_io.BufferedReader name=5> *** stdout *** #
Signed-off-by: Li Zhijian zhijianx.li@intel.com --- tools/testing/selftests/tc-testing/config | 1 + tools/testing/selftests/tc-testing/tdc.sh | 1 + 2 files changed, 2 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config index b1cd7efa4512..a3239d5e40c7 100644 --- a/tools/testing/selftests/tc-testing/config +++ b/tools/testing/selftests/tc-testing/config @@ -61,6 +61,7 @@ CONFIG_NET_SCH_FIFO=y CONFIG_NET_SCH_ETS=m CONFIG_NET_SCH_RED=m CONFIG_NET_SCH_FQ_PIE=m +CONFIG_NETDEVSIM=m
# ## Network testing diff --git a/tools/testing/selftests/tc-testing/tdc.sh b/tools/testing/selftests/tc-testing/tdc.sh index 7fe38c76db44..afb0cd86fa3d 100755 --- a/tools/testing/selftests/tc-testing/tdc.sh +++ b/tools/testing/selftests/tc-testing/tdc.sh @@ -1,5 +1,6 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0
+modprobe netdevsim ./tdc.py -c actions --nobuildebpf ./tdc.py -c qdisc
hi Li,
On Wed, Nov 17, 2021 at 01:45:15PM +0800, Li Zhijian wrote:
Mark the summary result as FAIL to prevent from confusing the selftest framework if some of them are failed.
Previously, the selftest framework always treats it as *ok* even though some of them are failed actually. That's because the script tdc.sh always return 0.
yes, also tdc was lacking support for KSFT_SKIP for a long time.
# All test results: # # 1..97 # ok 1 83be - Create FQ-PIE with invalid number of flows # ok 2 8b6e - Create RED with no flags [...snip] # ok 6 5f15 - Create RED with flags ECN, harddrop # ok 7 53e8 - Create RED with flags ECN, nodrop # ok 8 d091 - Fail to create RED with only nodrop flag # ok 9 af8e - Create RED with flags ECN, nodrop, harddrop # not ok 10 ce7d - Add mq Qdisc to multi-queue device (4 queues) # Could not match regex pattern. Verify command output: # qdisc mq 1: root # qdisc fq_codel 0: parent 1:4 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64 # qdisc fq_codel 0: parent 1:3 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64 [...snip] # ok 96 6979 - Change quantum of a strict ETS band # ok 97 9a7d - Change ETS strict band without quantum # # # # ok 1 selftests: tc-testing: tdc.sh <<< summary result
CC: Philip Li philip.li@intel.com Reported-by: kernel test robot lkp@intel.com Signed-off-by: Li Zhijian zhijianx.li@intel.com
Looks good to me!
Acked-by: Davide Caratti dcaratti@redhat.com
On Wed, 17 Nov 2021 09:55:14 +0100 Davide Caratti wrote:
On Wed, Nov 17, 2021 at 01:45:15PM +0800, Li Zhijian wrote:
Mark the summary result as FAIL to prevent from confusing the selftest framework if some of them are failed.
Previously, the selftest framework always treats it as *ok* even though some of them are failed actually. That's because the script tdc.sh always return 0.
yes, also tdc was lacking support for KSFT_SKIP for a long time.
Should this go via netdev? Is the risk of conflicts low enough so it doesn't matter?
We should probably add a MAINTAINERS entry for tdc. Adding Jamal.
On 2021-11-17 09:05, Jakub Kicinski wrote:
On Wed, 17 Nov 2021 09:55:14 +0100 Davide Caratti wrote:
On Wed, Nov 17, 2021 at 01:45:15PM +0800, Li Zhijian wrote:
Mark the summary result as FAIL to prevent from confusing the selftest framework if some of them are failed.
Previously, the selftest framework always treats it as *ok* even though some of them are failed actually. That's because the script tdc.sh always return 0.
yes, also tdc was lacking support for KSFT_SKIP for a long time.
Should this go via netdev? Is the risk of conflicts low enough so it doesn't matter?
Yes, discussions should at minimal Cc netdev + tc maintainers.
We should probably add a MAINTAINERS entry for tdc. Adding Jamal.
Did you mean adding a maintainer for tdc or just generally point who/what to involve when making changes? Typically the mailing list should be sufficient. Outside the list, at the moment, any outstanding issues on tdc are discussed/resolved in the monthly TC meetups (where all the stake holders show up)...
cheers, jamal
On Wed, 17 Nov 2021 11:41:18 -0500 Jamal Hadi Salim wrote:
Did you mean adding a maintainer for tdc or just generally point who/what to involve when making changes? Typically the mailing list should be sufficient. Outside the list, at the moment, any outstanding issues on tdc are discussed/resolved in the monthly TC meetups (where all the stake holders show up)...
I'm mostly interested in the code review and merging part.
Would be great to have a MAINTAINERS entry with a set of folks who can review patches, so that get_maintainers.pl can do its job.
At the very least to make sure netdev is CCed.
On 2021-11-17 11:48, Jakub Kicinski wrote:
On Wed, 17 Nov 2021 11:41:18 -0500 Jamal Hadi Salim wrote:
Did you mean adding a maintainer for tdc or just generally point who/what to involve when making changes? Typically the mailing list should be sufficient. Outside the list, at the moment, any outstanding issues on tdc are discussed/resolved in the monthly TC meetups (where all the stake holders show up)...
I'm mostly interested in the code review and merging part.
Would be great to have a MAINTAINERS entry with a set of folks who can review patches, so that get_maintainers.pl can do its job.
At the very least to make sure netdev is CCed.
ACK.
cheers, jamal
CCed netdev
Kindly ping
On 18/11/2021 00:51, Jamal Hadi Salim wrote:
On 2021-11-17 11:48, Jakub Kicinski wrote:
On Wed, 17 Nov 2021 11:41:18 -0500 Jamal Hadi Salim wrote:
Did you mean adding a maintainer for tdc or just generally point who/what to involve when making changes? Typically the mailing list should be sufficient. Outside the list, at the moment, any outstanding issues on tdc are discussed/resolved in the monthly TC meetups (where all the stake holders show up)...
I'm mostly interested in the code review and merging part.
Would be great to have a MAINTAINERS entry with a set of folks who can review patches, so that get_maintainers.pl can do its job.
At the very least to make sure netdev is CCed.
ACK.
cheers, jamal
linux-kselftest-mirror@lists.linaro.org