[Patch net v2 0/4] net_sched: two security bug fixes and test cases

[Patch net v2 0/4] net_sched: two security bug fixes and test cases

[Cong Wang]

From: Cong Wang <cong.wang@bytedance.com>

This patchset contains two bug fixes reported in security mailing list,
and test cases for both of them.

---
v2: replaced dummy2 with $DUMMY in pfifo_head_drop test
    reduced the number of ping's in pfifo_head_drop test
    improved commit messages

Cong Wang (2):
  netem: update sch->q.qlen before qdisc_tree_reduce_backlog()
  selftests/tc-testing: add a test case for qdisc_tree_reduce_backlog()

Quang Le (2):
  pfifo_tail_enqueue: Drop new packet when sch->limit == 0
  selftests/tc-testing: Add a test case for pfifo_head_drop qdisc when
    limit==0

 net/sched/sch_fifo.c                          |  3 ++
 net/sched/sch_netem.c                         |  2 +-
 .../tc-testing/tc-tests/infra/qdiscs.json     | 34 ++++++++++++++++++-
 .../tc-testing/tc-tests/qdiscs/fifo.json      | 25 ++++++++++++++
 4 files changed, 62 insertions(+), 2 deletions(-)

-- 
2.34.1

[Patch net v2 1/4] pfifo_tail_enqueue: Drop new packet when sch->limit == 0

[Cong Wang]

From: Quang Le <quanglex97@gmail.com>

Expected behaviour:
In case we reach scheduler's limit, pfifo_tail_enqueue() will drop a
packet in scheduler's queue and decrease scheduler's qlen by one.
Then, pfifo_tail_enqueue() enqueue new packet and increase
scheduler's qlen by one. Finally, pfifo_tail_enqueue() return
`NET_XMIT_CN` status code.

Weird behaviour:
In case we set `sch->limit == 0` and trigger pfifo_tail_enqueue() on a
scheduler that has no packet, the 'drop a packet' step will do nothing.
This means the scheduler's qlen still has value equal 0.
Then, we continue to enqueue new packet and increase scheduler's qlen by
one. In summary, we can leverage pfifo_tail_enqueue() to increase qlen by
one and return `NET_XMIT_CN` status code.

The problem is:
Let's say we have two qdiscs: Qdisc_A and Qdisc_B.
 - Qdisc_A's type must have '->graft()' function to create parent/child relationship.
   Let's say Qdisc_A's type is `hfsc`. Enqueue packet to this qdisc will trigger `hfsc_enqueue`.
 - Qdisc_B's type is pfifo_head_drop. Enqueue packet to this qdisc will trigger `pfifo_tail_enqueue`.
 - Qdisc_B is configured to have `sch->limit == 0`.
 - Qdisc_A is configured to route the enqueued's packet to Qdisc_B.

Enqueue packet through Qdisc_A will lead to:
 - hfsc_enqueue(Qdisc_A) -> pfifo_tail_enqueue(Qdisc_B)
 - Qdisc_B->q.qlen += 1
 - pfifo_tail_enqueue() return `NET_XMIT_CN`
 - hfsc_enqueue() check for `NET_XMIT_SUCCESS` and see `NET_XMIT_CN` => hfsc_enqueue() don't increase qlen of Qdisc_A.

The whole process lead to a situation where Qdisc_A->q.qlen == 0 and Qdisc_B->q.qlen == 1.
Replace 'hfsc' with other type (for example: 'drr') still lead to the same problem.
This violate the design where parent's qlen should equal to the sum of its childrens'qlen.

Bug impact: This issue can be used for user->kernel privilege escalation when it is reachable.

Reported-by: Quang Le <quanglex97@gmail.com>
Signed-off-by: Quang Le <quanglex97@gmail.com>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
 net/sched/sch_fifo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c
index b50b2c2cc09b..e6bfd39ff339 100644
--- a/net/sched/sch_fifo.c
+++ b/net/sched/sch_fifo.c
@@ -40,6 +40,9 @@ static int pfifo_tail_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 {
 	unsigned int prev_backlog;
 
+	if (unlikely(READ_ONCE(sch->limit) == 0))
+		return qdisc_drop(skb, sch, to_free);
+
 	if (likely(sch->q.qlen < READ_ONCE(sch->limit)))
 		return qdisc_enqueue_tail(skb, sch);
 
-- 
2.34.1

[Patch net v2 2/4] selftests/tc-testing: Add a test case for pfifo_head_drop qdisc when limit==0

[Cong Wang]

From: Quang Le <quanglex97@gmail.com>

When limit == 0, pfifo_tail_enqueue() must drop new packet and
increase dropped packets count of the qdisc.

All test results:

1..16
ok 1 a519 - Add bfifo qdisc with system default parameters on egress
ok 2 585c - Add pfifo qdisc with system default parameters on egress
ok 3 a86e - Add bfifo qdisc with system default parameters on egress with handle of maximum value
ok 4 9ac8 - Add bfifo qdisc on egress with queue size of 3000 bytes
ok 5 f4e6 - Add pfifo qdisc on egress with queue size of 3000 packets
ok 6 b1b1 - Add bfifo qdisc with system default parameters on egress with invalid handle exceeding maximum value
ok 7 8d5e - Add bfifo qdisc on egress with unsupported argument
ok 8 7787 - Add pfifo qdisc on egress with unsupported argument
ok 9 c4b6 - Replace bfifo qdisc on egress with new queue size
ok 10 3df6 - Replace pfifo qdisc on egress with new queue size
ok 11 7a67 - Add bfifo qdisc on egress with queue size in invalid format
ok 12 1298 - Add duplicate bfifo qdisc on egress
ok 13 45a0 - Delete nonexistent bfifo qdisc
ok 14 972b - Add prio qdisc on egress with invalid format for handles
ok 15 4d39 - Delete bfifo qdisc twice
ok 16 d774 - Check pfifo_head_drop qdisc enqueue behaviour when limit == 0

Signed-off-by: Quang Le <quanglex97@gmail.com>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
 .../tc-testing/tc-tests/qdiscs/fifo.json      | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json
index ae3d286a32b2..94f6456ab460 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json
@@ -313,6 +313,31 @@
         "matchPattern": "qdisc bfifo 1: root",
         "matchCount": "0",
         "teardown": [
+	]
+    },
+    {
+        "id": "d774",
+        "name": "Check pfifo_head_drop qdisc enqueue behaviour when limit == 0",
+        "category": [
+            "qdisc",
+            "pfifo_head_drop"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$IP link add dev $DUMMY mtu 1279 type dummy || true",
+            "$IP addr add 10.10.10.10/24 dev $DUMMY || true",
+            "$TC qdisc add dev $DUMMY root handle 1: pfifo_head_drop limit 0",
+            "$IP link set dev $DUMMY up || true"
+        ],
+        "cmdUnderTest": "ping -c2 -W0.01 -I $DUMMY 10.10.10.1",
+        "expExitCode": "1",
+        "verifyCmd": "$TC -s qdisc show dev $DUMMY",
+        "matchPattern": "dropped 2",
+        "matchCount": "1",
+        "teardown": [
+            "$IP link del dev $DUMMY"
         ]
     }
 ]
-- 
2.34.1

[Patch net v2 3/4] netem: update sch->q.qlen before qdisc_tree_reduce_backlog()

[Cong Wang]

From: Cong Wang <cong.wang@bytedance.com>

qdisc_tree_reduce_backlog() notifies parent qdisc only if child
qdisc becomes empty, therefore we need to reduce the backlog of the
child qdisc before calling it. Otherwise it would become a nop and
result in UAF in DRR case (which is integrated in the following patch).

Fixes: f8d4bc455047 ("net/sched: netem: account for backlog updates from child qdisc")
Cc: Martin Ottens <martin.ottens@fau.de>
Reported-by: Mingi Cho <mincho@theori.io>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
 net/sched/sch_netem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 71ec9986ed37..fdd79d3ccd8c 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -749,9 +749,9 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)
 				if (err != NET_XMIT_SUCCESS) {
 					if (net_xmit_drop_count(err))
 						qdisc_qstats_drop(sch);
-					qdisc_tree_reduce_backlog(sch, 1, pkt_len);
 					sch->qstats.backlog -= pkt_len;
 					sch->q.qlen--;
+					qdisc_tree_reduce_backlog(sch, 1, pkt_len);
 				}
 				goto tfifo_dequeue;
 			}
-- 
2.34.1

[Patch net v2 4/4] selftests/tc-testing: add a test case for qdisc_tree_reduce_backlog()

[Cong Wang]

From: Cong Wang <cong.wang@bytedance.com>

Integrate the test case provided by Mingi Cho into TDC.

All test results:

1..4
ok 1 ca5e - Check class delete notification for ffff:
ok 2 e4b7 - Check class delete notification for root ffff:
ok 3 33a9 - Check ingress is not searchable on backlog update
ok 4 a4b9 - Test class qlen notification

Cc: Mingi Cho <mincho@theori.io>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
 .../tc-testing/tc-tests/infra/qdiscs.json     | 34 ++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json b/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
index d3dd65b05b5f..9044ac054167 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
@@ -94,5 +94,37 @@
             "$TC qdisc del dev $DUMMY ingress",
             "$IP addr del 10.10.10.10/24 dev $DUMMY"
         ]
-    }
+    },
+    {
+	"id": "a4b9",
+	"name": "Test class qlen notification",
+	"category": [
+	    "qdisc"
+	],
+	"plugins": {
+	    "requires": "nsPlugin"
+	},
+	"setup": [
+            "$IP link set dev $DUMMY up || true",
+            "$IP addr add 10.10.10.10/24 dev $DUMMY || true",
+            "$TC qdisc add dev $DUMMY root handle 1: drr",
+            "$TC filter add dev $DUMMY parent 1: basic classid 1:1",
+            "$TC class add dev $DUMMY parent 1: classid 1:1 drr",
+            "$TC qdisc add dev $DUMMY parent 1:1 handle 2: netem",
+            "$TC qdisc add dev $DUMMY parent 2: handle 3: drr",
+            "$TC filter add dev $DUMMY parent 3: basic action drop",
+            "$TC class add dev $DUMMY parent 3: classid 3:1 drr",
+            "$TC class del dev $DUMMY classid 1:1",
+            "$TC class add dev $DUMMY parent 1: classid 1:1 drr"
+        ],
+        "cmdUnderTest": "ping -c1 -W0.01 -I $DUMMY 10.10.10.1",
+        "expExitCode": "1",
+        "verifyCmd": "$TC qdisc ls dev $DUMMY",
+        "matchPattern": "drr 1: root",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY root handle 1: drr",
+            "$IP addr del 10.10.10.10/24 dev $DUMMY"
+        ]
+   }
 ]
-- 
2.34.1

Re: [Patch net v2 2/4] selftests/tc-testing: Add a test case for pfifo_head_drop qdisc when limit==0

[Jakub Kicinski]

On Sat, 25 Jan 2025 20:12:22 -0800 Cong Wang wrote:
> From: Quang Le <quanglex97@gmail.com>
> 
> When limit == 0, pfifo_tail_enqueue() must drop new packet and
> increase dropped packets count of the qdisc.
> 
> All test results:
> 
> 1..16
> ok 1 a519 - Add bfifo qdisc with system default parameters on egress
> ok 2 585c - Add pfifo qdisc with system default parameters on egress
> ok 3 a86e - Add bfifo qdisc with system default parameters on egress with handle of maximum value
> ok 4 9ac8 - Add bfifo qdisc on egress with queue size of 3000 bytes
> ok 5 f4e6 - Add pfifo qdisc on egress with queue size of 3000 packets
> ok 6 b1b1 - Add bfifo qdisc with system default parameters on egress with invalid handle exceeding maximum value
> ok 7 8d5e - Add bfifo qdisc on egress with unsupported argument
> ok 8 7787 - Add pfifo qdisc on egress with unsupported argument
> ok 9 c4b6 - Replace bfifo qdisc on egress with new queue size
> ok 10 3df6 - Replace pfifo qdisc on egress with new queue size
> ok 11 7a67 - Add bfifo qdisc on egress with queue size in invalid format
> ok 12 1298 - Add duplicate bfifo qdisc on egress
> ok 13 45a0 - Delete nonexistent bfifo qdisc
> ok 14 972b - Add prio qdisc on egress with invalid format for handles
> ok 15 4d39 - Delete bfifo qdisc twice
> ok 16 d774 - Check pfifo_head_drop qdisc enqueue behaviour when limit == 0

Same problem as on v1:

# Could not match regex pattern. Verify command output:
# qdisc pfifo_head_drop 1: root refcnt 2 limit 0p
#  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) 
#  backlog 0b 0p requeues 0

https://github.com/p4tc-dev/tc-executor/blob/storage/artifacts/966506/1-tdc-sh/stdout

Did you run the full suite? I wonder if some other test leaks an
interface with a 10.x network.
-- 
pw-bot: cr

Re: [Patch net v2 2/4] selftests/tc-testing: Add a test case for pfifo_head_drop qdisc when limit==0

[Cong Wang]

On Mon, Jan 27, 2025 at 8:57 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sat, 25 Jan 2025 20:12:22 -0800 Cong Wang wrote:
> > From: Quang Le <quanglex97@gmail.com>
> >
> > When limit == 0, pfifo_tail_enqueue() must drop new packet and
> > increase dropped packets count of the qdisc.
> >
> > All test results:
> >
> > 1..16
> > ok 1 a519 - Add bfifo qdisc with system default parameters on egress
> > ok 2 585c - Add pfifo qdisc with system default parameters on egress
> > ok 3 a86e - Add bfifo qdisc with system default parameters on egress with handle of maximum value
> > ok 4 9ac8 - Add bfifo qdisc on egress with queue size of 3000 bytes
> > ok 5 f4e6 - Add pfifo qdisc on egress with queue size of 3000 packets
> > ok 6 b1b1 - Add bfifo qdisc with system default parameters on egress with invalid handle exceeding maximum value
> > ok 7 8d5e - Add bfifo qdisc on egress with unsupported argument
> > ok 8 7787 - Add pfifo qdisc on egress with unsupported argument
> > ok 9 c4b6 - Replace bfifo qdisc on egress with new queue size
> > ok 10 3df6 - Replace pfifo qdisc on egress with new queue size
> > ok 11 7a67 - Add bfifo qdisc on egress with queue size in invalid format
> > ok 12 1298 - Add duplicate bfifo qdisc on egress
> > ok 13 45a0 - Delete nonexistent bfifo qdisc
> > ok 14 972b - Add prio qdisc on egress with invalid format for handles
> > ok 15 4d39 - Delete bfifo qdisc twice
> > ok 16 d774 - Check pfifo_head_drop qdisc enqueue behaviour when limit == 0
>
> Same problem as on v1:
>
> # Could not match regex pattern. Verify command output:
> # qdisc pfifo_head_drop 1: root refcnt 2 limit 0p
> #  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> #  backlog 0b 0p requeues 0
>
> https://github.com/p4tc-dev/tc-executor/blob/storage/artifacts/966506/1-tdc-sh/stdout
>
> Did you run the full suite? I wonder if some other test leaks an
> interface with a 10.x network.

No, I only ran the tests shown above, I will run all the TDC tests.

This is indeed a good hint.

Thanks!

Re: [Patch net v2 2/4] selftests/tc-testing: Add a test case for pfifo_head_drop qdisc when limit==0

[Cong Wang]

On Mon, Jan 27, 2025 at 5:08 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Mon, Jan 27, 2025 at 8:57 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Sat, 25 Jan 2025 20:12:22 -0800 Cong Wang wrote:
> > > From: Quang Le <quanglex97@gmail.com>
> > >
> > > When limit == 0, pfifo_tail_enqueue() must drop new packet and
> > > increase dropped packets count of the qdisc.
> > >
> > > All test results:
> > >
> > > 1..16
> > > ok 1 a519 - Add bfifo qdisc with system default parameters on egress
> > > ok 2 585c - Add pfifo qdisc with system default parameters on egress
> > > ok 3 a86e - Add bfifo qdisc with system default parameters on egress with handle of maximum value
> > > ok 4 9ac8 - Add bfifo qdisc on egress with queue size of 3000 bytes
> > > ok 5 f4e6 - Add pfifo qdisc on egress with queue size of 3000 packets
> > > ok 6 b1b1 - Add bfifo qdisc with system default parameters on egress with invalid handle exceeding maximum value
> > > ok 7 8d5e - Add bfifo qdisc on egress with unsupported argument
> > > ok 8 7787 - Add pfifo qdisc on egress with unsupported argument
> > > ok 9 c4b6 - Replace bfifo qdisc on egress with new queue size
> > > ok 10 3df6 - Replace pfifo qdisc on egress with new queue size
> > > ok 11 7a67 - Add bfifo qdisc on egress with queue size in invalid format
> > > ok 12 1298 - Add duplicate bfifo qdisc on egress
> > > ok 13 45a0 - Delete nonexistent bfifo qdisc
> > > ok 14 972b - Add prio qdisc on egress with invalid format for handles
> > > ok 15 4d39 - Delete bfifo qdisc twice
> > > ok 16 d774 - Check pfifo_head_drop qdisc enqueue behaviour when limit == 0
> >
> > Same problem as on v1:
> >
> > # Could not match regex pattern. Verify command output:
> > # qdisc pfifo_head_drop 1: root refcnt 2 limit 0p
> > #  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> > #  backlog 0b 0p requeues 0
> >
> > https://github.com/p4tc-dev/tc-executor/blob/storage/artifacts/966506/1-tdc-sh/stdout
> >
> > Did you run the full suite? I wonder if some other test leaks an
> > interface with a 10.x network.
>
> No, I only ran the tests shown above, I will run all the TDC tests.

Hmm, I just got another error which prevents me from starting all the tests:

# -----> prepare stage *** Could not execute: "$TC qdisc replace dev
$ETH 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 flags 0x2"
#
# -----> prepare stage *** Error message: "Error: Device does not have
a PTP clock.
# "
#
# -----> prepare stage *** Aborting test run.

Let me see if I can workaround it before looking into it.

Thanks.

Re: [Patch net v2 2/4] selftests/tc-testing: Add a test case for pfifo_head_drop qdisc when limit==0

[Jakub Kicinski]

On Mon, 27 Jan 2025 20:25:09 -0800 Cong Wang wrote:
> > > Same problem as on v1:
> > >
> > > # Could not match regex pattern. Verify command output:
> > > # qdisc pfifo_head_drop 1: root refcnt 2 limit 0p
> > > #  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> > > #  backlog 0b 0p requeues 0
> > >
> > > https://github.com/p4tc-dev/tc-executor/blob/storage/artifacts/966506/1-tdc-sh/stdout
> > >
> > > Did you run the full suite? I wonder if some other test leaks an
> > > interface with a 10.x network.  
> >
> > No, I only ran the tests shown above, I will run all the TDC tests.  
> 
> Hmm, I just got another error which prevents me from starting all the tests:
> 
> # -----> prepare stage *** Could not execute: "$TC qdisc replace dev
> $ETH 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 flags 0x2"
> #
> # -----> prepare stage *** Error message: "Error: Device does not have
> a PTP clock.
> # "
> #
> # -----> prepare stage *** Aborting test run.
> 
> Let me see if I can workaround it before looking into it.

CC Pedro, in case he has cycles to take a look.

Re: [Patch net v2 2/4] selftests/tc-testing: Add a test case for pfifo_head_drop qdisc when limit==0

[Pedro Tammela]

On 26/01/2025 01:12, Cong Wang wrote:
> From: Quang Le <quanglex97@gmail.com>
> 
> When limit == 0, pfifo_tail_enqueue() must drop new packet and
> increase dropped packets count of the qdisc.
> 
> All test results:
> 
> 1..16
> ok 1 a519 - Add bfifo qdisc with system default parameters on egress
> ok 2 585c - Add pfifo qdisc with system default parameters on egress
> ok 3 a86e - Add bfifo qdisc with system default parameters on egress with handle of maximum value
> ok 4 9ac8 - Add bfifo qdisc on egress with queue size of 3000 bytes
> ok 5 f4e6 - Add pfifo qdisc on egress with queue size of 3000 packets
> ok 6 b1b1 - Add bfifo qdisc with system default parameters on egress with invalid handle exceeding maximum value
> ok 7 8d5e - Add bfifo qdisc on egress with unsupported argument
> ok 8 7787 - Add pfifo qdisc on egress with unsupported argument
> ok 9 c4b6 - Replace bfifo qdisc on egress with new queue size
> ok 10 3df6 - Replace pfifo qdisc on egress with new queue size
> ok 11 7a67 - Add bfifo qdisc on egress with queue size in invalid format
> ok 12 1298 - Add duplicate bfifo qdisc on egress
> ok 13 45a0 - Delete nonexistent bfifo qdisc
> ok 14 972b - Add prio qdisc on egress with invalid format for handles
> ok 15 4d39 - Delete bfifo qdisc twice
> ok 16 d774 - Check pfifo_head_drop qdisc enqueue behaviour when limit == 0
> 
> Signed-off-by: Quang Le <quanglex97@gmail.com>
> Signed-off-by: Cong Wang <cong.wang@bytedance.com>
> ---
>   .../tc-testing/tc-tests/qdiscs/fifo.json      | 25 +++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json
> index ae3d286a32b2..94f6456ab460 100644
> --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json
> +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json
> @@ -313,6 +313,31 @@
>           "matchPattern": "qdisc bfifo 1: root",
>           "matchCount": "0",
>           "teardown": [
> +	]
> +    },
> +    {
> +        "id": "d774",
> +        "name": "Check pfifo_head_drop qdisc enqueue behaviour when limit == 0",
> +        "category": [
> +            "qdisc",
> +            "pfifo_head_drop"
> +        ],
> +        "plugins": {
> +            "requires": "nsPlugin"
> +        },
> +        "setup": [
> +            "$IP link add dev $DUMMY mtu 1279 type dummy || true",

You don't need to manually add or remove a dummy device for tdc anymore.
The nsPlugin is responsible for it.

I ran the suite with both of the tests without the link add/del and it's 
working!

Can you try it?

> +            "$IP addr add 10.10.10.10/24 dev $DUMMY || true",
> +            "$TC qdisc add dev $DUMMY root handle 1: pfifo_head_drop limit 0",
> +            "$IP link set dev $DUMMY up || true"
> +        ],
> +        "cmdUnderTest": "ping -c2 -W0.01 -I $DUMMY 10.10.10.1",
> +        "expExitCode": "1",
> +        "verifyCmd": "$TC -s qdisc show dev $DUMMY",
> +        "matchPattern": "dropped 2",
> +        "matchCount": "1",
> +        "teardown": [
> +            "$IP link del dev $DUMMY"
>           ]
>       }
>   ]

Re: [Patch net v2 2/4] selftests/tc-testing: Add a test case for pfifo_head_drop qdisc when limit==0

[Cong Wang]

On Tue, Jan 28, 2025 at 08:19:54PM -0300, Pedro Tammela wrote:
> On 26/01/2025 01:12, Cong Wang wrote:
> > From: Quang Le <quanglex97@gmail.com>
> > 
> > When limit == 0, pfifo_tail_enqueue() must drop new packet and
> > increase dropped packets count of the qdisc.
> > 
> > All test results:
> > 
> > 1..16
> > ok 1 a519 - Add bfifo qdisc with system default parameters on egress
> > ok 2 585c - Add pfifo qdisc with system default parameters on egress
> > ok 3 a86e - Add bfifo qdisc with system default parameters on egress with handle of maximum value
> > ok 4 9ac8 - Add bfifo qdisc on egress with queue size of 3000 bytes
> > ok 5 f4e6 - Add pfifo qdisc on egress with queue size of 3000 packets
> > ok 6 b1b1 - Add bfifo qdisc with system default parameters on egress with invalid handle exceeding maximum value
> > ok 7 8d5e - Add bfifo qdisc on egress with unsupported argument
> > ok 8 7787 - Add pfifo qdisc on egress with unsupported argument
> > ok 9 c4b6 - Replace bfifo qdisc on egress with new queue size
> > ok 10 3df6 - Replace pfifo qdisc on egress with new queue size
> > ok 11 7a67 - Add bfifo qdisc on egress with queue size in invalid format
> > ok 12 1298 - Add duplicate bfifo qdisc on egress
> > ok 13 45a0 - Delete nonexistent bfifo qdisc
> > ok 14 972b - Add prio qdisc on egress with invalid format for handles
> > ok 15 4d39 - Delete bfifo qdisc twice
> > ok 16 d774 - Check pfifo_head_drop qdisc enqueue behaviour when limit == 0
> > 
> > Signed-off-by: Quang Le <quanglex97@gmail.com>
> > Signed-off-by: Cong Wang <cong.wang@bytedance.com>
> > ---
> >   .../tc-testing/tc-tests/qdiscs/fifo.json      | 25 +++++++++++++++++++
> >   1 file changed, 25 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json
> > index ae3d286a32b2..94f6456ab460 100644
> > --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json
> > +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fifo.json
> > @@ -313,6 +313,31 @@
> >           "matchPattern": "qdisc bfifo 1: root",
> >           "matchCount": "0",
> >           "teardown": [
> > +	]
> > +    },
> > +    {
> > +        "id": "d774",
> > +        "name": "Check pfifo_head_drop qdisc enqueue behaviour when limit == 0",
> > +        "category": [
> > +            "qdisc",
> > +            "pfifo_head_drop"
> > +        ],
> > +        "plugins": {
> > +            "requires": "nsPlugin"
> > +        },
> > +        "setup": [
> > +            "$IP link add dev $DUMMY mtu 1279 type dummy || true",
> 
> You don't need to manually add or remove a dummy device for tdc anymore.
> The nsPlugin is responsible for it.

Thanks for the hint!

> 
> I ran the suite with both of the tests without the link add/del and it's
> working!
> 
> Can you try it?

I tried it, but I saw the following error. I am not sure whether this
error is related to these patches.

multiprocessing.pool.RemoteTraceback:
# """
# Traceback (most recent call last):
#   File "/usr/lib64/python3.12/multiprocessing/pool.py", line 125, in worker
#     result = (True, func(*args, **kwds))
#                     ^^^^^^^^^^^^^^^^^^^
#   File "/usr/lib64/python3.12/multiprocessing/pool.py", line 48, in mapstar
#     return list(map(*args))
#            ^^^^^^^^^^^^^^^^
#   File "/host/tools/testing/selftests/tc-testing/./tdc.py", line 601, in __mp_runner
#     (_, tsr) = test_runner(mp_pm, mp_args, tests)
#                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "/host/tools/testing/selftests/tc-testing/./tdc.py", line 535, in test_runner
#     res = run_one_test(pm, args, index, tidx)
#           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "/host/tools/testing/selftests/tc-testing/./tdc.py", line 418, in run_one_test
#     pm.call_pre_case(tidx)
#   File "/host/tools/testing/selftests/tc-testing/./tdc.py", line 142, in call_pre_case
#     pgn_inst.pre_case(caseinfo, test_skip)
#   File "/host/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py", line 63, in pre_case
#     self.prepare_test(test)
#   File "/host/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py", line 36, in prepare_test
#     self._nl_ns_create()
#   File "/host/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py", line 130, in _nl_ns_create
#     ip.link('add', ifname=dev1, kind='veth', peer={'ifname': dev0, 'net_ns_fd':'/proc/1/ns/net'})
#   File "/usr/local/lib/python3.12/site-packages/pyroute2/iproute/linux.py", line 1730, in link
#     ret = self.nlm_request(msg, msg_type=msg_type, msg_flags=msg_flags)
#           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "/usr/local/lib/python3.12/site-packages/pyroute2/netlink/nlsocket.py", line 875, in nlm_request
#     return tuple(self._genlm_request(*argv, **kwarg))
#            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "/usr/local/lib/python3.12/site-packages/pyroute2/netlink/nlsocket.py", line 1248, in nlm_request
#     for msg in self.get(
#                ^^^^^^^^^
#   File "/usr/local/lib/python3.12/site-packages/pyroute2/netlink/nlsocket.py", line 878, in get
#     return tuple(self._genlm_get(*argv, **kwarg))
#            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "/usr/local/lib/python3.12/site-packages/pyroute2/netlink/nlsocket.py", line 555, in get
#     raise msg['header']['error']
# pyroute2.netlink.exceptions.NetlinkError: (17, 'File exists')
# """
#
# The above exception was the direct cause of the following exception:
#
# Traceback (most recent call last):
#   File "/host/tools/testing/selftests/tc-testing/./tdc.py", line 1027, in <module>
#     main()
#   File "/host/tools/testing/selftests/tc-testing/./tdc.py", line 1021, in main
#     set_operation_mode(pm, parser, args, remaining)
#   File "/host/tools/testing/selftests/tc-testing/./tdc.py", line 963, in set_operation_mode
#     catresults = test_runner_mp(pm, args, alltests)
#                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "/host/tools/testing/selftests/tc-testing/./tdc.py", line 623, in test_runner_mp
#     pres = p.map(__mp_runner, batches)
#            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "/usr/lib64/python3.12/multiprocessing/pool.py", line 367, in map
#     return self._map_async(func, iterable, mapstar, chunksize).get()
#            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "/usr/lib64/python3.12/multiprocessing/pool.py", line 774, in get
#     raise self._value
# pyroute2.netlink.exceptions.NetlinkError: (17, 'File exists')