Merge branch 'net_sched-adapt-qdiscs-for-reentrant-enqueue-cases'

Victor Nogueira says:

====================
net_sched: Adapt qdiscs for reentrant enqueue cases

As described in Gerrard's report [1], there are cases where netem can
make the qdisc enqueue callback reentrant. Some qdiscs (drr, hfsc, ets,
qfq) break whenever the enqueue callback has reentrant behaviour.
This series addresses these issues by adding extra checks that cater for
these reentrant corner cases. This series has passed all relevant test
cases in the TDC suite.

[1] https://lore.kernel.org/netdev/CAHcdcOm+03OD2j6R0=YHKqmy=VgJ8xEOKuP6c7mSgnp-TEJJbw@mail.gmail.com/
====================

Link: https://patch.msgid.link/20250425220710.3964791-1-victor@mojatatu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski
2025-04-28 15:55:11 -07:00
5 changed files with 206 additions and 11 deletions
+6 -3
View File
@@ -35,6 +35,11 @@ struct drr_sched {
struct Qdisc_class_hash clhash;
};
static bool cl_is_active(struct drr_class *cl)
{
return !list_empty(&cl->alist);
}
static struct drr_class *drr_find_class(struct Qdisc *sch, u32 classid)
{
struct drr_sched *q = qdisc_priv(sch);
@@ -337,7 +342,6 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct drr_sched *q = qdisc_priv(sch);
struct drr_class *cl;
int err = 0;
bool first;
cl = drr_classify(skb, sch, &err);
if (cl == NULL) {
@@ -347,7 +351,6 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
return err;
}
first = !cl->qdisc->q.qlen;
err = qdisc_enqueue(skb, cl->qdisc, to_free);
if (unlikely(err != NET_XMIT_SUCCESS)) {
if (net_xmit_drop_count(err)) {
@@ -357,7 +360,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
return err;
}
if (first) {
if (!cl_is_active(cl)) {
list_add_tail(&cl->alist, &q->active);
cl->deficit = cl->quantum;
}
+6 -3
View File
@@ -74,6 +74,11 @@ static const struct nla_policy ets_class_policy[TCA_ETS_MAX + 1] = {
[TCA_ETS_QUANTA_BAND] = { .type = NLA_U32 },
};
static bool cl_is_active(struct ets_class *cl)
{
return !list_empty(&cl->alist);
}
static int ets_quantum_parse(struct Qdisc *sch, const struct nlattr *attr,
unsigned int *quantum,
struct netlink_ext_ack *extack)
@@ -416,7 +421,6 @@ static int ets_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct ets_sched *q = qdisc_priv(sch);
struct ets_class *cl;
int err = 0;
bool first;
cl = ets_classify(skb, sch, &err);
if (!cl) {
@@ -426,7 +430,6 @@ static int ets_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
return err;
}
first = !cl->qdisc->q.qlen;
err = qdisc_enqueue(skb, cl->qdisc, to_free);
if (unlikely(err != NET_XMIT_SUCCESS)) {
if (net_xmit_drop_count(err)) {
@@ -436,7 +439,7 @@ static int ets_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
return err;
}
if (first && !ets_class_is_strict(q, cl)) {
if (!cl_is_active(cl) && !ets_class_is_strict(q, cl)) {
list_add_tail(&cl->alist, &q->active);
cl->deficit = cl->quantum;
}
+1 -1
View File
@@ -1569,7 +1569,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
return err;
}
if (first) {
if (first && !cl->cl_nactive) {
if (cl->cl_flags & HFSC_RSC)
init_ed(cl, len);
if (cl->cl_flags & HFSC_FSC)
+7 -4
View File
@@ -202,6 +202,11 @@ struct qfq_sched {
*/
enum update_reason {enqueue, requeue};
static bool cl_is_active(struct qfq_class *cl)
{
return !list_empty(&cl->alist);
}
static struct qfq_class *qfq_find_class(struct Qdisc *sch, u32 classid)
{
struct qfq_sched *q = qdisc_priv(sch);
@@ -1215,7 +1220,6 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct qfq_class *cl;
struct qfq_aggregate *agg;
int err = 0;
bool first;
cl = qfq_classify(skb, sch, &err);
if (cl == NULL) {
@@ -1237,7 +1241,6 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
}
gso_segs = skb_is_gso(skb) ? skb_shinfo(skb)->gso_segs : 1;
first = !cl->qdisc->q.qlen;
err = qdisc_enqueue(skb, cl->qdisc, to_free);
if (unlikely(err != NET_XMIT_SUCCESS)) {
pr_debug("qfq_enqueue: enqueue failed %d\n", err);
@@ -1253,8 +1256,8 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
++sch->q.qlen;
agg = cl->agg;
/* if the queue was not empty, then done here */
if (!first) {
/* if the class is active, then done here */
if (cl_is_active(cl)) {
if (unlikely(skb == cl->qdisc->ops->peek(cl->qdisc)) &&
list_first_entry(&agg->active, struct qfq_class, alist)
== cl && cl->deficit < len)
@@ -352,5 +352,191 @@
"$TC qdisc del dev $DUMMY handle 1:0 root",
"$IP addr del 10.10.10.10/24 dev $DUMMY || true"
]
},
{
"id": "90ec",
"name": "Test DRR's enqueue reentrant behaviour with netem",
"category": [
"qdisc",
"drr"
],
"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 handle 1:0 root drr",
"$TC class replace dev $DUMMY parent 1:0 classid 1:1 drr",
"$TC qdisc add dev $DUMMY parent 1:1 handle 2:0 netem duplicate 100%",
"$TC filter add dev $DUMMY parent 1:0 protocol ip prio 1 u32 match ip protocol 1 0xff flowid 1:1"
],
"cmdUnderTest": "ping -c 1 -I $DUMMY 10.10.10.1 > /dev/null || true",
"expExitCode": "0",
"verifyCmd": "$TC -j -s qdisc ls dev $DUMMY handle 1:0",
"matchJSON": [
{
"kind": "drr",
"handle": "1:",
"bytes": 196,
"packets": 2
}
],
"matchCount": "1",
"teardown": [
"$TC qdisc del dev $DUMMY handle 1:0 root",
"$IP addr del 10.10.10.10/24 dev $DUMMY || true"
]
},
{
"id": "1f1f",
"name": "Test ETS's enqueue reentrant behaviour with netem",
"category": [
"qdisc",
"ets"
],
"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 handle 1:0 root ets bands 2",
"$TC class replace dev $DUMMY parent 1:0 classid 1:1 ets quantum 1500",
"$TC qdisc add dev $DUMMY parent 1:1 handle 2:0 netem duplicate 100%",
"$TC filter add dev $DUMMY parent 1:0 protocol ip prio 1 u32 match ip protocol 1 0xff flowid 1:1"
],
"cmdUnderTest": "ping -c 1 -I $DUMMY 10.10.10.1 > /dev/null || true",
"expExitCode": "0",
"verifyCmd": "$TC -j -s class show dev $DUMMY",
"matchJSON": [
{
"class": "ets",
"handle": "1:1",
"stats": {
"bytes": 196,
"packets": 2
}
}
],
"matchCount": "1",
"teardown": [
"$TC qdisc del dev $DUMMY handle 1:0 root",
"$IP addr del 10.10.10.10/24 dev $DUMMY || true"
]
},
{
"id": "5e6d",
"name": "Test QFQ's enqueue reentrant behaviour with netem",
"category": [
"qdisc",
"qfq"
],
"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 handle 1:0 root qfq",
"$TC class replace dev $DUMMY parent 1:0 classid 1:1 qfq weight 100 maxpkt 1500",
"$TC qdisc add dev $DUMMY parent 1:1 handle 2:0 netem duplicate 100%",
"$TC filter add dev $DUMMY parent 1:0 protocol ip prio 1 u32 match ip protocol 1 0xff flowid 1:1"
],
"cmdUnderTest": "ping -c 1 -I $DUMMY 10.10.10.1 > /dev/null || true",
"expExitCode": "0",
"verifyCmd": "$TC -j -s qdisc ls dev $DUMMY handle 1:0",
"matchJSON": [
{
"kind": "qfq",
"handle": "1:",
"bytes": 196,
"packets": 2
}
],
"matchCount": "1",
"teardown": [
"$TC qdisc del dev $DUMMY handle 1:0 root",
"$IP addr del 10.10.10.10/24 dev $DUMMY || true"
]
},
{
"id": "bf1d",
"name": "Test HFSC's enqueue reentrant behaviour with netem",
"category": [
"qdisc",
"hfsc"
],
"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 handle 1:0 root hfsc",
"$TC class add dev $DUMMY parent 1:0 classid 1:1 hfsc ls m2 10Mbit",
"$TC qdisc add dev $DUMMY parent 1:1 handle 2:0 netem duplicate 100%",
"$TC filter add dev $DUMMY parent 1:0 protocol ip prio 1 u32 match ip dst 10.10.10.1/32 flowid 1:1",
"$TC class add dev $DUMMY parent 1:0 classid 1:2 hfsc ls m2 10Mbit",
"$TC qdisc add dev $DUMMY parent 1:2 handle 3:0 netem duplicate 100%",
"$TC filter add dev $DUMMY parent 1:0 protocol ip prio 2 u32 match ip dst 10.10.10.2/32 flowid 1:2",
"ping -c 1 10.10.10.1 -I$DUMMY > /dev/null || true",
"$TC filter del dev $DUMMY parent 1:0 protocol ip prio 1",
"$TC class del dev $DUMMY classid 1:1"
],
"cmdUnderTest": "ping -c 1 10.10.10.2 -I$DUMMY > /dev/null || true",
"expExitCode": "0",
"verifyCmd": "$TC -j -s qdisc ls dev $DUMMY handle 1:0",
"matchJSON": [
{
"kind": "hfsc",
"handle": "1:",
"bytes": 392,
"packets": 4
}
],
"matchCount": "1",
"teardown": [
"$TC qdisc del dev $DUMMY handle 1:0 root",
"$IP addr del 10.10.10.10/24 dev $DUMMY || true"
]
},
{
"id": "7c3b",
"name": "Test nested DRR's enqueue reentrant behaviour with netem",
"category": [
"qdisc",
"drr"
],
"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 handle 1:0 root drr",
"$TC class add dev $DUMMY parent 1:0 classid 1:1 drr",
"$TC filter add dev $DUMMY parent 1:0 protocol ip prio 1 u32 match ip protocol 1 0xff flowid 1:1",
"$TC qdisc add dev $DUMMY handle 2:0 parent 1:1 drr",
"$TC class add dev $DUMMY classid 2:1 parent 2:0 drr",
"$TC filter add dev $DUMMY parent 2:0 protocol ip prio 1 u32 match ip protocol 1 0xff flowid 2:1",
"$TC qdisc add dev $DUMMY parent 2:1 handle 3:0 netem duplicate 100%"
],
"cmdUnderTest": "ping -c 1 -I $DUMMY 10.10.10.1 > /dev/null || true",
"expExitCode": "0",
"verifyCmd": "$TC -j -s qdisc ls dev $DUMMY handle 1:0",
"matchJSON": [
{
"kind": "drr",
"handle": "1:",
"bytes": 196,
"packets": 2
}
],
"matchCount": "1",
"teardown": [
"$TC qdisc del dev $DUMMY handle 1:0 root",
"$IP addr del 10.10.10.10/24 dev $DUMMY || true"
]
}
]