Re: [PATCH] [v2] net: sched: gred: dynamically allocate tc_gred_qopt_offload

On Tue, 19 Oct 2021 21:15:29 +0200 Arnd Bergmann wrote:

From: Arnd Bergmann <arnd@arndb.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 per qdisc to avoid this.

Fixes: 50dc9a8572aa ("net: sched: Merge Qdisc::bstats and Qdisc::cpu_bstats data types")
Fixes: 67c9e6270f30 ("net: sched: Protect Qdisc::bstats with u64_stats")
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Hi Jakub,

Not sure if this is what you had in mind, if not it might be easier
if you do it yourself. In particular, adding tc_gred_qopt_offload
to gred_sched directly rather than as a pointer may be easier here,
but that may have other downsides.

This is exactly what I had in mind, thanks!

Two minor nits if you're willing to respin, if you feel like you've
spent enough time on this already we can marge as is :)

-		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);
+
+	return;

return no longer needed

 }
 
 static int gred_offload_dump_stats(struct Qdisc *sch)

The ops should not change, so I think we can do

	if (qdisc_dev(sch)->netdev_ops->ndo_setup_tc) {

+	table->opt = kzalloc(sizeof(table->opt), GFP_KERNEL);
+	if (!table->opt)
+		return -ENOMEM;
+
 	return gred_change_table_def(sch, tb[TCA_GRED_DPS], extack);
 }