All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>, netdev@vger.kernel.org
Cc: Jiri Pirko <jiri@resnulli.us>,
	Pedro Tammela <pctammela@mojatatu.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Jamal Hadi Salim <jhs@mojatatu.com>,
	linux-kernel@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
	intel-wired-lan@lists.osuosl.org,
	Maxim Georgiev <glipus@gmail.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Peilin Ye <yepeilin.cs@gmail.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Zhengchao Shao <shaozhengchao@huawei.com>,
	Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [Intel-wired-lan] [PATCH v3 net-next 09/10] selftests/tc-testing: test that taprio can only be attached as root
Date: Wed, 02 Aug 2023 16:29:55 -0700	[thread overview]
Message-ID: <87pm4510r0.fsf@intel.com> (raw)
In-Reply-To: <20230801182421.1997560-10-vladimir.oltean@nxp.com>

Vladimir Oltean <vladimir.oltean@nxp.com> writes:

> Check that the "Can only be attached as root qdisc" error message from
> taprio is effective by attempting to attach it to a class of another
> taprio qdisc. That operation should fail.
>
> In the bug that was squashed by change "net/sched: taprio: try again to
> report q->qdiscs[] to qdisc_leaf()", grafting a child taprio to a root
> software taprio would be misinterpreted as a change() to the root
> taprio. Catch this by looking at whether the base-time of the root
> taprio has changed to follow the base-time of the child taprio,
> something which should have absolutely never happened assuming correct
> semantics.
>

This test is somehow flaky (all others are fine), 1 in ~4 times, it fails.

Taking a look at the test I couldn't quickly find out the reason for the
flakyness.

Here's the verbose output of one of the failures:

vcgomes@otc-cfl-clr-30 ~/src/net-next/tools/testing/selftests/tc-testing $ sudo ./tdc.py -e 39b4 -v
 -- ns/SubPlugin.__init__
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip link add v0p0 type veth peer name v0p1]
_exec_cmd:  command "/sbin/ip link add v0p0 type veth peer name v0p1"
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip link set v0p0 up]
_exec_cmd:  command "/sbin/ip link set v0p0 up"
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip netns add tcut]
_exec_cmd:  command "/sbin/ip netns add tcut"
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip link set v0p1 netns tcut]
_exec_cmd:  command "/sbin/ip link set v0p1 netns tcut"
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip -n tcut link set v0p1 up]
_exec_cmd:  command "/sbin/ip -n tcut link set v0p1 up"
	====================
=====> Test 39b4: Reject grafting taprio as child qdisc of software taprio
-----> prepare stage
ns/SubPlugin.adjust_command
adjust_command:  stage is setup; inserting netns stuff in command [echo "1 1 8" > /sys/bus/netdevsim/new_device] list [['echo', '"1', '1', '8"', '>', '/sys/bus/netdevsim/new_device']]
adjust_command:  return command [/sbin/ip netns exec tcut echo "1 1 8" > /sys/bus/netdevsim/new_device]
command "/sbin/ip netns exec tcut echo "1 1 8" > /sys/bus/netdevsim/new_device"
ns/SubPlugin.adjust_command
adjust_command:  stage is setup; inserting netns stuff in command [/sbin/tc qdisc replace dev eth0 handle 8001: parent root stab overhead 24 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 0 sched-entry S ff 20000000 clockid CLOCK_TAI] list [['/sbin/tc', 'qdisc', 'replace', 'dev', 'eth0', 'handle', '8001:', 'parent', 'root', 'stab', 'overhead', '24', 'taprio', 'num_tc', '8', 'map', '0', '1', '2', '3', '4', '5', '6', '7', 'queues', '1@0', '1@1', '1@2', '1@3', '1@4', '1@5', '1@6', '1@7', 'base-time', '0', 'sched-entry', 'S', 'ff', '20000000', 'clockid', 'CLOCK_TAI']]
adjust_command:  return command [/sbin/ip netns exec tcut /sbin/tc qdisc replace dev eth0 handle 8001: parent root stab overhead 24 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 0 sched-entry S ff 20000000 clockid CLOCK_TAI]
command "/sbin/ip netns exec tcut /sbin/tc qdisc replace dev eth0 handle 8001: parent root stab overhead 24 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 0 sched-entry S ff 20000000 clockid CLOCK_TAI"
-----> execute stage
ns/SubPlugin.adjust_command
adjust_command:  stage is execute; inserting netns stuff in command [/sbin/tc qdisc replace dev eth0 parent 8001:7 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 200 sched-entry S ff 20000000 clockid CLOCK_TAI] list [['/sbin/tc', 'qdisc', 'replace', 'dev', 'eth0', 'parent', '8001:7', 'taprio', 'num_tc', '8', 'map', '0', '1', '2', '3', '4', '5', '6', '7', 'queues', '1@0', '1@1', '1@2', '1@3', '1@4', '1@5', '1@6', '1@7', 'base-time', '200', 'sched-entry', 'S', 'ff', '20000000', 'clockid', 'CLOCK_TAI']]
adjust_command:  return command [/sbin/ip netns exec tcut /sbin/tc qdisc replace dev eth0 parent 8001:7 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 200 sched-entry S ff 20000000 clockid CLOCK_TAI]
command "/sbin/ip netns exec tcut /sbin/tc qdisc replace dev eth0 parent 8001:7 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 200 sched-entry S ff 20000000 clockid CLOCK_TAI"
-----> verify stage
ns/SubPlugin.adjust_command
adjust_command:  stage is verify; inserting netns stuff in command [/sbin/tc -j qdisc show dev eth0 root | jq '.[].options.base_time'] list [['/sbin/tc', '-j', 'qdisc', 'show', 'dev', 'eth0', 'root', '|', 'jq', "'.[].options.base_time'"]]
adjust_command:  return command [/sbin/ip netns exec tcut /sbin/tc -j qdisc show dev eth0 root | jq '.[].options.base_time']
command "/sbin/ip netns exec tcut /sbin/tc -j qdisc show dev eth0 root | jq '.[].options.base_time'"
-----> teardown stage
ns/SubPlugin.adjust_command
adjust_command:  stage is teardown; inserting netns stuff in command [/sbin/tc qdisc del dev eth0 root] list [['/sbin/tc', 'qdisc', 'del', 'dev', 'eth0', 'root']]
adjust_command:  return command [/sbin/ip netns exec tcut /sbin/tc qdisc del dev eth0 root]
command "/sbin/ip netns exec tcut /sbin/tc qdisc del dev eth0 root"
ns/SubPlugin.adjust_command
adjust_command:  stage is teardown; inserting netns stuff in command [echo "1" > /sys/bus/netdevsim/del_device] list [['echo', '"1"', '>', '/sys/bus/netdevsim/del_device']]
adjust_command:  return command [/sbin/ip netns exec tcut echo "1" > /sys/bus/netdevsim/del_device]
command "/sbin/ip netns exec tcut echo "1" > /sys/bus/netdevsim/del_device"
ns/SubPlugin.post_suite
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip netns delete tcut]
_exec_cmd:  command "/sbin/ip netns delete tcut"

All test results:

1..1
not ok 1 39b4 - Reject grafting taprio as child qdisc of software taprio
	Could not match regex pattern. Verify command output:
parse error: Objects must consist of key:value pairs at line 1, column 334


Cheers,
-- 
Vinicius
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

WARNING: multiple messages have this Message-ID (diff)
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>, netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Jamal Hadi Salim <jhs@mojatatu.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>,
	linux-kernel@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
	Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>,
	Peilin Ye <yepeilin.cs@gmail.com>,
	Pedro Tammela <pctammela@mojatatu.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Zhengchao Shao <shaozhengchao@huawei.com>,
	Maxim Georgiev <glipus@gmail.com>
Subject: Re: [PATCH v3 net-next 09/10] selftests/tc-testing: test that taprio can only be attached as root
Date: Wed, 02 Aug 2023 16:29:55 -0700	[thread overview]
Message-ID: <87pm4510r0.fsf@intel.com> (raw)
In-Reply-To: <20230801182421.1997560-10-vladimir.oltean@nxp.com>

Vladimir Oltean <vladimir.oltean@nxp.com> writes:

> Check that the "Can only be attached as root qdisc" error message from
> taprio is effective by attempting to attach it to a class of another
> taprio qdisc. That operation should fail.
>
> In the bug that was squashed by change "net/sched: taprio: try again to
> report q->qdiscs[] to qdisc_leaf()", grafting a child taprio to a root
> software taprio would be misinterpreted as a change() to the root
> taprio. Catch this by looking at whether the base-time of the root
> taprio has changed to follow the base-time of the child taprio,
> something which should have absolutely never happened assuming correct
> semantics.
>

This test is somehow flaky (all others are fine), 1 in ~4 times, it fails.

Taking a look at the test I couldn't quickly find out the reason for the
flakyness.

Here's the verbose output of one of the failures:

vcgomes@otc-cfl-clr-30 ~/src/net-next/tools/testing/selftests/tc-testing $ sudo ./tdc.py -e 39b4 -v
 -- ns/SubPlugin.__init__
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip link add v0p0 type veth peer name v0p1]
_exec_cmd:  command "/sbin/ip link add v0p0 type veth peer name v0p1"
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip link set v0p0 up]
_exec_cmd:  command "/sbin/ip link set v0p0 up"
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip netns add tcut]
_exec_cmd:  command "/sbin/ip netns add tcut"
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip link set v0p1 netns tcut]
_exec_cmd:  command "/sbin/ip link set v0p1 netns tcut"
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip -n tcut link set v0p1 up]
_exec_cmd:  command "/sbin/ip -n tcut link set v0p1 up"
	====================
=====> Test 39b4: Reject grafting taprio as child qdisc of software taprio
-----> prepare stage
ns/SubPlugin.adjust_command
adjust_command:  stage is setup; inserting netns stuff in command [echo "1 1 8" > /sys/bus/netdevsim/new_device] list [['echo', '"1', '1', '8"', '>', '/sys/bus/netdevsim/new_device']]
adjust_command:  return command [/sbin/ip netns exec tcut echo "1 1 8" > /sys/bus/netdevsim/new_device]
command "/sbin/ip netns exec tcut echo "1 1 8" > /sys/bus/netdevsim/new_device"
ns/SubPlugin.adjust_command
adjust_command:  stage is setup; inserting netns stuff in command [/sbin/tc qdisc replace dev eth0 handle 8001: parent root stab overhead 24 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 0 sched-entry S ff 20000000 clockid CLOCK_TAI] list [['/sbin/tc', 'qdisc', 'replace', 'dev', 'eth0', 'handle', '8001:', 'parent', 'root', 'stab', 'overhead', '24', 'taprio', 'num_tc', '8', 'map', '0', '1', '2', '3', '4', '5', '6', '7', 'queues', '1@0', '1@1', '1@2', '1@3', '1@4', '1@5', '1@6', '1@7', 'base-time', '0', 'sched-entry', 'S', 'ff', '20000000', 'clockid', 'CLOCK_TAI']]
adjust_command:  return command [/sbin/ip netns exec tcut /sbin/tc qdisc replace dev eth0 handle 8001: parent root stab overhead 24 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 0 sched-entry S ff 20000000 clockid CLOCK_TAI]
command "/sbin/ip netns exec tcut /sbin/tc qdisc replace dev eth0 handle 8001: parent root stab overhead 24 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 0 sched-entry S ff 20000000 clockid CLOCK_TAI"
-----> execute stage
ns/SubPlugin.adjust_command
adjust_command:  stage is execute; inserting netns stuff in command [/sbin/tc qdisc replace dev eth0 parent 8001:7 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 200 sched-entry S ff 20000000 clockid CLOCK_TAI] list [['/sbin/tc', 'qdisc', 'replace', 'dev', 'eth0', 'parent', '8001:7', 'taprio', 'num_tc', '8', 'map', '0', '1', '2', '3', '4', '5', '6', '7', 'queues', '1@0', '1@1', '1@2', '1@3', '1@4', '1@5', '1@6', '1@7', 'base-time', '200', 'sched-entry', 'S', 'ff', '20000000', 'clockid', 'CLOCK_TAI']]
adjust_command:  return command [/sbin/ip netns exec tcut /sbin/tc qdisc replace dev eth0 parent 8001:7 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 200 sched-entry S ff 20000000 clockid CLOCK_TAI]
command "/sbin/ip netns exec tcut /sbin/tc qdisc replace dev eth0 parent 8001:7 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 200 sched-entry S ff 20000000 clockid CLOCK_TAI"
-----> verify stage
ns/SubPlugin.adjust_command
adjust_command:  stage is verify; inserting netns stuff in command [/sbin/tc -j qdisc show dev eth0 root | jq '.[].options.base_time'] list [['/sbin/tc', '-j', 'qdisc', 'show', 'dev', 'eth0', 'root', '|', 'jq', "'.[].options.base_time'"]]
adjust_command:  return command [/sbin/ip netns exec tcut /sbin/tc -j qdisc show dev eth0 root | jq '.[].options.base_time']
command "/sbin/ip netns exec tcut /sbin/tc -j qdisc show dev eth0 root | jq '.[].options.base_time'"
-----> teardown stage
ns/SubPlugin.adjust_command
adjust_command:  stage is teardown; inserting netns stuff in command [/sbin/tc qdisc del dev eth0 root] list [['/sbin/tc', 'qdisc', 'del', 'dev', 'eth0', 'root']]
adjust_command:  return command [/sbin/ip netns exec tcut /sbin/tc qdisc del dev eth0 root]
command "/sbin/ip netns exec tcut /sbin/tc qdisc del dev eth0 root"
ns/SubPlugin.adjust_command
adjust_command:  stage is teardown; inserting netns stuff in command [echo "1" > /sys/bus/netdevsim/del_device] list [['echo', '"1"', '>', '/sys/bus/netdevsim/del_device']]
adjust_command:  return command [/sbin/ip netns exec tcut echo "1" > /sys/bus/netdevsim/del_device]
command "/sbin/ip netns exec tcut echo "1" > /sys/bus/netdevsim/del_device"
ns/SubPlugin.post_suite
ns/SubPlugin.adjust_command
adjust_command:  return command [/sbin/ip netns delete tcut]
_exec_cmd:  command "/sbin/ip netns delete tcut"

All test results:

1..1
not ok 1 39b4 - Reject grafting taprio as child qdisc of software taprio
	Could not match regex pattern. Verify command output:
parse error: Objects must consist of key:value pairs at line 1, column 334


Cheers,
-- 
Vinicius

  reply	other threads:[~2023-08-02 23:30 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-01 18:24 [Intel-wired-lan] [PATCH v3 net-next 00/10] Improve the taprio qdisc's relationship with its children Vladimir Oltean
2023-08-01 18:24 ` Vladimir Oltean
2023-08-01 18:24 ` [Intel-wired-lan] [PATCH v3 net-next 01/10] net/sched: taprio: don't access q->qdiscs[] in unoffloaded mode during attach() Vladimir Oltean
2023-08-01 18:24   ` Vladimir Oltean
2023-08-01 18:24 ` [Intel-wired-lan] [PATCH v3 net-next 02/10] net/sched: taprio: keep child Qdisc refcount elevated at 2 in offload mode Vladimir Oltean
2023-08-01 18:24   ` Vladimir Oltean
2023-08-01 18:24 ` [Intel-wired-lan] [PATCH v3 net-next 03/10] net/sched: taprio: try again to report q->qdiscs[] to qdisc_leaf() Vladimir Oltean
2023-08-01 18:24   ` Vladimir Oltean
2023-08-01 18:24 ` [Intel-wired-lan] [PATCH v3 net-next 04/10] net/sched: taprio: delete misleading comment about preallocating child qdiscs Vladimir Oltean
2023-08-01 18:24   ` Vladimir Oltean
2023-08-01 18:24 ` [Intel-wired-lan] [PATCH v3 net-next 05/10] net/sched: taprio: dump class stats for the actual q->qdiscs[] Vladimir Oltean
2023-08-01 18:24   ` Vladimir Oltean
2023-08-01 18:24 ` [Intel-wired-lan] [PATCH v3 net-next 06/10] net: ptp: create a mock-up PTP Hardware Clock driver Vladimir Oltean
2023-08-01 18:24   ` Vladimir Oltean
2023-08-02 12:18   ` [Intel-wired-lan] " Kurt Kanzenbach
2023-08-02 12:18     ` Kurt Kanzenbach
2023-08-02 12:43     ` [Intel-wired-lan] " Vladimir Oltean
2023-08-02 12:43       ` Vladimir Oltean
2023-08-02 21:58   ` [Intel-wired-lan] " Vinicius Costa Gomes
2023-08-02 21:58     ` Vinicius Costa Gomes
2023-08-03 14:45     ` [Intel-wired-lan] " Vladimir Oltean
2023-08-03 14:45       ` Vladimir Oltean
2023-08-01 18:24 ` [Intel-wired-lan] [PATCH v3 net-next 07/10] net: netdevsim: use mock PHC driver Vladimir Oltean
2023-08-01 18:24   ` Vladimir Oltean
2023-08-01 18:24 ` [Intel-wired-lan] [PATCH v3 net-next 08/10] net: netdevsim: mimic tc-taprio offload Vladimir Oltean
2023-08-01 18:24   ` Vladimir Oltean
2023-08-01 18:24 ` [Intel-wired-lan] [PATCH v3 net-next 09/10] selftests/tc-testing: test that taprio can only be attached as root Vladimir Oltean
2023-08-01 18:24   ` Vladimir Oltean
2023-08-02 23:29   ` Vinicius Costa Gomes [this message]
2023-08-02 23:29     ` Vinicius Costa Gomes
2023-08-03 14:33     ` [Intel-wired-lan] " Vladimir Oltean
2023-08-03 14:33       ` Vladimir Oltean
2023-08-03 18:43       ` [Intel-wired-lan] " Vinicius Costa Gomes
2023-08-03 18:43         ` Vinicius Costa Gomes
2023-08-07 19:06         ` [Intel-wired-lan] " Vladimir Oltean
2023-08-07 19:06           ` Vladimir Oltean
2023-08-03 17:21   ` [Intel-wired-lan] " Victor Nogueira
2023-08-03 17:21     ` Victor Nogueira
2023-08-04 11:14     ` [Intel-wired-lan] " Vladimir Oltean
2023-08-04 11:14       ` Vladimir Oltean
2023-08-01 18:24 ` [Intel-wired-lan] [PATCH v3 net-next 10/10] selftests/tc-testing: verify that a qdisc can be grafted onto a taprio class Vladimir Oltean
2023-08-01 18:24   ` Vladimir Oltean
2023-08-03 10:04 ` [Intel-wired-lan] [PATCH v3 net-next 00/10] Improve the taprio qdisc's relationship with its children Paolo Abeni
2023-08-03 10:04   ` Paolo Abeni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pm4510r0.fsf@intel.com \
    --to=vinicius.gomes@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=glipus@gmail.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pctammela@mojatatu.com \
    --cc=richardcochran@gmail.com \
    --cc=shaozhengchao@huawei.com \
    --cc=vladimir.oltean@nxp.com \
    --cc=xiyou.wangcong@gmail.com \
    --cc=yepeilin.cs@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.