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(¶ms->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.