| Message ID | 20211117054517.31847-1-zhijianx.li@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 96f3896780153214040a6747974bebc1355307c0 |
| Headers | show |
| Series | [v2,1/3] selftests/tc-testing: add exit code | expand |
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 >
On Fri, 3 Dec 2021 10:21:31 +0800 Li Zhijian wrote:
> CCed netdev
Please repost the patches.
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()
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(-)