netdev - RE: [patch net-next v2 2/4] net/sched: Introduce sample tc action

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DB3PR05MB0764317D72ACC550EB2DE3E8AC760@DB3PR05MB0764.eurprd05.prod.outlook.com>
Date:   Fri, 27 Jan 2017 06:09:20 +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: 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.

I will send a fixup patch in the following days. Thanks!

>
>> +
>> +       if (ret == ACT_P_CREATED)
>> +               tcf_hash_insert(tn, *a);
>> +       return ret;
>> +}
>> +
>
>
>Thanks.