netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v3 0/4] net/sched: fixes for sch_qfq
@ 2023-07-11 21:00 Pedro Tammela
  2023-07-11 21:01 ` [PATCH net v3 1/4] net/sched: sch_qfq: reintroduce lmax bound check for MTU Pedro Tammela
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Pedro Tammela @ 2023-07-11 21:00 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, shuah,
	shaozhengchao, victor, simon.horman, paolo.valente, Pedro Tammela

Patch 1 fixes a regression introduced in 6.4 where the MTU size could be
bigger than 'lmax'.

Patch 3 fixes an issue where the code doesn't account for qdisc_pkt_len()
returning a size bigger then 'lmax'.

Patches 2 and 4 are selftests for the issues above.

v2 -> v3:
 - Added Eric tags
 - Addressed issue in patch 4 pointed by Shaozheng

v1 -> v2:
 - Added another fix and selftest for sch_qfq
 - Addressed comment by Simon
 - Added Jamal acks and Shaozheng tested by

Pedro Tammela (4):
  net/sched: sch_qfq: reintroduce lmax bound check for MTU
  selftests: tc-testing: add tests for qfq mtu sanity check
  net/sched: sch_qfq: account for stab overhead in qfq_enqueue
  selftests: tc-testing: add test for qfq with stab overhead

 net/sched/sch_qfq.c                           | 18 +++-
 .../tc-testing/tc-tests/qdiscs/qfq.json       | 86 +++++++++++++++++++
 2 files changed, 101 insertions(+), 3 deletions(-)

-- 
2.39.2


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

* [PATCH net v3 1/4] net/sched: sch_qfq: reintroduce lmax bound check for MTU
  2023-07-11 21:00 [PATCH net v3 0/4] net/sched: fixes for sch_qfq Pedro Tammela
@ 2023-07-11 21:01 ` Pedro Tammela
  2023-07-13  8:31   ` Simon Horman
  2023-07-11 21:01 ` [PATCH net v3 2/4] selftests: tc-testing: add tests for qfq mtu sanity check Pedro Tammela
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Pedro Tammela @ 2023-07-11 21:01 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, shuah,
	shaozhengchao, victor, simon.horman, paolo.valente, Pedro Tammela

25369891fcef deletes a check for the case where no 'lmax' is
specified which 3037933448f6 previously fixed as 'lmax'
could be set to the device's MTU without any bound checking
for QFQ_LMAX_MIN and QFQ_LMAX_MAX. Therefore, reintroduce the check.

Fixes: 25369891fcef ("net/sched: sch_qfq: refactor parsing of netlink parameters")
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 net/sched/sch_qfq.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index dfd9a99e6257..63a5b277c117 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -423,10 +423,17 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 	else
 		weight = 1;
 
-	if (tb[TCA_QFQ_LMAX])
+	if (tb[TCA_QFQ_LMAX]) {
 		lmax = nla_get_u32(tb[TCA_QFQ_LMAX]);
-	else
+	} else {
+		/* MTU size is user controlled */
 		lmax = psched_mtu(qdisc_dev(sch));
+		if (lmax < QFQ_MIN_LMAX || lmax > QFQ_MAX_LMAX) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "MTU size out of bounds for qfq");
+			return -EINVAL;
+		}
+	}
 
 	inv_w = ONE_FP / weight;
 	weight = ONE_FP / inv_w;
-- 
2.39.2


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

* [PATCH net v3 2/4] selftests: tc-testing: add tests for qfq mtu sanity check
  2023-07-11 21:00 [PATCH net v3 0/4] net/sched: fixes for sch_qfq Pedro Tammela
  2023-07-11 21:01 ` [PATCH net v3 1/4] net/sched: sch_qfq: reintroduce lmax bound check for MTU Pedro Tammela
@ 2023-07-11 21:01 ` Pedro Tammela
  2023-07-13  8:33   ` Simon Horman
  2023-07-13  9:01   ` shaozhengchao
  2023-07-11 21:01 ` [PATCH net v3 3/4] net/sched: sch_qfq: account for stab overhead in qfq_enqueue Pedro Tammela
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Pedro Tammela @ 2023-07-11 21:01 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, shuah,
	shaozhengchao, victor, simon.horman, paolo.valente, Pedro Tammela

QFQ only supports a certain bound of MTU size so make sure
we check for this requirement in the tests.

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 .../tc-testing/tc-tests/qdiscs/qfq.json       | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
index 147899a868d3..965da7622dac 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
@@ -213,5 +213,53 @@
             "$TC qdisc del dev $DUMMY handle 1: root",
             "$IP link del dev $DUMMY type dummy"
         ]
+    },
+    {
+        "id": "85ee",
+        "name": "QFQ with big MTU",
+        "category": [
+            "qdisc",
+            "qfq"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$IP link add dev $DUMMY type dummy || /bin/true",
+            "$IP link set dev $DUMMY mtu 2147483647 || /bin/true",
+            "$TC qdisc add dev $DUMMY handle 1: root qfq"
+        ],
+        "cmdUnderTest": "$TC class add dev $DUMMY parent 1: classid 1:1 qfq weight 100",
+        "expExitCode": "2",
+        "verifyCmd": "$TC class show dev $DUMMY",
+        "matchPattern": "class qfq 1:",
+        "matchCount": "0",
+        "teardown": [
+            "$IP link del dev $DUMMY type dummy"
+        ]
+    },
+    {
+        "id": "ddfa",
+        "name": "QFQ with small MTU",
+        "category": [
+            "qdisc",
+            "qfq"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$IP link add dev $DUMMY type dummy || /bin/true",
+            "$IP link set dev $DUMMY mtu 256 || /bin/true",
+            "$TC qdisc add dev $DUMMY handle 1: root qfq"
+        ],
+        "cmdUnderTest": "$TC class add dev $DUMMY parent 1: classid 1:1 qfq weight 100",
+        "expExitCode": "2",
+        "verifyCmd": "$TC class show dev $DUMMY",
+        "matchPattern": "class qfq 1:",
+        "matchCount": "0",
+        "teardown": [
+            "$IP link del dev $DUMMY type dummy"
+        ]
     }
 ]
-- 
2.39.2


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

* [PATCH net v3 3/4] net/sched: sch_qfq: account for stab overhead in qfq_enqueue
  2023-07-11 21:00 [PATCH net v3 0/4] net/sched: fixes for sch_qfq Pedro Tammela
  2023-07-11 21:01 ` [PATCH net v3 1/4] net/sched: sch_qfq: reintroduce lmax bound check for MTU Pedro Tammela
  2023-07-11 21:01 ` [PATCH net v3 2/4] selftests: tc-testing: add tests for qfq mtu sanity check Pedro Tammela
@ 2023-07-11 21:01 ` Pedro Tammela
  2023-07-13  8:35   ` Simon Horman
  2023-07-13  8:56   ` Paolo Abeni
  2023-07-11 21:01 ` [PATCH net v3 4/4] selftests: tc-testing: add test for qfq with stab overhead Pedro Tammela
  2023-07-13  9:50 ` [PATCH net v3 0/4] net/sched: fixes for sch_qfq patchwork-bot+netdevbpf
  4 siblings, 2 replies; 13+ messages in thread
From: Pedro Tammela @ 2023-07-11 21:01 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, shuah,
	shaozhengchao, victor, simon.horman, paolo.valente, Pedro Tammela,
	Lion

Lion says:
-------
In the QFQ scheduler a similar issue to CVE-2023-31436
persists.

Consider the following code in net/sched/sch_qfq.c:

static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
                struct sk_buff **to_free)
{
     unsigned int len = qdisc_pkt_len(skb), gso_segs;

    // ...

     if (unlikely(cl->agg->lmax < len)) {
         pr_debug("qfq: increasing maxpkt from %u to %u for class %u",
              cl->agg->lmax, len, cl->common.classid);
         err = qfq_change_agg(sch, cl, cl->agg->class_weight, len);
         if (err) {
             cl->qstats.drops++;
             return qdisc_drop(skb, sch, to_free);
         }

    // ...

     }

Similarly to CVE-2023-31436, "lmax" is increased without any bounds
checks according to the packet length "len". Usually this would not
impose a problem because packet sizes are naturally limited.

This is however not the actual packet length, rather the
"qdisc_pkt_len(skb)" which might apply size transformations according to
"struct qdisc_size_table" as created by "qdisc_get_stab()" in
net/sched/sch_api.c if the TCA_STAB option was set when modifying the qdisc.

A user may choose virtually any size using such a table.

As a result the same issue as in CVE-2023-31436 can occur, allowing heap
out-of-bounds read / writes in the kmalloc-8192 cache.
-------

We can create the issue with the following commands:

tc qdisc add dev $DEV root handle 1: stab mtu 2048 tsize 512 mpu 0 \
overhead 999999999 linklayer ethernet qfq
tc class add dev $DEV parent 1: classid 1:1 htb rate 6mbit burst 15k
tc filter add dev $DEV parent 1: matchall classid 1:1
ping -I $DEV 1.1.1.2

This is caused by incorrectly assuming that qdisc_pkt_len() returns a
length within the QFQ_MIN_LMAX < len < QFQ_MAX_LMAX.

Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
Reported-by: Lion <nnamrec@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 net/sched/sch_qfq.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 63a5b277c117..befaf74b33ca 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -381,8 +381,13 @@ static int qfq_change_agg(struct Qdisc *sch, struct qfq_class *cl, u32 weight,
 			   u32 lmax)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
-	struct qfq_aggregate *new_agg = qfq_find_agg(q, lmax, weight);
+	struct qfq_aggregate *new_agg;
 
+	/* 'lmax' can range from [QFQ_MIN_LMAX, pktlen + stab overhead] */
+	if (lmax > QFQ_MAX_LMAX)
+		return -EINVAL;
+
+	new_agg = qfq_find_agg(q, lmax, weight);
 	if (new_agg == NULL) { /* create new aggregate */
 		new_agg = kzalloc(sizeof(*new_agg), GFP_ATOMIC);
 		if (new_agg == NULL)
-- 
2.39.2


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

* [PATCH net v3 4/4] selftests: tc-testing: add test for qfq with stab overhead
  2023-07-11 21:00 [PATCH net v3 0/4] net/sched: fixes for sch_qfq Pedro Tammela
                   ` (2 preceding siblings ...)
  2023-07-11 21:01 ` [PATCH net v3 3/4] net/sched: sch_qfq: account for stab overhead in qfq_enqueue Pedro Tammela
@ 2023-07-11 21:01 ` Pedro Tammela
  2023-07-13  8:36   ` Simon Horman
  2023-07-13  9:05   ` shaozhengchao
  2023-07-13  9:50 ` [PATCH net v3 0/4] net/sched: fixes for sch_qfq patchwork-bot+netdevbpf
  4 siblings, 2 replies; 13+ messages in thread
From: Pedro Tammela @ 2023-07-11 21:01 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, shuah,
	shaozhengchao, victor, simon.horman, paolo.valente, Pedro Tammela

A packet with stab overhead greater than QFQ_MAX_LMAX should be dropped
by the QFQ qdisc as it can't handle such lengths.

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 .../tc-testing/tc-tests/qdiscs/qfq.json       | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
index 965da7622dac..976dffda4654 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
@@ -261,5 +261,43 @@
         "teardown": [
             "$IP link del dev $DUMMY type dummy"
         ]
+    },
+    {
+        "id": "5993",
+        "name": "QFQ with stab overhead greater than max packet len",
+        "category": [
+            "qdisc",
+            "qfq",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            "$IP link add dev $DUMMY type dummy || /bin/true",
+            "$IP link set dev $DUMMY up || /bin/true",
+            "$TC qdisc add dev $DUMMY handle 1: stab mtu 2048 tsize 512 mpu 0 overhead 999999999 linklayer ethernet root qfq",
+            "$TC class add dev $DUMMY parent 1: classid 1:1 qfq weight 100",
+            "$TC qdisc add dev $DEV1 clsact",
+            "$TC filter add dev $DEV1 ingress protocol ip flower dst_ip 1.3.3.7/32 action mirred egress mirror dev $DUMMY"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DUMMY parent 1: matchall classid 1:1",
+        "scapy": [
+            {
+                "iface": "$DEV0",
+                "count": 22,
+                "packet": "Ether(type=0x800)/IP(src='10.0.0.10',dst='1.3.3.7')/TCP(sport=5000,dport=10)"
+            }
+        ],
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s qdisc ls dev $DUMMY",
+        "matchPattern": "dropped 22",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root qfq"
+        ]
     }
 ]
-- 
2.39.2


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

* Re: [PATCH net v3 1/4] net/sched: sch_qfq: reintroduce lmax bound check for MTU
  2023-07-11 21:01 ` [PATCH net v3 1/4] net/sched: sch_qfq: reintroduce lmax bound check for MTU Pedro Tammela
@ 2023-07-13  8:31   ` Simon Horman
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Horman @ 2023-07-13  8:31 UTC (permalink / raw)
  To: Pedro Tammela
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
	shuah, shaozhengchao, victor, paolo.valente

On Tue, Jul 11, 2023 at 06:01:00PM -0300, Pedro Tammela wrote:
> 25369891fcef deletes a check for the case where no 'lmax' is
> specified which 3037933448f6 previously fixed as 'lmax'
> could be set to the device's MTU without any bound checking
> for QFQ_LMAX_MIN and QFQ_LMAX_MAX. Therefore, reintroduce the check.
> 
> Fixes: 25369891fcef ("net/sched: sch_qfq: refactor parsing of netlink parameters")
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


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

* Re: [PATCH net v3 2/4] selftests: tc-testing: add tests for qfq mtu sanity check
  2023-07-11 21:01 ` [PATCH net v3 2/4] selftests: tc-testing: add tests for qfq mtu sanity check Pedro Tammela
@ 2023-07-13  8:33   ` Simon Horman
  2023-07-13  9:01   ` shaozhengchao
  1 sibling, 0 replies; 13+ messages in thread
From: Simon Horman @ 2023-07-13  8:33 UTC (permalink / raw)
  To: Pedro Tammela
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
	shuah, shaozhengchao, victor, paolo.valente

On Tue, Jul 11, 2023 at 06:01:01PM -0300, Pedro Tammela wrote:
> QFQ only supports a certain bound of MTU size so make sure
> we check for this requirement in the tests.
> 
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


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

* Re: [PATCH net v3 3/4] net/sched: sch_qfq: account for stab overhead in qfq_enqueue
  2023-07-11 21:01 ` [PATCH net v3 3/4] net/sched: sch_qfq: account for stab overhead in qfq_enqueue Pedro Tammela
@ 2023-07-13  8:35   ` Simon Horman
  2023-07-13  8:56   ` Paolo Abeni
  1 sibling, 0 replies; 13+ messages in thread
From: Simon Horman @ 2023-07-13  8:35 UTC (permalink / raw)
  To: Pedro Tammela
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
	shuah, shaozhengchao, victor, paolo.valente, Lion

On Tue, Jul 11, 2023 at 06:01:02PM -0300, Pedro Tammela wrote:
> Lion says:
> -------
> In the QFQ scheduler a similar issue to CVE-2023-31436
> persists.
> 
> Consider the following code in net/sched/sch_qfq.c:
> 
> static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
>                 struct sk_buff **to_free)
> {
>      unsigned int len = qdisc_pkt_len(skb), gso_segs;
> 
>     // ...
> 
>      if (unlikely(cl->agg->lmax < len)) {
>          pr_debug("qfq: increasing maxpkt from %u to %u for class %u",
>               cl->agg->lmax, len, cl->common.classid);
>          err = qfq_change_agg(sch, cl, cl->agg->class_weight, len);
>          if (err) {
>              cl->qstats.drops++;
>              return qdisc_drop(skb, sch, to_free);
>          }
> 
>     // ...
> 
>      }
> 
> Similarly to CVE-2023-31436, "lmax" is increased without any bounds
> checks according to the packet length "len". Usually this would not
> impose a problem because packet sizes are naturally limited.
> 
> This is however not the actual packet length, rather the
> "qdisc_pkt_len(skb)" which might apply size transformations according to
> "struct qdisc_size_table" as created by "qdisc_get_stab()" in
> net/sched/sch_api.c if the TCA_STAB option was set when modifying the qdisc.
> 
> A user may choose virtually any size using such a table.
> 
> As a result the same issue as in CVE-2023-31436 can occur, allowing heap
> out-of-bounds read / writes in the kmalloc-8192 cache.
> -------
> 
> We can create the issue with the following commands:
> 
> tc qdisc add dev $DEV root handle 1: stab mtu 2048 tsize 512 mpu 0 \
> overhead 999999999 linklayer ethernet qfq
> tc class add dev $DEV parent 1: classid 1:1 htb rate 6mbit burst 15k
> tc filter add dev $DEV parent 1: matchall classid 1:1
> ping -I $DEV 1.1.1.2
> 
> This is caused by incorrectly assuming that qdisc_pkt_len() returns a
> length within the QFQ_MIN_LMAX < len < QFQ_MAX_LMAX.
> 
> Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
> Reported-by: Lion <nnamrec@gmail.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


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

* Re: [PATCH net v3 4/4] selftests: tc-testing: add test for qfq with stab overhead
  2023-07-11 21:01 ` [PATCH net v3 4/4] selftests: tc-testing: add test for qfq with stab overhead Pedro Tammela
@ 2023-07-13  8:36   ` Simon Horman
  2023-07-13  9:05   ` shaozhengchao
  1 sibling, 0 replies; 13+ messages in thread
From: Simon Horman @ 2023-07-13  8:36 UTC (permalink / raw)
  To: Pedro Tammela
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
	shuah, shaozhengchao, victor, paolo.valente

On Tue, Jul 11, 2023 at 06:01:03PM -0300, Pedro Tammela wrote:
> A packet with stab overhead greater than QFQ_MAX_LMAX should be dropped
> by the QFQ qdisc as it can't handle such lengths.
> 
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


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

* Re: [PATCH net v3 3/4] net/sched: sch_qfq: account for stab overhead in qfq_enqueue
  2023-07-11 21:01 ` [PATCH net v3 3/4] net/sched: sch_qfq: account for stab overhead in qfq_enqueue Pedro Tammela
  2023-07-13  8:35   ` Simon Horman
@ 2023-07-13  8:56   ` Paolo Abeni
  1 sibling, 0 replies; 13+ messages in thread
From: Paolo Abeni @ 2023-07-13  8:56 UTC (permalink / raw)
  To: Pedro Tammela, netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, shuah,
	shaozhengchao, victor, simon.horman, paolo.valente, Lion

On Tue, 2023-07-11 at 18:01 -0300, Pedro Tammela wrote:
> Lion says:
> -------

For the records, I feared the above separator could confuse git am -
dropping the remaining part of the commit message - but empirical
testing showed no issues;).

Cheers,

Paolo


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

* Re: [PATCH net v3 2/4] selftests: tc-testing: add tests for qfq mtu sanity check
  2023-07-11 21:01 ` [PATCH net v3 2/4] selftests: tc-testing: add tests for qfq mtu sanity check Pedro Tammela
  2023-07-13  8:33   ` Simon Horman
@ 2023-07-13  9:01   ` shaozhengchao
  1 sibling, 0 replies; 13+ messages in thread
From: shaozhengchao @ 2023-07-13  9:01 UTC (permalink / raw)
  To: Pedro Tammela, netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, shuah,
	victor, simon.horman, paolo.valente



On 2023/7/12 5:01, Pedro Tammela wrote:
> QFQ only supports a certain bound of MTU size so make sure
> we check for this requirement in the tests.
> 
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
> ---
>   .../tc-testing/tc-tests/qdiscs/qfq.json       | 48 +++++++++++++++++++
>   1 file changed, 48 insertions(+)
> 
> diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
> index 147899a868d3..965da7622dac 100644
> --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
> +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
> @@ -213,5 +213,53 @@
>               "$TC qdisc del dev $DUMMY handle 1: root",
>               "$IP link del dev $DUMMY type dummy"
>           ]
> +    },
> +    {
> +        "id": "85ee",
> +        "name": "QFQ with big MTU",
> +        "category": [
> +            "qdisc",
> +            "qfq"
> +        ],
> +        "plugins": {
> +            "requires": "nsPlugin"
> +        },
> +        "setup": [
> +            "$IP link add dev $DUMMY type dummy || /bin/true",
> +            "$IP link set dev $DUMMY mtu 2147483647 || /bin/true",
> +            "$TC qdisc add dev $DUMMY handle 1: root qfq"
> +        ],
> +        "cmdUnderTest": "$TC class add dev $DUMMY parent 1: classid 1:1 qfq weight 100",
> +        "expExitCode": "2",
> +        "verifyCmd": "$TC class show dev $DUMMY",
> +        "matchPattern": "class qfq 1:",
> +        "matchCount": "0",
> +        "teardown": [
> +            "$IP link del dev $DUMMY type dummy"
> +        ]
> +    },
> +    {
> +        "id": "ddfa",
> +        "name": "QFQ with small MTU",
> +        "category": [
> +            "qdisc",
> +            "qfq"
> +        ],
> +        "plugins": {
> +            "requires": "nsPlugin"
> +        },
> +        "setup": [
> +            "$IP link add dev $DUMMY type dummy || /bin/true",
> +            "$IP link set dev $DUMMY mtu 256 || /bin/true",
> +            "$TC qdisc add dev $DUMMY handle 1: root qfq"
> +        ],
> +        "cmdUnderTest": "$TC class add dev $DUMMY parent 1: classid 1:1 qfq weight 100",
> +        "expExitCode": "2",
> +        "verifyCmd": "$TC class show dev $DUMMY",
> +        "matchPattern": "class qfq 1:",
> +        "matchCount": "0",
> +        "teardown": [
> +            "$IP link del dev $DUMMY type dummy"
> +        ]
>       }
>   ]

Tested-by: Zhengchao Shao <shaozhengchao@huawei.com>

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

* Re: [PATCH net v3 4/4] selftests: tc-testing: add test for qfq with stab overhead
  2023-07-11 21:01 ` [PATCH net v3 4/4] selftests: tc-testing: add test for qfq with stab overhead Pedro Tammela
  2023-07-13  8:36   ` Simon Horman
@ 2023-07-13  9:05   ` shaozhengchao
  1 sibling, 0 replies; 13+ messages in thread
From: shaozhengchao @ 2023-07-13  9:05 UTC (permalink / raw)
  To: Pedro Tammela, netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, shuah,
	victor, simon.horman, paolo.valente



On 2023/7/12 5:01, Pedro Tammela wrote:
> A packet with stab overhead greater than QFQ_MAX_LMAX should be dropped
> by the QFQ qdisc as it can't handle such lengths.
> 
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
> ---
>   .../tc-testing/tc-tests/qdiscs/qfq.json       | 38 +++++++++++++++++++
>   1 file changed, 38 insertions(+)
> 
> diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
> index 965da7622dac..976dffda4654 100644
> --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
> +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
> @@ -261,5 +261,43 @@
>           "teardown": [
>               "$IP link del dev $DUMMY type dummy"
>           ]
> +    },
> +    {
> +        "id": "5993",
> +        "name": "QFQ with stab overhead greater than max packet len",
> +        "category": [
> +            "qdisc",
> +            "qfq",
> +            "scapy"
> +        ],
> +        "plugins": {
> +            "requires": [
> +                "nsPlugin",
> +                "scapyPlugin"
> +            ]
> +        },
> +        "setup": [
> +            "$IP link add dev $DUMMY type dummy || /bin/true",
> +            "$IP link set dev $DUMMY up || /bin/true",
> +            "$TC qdisc add dev $DUMMY handle 1: stab mtu 2048 tsize 512 mpu 0 overhead 999999999 linklayer ethernet root qfq",
> +            "$TC class add dev $DUMMY parent 1: classid 1:1 qfq weight 100",
> +            "$TC qdisc add dev $DEV1 clsact",
> +            "$TC filter add dev $DEV1 ingress protocol ip flower dst_ip 1.3.3.7/32 action mirred egress mirror dev $DUMMY"
> +        ],
> +        "cmdUnderTest": "$TC filter add dev $DUMMY parent 1: matchall classid 1:1",
> +        "scapy": [
> +            {
> +                "iface": "$DEV0",
> +                "count": 22,
> +                "packet": "Ether(type=0x800)/IP(src='10.0.0.10',dst='1.3.3.7')/TCP(sport=5000,dport=10)"
> +            }
> +        ],
> +        "expExitCode": "0",
> +        "verifyCmd": "$TC -s qdisc ls dev $DUMMY",
> +        "matchPattern": "dropped 22",
> +        "matchCount": "1",
> +        "teardown": [
> +            "$TC qdisc del dev $DUMMY handle 1: root qfq"
> +        ]
>       }
>   ]
Hi Pedro:
I'm confused about whether the following output is necessary during the
test.
Sent 1 packets.
.
Sent 1 packets.
.
...
Otherwise, it looks good to me.

Tested-by: Zhengchao Shao <shaozhengchao@huawei.com>


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

* Re: [PATCH net v3 0/4] net/sched: fixes for sch_qfq
  2023-07-11 21:00 [PATCH net v3 0/4] net/sched: fixes for sch_qfq Pedro Tammela
                   ` (3 preceding siblings ...)
  2023-07-11 21:01 ` [PATCH net v3 4/4] selftests: tc-testing: add test for qfq with stab overhead Pedro Tammela
@ 2023-07-13  9:50 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-07-13  9:50 UTC (permalink / raw)
  To: Pedro Tammela
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
	shuah, shaozhengchao, victor, simon.horman, paolo.valente

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 11 Jul 2023 18:00:59 -0300 you wrote:
> Patch 1 fixes a regression introduced in 6.4 where the MTU size could be
> bigger than 'lmax'.
> 
> Patch 3 fixes an issue where the code doesn't account for qdisc_pkt_len()
> returning a size bigger then 'lmax'.
> 
> Patches 2 and 4 are selftests for the issues above.
> 
> [...]

Here is the summary with links:
  - [net,v3,1/4] net/sched: sch_qfq: reintroduce lmax bound check for MTU
    https://git.kernel.org/netdev/net/c/158810b261d0
  - [net,v3,2/4] selftests: tc-testing: add tests for qfq mtu sanity check
    https://git.kernel.org/netdev/net/c/c5a06fdc618d
  - [net,v3,3/4] net/sched: sch_qfq: account for stab overhead in qfq_enqueue
    https://git.kernel.org/netdev/net/c/3e337087c3b5
  - [net,v3,4/4] selftests: tc-testing: add test for qfq with stab overhead
    https://git.kernel.org/netdev/net/c/137f6219da59

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2023-07-13  9:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-11 21:00 [PATCH net v3 0/4] net/sched: fixes for sch_qfq Pedro Tammela
2023-07-11 21:01 ` [PATCH net v3 1/4] net/sched: sch_qfq: reintroduce lmax bound check for MTU Pedro Tammela
2023-07-13  8:31   ` Simon Horman
2023-07-11 21:01 ` [PATCH net v3 2/4] selftests: tc-testing: add tests for qfq mtu sanity check Pedro Tammela
2023-07-13  8:33   ` Simon Horman
2023-07-13  9:01   ` shaozhengchao
2023-07-11 21:01 ` [PATCH net v3 3/4] net/sched: sch_qfq: account for stab overhead in qfq_enqueue Pedro Tammela
2023-07-13  8:35   ` Simon Horman
2023-07-13  8:56   ` Paolo Abeni
2023-07-11 21:01 ` [PATCH net v3 4/4] selftests: tc-testing: add test for qfq with stab overhead Pedro Tammela
2023-07-13  8:36   ` Simon Horman
2023-07-13  9:05   ` shaozhengchao
2023-07-13  9:50 ` [PATCH net v3 0/4] net/sched: fixes for sch_qfq patchwork-bot+netdevbpf

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).