linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] selftests/tc-testing: add exit code
@ 2021-11-17  5:45 Li Zhijian
  2021-11-17  5:45 ` [PATCH v2 2/3] selftests/tc-testing: add missing config Li Zhijian
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Li Zhijian @ 2021-11-17  5:45 UTC (permalink / raw)
  To: shuah, kuba, dcaratti, linux-kselftest
  Cc: lizhijian, linux-kernel, lkp, philip.li, Li Zhijian

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()
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] selftests/tc-testing: add missing config
  2021-11-17  5:45 [PATCH v2 1/3] selftests/tc-testing: add exit code Li Zhijian
@ 2021-11-17  5:45 ` Li Zhijian
  2021-11-17  5:45 ` [PATCH v2 3/3] selftests/tc-testing: Fix cannot create /sys/bus/netdevsim/new_device: Directory nonexistent Li Zhijian
  2021-11-17  8:55 ` [PATCH v2 1/3] selftests/tc-testing: add exit code Davide Caratti
  2 siblings, 0 replies; 10+ messages in thread
From: Li Zhijian @ 2021-11-17  5:45 UTC (permalink / raw)
  To: shuah, kuba, dcaratti, linux-kselftest
  Cc: lizhijian, linux-kernel, lkp, philip.li, Li Zhijian

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
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] selftests/tc-testing: Fix cannot create /sys/bus/netdevsim/new_device: Directory nonexistent
  2021-11-17  5:45 [PATCH v2 1/3] selftests/tc-testing: add exit code Li Zhijian
  2021-11-17  5:45 ` [PATCH v2 2/3] selftests/tc-testing: add missing config Li Zhijian
@ 2021-11-17  5:45 ` Li Zhijian
  2021-11-17  8:55 ` [PATCH v2 1/3] selftests/tc-testing: add exit code Davide Caratti
  2 siblings, 0 replies; 10+ messages in thread
From: Li Zhijian @ 2021-11-17  5:45 UTC (permalink / raw)
  To: shuah, kuba, dcaratti, linux-kselftest
  Cc: lizhijian, linux-kernel, lkp, philip.li, Li Zhijian

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
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] selftests/tc-testing: add exit code
  2021-11-17  5:45 [PATCH v2 1/3] selftests/tc-testing: add exit code Li Zhijian
  2021-11-17  5:45 ` [PATCH v2 2/3] selftests/tc-testing: add missing config Li Zhijian
  2021-11-17  5:45 ` [PATCH v2 3/3] selftests/tc-testing: Fix cannot create /sys/bus/netdevsim/new_device: Directory nonexistent Li Zhijian
@ 2021-11-17  8:55 ` Davide Caratti
  2021-11-17 14:05   ` Jakub Kicinski
  2 siblings, 1 reply; 10+ messages in thread
From: Davide Caratti @ 2021-11-17  8:55 UTC (permalink / raw)
  To: Li Zhijian
  Cc: shuah, kuba, linux-kselftest, lizhijian, linux-kernel, lkp, philip.li

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>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] selftests/tc-testing: add exit code
  2021-11-17  8:55 ` [PATCH v2 1/3] selftests/tc-testing: add exit code Davide Caratti
@ 2021-11-17 14:05   ` Jakub Kicinski
  2021-11-17 16:41     ` Jamal Hadi Salim
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2021-11-17 14:05 UTC (permalink / raw)
  To: Davide Caratti, Jamal Hadi Salim
  Cc: Li Zhijian, shuah, linux-kselftest, lizhijian, linux-kernel, lkp,
	philip.li

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] selftests/tc-testing: add exit code
  2021-11-17 14:05   ` Jakub Kicinski
@ 2021-11-17 16:41     ` Jamal Hadi Salim
  2021-11-17 16:48       ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Jamal Hadi Salim @ 2021-11-17 16:41 UTC (permalink / raw)
  To: Jakub Kicinski, Davide Caratti
  Cc: Li Zhijian, shuah, linux-kselftest, lizhijian, linux-kernel, lkp,
	philip.li

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] selftests/tc-testing: add exit code
  2021-11-17 16:41     ` Jamal Hadi Salim
@ 2021-11-17 16:48       ` Jakub Kicinski
  2021-11-17 16:51         ` Jamal Hadi Salim
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2021-11-17 16:48 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: Davide Caratti, Li Zhijian, shuah, linux-kselftest, lizhijian,
	linux-kernel, lkp, philip.li

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] selftests/tc-testing: add exit code
  2021-11-17 16:48       ` Jakub Kicinski
@ 2021-11-17 16:51         ` Jamal Hadi Salim
  2021-12-03  2:21           ` Li Zhijian
  0 siblings, 1 reply; 10+ messages in thread
From: Jamal Hadi Salim @ 2021-11-17 16:51 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Davide Caratti, Li Zhijian, shuah, linux-kselftest, lizhijian,
	linux-kernel, lkp, philip.li

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] selftests/tc-testing: add exit code
  2021-11-17 16:51         ` Jamal Hadi Salim
@ 2021-12-03  2:21           ` Li Zhijian
  2021-12-03  2:47             ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Li Zhijian @ 2021-12-03  2:21 UTC (permalink / raw)
  To: Jamal Hadi Salim, Jakub Kicinski
  Cc: Davide Caratti, shuah, linux-kselftest, lizhijian, linux-kernel,
	lkp, philip.li, Networking

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
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] selftests/tc-testing: add exit code
  2021-12-03  2:21           ` Li Zhijian
@ 2021-12-03  2:47             ` Jakub Kicinski
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2021-12-03  2:47 UTC (permalink / raw)
  To: Li Zhijian
  Cc: Jamal Hadi Salim, Davide Caratti, shuah, linux-kselftest,
	lizhijian, linux-kernel, lkp, philip.li, Networking

On Fri, 3 Dec 2021 10:21:31 +0800 Li Zhijian wrote:
> CCed netdev

Please repost the patches.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-12-03  2:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17  5:45 [PATCH v2 1/3] selftests/tc-testing: add exit code Li Zhijian
2021-11-17  5:45 ` [PATCH v2 2/3] selftests/tc-testing: add missing config Li Zhijian
2021-11-17  5:45 ` [PATCH v2 3/3] selftests/tc-testing: Fix cannot create /sys/bus/netdevsim/new_device: Directory nonexistent Li Zhijian
2021-11-17  8:55 ` [PATCH v2 1/3] selftests/tc-testing: add exit code Davide Caratti
2021-11-17 14:05   ` Jakub Kicinski
2021-11-17 16:41     ` Jamal Hadi Salim
2021-11-17 16:48       ` Jakub Kicinski
2021-11-17 16:51         ` Jamal Hadi Salim
2021-12-03  2:21           ` Li Zhijian
2021-12-03  2:47             ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).