* [PATCH nf] netfilter: conntrack: warn the user if there is a better helper to use
@ 2015-05-21 13:57 Marcelo Ricardo Leitner
2015-06-09 17:01 ` Marcelo Ricardo Leitner
2015-06-12 12:24 ` Pablo Neira Ayuso
0 siblings, 2 replies; 4+ messages in thread
From: Marcelo Ricardo Leitner @ 2015-05-21 13:57 UTC (permalink / raw)
To: netfilter-devel; +Cc: daniel, fw
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
After db29a9508a92 ("netfilter: conntrack: disable generic tracking for
known protocols"), if the specific helper is built but not loaded
(a standard for most distributions) systems with a restrictive firewall
but weak configuration regarding netfilter modules to load, will
silently stop working.
This patch then puts a warning message so the sysadmin knows where to
start looking into. It's a pr_warn_once regardless of protocol itself
but it should be enough to give a hint on where to look.
Cc: Florian Westphal <fw@strlen.de>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
net/netfilter/nf_conntrack_proto_generic.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
index 60865f1103099383c4263a1a56e691b3c86c3720..2281be419a74b6d8abe0fd7da8d7e8b35d304600 100644
--- a/net/netfilter/nf_conntrack_proto_generic.c
+++ b/net/netfilter/nf_conntrack_proto_generic.c
@@ -90,7 +90,13 @@ static int generic_packet(struct nf_conn *ct,
static bool generic_new(struct nf_conn *ct, const struct sk_buff *skb,
unsigned int dataoff, unsigned int *timeouts)
{
- return nf_generic_should_process(nf_ct_protonum(ct));
+ bool ret;
+
+ ret = nf_generic_should_process(nf_ct_protonum(ct));
+ if (!ret)
+ pr_warn_once("conntrack: generic helper won't handle protocol %d. Please consider loading the specific helper module.\n",
+ nf_ct_protonum(ct));
+ return ret;
}
#if IS_ENABLED(CONFIG_NF_CT_NETLINK_TIMEOUT)
--
2.4.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nf] netfilter: conntrack: warn the user if there is a better helper to use
2015-05-21 13:57 [PATCH nf] netfilter: conntrack: warn the user if there is a better helper to use Marcelo Ricardo Leitner
@ 2015-06-09 17:01 ` Marcelo Ricardo Leitner
2015-06-12 12:24 ` Pablo Neira Ayuso
1 sibling, 0 replies; 4+ messages in thread
From: Marcelo Ricardo Leitner @ 2015-06-09 17:01 UTC (permalink / raw)
To: netfilter-devel; +Cc: daniel, fw
Ahm, ping? :)
On Thu, May 21, 2015 at 10:57:12AM -0300, Marcelo Ricardo Leitner wrote:
> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>
> After db29a9508a92 ("netfilter: conntrack: disable generic tracking for
> known protocols"), if the specific helper is built but not loaded
> (a standard for most distributions) systems with a restrictive firewall
> but weak configuration regarding netfilter modules to load, will
> silently stop working.
>
> This patch then puts a warning message so the sysadmin knows where to
> start looking into. It's a pr_warn_once regardless of protocol itself
> but it should be enough to give a hint on where to look.
>
> Cc: Florian Westphal <fw@strlen.de>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/netfilter/nf_conntrack_proto_generic.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
> index 60865f1103099383c4263a1a56e691b3c86c3720..2281be419a74b6d8abe0fd7da8d7e8b35d304600 100644
> --- a/net/netfilter/nf_conntrack_proto_generic.c
> +++ b/net/netfilter/nf_conntrack_proto_generic.c
> @@ -90,7 +90,13 @@ static int generic_packet(struct nf_conn *ct,
> static bool generic_new(struct nf_conn *ct, const struct sk_buff *skb,
> unsigned int dataoff, unsigned int *timeouts)
> {
> - return nf_generic_should_process(nf_ct_protonum(ct));
> + bool ret;
> +
> + ret = nf_generic_should_process(nf_ct_protonum(ct));
> + if (!ret)
> + pr_warn_once("conntrack: generic helper won't handle protocol %d. Please consider loading the specific helper module.\n",
> + nf_ct_protonum(ct));
> + return ret;
> }
>
> #if IS_ENABLED(CONFIG_NF_CT_NETLINK_TIMEOUT)
> --
> 2.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf] netfilter: conntrack: warn the user if there is a better helper to use
2015-05-21 13:57 [PATCH nf] netfilter: conntrack: warn the user if there is a better helper to use Marcelo Ricardo Leitner
2015-06-09 17:01 ` Marcelo Ricardo Leitner
@ 2015-06-12 12:24 ` Pablo Neira Ayuso
2015-06-12 13:50 ` Marcelo Ricardo Leitner
1 sibling, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2015-06-12 12:24 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: netfilter-devel, daniel, fw
On Thu, May 21, 2015 at 10:57:12AM -0300, Marcelo Ricardo Leitner wrote:
> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>
> After db29a9508a92 ("netfilter: conntrack: disable generic tracking for
> known protocols"), if the specific helper is built but not loaded
> (a standard for most distributions) systems with a restrictive firewall
> but weak configuration regarding netfilter modules to load, will
> silently stop working.
>
> This patch then puts a warning message so the sysadmin knows where to
> start looking into. It's a pr_warn_once regardless of protocol itself
> but it should be enough to give a hint on where to look.
Applied to nf-next.
I'd rather see some evaluation on getting these helpers into the
nf_conntrack module in terms of extra size, just as we do for tcp, udp
and icmp. Moreover, these trackers (specifically DCCP and SCTP) got
not much care so some extra review would be good if we decide to get
this into core.
I'm telling this because assuming that people will look at this warn
once still seem weak assumption to me.
Thanks for your patience.
> Cc: Florian Westphal <fw@strlen.de>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/netfilter/nf_conntrack_proto_generic.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
> index 60865f1103099383c4263a1a56e691b3c86c3720..2281be419a74b6d8abe0fd7da8d7e8b35d304600 100644
> --- a/net/netfilter/nf_conntrack_proto_generic.c
> +++ b/net/netfilter/nf_conntrack_proto_generic.c
> @@ -90,7 +90,13 @@ static int generic_packet(struct nf_conn *ct,
> static bool generic_new(struct nf_conn *ct, const struct sk_buff *skb,
> unsigned int dataoff, unsigned int *timeouts)
> {
> - return nf_generic_should_process(nf_ct_protonum(ct));
> + bool ret;
> +
> + ret = nf_generic_should_process(nf_ct_protonum(ct));
> + if (!ret)
> + pr_warn_once("conntrack: generic helper won't handle protocol %d. Please consider loading the specific helper module.\n",
> + nf_ct_protonum(ct));
> + return ret;
> }
>
> #if IS_ENABLED(CONFIG_NF_CT_NETLINK_TIMEOUT)
> --
> 2.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf] netfilter: conntrack: warn the user if there is a better helper to use
2015-06-12 12:24 ` Pablo Neira Ayuso
@ 2015-06-12 13:50 ` Marcelo Ricardo Leitner
0 siblings, 0 replies; 4+ messages in thread
From: Marcelo Ricardo Leitner @ 2015-06-12 13:50 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, daniel, fw
On Fri, Jun 12, 2015 at 02:24:14PM +0200, Pablo Neira Ayuso wrote:
> On Thu, May 21, 2015 at 10:57:12AM -0300, Marcelo Ricardo Leitner wrote:
> > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> >
> > After db29a9508a92 ("netfilter: conntrack: disable generic tracking for
> > known protocols"), if the specific helper is built but not loaded
> > (a standard for most distributions) systems with a restrictive firewall
> > but weak configuration regarding netfilter modules to load, will
> > silently stop working.
> >
> > This patch then puts a warning message so the sysadmin knows where to
> > start looking into. It's a pr_warn_once regardless of protocol itself
> > but it should be enough to give a hint on where to look.
>
> Applied to nf-next.
>
> I'd rather see some evaluation on getting these helpers into the
> nf_conntrack module in terms of extra size, just as we do for tcp, udp
> and icmp. Moreover, these trackers (specifically DCCP and SCTP) got
> not much care so some extra review would be good if we decide to get
> this into core.
That would be much better yes.
> I'm telling this because assuming that people will look at this warn
> once still seem weak assumption to me.
>
> Thanks for your patience.
Ok, thanks Pablo.
Marcelo
> > Cc: Florian Westphal <fw@strlen.de>
> > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > ---
> > net/netfilter/nf_conntrack_proto_generic.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
> > index 60865f1103099383c4263a1a56e691b3c86c3720..2281be419a74b6d8abe0fd7da8d7e8b35d304600 100644
> > --- a/net/netfilter/nf_conntrack_proto_generic.c
> > +++ b/net/netfilter/nf_conntrack_proto_generic.c
> > @@ -90,7 +90,13 @@ static int generic_packet(struct nf_conn *ct,
> > static bool generic_new(struct nf_conn *ct, const struct sk_buff *skb,
> > unsigned int dataoff, unsigned int *timeouts)
> > {
> > - return nf_generic_should_process(nf_ct_protonum(ct));
> > + bool ret;
> > +
> > + ret = nf_generic_should_process(nf_ct_protonum(ct));
> > + if (!ret)
> > + pr_warn_once("conntrack: generic helper won't handle protocol %d. Please consider loading the specific helper module.\n",
> > + nf_ct_protonum(ct));
> > + return ret;
> > }
> >
> > #if IS_ENABLED(CONFIG_NF_CT_NETLINK_TIMEOUT)
> > --
> > 2.4.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-06-12 13:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-21 13:57 [PATCH nf] netfilter: conntrack: warn the user if there is a better helper to use Marcelo Ricardo Leitner
2015-06-09 17:01 ` Marcelo Ricardo Leitner
2015-06-12 12:24 ` Pablo Neira Ayuso
2015-06-12 13:50 ` Marcelo Ricardo Leitner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).