From: Andrew Vagin <avagin@parallels•com>
To: Pablo Neira Ayuso <pablo@netfilter•org>
Cc: Andrey Vagin <avagin@openvz•org>, <linux-kernel@vger•kernel.org>,
<netfilter-devel@vger•kernel.org>, <netfilter@vger•kernel.org>,
<coreteam@netfilter•org>, <netdev@vger•kernel.org>,
<vvs@parallels•com>, Patrick McHardy <kaber@trash•net>,
Jozsef Kadlecsik <kadlec@blackhole•kfki.hu>,
"David S. Miller" <davem@davemloft•net>
Subject: Re: [PATCH] [RFC] netfilter: ct: flush net_gre->keymap_list only in nf_conntrack_proto_gre
Date: Tue, 8 Apr 2014 10:56:00 +0400 [thread overview]
Message-ID: <20140408065600.GA15309@paralelels.com> (raw)
In-Reply-To: <20140407145158.GA25974@localhost>
On Mon, Apr 07, 2014 at 04:51:58PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Mar 31, 2014 at 06:14:18PM +0400, Andrey Vagin wrote:
> > nf_ct_gre_keymap_flush() removes a nf_ct_gre_keymap object from
> > net_gre->keymap_list and frees the object. But it doesn't clean
> > a reference on this object from ct_pptp_info->keymap[dir].
> > Then nf_ct_gre_keymap_destroy() may release the same object again.
> >
> > So nf_ct_gre_keymap_flush() can be called only when we are sure that
> > when nf_ct_gre_keymap_destroy will not be called.
> >
> > nf_ct_gre_keymap is created by nf_ct_gre_keymap_add() and the right way
> > to destroy it is to call nf_ct_gre_keymap_destroy().
>
> I think you use call nf_ct_iterate_cleanup() to get rid of all GRE
> conntrack entries that have a ct->master, which means that they were
> created by PPTP. That implicitly calls gre_destroy, which releases the
> keymap object.
I'm not sure that I understand your comment. Currently I change nothing
about nf_ct_iterate_cleanup(). In this sentence I point to the right
procedure of destroying a gre conntrack.
nf_ct_iterate_cleanup() is called from nf_ct_l4proto_pernet_unregister().
[ 1858.693250] <idle>-0 1..s. 118795582us : nf_ct_gre_keymap_add <-exp_gre
[ 1858.693250] <idle>-0 1..s. 118795598us : nf_ct_gre_keymap_add <-exp_gre
[ 1858.693250] <idle>-0 1..s. 118795652us : nf_ct_gre_keymap_add <-exp_gre
[ 1858.693250] <idle>-0 1..s. 118795683us : nf_ct_gre_keymap_add <-exp_gre
[ 1858.693250] kworker/-44 1.... 133620272us : nf_conntrack_pptp_net_exit <-ops_exit_list.isra.1
[ 1858.693250] kworker/-44 1.... 133620274us : nf_ct_gre_keymap_flush <-nf_conntrack_pptp_net_exit
[ 1858.693250] kworker/-44 1.... 133621145us : proto_gre_net_exit <-ops_exit_list.isra.1
[ 1858.693250] kworker/-44 1.... 133621145us : nf_ct_l4proto_pernet_unregister <-proto_gre_net_exit
[ 1858.693250] kworker/-44 1.... 133621146us : nf_ct_iterate_cleanup <-nf_ct_l4proto_pernet_unregister
[ 1858.693250] kworker/-44 1.... 133621187us : nf_ct_gre_keymap_destroy <-gre_destroy
>
> > This patch marks nf_ct_gre_keymap_flush() as static, so this patch can
> > break compilation of third party modules, which use
> > nf_ct_gre_keymap_flush. I'm not sure this is the right way to deprecate
> > this function.
>
> No problem with that, we don't mind about out of tree modules. With
> the approach I'm proposing I think you will also need to make this
> function static as it won't be used by PPTP anymore.
I have made this function static. If you don't have other comment, could
you take this patch?
Thanks,
Andrey
next prev parent reply other threads:[~2014-04-08 6:56 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-31 14:14 [PATCH] [RFC] netfilter: ct: flush net_gre->keymap_list only in nf_conntrack_proto_gre Andrey Vagin
2014-04-07 14:51 ` Pablo Neira Ayuso
2014-04-08 6:56 ` Andrew Vagin [this message]
2014-04-08 7:27 ` Pablo Neira Ayuso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140408065600.GA15309@paralelels.com \
--to=avagin@parallels$(echo .)com \
--cc=avagin@openvz$(echo .)org \
--cc=coreteam@netfilter$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=kaber@trash$(echo .)net \
--cc=kadlec@blackhole$(echo .)kfki.hu \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=netfilter-devel@vger$(echo .)kernel.org \
--cc=netfilter@vger$(echo .)kernel.org \
--cc=pablo@netfilter$(echo .)org \
--cc=vvs@parallels$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox