[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAM_iQpUHrhkVtgrA37btQ9sn0UPeYgkqnRx=3HZoLan-soT8hg@mail.gmail.com>
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.
Powered by blists - more mailing lists