| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 30 Jan 2017 16:49:30 +0000
From: Yotam Gigi <yotamg@...lanox.com>
To: Cong Wang <xiyou.wangcong@...il.com>, Jiri Pirko <jiri@...nulli.us>
CC: Linux Kernel Network Developers <netdev@...r.kernel.org>,
David Miller <davem@...emloft.net>,
Ido Schimmel <idosch@...lanox.com>,
Elad Raz <eladr@...lanox.com>,
Nogah Frankel <nogahf@...lanox.com>,
Or Gerlitz <ogerlitz@...lanox.com>,
Jamal Hadi Salim <jhs@...atatu.com>,
"geert+renesas@...der.be" <geert+renesas@...der.be>,
Stephen Hemminger <stephen@...workplumber.org>,
Guenter Roeck <linux@...ck-us.net>,
"Roopa Prabhu" <roopa@...ulusnetworks.com>,
John Fastabend <john.fastabend@...il.com>,
Simon Horman <simon.horman@...ronome.com>,
"Roman Mashak" <mrv@...atatu.com>
Subject: RE: [patch net-next v2 2/4] net/sched: Introduce sample tc action
>-----Original Message-----
>From: Yotam Gigi
>Sent: Friday, January 27, 2017 8:09 AM
>To: 'Cong Wang' <xiyou.wangcong@...il.com>; Jiri Pirko <jiri@...nulli.us>
>Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>; David Miller
><davem@...emloft.net>; Ido Schimmel <idosch@...lanox.com>; Elad Raz
><eladr@...lanox.com>; Nogah Frankel <nogahf@...lanox.com>; Or Gerlitz
><ogerlitz@...lanox.com>; Jamal Hadi Salim <jhs@...atatu.com>;
>geert+renesas@...der.be; Stephen Hemminger <stephen@...workplumber.org>;
>Guenter Roeck <linux@...ck-us.net>; Roopa Prabhu
><roopa@...ulusnetworks.com>; John Fastabend <john.fastabend@...il.com>;
>Simon Horman <simon.horman@...ronome.com>; Roman Mashak
><mrv@...atatu.com>
>Subject: RE: [patch net-next v2 2/4] net/sched: Introduce sample tc action
>
>>-----Original Message-----
>>From: Cong Wang [mailto:xiyou.wangcong@...il.com]
>>Sent: Thursday, January 26, 2017 1:30 AM
>>To: Jiri Pirko <jiri@...nulli.us>
>>Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>; David Miller
>><davem@...emloft.net>; Yotam Gigi <yotamg@...lanox.com>; Ido Schimmel
>><idosch@...lanox.com>; Elad Raz <eladr@...lanox.com>; Nogah Frankel
>><nogahf@...lanox.com>; Or Gerlitz <ogerlitz@...lanox.com>; Jamal Hadi Salim
>><jhs@...atatu.com>; geert+renesas@...der.be; Stephen Hemminger
>><stephen@...workplumber.org>; Guenter Roeck <linux@...ck-us.net>; Roopa
>>Prabhu <roopa@...ulusnetworks.com>; John Fastabend
>><john.fastabend@...il.com>; Simon Horman
><simon.horman@...ronome.com>;
>>Roman Mashak <mrv@...atatu.com>
>>Subject: Re: [patch net-next v2 2/4] net/sched: Introduce sample tc action
>>
>>On Mon, Jan 23, 2017 at 2:07 AM, Jiri Pirko <jiri@...nulli.us> wrote:
>>> +
>>> +static int tcf_sample_init(struct net *net, struct nlattr *nla,
>>> + struct nlattr *est, struct tc_action **a, int ovr,
>>> + int bind)
>>> +{
>>> + struct tc_action_net *tn = net_generic(net, sample_net_id);
>>> + struct nlattr *tb[TCA_SAMPLE_MAX + 1];
>>> + struct psample_group *psample_group;
>>> + struct tc_sample *parm;
>>> + struct tcf_sample *s;
>>> + bool exists = false;
>>> + int ret;
>>> +
>>> + if (!nla)
>>> + return -EINVAL;
>>> + ret = nla_parse_nested(tb, TCA_SAMPLE_MAX, nla, sample_policy);
>>> + if (ret < 0)
>>> + return ret;
>>> + if (!tb[TCA_SAMPLE_PARMS] || !tb[TCA_SAMPLE_RATE] ||
>>> + !tb[TCA_SAMPLE_PSAMPLE_GROUP])
>>> + return -EINVAL;
>>> +
>>> + parm = nla_data(tb[TCA_SAMPLE_PARMS]);
>>> +
>>> + exists = tcf_hash_check(tn, parm->index, a, bind);
>>> + if (exists && bind)
>>> + return 0;
>>> +
>>> + if (!exists) {
>>> + ret = tcf_hash_create(tn, parm->index, est, a,
>>> + &act_sample_ops, bind, false);
>>> + if (ret)
>>> + return ret;
>>> + ret = ACT_P_CREATED;
>>> + } else {
>>> + tcf_hash_release(*a, bind);
>>> + if (!ovr)
>>> + return -EEXIST;
>>> + }
>>> + s = to_sample(*a);
>>> +
>>> + ASSERT_RTNL();
>>
>>Copy-n-paste from mirred aciton? This is not needed for you, mirred
>>needs it because of target netdevice.
>
>Ho, you are right. I will remove it.
>
>>
>>
>>> + s->tcf_action = parm->action;
>>> + s->rate = nla_get_u32(tb[TCA_SAMPLE_RATE]);
>>> + s->psample_group_num =
>>nla_get_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]);
>>> + psample_group = psample_group_get(net, s->psample_group_num);
>>> + if (!psample_group)
>>> + return -ENOMEM;
>>
>>I don't think you can just return here, needs tcf_hash_cleanup() for create
>>case, right?
>
>Will fix.
>
>>
>>
>>> + RCU_INIT_POINTER(s->psample_group, psample_group);
>>> +
>>> + if (tb[TCA_SAMPLE_TRUNC_SIZE]) {
>>> + s->truncate = true;
>>> + s->trunc_size = nla_get_u32(tb[TCA_SAMPLE_TRUNC_SIZE]);
>>> + }
>>
>>
>>Do you need tcf_lock here if RCU only protects ->psample_group??
>>
>
>You are right. I need to protect in case of update.
Cong, after some thinking I think I don't really need the tcf_lock here. I
don't really care if the truncate, trunc_size, rate and tcf_action are
consistent among themselves - the only parameter that I care about is the
psample_group pointer, and it is protected via RCU. As far as I see, there is
no need to lock here.
I do need to take the tcf_lock to protect the statistics update in the
tcf_sample_act code, as far as I see.
Am I missing something?
>
>I will send a fixup patch in the following days. Thanks!
>
>>
>>> +
>>> + if (ret == ACT_P_CREATED)
>>> + tcf_hash_insert(tn, *a);
>>> + return ret;
>>> +}
>>> +
>>
>>
>>Thanks.
Powered by blists - more mailing lists