Commit 3a62fed2 authored by Davide Caratti's avatar Davide Caratti Committed by David S. Miller

net/sched: fq_pie: re-factor fix for fq_pie endless loop

the patch that fixed an endless loop in_fq_pie_init() was not considering
that 65535 is a valid class id. The correct bugfix for this infinite loop
is to change 'idx' to become an u32, like Colin proposed in the past [1].

Fix this as follows:
 - restore 65536 as maximum possible values of 'flows_cnt'
 - use u32 'idx' when iterating on 'q->flows'
 - fix the TDC selftest

This reverts commit bb2f930d.

[1] https://lore.kernel.org/netdev/20210407163808.499027-1-colin.king@canonical.com/

CC: Colin Ian King <colin.king@canonical.com>
CC: stable@vger.kernel.org
Fixes: bb2f930d ("net/sched: fix infinite loop in sch_fq_pie")
Fixes: ec97ecf1 ("net: sched: add Flow Queue PIE packet scheduler")
Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 5eff1461
...@@ -297,9 +297,9 @@ static int fq_pie_change(struct Qdisc *sch, struct nlattr *opt, ...@@ -297,9 +297,9 @@ static int fq_pie_change(struct Qdisc *sch, struct nlattr *opt,
goto flow_error; goto flow_error;
} }
q->flows_cnt = nla_get_u32(tb[TCA_FQ_PIE_FLOWS]); q->flows_cnt = nla_get_u32(tb[TCA_FQ_PIE_FLOWS]);
if (!q->flows_cnt || q->flows_cnt >= 65536) { if (!q->flows_cnt || q->flows_cnt > 65536) {
NL_SET_ERR_MSG_MOD(extack, NL_SET_ERR_MSG_MOD(extack,
"Number of flows must range in [1..65535]"); "Number of flows must range in [1..65536]");
goto flow_error; goto flow_error;
} }
} }
...@@ -367,7 +367,7 @@ static void fq_pie_timer(struct timer_list *t) ...@@ -367,7 +367,7 @@ static void fq_pie_timer(struct timer_list *t)
struct fq_pie_sched_data *q = from_timer(q, t, adapt_timer); struct fq_pie_sched_data *q = from_timer(q, t, adapt_timer);
struct Qdisc *sch = q->sch; struct Qdisc *sch = q->sch;
spinlock_t *root_lock; /* to lock qdisc for probability calculations */ spinlock_t *root_lock; /* to lock qdisc for probability calculations */
u16 idx; u32 idx;
root_lock = qdisc_lock(qdisc_root_sleeping(sch)); root_lock = qdisc_lock(qdisc_root_sleeping(sch));
spin_lock(root_lock); spin_lock(root_lock);
...@@ -388,7 +388,7 @@ static int fq_pie_init(struct Qdisc *sch, struct nlattr *opt, ...@@ -388,7 +388,7 @@ static int fq_pie_init(struct Qdisc *sch, struct nlattr *opt,
{ {
struct fq_pie_sched_data *q = qdisc_priv(sch); struct fq_pie_sched_data *q = qdisc_priv(sch);
int err; int err;
u16 idx; u32 idx;
pie_params_init(&q->p_params); pie_params_init(&q->p_params);
sch->limit = 10 * 1024; sch->limit = 10 * 1024;
...@@ -500,7 +500,7 @@ static int fq_pie_dump_stats(struct Qdisc *sch, struct gnet_dump *d) ...@@ -500,7 +500,7 @@ static int fq_pie_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
static void fq_pie_reset(struct Qdisc *sch) static void fq_pie_reset(struct Qdisc *sch)
{ {
struct fq_pie_sched_data *q = qdisc_priv(sch); struct fq_pie_sched_data *q = qdisc_priv(sch);
u16 idx; u32 idx;
INIT_LIST_HEAD(&q->new_flows); INIT_LIST_HEAD(&q->new_flows);
INIT_LIST_HEAD(&q->old_flows); INIT_LIST_HEAD(&q->old_flows);
......
...@@ -9,11 +9,11 @@ ...@@ -9,11 +9,11 @@
"setup": [ "setup": [
"$IP link add dev $DUMMY type dummy || /bin/true" "$IP link add dev $DUMMY type dummy || /bin/true"
], ],
"cmdUnderTest": "$TC qdisc add dev $DUMMY root fq_pie flows 65536", "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root fq_pie flows 65536",
"expExitCode": "2", "expExitCode": "0",
"verifyCmd": "$TC qdisc show dev $DUMMY", "verifyCmd": "$TC qdisc show dev $DUMMY",
"matchPattern": "qdisc", "matchPattern": "qdisc fq_pie 1: root refcnt 2 limit 10240p flows 65536",
"matchCount": "0", "matchCount": "1",
"teardown": [ "teardown": [
"$IP link del dev $DUMMY" "$IP link del dev $DUMMY"
] ]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment