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]

Date:   Wed, 25 Jan 2017 15:29:47 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
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>, eladr@...lanox.com,
        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.


> +       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?


> +       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??


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


Thanks.