netdev - Re: [PATCH nf-next 3/9] netfilter: conntrack: don't attempt to iterate over empty table

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: <20160503175559.GJ2395@breakpoint.cc>
Date:	Tue, 3 May 2016 19:55:59 +0200
From:	Florian Westphal <fw@...len.de>
To:	Pablo Neira Ayuso <pablo@...filter.org>
Cc:	Florian Westphal <fw@...len.de>, netfilter-devel@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH nf-next 3/9] netfilter: conntrack: don't attempt to
 iterate over empty table

Pablo Neira Ayuso <pablo@...filter.org> wrote:
> > I was thinking of the cleanup we do in the netns exit path
> > (in nf_conntrack_cleanup_net_list() ).
> 
> Right, but in that path we still have entries in the table.

Not necessarily, they might have already been removed
(timeout, close).

> > If you don't like this I can move the check here:
> > 
> > i_see_dead_people:
> >     busy = 0;
> >     list_for_each_entry(net, net_exit_list, exit_list) {
> >     // here
> >     if (atomic_read .. > 0)
> >        nf_ct_iterate_cleanup(net, kill_all, ...
> 
> I don't mind about placing this or there, as I said, my question is
> how often we will hit this optimization in a real scenario.
> 
> If you think the answer is often, then this will help.

I think the extra atomic_read in this code does no harm and
saves us the entire scan.  Also, in the exit path, when we hit the
'i_see_dead_people' label we restart the entire loop, so if we
have 200 netns on the list and the last one caused that restart,
we re-iterate needlesly for 199 netns...

> Otherwise, every time we'll go container destruction path, we'll hit
> slow path, ie.  scanning the full table.

Yes, but I see no other choice.