2021-11-17 05:48:53

by Li Zhijian

[permalink] [raw]
Subject: [PATCH v2 1/3] selftests/tc-testing: add exit code

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 <[email protected]>
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Li Zhijian <[email protected]>
---
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



2021-11-17 05:48:57

by Li Zhijian

[permalink] [raw]
Subject: [PATCH v2 2/3] selftests/tc-testing: add missing config

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 <[email protected]>
---
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


2021-11-17 05:49:08

by Li Zhijian

[permalink] [raw]
Subject: [PATCH v2 3/3] selftests/tc-testing: Fix cannot create /sys/bus/netdevsim/new_device: Directory nonexistent

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 <[email protected]>
---
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


2021-11-17 08:55:27

by Davide Caratti

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] selftests/tc-testing: add exit code

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 <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Li Zhijian <[email protected]>

Looks good to me!

Acked-by: Davide Caratti <[email protected]>


2021-11-17 14:05:40

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] selftests/tc-testing: add exit code

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.

2021-11-17 16:41:26

by Jamal Hadi Salim

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] selftests/tc-testing: add exit code

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



2021-11-17 16:49:18

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] selftests/tc-testing: add exit code

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.

2021-11-17 16:51:48

by Jamal Hadi Salim

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] selftests/tc-testing: add exit code

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


2021-12-03 02:22:06

by Li Zhijian

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] selftests/tc-testing: add exit code

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
>


2021-12-03 02:47:20

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] selftests/tc-testing: add exit code

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

Please repost the patches.