netdev - Re: [Patch net 01/16] net_sched: introduce a workqueue for RCU callbacks of tc filter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1509077103.11887.23.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Thu, 26 Oct 2017 21:05:03 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     netdev@...r.kernel.org, Chris Mi <chrism@...lanox.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jiri Pirko <jiri@...nulli.us>,
        John Fastabend <john.fastabend@...il.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [Patch net 01/16] net_sched: introduce a workqueue for RCU
 callbacks of tc filter

On Thu, 2017-10-26 at 18:24 -0700, Cong Wang wrote:
> ...

> On the other hand, this makes tcf_block_put() ugly and
> harder to understand. Since David and Eric strongly dislike
> adding synchronize_rcu(), this is probably the only
> solution that could make everyone happy.


...

> +static void tcf_block_put_deferred(struct work_struct *work)
> +{
> +	struct tcf_block *block = container_of(work, struct tcf_block, work);
> +	struct tcf_chain *chain;
>  
> +	rtnl_lock();
>  	/* Hold a refcnt for all chains, except 0, in case they are gone. */
>  	list_for_each_entry(chain, &block->chain_list, list)
>  		if (chain->index)
> @@ -292,13 +308,27 @@ void tcf_block_put(struct tcf_block *block)
>  	list_for_each_entry(chain, &block->chain_list, list)
>  		tcf_chain_flush(chain);
>  
> -	/* Wait for RCU callbacks to release the reference count. */
> +	INIT_WORK(&block->work, tcf_block_put_final);
> +	/* Wait for RCU callbacks to release the reference count and make
> +	 * sure their works have been queued before this.
> +	 */
>  	rcu_barrier();
> +	tcf_queue_work(&block->work);
> +	rtnl_unlock();
> +}


On a loaded server, rcu_barrier() typically takes 4 ms.

Way better than synchronize_rcu() (about 90 ms) but still an issue when
holding RTNL.

We have thousands of filters, and management daemon restarts and rebuild
TC hierarchy from scratch.

Simply getting rid of 1000 old filters might block RTNL for a while, or
maybe I misunderstood your patches.

Thanks.