On Wed, Sep 7, 2016 at 1:08 AM, Hadar Hen Zion <had...@mellanox.com> wrote:
> +struct tcf_tunnel_key_params {
> +       struct rcu_head         rcu;
> +       int                     tcft_action;
> +       int                     action;
> +       struct metadata_dst     *tcft_enc_metadata;
> +};
> +
> +struct tcf_tunnel_key {
> +       struct tc_action              common;
> +       struct tcf_tunnel_key_params __rcu *params;
> +};
> +
...

This is unnecessary if we make the tc action API aware of RCU.

> +
> +static void tunnel_key_release(struct tc_action *a, int bind)
> +{
> +       struct tcf_tunnel_key *t = to_tunnel_key(a);
> +       struct tcf_tunnel_key_params *params;
> +
> +       rcu_read_lock();
> +       params = rcu_dereference(t->params);
> +
> +       if (params->tcft_action == TCA_TUNNEL_KEY_ACT_SET)
> +               dst_release(&params->tcft_enc_metadata->dst);
> +
> +       rcu_read_unlock();
> +

So you allocate memory for t->params in ->init() but not
release it here?

Also, ->cleanup() should be called with RTNL, no need to
take read lock here.

BTW, again you do NOT need to make it RCU, the whole
tc action API should be, as my patchset does, I will take care
of this as a part of my patchset. Eric is wasting your time on
this, with no benefits, the code will be replaced soon.

Reply via email to