* [PATCH net] net/sched: cls_flower: allocate mask dynamically in fl_change()
@ 2019-01-16 15:02 Ivan Vecera
2019-01-16 15:24 ` Jiri Pirko
0 siblings, 1 reply; 3+ messages in thread
From: Ivan Vecera @ 2019-01-16 15:02 UTC (permalink / raw)
To: netdev; +Cc: Petr Oros, Jiri Pirko, Paul Blakey
Recent changes (especially 05cd271fd61a ("cls_flower: Support multiple
masks per priority")) in the fl_flow_mask structure grow it and its
current size e.g. on x86_64 with defconfig is 760 bytes and more than
1024 bytes with some debug options enabled. Prior the mentioned commit
its size was 176 bytes (using defconfig on x86_64).
With regard to this fact it's reasonable to allocate this structure
dynamically in fl_change() to reduce its stack size.
Fixes: 05cd271fd61a ("cls_flower: Support multiple masks per priority")
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: Paul Blakey <paulb@mellanox.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
net/sched/cls_flower.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index dad04e710493..47ac88cc34fb 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1291,16 +1291,22 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
struct cls_fl_filter *fold = *arg;
struct cls_fl_filter *fnew;
struct nlattr **tb;
- struct fl_flow_mask mask = {};
+ struct fl_flow_mask *mask;
int err;
if (!tca[TCA_OPTIONS])
return -EINVAL;
- tb = kcalloc(TCA_FLOWER_MAX + 1, sizeof(struct nlattr *), GFP_KERNEL);
- if (!tb)
+ mask = kcalloc(1, sizeof(struct fl_flow_mask), GFP_KERNEL);
+ if (!mask)
return -ENOBUFS;
+ tb = kcalloc(TCA_FLOWER_MAX + 1, sizeof(struct nlattr *), GFP_KERNEL);
+ if (!tb) {
+ err = -ENOBUFS;
+ goto errout_mask_alloc;
+ }
+
err = nla_parse_nested(tb, TCA_FLOWER_MAX, tca[TCA_OPTIONS],
fl_policy, NULL);
if (err < 0)
@@ -1343,12 +1349,12 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
}
}
- err = fl_set_parms(net, tp, fnew, &mask, base, tb, tca[TCA_RATE], ovr,
+ err = fl_set_parms(net, tp, fnew, mask, base, tb, tca[TCA_RATE], ovr,
tp->chain->tmplt_priv, extack);
if (err)
goto errout_idr;
- err = fl_check_assign_mask(head, fnew, fold, &mask);
+ err = fl_check_assign_mask(head, fnew, fold, mask);
if (err)
goto errout_idr;
@@ -1392,6 +1398,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
}
kfree(tb);
+ kfree(mask);
return 0;
errout_mask:
@@ -1405,6 +1412,8 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
kfree(fnew);
errout_tb:
kfree(tb);
+errout_mask_alloc:
+ kfree(mask);
return err;
}
--
2.19.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net] net/sched: cls_flower: allocate mask dynamically in fl_change()
2019-01-16 15:02 [PATCH net] net/sched: cls_flower: allocate mask dynamically in fl_change() Ivan Vecera
@ 2019-01-16 15:24 ` Jiri Pirko
2019-01-16 15:49 ` Ivan Vecera
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Pirko @ 2019-01-16 15:24 UTC (permalink / raw)
To: Ivan Vecera; +Cc: netdev, Petr Oros, Paul Blakey
Wed, Jan 16, 2019 at 04:02:10PM CET, ivecera@redhat.com wrote:
>Recent changes (especially 05cd271fd61a ("cls_flower: Support multiple
>masks per priority")) in the fl_flow_mask structure grow it and its
>current size e.g. on x86_64 with defconfig is 760 bytes and more than
>1024 bytes with some debug options enabled. Prior the mentioned commit
>its size was 176 bytes (using defconfig on x86_64).
>With regard to this fact it's reasonable to allocate this structure
>dynamically in fl_change() to reduce its stack size.
>
>Fixes: 05cd271fd61a ("cls_flower: Support multiple masks per priority")
Not sure this is -net material. But I don't mind.
>Cc: Jiri Pirko <jiri@resnulli.us>
>Cc: Paul Blakey <paulb@mellanox.com>
>Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>---
> net/sched/cls_flower.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
>diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>index dad04e710493..47ac88cc34fb 100644
>--- a/net/sched/cls_flower.c
>+++ b/net/sched/cls_flower.c
>@@ -1291,16 +1291,22 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> struct cls_fl_filter *fold = *arg;
> struct cls_fl_filter *fnew;
> struct nlattr **tb;
>- struct fl_flow_mask mask = {};
>+ struct fl_flow_mask *mask;
> int err;
>
> if (!tca[TCA_OPTIONS])
> return -EINVAL;
>
>- tb = kcalloc(TCA_FLOWER_MAX + 1, sizeof(struct nlattr *), GFP_KERNEL);
>- if (!tb)
>+ mask = kcalloc(1, sizeof(struct fl_flow_mask), GFP_KERNEL);
kzalloc?
Otherwise this looks fine.
Acked-by: Jiri Pirko <jiri@mellanox.com>
>+ if (!mask)
> return -ENOBUFS;
>
>+ tb = kcalloc(TCA_FLOWER_MAX + 1, sizeof(struct nlattr *), GFP_KERNEL);
>+ if (!tb) {
>+ err = -ENOBUFS;
>+ goto errout_mask_alloc;
>+ }
>+
> err = nla_parse_nested(tb, TCA_FLOWER_MAX, tca[TCA_OPTIONS],
> fl_policy, NULL);
> if (err < 0)
>@@ -1343,12 +1349,12 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> }
> }
>
>- err = fl_set_parms(net, tp, fnew, &mask, base, tb, tca[TCA_RATE], ovr,
>+ err = fl_set_parms(net, tp, fnew, mask, base, tb, tca[TCA_RATE], ovr,
> tp->chain->tmplt_priv, extack);
> if (err)
> goto errout_idr;
>
>- err = fl_check_assign_mask(head, fnew, fold, &mask);
>+ err = fl_check_assign_mask(head, fnew, fold, mask);
> if (err)
> goto errout_idr;
>
>@@ -1392,6 +1398,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> }
>
> kfree(tb);
>+ kfree(mask);
> return 0;
>
> errout_mask:
>@@ -1405,6 +1412,8 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> kfree(fnew);
> errout_tb:
> kfree(tb);
>+errout_mask_alloc:
>+ kfree(mask);
> return err;
> }
>
>--
>2.19.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] net/sched: cls_flower: allocate mask dynamically in fl_change()
2019-01-16 15:24 ` Jiri Pirko
@ 2019-01-16 15:49 ` Ivan Vecera
0 siblings, 0 replies; 3+ messages in thread
From: Ivan Vecera @ 2019-01-16 15:49 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, Petr Oros, Paul Blakey
On 16. 01. 19 16:24, Jiri Pirko wrote:
> Wed, Jan 16, 2019 at 04:02:10PM CET, ivecera@redhat.com wrote:
>> Recent changes (especially 05cd271fd61a ("cls_flower: Support multiple
>> masks per priority")) in the fl_flow_mask structure grow it and its
>> current size e.g. on x86_64 with defconfig is 760 bytes and more than
>> 1024 bytes with some debug options enabled. Prior the mentioned commit
>> its size was 176 bytes (using defconfig on x86_64).
>> With regard to this fact it's reasonable to allocate this structure
>> dynamically in fl_change() to reduce its stack size.
>>
>> Fixes: 05cd271fd61a ("cls_flower: Support multiple masks per priority")
>
> Not sure this is -net material. But I don't mind.
>
>
>> Cc: Jiri Pirko <jiri@resnulli.us>
>> Cc: Paul Blakey <paulb@mellanox.com>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> ---
>> net/sched/cls_flower.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>> index dad04e710493..47ac88cc34fb 100644
>> --- a/net/sched/cls_flower.c
>> +++ b/net/sched/cls_flower.c
>> @@ -1291,16 +1291,22 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
>> struct cls_fl_filter *fold = *arg;
>> struct cls_fl_filter *fnew;
>> struct nlattr **tb;
>> - struct fl_flow_mask mask = {};
>> + struct fl_flow_mask *mask;
>> int err;
>>
>> if (!tca[TCA_OPTIONS])
>> return -EINVAL;
>>
>> - tb = kcalloc(TCA_FLOWER_MAX + 1, sizeof(struct nlattr *), GFP_KERNEL);
>> - if (!tb)
>> + mask = kcalloc(1, sizeof(struct fl_flow_mask), GFP_KERNEL);
>
> kzalloc?
Sure.. excuse.
> Otherwise this looks fine.
> Acked-by: Jiri Pirko <jiri@mellanox.com>
Thx.
>
>
>
>> + if (!mask)
>> return -ENOBUFS;
>>
>> + tb = kcalloc(TCA_FLOWER_MAX + 1, sizeof(struct nlattr *), GFP_KERNEL);
>> + if (!tb) {
>> + err = -ENOBUFS;
>> + goto errout_mask_alloc;
>> + }
>> +
>> err = nla_parse_nested(tb, TCA_FLOWER_MAX, tca[TCA_OPTIONS],
>> fl_policy, NULL);
>> if (err < 0)
>> @@ -1343,12 +1349,12 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
>> }
>> }
>>
>> - err = fl_set_parms(net, tp, fnew, &mask, base, tb, tca[TCA_RATE], ovr,
>> + err = fl_set_parms(net, tp, fnew, mask, base, tb, tca[TCA_RATE], ovr,
>> tp->chain->tmplt_priv, extack);
>> if (err)
>> goto errout_idr;
>>
>> - err = fl_check_assign_mask(head, fnew, fold, &mask);
>> + err = fl_check_assign_mask(head, fnew, fold, mask);
>> if (err)
>> goto errout_idr;
>>
>> @@ -1392,6 +1398,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
>> }
>>
>> kfree(tb);
>> + kfree(mask);
>> return 0;
>>
>> errout_mask:
>> @@ -1405,6 +1412,8 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
>> kfree(fnew);
>> errout_tb:
>> kfree(tb);
>> +errout_mask_alloc:
>> + kfree(mask);
>> return err;
>> }
>>
>> --
>> 2.19.2
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-16 15:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-16 15:02 [PATCH net] net/sched: cls_flower: allocate mask dynamically in fl_change() Ivan Vecera
2019-01-16 15:24 ` Jiri Pirko
2019-01-16 15:49 ` Ivan Vecera
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).