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

[Amir Vadai <amir@vadai.me>]

On Tue, Aug 30, 2016 at 08:05:03AM -0400, Jamal Hadi Salim wrote:
> On 16-08-30 07:03 AM, Amir Vadai wrote:
> > On Sun, Aug 28, 2016 at 10:04:21PM -0700, Cong Wang wrote:
> > > On Fri, Aug 26, 2016 at 12:16 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > > On Fri, 2016-08-26 at 11:26 -0700, Cong Wang wrote:
> 
> 
> > Regarding the specific action in this patchset, correct me if I'm wrong,
> > but I think that the lock could be removed safely.
> > 
> 
> From what Eric suggested (refer to my posting on skbmod),
> this becomes:
> 
> +struct tcf_tunnel_key_p {
> +	int			tcft_action;
> +	struct metadata_dst     *tcft_enc_metadata;
> +};
> 
> /* rcu protected */
> +struct tcf_tunnel_key {
> +	struct tc_action	common;
> +       struct tcf_tunnel_key_p *p;
> +};
> 
> At init() - always alloc struct tcf_tunnel_key_p, new
> 
> old = rtnl_dereference(mykey->p);
> if (ovr)
>     spin_lock_bh(&mykey->tcf_lock);
Thanks for the detailed example :)

what are we protecting with this spin lock here? isn't concurrent init()
calls are protected by the rtnl lock?


> ... update all params here ..
> rcu_assign_pointer(mykey->p, new);
> if (ovr) {
>      spin_unlock_bh(&mykey->tcf_lock);
>      synchronize_rcu();
> }
> 
> kfree(old);
> 
> at act():
> 
> rcu_read_lock();
> struct tcf_tunnel_key_p *p = rcu_dereference(mykey->p);
> ... use p here ...
> rcu_read_unlock();
> 
> Cong was looking to do something more generic for all actions.
> 
> cheers,
> jamal