Re: [PATCH net-next V6 4/4] net/sched: Introduce act_tunnel_key

On Wed, Sep 7, 2016 at 7:27 PM, Cong Wang [off-list ref] wrote:

On Wed, Sep 7, 2016 at 1:08 AM, Hadar Hen Zion [off-list ref] wrote:

quoted

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

quoted

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

Right, I'll fix it in the next version.

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

RTNL lock isn't taken when cleanup is called.

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.