On Mon, Jan 23, 2017 at 2:07 AM, Jiri Pirko <j...@resnulli.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.
