On Tue, May 03, 2016 at 08:12:50PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Apr 28, 2016 at 07:13:44PM +0200, Florian Westphal wrote:
> > The iteration process is lockless, so we test if the conntrack object is
> > eligible for printing (e.g. is AF_INET) after obtaining the reference
> > count.
> >
> > Once we put all conntracks into same hash table we might see more
> > entries that need to be skipped.
> >
> > So add a helper and first perform the test in a lockless fashion
> > for fast skip.
> >
> > Once we obtain the reference count, just repeat the check.
> >
> > Signed-off-by: Florian Westphal <f...@strlen.de>
> > ---
> > .../netfilter/nf_conntrack_l3proto_ipv4_compat.c | 24
> > +++++++++++++++++-----
> > 1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> > b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> > index f0dfe92..483cf79 100644
> > --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> > +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> > @@ -114,6 +114,19 @@ static inline void ct_show_secctx(struct seq_file *s,
> > const struct nf_conn *ct)
> > }
> > #endif
> >
> > +static bool ct_seq_should_skip(const struct nf_conn *ct,
> > + const struct nf_conntrack_tuple_hash *hash)
> > +{
> > + /* we only want to print DIR_ORIGINAL */
> > + if (NF_CT_DIRECTION(hash))
> > + return true;
> > +
> > + if (nf_ct_l3num(ct) != AF_INET)
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > static int ct_seq_show(struct seq_file *s, void *v)
> > {
> > struct nf_conntrack_tuple_hash *hash = v;
> > @@ -123,14 +136,15 @@ static int ct_seq_show(struct seq_file *s, void *v)
> > int ret = 0;
> >
> > NF_CT_ASSERT(ct);
> > - if (unlikely(!atomic_inc_not_zero(&ct->ct_general.use)))
> > + if (ct_seq_should_skip(ct, hash))
> > return 0;
> >
> > + if (unlikely(!atomic_inc_not_zero(&ct->ct_general.use)))
> > + return 0;
> >
> > - /* we only want to print DIR_ORIGINAL */
> > - if (NF_CT_DIRECTION(hash))
> > - goto release;
> > - if (nf_ct_l3num(ct) != AF_INET)
> > + /* check if we raced w. object reuse */
> > + if (!nf_ct_is_confirmed(ct) ||
>
> This refactoring includes this new check, is this intentional?
It seems this check was previously missing, I can just amend the
commit log with a couple of lines to document that this patch also
includes this missing check. No problem.
