| Message ID | 20200529201413.397679-1-arnd@arndb.de |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | flow_dissector: work around stack frame size warning | expand |
On Fri, May 29, 2020 at 1:14 PM Arnd Bergmann <arnd@arndb.de> wrote: > > The fl_flow_key structure is around 500 bytes, so having two of them > on the stack in one function now exceeds the warning limit after an > otherwise correct change: > > net/sched/cls_flower.c:298:12: error: stack frame size of 1056 bytes in function 'fl_classify' [-Werror,-Wframe-larger-than=] > > I suspect the fl_classify function could be reworked to only have one > of them on the stack and modify it in place, but I could not work out > how to do that. > > As a somewhat hacky workaround, move one of them into an out-of-line > function to reduce its scope. This does not necessarily reduce the stack > usage of the outer function, but at least the second copy is removed > from the stack during most of it and does not add up to whatever is > called from there. > > I now see 552 bytes of stack usage for fl_classify(), plus 528 bytes > for fl_mask_lookup(). > > Fixes: 58cff782cc55 ("flow_dissector: Parse multiple MPLS Label Stack Entries") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> I think this is probably the quickest way to amend this warning, so: Acked-by: Cong Wang <xiyou.wangcong@gmail.com> Thanks.
On Fri, May 29, 2020 at 10:13:58PM +0200, Arnd Bergmann wrote: > The fl_flow_key structure is around 500 bytes, so having two of them > on the stack in one function now exceeds the warning limit after an > otherwise correct change: > > net/sched/cls_flower.c:298:12: error: stack frame size of 1056 bytes in function 'fl_classify' [-Werror,-Wframe-larger-than=] > > I suspect the fl_classify function could be reworked to only have one > of them on the stack and modify it in place, but I could not work out > how to do that. > > As a somewhat hacky workaround, move one of them into an out-of-line > function to reduce its scope. This does not necessarily reduce the stack > usage of the outer function, but at least the second copy is removed > from the stack during most of it and does not add up to whatever is > called from there. > > I now see 552 bytes of stack usage for fl_classify(), plus 528 bytes > for fl_mask_lookup(). > > Fixes: 58cff782cc55 ("flow_dissector: Parse multiple MPLS Label Stack Entries") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Sorry, I didn't see that, as my .config has CONFIG_FRAME_WARN=2048, which seems to be the default for x86_64. Acked-by: Guillaume Nault <gnault@redhat.com>
From: Arnd Bergmann <arnd@arndb.de> Date: Fri, 29 May 2020 22:13:58 +0200 > The fl_flow_key structure is around 500 bytes, so having two of them > on the stack in one function now exceeds the warning limit after an > otherwise correct change: > > net/sched/cls_flower.c:298:12: error: stack frame size of 1056 bytes in function 'fl_classify' [-Werror,-Wframe-larger-than=] > > I suspect the fl_classify function could be reworked to only have one > of them on the stack and modify it in place, but I could not work out > how to do that. > > As a somewhat hacky workaround, move one of them into an out-of-line > function to reduce its scope. This does not necessarily reduce the stack > usage of the outer function, but at least the second copy is removed > from the stack during most of it and does not add up to whatever is > called from there. > > I now see 552 bytes of stack usage for fl_classify(), plus 528 bytes > for fl_mask_lookup(). > > Fixes: 58cff782cc55 ("flow_dissector: Parse multiple MPLS Label Stack Entries") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Applied, thanks.
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index 96f5999281e0..030896eadd11 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -272,14 +272,16 @@ static struct cls_fl_filter *fl_lookup_range(struct fl_flow_mask *mask, return NULL; } -static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask, - struct fl_flow_key *mkey, - struct fl_flow_key *key) +static noinline_for_stack +struct cls_fl_filter *fl_mask_lookup(struct fl_flow_mask *mask, struct fl_flow_key *key) { + struct fl_flow_key mkey; + + fl_set_masked_key(&mkey, key, mask); if ((mask->flags & TCA_FLOWER_MASK_FLAGS_RANGE)) - return fl_lookup_range(mask, mkey, key); + return fl_lookup_range(mask, &mkey, key); - return __fl_lookup(mask, mkey); + return __fl_lookup(mask, &mkey); } static u16 fl_ct_info_to_flower_map[] = { @@ -299,7 +301,6 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp, struct tcf_result *res) { struct cls_fl_head *head = rcu_dereference_bh(tp->root); - struct fl_flow_key skb_mkey; struct fl_flow_key skb_key; struct fl_flow_mask *mask; struct cls_fl_filter *f; @@ -319,9 +320,7 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp, ARRAY_SIZE(fl_ct_info_to_flower_map)); skb_flow_dissect(skb, &mask->dissector, &skb_key, 0); - fl_set_masked_key(&skb_mkey, &skb_key, mask); - - f = fl_lookup(mask, &skb_mkey, &skb_key); + f = fl_mask_lookup(mask, &skb_key); if (f && !tc_skip_sw(f->flags)) { *res = f->res; return tcf_exts_exec(skb, &f->exts, res);
The fl_flow_key structure is around 500 bytes, so having two of them on the stack in one function now exceeds the warning limit after an otherwise correct change: net/sched/cls_flower.c:298:12: error: stack frame size of 1056 bytes in function 'fl_classify' [-Werror,-Wframe-larger-than=] I suspect the fl_classify function could be reworked to only have one of them on the stack and modify it in place, but I could not work out how to do that. As a somewhat hacky workaround, move one of them into an out-of-line function to reduce its scope. This does not necessarily reduce the stack usage of the outer function, but at least the second copy is removed from the stack during most of it and does not add up to whatever is called from there. I now see 552 bytes of stack usage for fl_classify(), plus 528 bytes for fl_mask_lookup(). Fixes: 58cff782cc55 ("flow_dissector: Parse multiple MPLS Label Stack Entries") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- net/sched/cls_flower.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)