[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20211019153739.446190-1-arnd@kernel.org>
Date: Tue, 19 Oct 2021 17:37:27 +0200
From: Arnd Bergmann <arnd@...nel.org>
To: Jamal Hadi Salim <jhs@...atatu.com>,
Cong Wang <xiyou.wangcong@...il.com>,
Jiri Pirko <jiri@...nulli.us>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
"Ahmed S. Darwish" <a.darwish@...utronix.de>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: Arnd Bergmann <arnd@...db.de>,
Zheng Yongjun <zhengyongjun3@...wei.com>,
Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: [PATCH net-next] net: sched: gred: dynamically allocate tc_gred_qopt_offload
From: Arnd Bergmann <arnd@...db.de>
The tc_gred_qopt_offload structure has grown too big to be on the
stack for 32-bit architectures after recent changes.
net/sched/sch_gred.c:903:13: error: stack frame size (1180) exceeds limit (1024) in 'gred_destroy' [-Werror,-Wframe-larger-than]
net/sched/sch_gred.c:310:13: error: stack frame size (1212) exceeds limit (1024) in 'gred_offload' [-Werror,-Wframe-larger-than]
Use dynamic allocation to avoid this. Unfortunately, this introduces
a new problem in gred_destroy(), which cannot recover from a failure
to allocate memory, and that may be worse than the potential
stack overflow risk.
Not sure what a better approach might be.
Fixes: 50dc9a8572aa ("net: sched: Merge Qdisc::bstats and Qdisc::cpu_bstats data types")
Fixes: 67c9e6270f30 ("net: sched: Protect Qdisc::bstats with u64_stats")
Signed-off-by: Arnd Bergmann <arnd@...db.de>
---
net/sched/sch_gred.c | 64 +++++++++++++++++++++++++-------------------
1 file changed, 36 insertions(+), 28 deletions(-)
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index 72de08ef8335..59c55c6cf3ea 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -307,46 +307,53 @@ static void gred_reset(struct Qdisc *sch)
}
}
-static void gred_offload(struct Qdisc *sch, enum tc_gred_command command)
+static int gred_offload(struct Qdisc *sch, enum tc_gred_command command)
{
struct gred_sched *table = qdisc_priv(sch);
struct net_device *dev = qdisc_dev(sch);
- struct tc_gred_qopt_offload opt = {
- .command = command,
- .handle = sch->handle,
- .parent = sch->parent,
- };
+ struct tc_gred_qopt_offload *opt;
if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
- return;
+ return -ENXIO;
+
+ opt = kzalloc(sizeof(*opt), GFP_KERNEL);
+ if (!opt)
+ return -ENOMEM;
+
+ opt->command = command;
+ opt->handle = sch->handle;
+ opt->parent = sch->parent;
if (command == TC_GRED_REPLACE) {
unsigned int i;
- opt.set.grio_on = gred_rio_mode(table);
- opt.set.wred_on = gred_wred_mode(table);
- opt.set.dp_cnt = table->DPs;
- opt.set.dp_def = table->def;
+ opt->set.grio_on = gred_rio_mode(table);
+ opt->set.wred_on = gred_wred_mode(table);
+ opt->set.dp_cnt = table->DPs;
+ opt->set.dp_def = table->def;
for (i = 0; i < table->DPs; i++) {
struct gred_sched_data *q = table->tab[i];
if (!q)
continue;
- opt.set.tab[i].present = true;
- opt.set.tab[i].limit = q->limit;
- opt.set.tab[i].prio = q->prio;
- opt.set.tab[i].min = q->parms.qth_min >> q->parms.Wlog;
- opt.set.tab[i].max = q->parms.qth_max >> q->parms.Wlog;
- opt.set.tab[i].is_ecn = gred_use_ecn(q);
- opt.set.tab[i].is_harddrop = gred_use_harddrop(q);
- opt.set.tab[i].probability = q->parms.max_P;
- opt.set.tab[i].backlog = &q->backlog;
+ opt->set.tab[i].present = true;
+ opt->set.tab[i].limit = q->limit;
+ opt->set.tab[i].prio = q->prio;
+ opt->set.tab[i].min = q->parms.qth_min >> q->parms.Wlog;
+ opt->set.tab[i].max = q->parms.qth_max >> q->parms.Wlog;
+ opt->set.tab[i].is_ecn = gred_use_ecn(q);
+ opt->set.tab[i].is_harddrop = gred_use_harddrop(q);
+ opt->set.tab[i].probability = q->parms.max_P;
+ opt->set.tab[i].backlog = &q->backlog;
}
- opt.set.qstats = &sch->qstats;
+ opt->set.qstats = &sch->qstats;
}
- dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_GRED, &opt);
+ dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_GRED, opt);
+ kfree(opt);
+
+ return 0;
}
static int gred_offload_dump_stats(struct Qdisc *sch)
@@ -470,8 +477,7 @@ static int gred_change_table_def(struct Qdisc *sch, struct nlattr *dps,
}
}
- gred_offload(sch, TC_GRED_REPLACE);
- return 0;
+ return gred_offload(sch, TC_GRED_REPLACE);
}
static inline int gred_change_vq(struct Qdisc *sch, int dp,
@@ -719,8 +725,7 @@ static int gred_change(struct Qdisc *sch, struct nlattr *opt,
sch_tree_unlock(sch);
kfree(prealloc);
- gred_offload(sch, TC_GRED_REPLACE);
- return 0;
+ return gred_offload(sch, TC_GRED_REPLACE);
err_unlock_free:
sch_tree_unlock(sch);
@@ -903,13 +908,16 @@ static int gred_dump(struct Qdisc *sch, struct sk_buff *skb)
static void gred_destroy(struct Qdisc *sch)
{
struct gred_sched *table = qdisc_priv(sch);
- int i;
+ int i, ret;
for (i = 0; i < table->DPs; i++) {
if (table->tab[i])
gred_destroy_vq(table->tab[i]);
}
- gred_offload(sch, TC_GRED_DESTROY);
+ ret = gred_offload(sch, TC_GRED_DESTROY);
+
+ WARN(ret, "%s: failed to disable offload: %pe\n",
+ __func__, ERR_PTR(ret));
}
static struct Qdisc_ops gred_qdisc_ops __read_mostly = {
--
2.29.2
Powered by blists - more mailing lists