Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1380608 > unrolled thread

[PATCH] netfilter: ctnetlink: add more #ifdef around unused code

Back to article view | Back to linux.kernel

Contents

  [PATCH] netfilter: ctnetlink: add more #ifdef around unused code Arnd Bergmann <arnd@arndb.de> - 2016-04-16 22:20 +0200
    Re: [PATCH] netfilter: ctnetlink: add more #ifdef around unused code Pablo Neira Ayuso <pablo@netfilter.org> - 2016-04-18 20:20 +0200
      Re: [PATCH] netfilter: ctnetlink: add more #ifdef around unused code Arnd Bergmann <arnd@arndb.de> - 2016-04-18 20:40 +0200
        Re: [PATCH] netfilter: ctnetlink: add more #ifdef around unused code Pablo Neira Ayuso <pablo@netfilter.org> - 2016-04-18 20:50 +0200
          Re: [PATCH] netfilter: ctnetlink: add more #ifdef around unused code Arnd Bergmann <arnd@arndb.de> - 2016-04-18 22:10 +0200
            Re: [PATCH] netfilter: ctnetlink: add more #ifdef around unused code Pablo Neira Ayuso <pablo@netfilter.org> - 2016-04-18 22:20 +0200

#1380608 — [PATCH] netfilter: ctnetlink: add more #ifdef around unused code

A recent patch removed many 'inline' annotations for static
functions in this file, which has caused warnings for functions
that are not used in a given configuration, in particular when
CONFIG_NF_CONNTRACK_EVENTS is disabled:

nf_conntrack_netlink.c:572:15: 'ctnetlink_timestamp_size' defined but not used
nf_conntrack_netlink.c:546:15: 'ctnetlink_acct_size' defined but not used
nf_conntrack_netlink.c:339:12: 'ctnetlink_label_size' defined but not used

I first tried to replace some of the existing #ifdefs with nicer
'if (IS_ENABLED())' checks, but ran into several other problems
with that, so this patch adds even more #ifdef conditionals to
avoid the remaining warnings. Another option would be to put
'__maybe_unused' annotations in place of the previous 'inline'
keyword.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 4054ff45454a ("netfilter: ctnetlink: remove unnecessary inlining")
---
 net/netfilter/nf_conntrack_netlink.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index caa4efe5930b..f893012986c7 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -336,6 +336,7 @@ nla_put_failure:
 #endif
 
 #ifdef CONFIG_NF_CONNTRACK_LABELS
+#ifdef CONFIG_NF_CONNTRACK_EVENTS
 static int ctnetlink_label_size(const struct nf_conn *ct)
 {
 	struct nf_conn_labels *labels = nf_ct_labels_find(ct);
@@ -344,6 +345,7 @@ static int ctnetlink_label_size(const struct nf_conn *ct)
 		return 0;
 	return nla_total_size(labels->words * sizeof(long));
 }
+#endif
 
 static int
 ctnetlink_dump_labels(struct sk_buff *skb, const struct nf_conn *ct)
@@ -526,6 +528,7 @@ nla_put_failure:
 	return -1;
 }
 
+#if defined(CONFIG_NF_CONNTRACK_EVENTS) || defined(CONFIG_NETFILTER_NETLINK_GLUE_CT)
 static size_t ctnetlink_proto_size(const struct nf_conn *ct)
 {
 	struct nf_conntrack_l3proto *l3proto;
@@ -543,16 +546,6 @@ static size_t ctnetlink_proto_size(const struct nf_conn *ct)
 	return len;
 }
 
-static size_t ctnetlink_acct_size(const struct nf_conn *ct)
-{
-	if (!nf_ct_ext_exist(ct, NF_CT_EXT_ACCT))
-		return 0;
-	return 2 * nla_total_size(0) /* CTA_COUNTERS_ORIG|REPL */
-	       + 2 * nla_total_size(sizeof(uint64_t)) /* CTA_COUNTERS_PACKETS */
-	       + 2 * nla_total_size(sizeof(uint64_t)) /* CTA_COUNTERS_BYTES */
-	       ;
-}
-
 static int ctnetlink_secctx_size(const struct nf_conn *ct)
 {
 #ifdef CONFIG_NF_CONNTRACK_SECMARK
@@ -568,6 +561,18 @@ static int ctnetlink_secctx_size(const struct nf_conn *ct)
 	return 0;
 #endif
 }
+#endif
+
+#ifdef CONFIG_NF_CONNTRACK_EVENTS
+static size_t ctnetlink_acct_size(const struct nf_conn *ct)
+{
+	if (!nf_ct_ext_exist(ct, NF_CT_EXT_ACCT))
+		return 0;
+	return 2 * nla_total_size(0) /* CTA_COUNTERS_ORIG|REPL */
+	       + 2 * nla_total_size(sizeof(uint64_t)) /* CTA_COUNTERS_PACKETS */
+	       + 2 * nla_total_size(sizeof(uint64_t)) /* CTA_COUNTERS_BYTES */
+	       ;
+}
 
 static size_t ctnetlink_timestamp_size(const struct nf_conn *ct)
 {
@@ -580,7 +585,6 @@ static size_t ctnetlink_timestamp_size(const struct nf_conn *ct)
 #endif
 }
 
-#ifdef CONFIG_NF_CONNTRACK_EVENTS
 static size_t ctnetlink_nlmsg_size(const struct nf_conn *ct)
 {
 	return NLMSG_ALIGN(sizeof(struct nfgenmsg))
-- 
2.7.0

[toc] | [next] | [standalone]

#1381956

On Sat, Apr 16, 2016 at 10:17:43PM +0200, Arnd Bergmann wrote:
> A recent patch removed many 'inline' annotations for static
> functions in this file, which has caused warnings for functions
> that are not used in a given configuration, in particular when
> CONFIG_NF_CONNTRACK_EVENTS is disabled:
> 
> nf_conntrack_netlink.c:572:15: 'ctnetlink_timestamp_size' defined but not used
> nf_conntrack_netlink.c:546:15: 'ctnetlink_acct_size' defined but not used
> nf_conntrack_netlink.c:339:12: 'ctnetlink_label_size' defined but not used

Arnd, thanks for the fix.

I'm planning to push this though:

http://patchwork.ozlabs.org/patch/610820/

This is restoring the inlines for the size calculation functions, but
I think that's ok. They are rather small and they're called from the
event notification path (ie. packet path), so the compiler just place
them out of the way when not needed and we calm down the gcc warning.

Thanks!

[toc] | [prev] | [next] | [standalone]

#1381968

On Monday 18 April 2016 20:16:59 Pablo Neira Ayuso wrote:
> On Sat, Apr 16, 2016 at 10:17:43PM +0200, Arnd Bergmann wrote:
> > A recent patch removed many 'inline' annotations for static
> > functions in this file, which has caused warnings for functions
> > that are not used in a given configuration, in particular when
> > CONFIG_NF_CONNTRACK_EVENTS is disabled:
> > 
> > nf_conntrack_netlink.c:572:15: 'ctnetlink_timestamp_size' defined but not used
> > nf_conntrack_netlink.c:546:15: 'ctnetlink_acct_size' defined but not used
> > nf_conntrack_netlink.c:339:12: 'ctnetlink_label_size' defined but not used
> 
> Arnd, thanks for the fix.
> 
> I'm planning to push this though:
> 
> http://patchwork.ozlabs.org/patch/610820/
> 
> This is restoring the inlines for the size calculation functions, but
> I think that's ok. They are rather small and they're called from the
> event notification path (ie. packet path), so the compiler just place
> them out of the way when not needed and we calm down the gcc warning.

Looks good. I'll put this in my randconfig builder to replace my own
patch and will let you know if you missed something.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1381971

On Mon, Apr 18, 2016 at 08:33:15PM +0200, Arnd Bergmann wrote:
> On Monday 18 April 2016 20:16:59 Pablo Neira Ayuso wrote:
> > On Sat, Apr 16, 2016 at 10:17:43PM +0200, Arnd Bergmann wrote:
> > > A recent patch removed many 'inline' annotations for static
> > > functions in this file, which has caused warnings for functions
> > > that are not used in a given configuration, in particular when
> > > CONFIG_NF_CONNTRACK_EVENTS is disabled:
> > > 
> > > nf_conntrack_netlink.c:572:15: 'ctnetlink_timestamp_size' defined but not used
> > > nf_conntrack_netlink.c:546:15: 'ctnetlink_acct_size' defined but not used
> > > nf_conntrack_netlink.c:339:12: 'ctnetlink_label_size' defined but not used
> > 
> > Arnd, thanks for the fix.
> > 
> > I'm planning to push this though:
> > 
> > http://patchwork.ozlabs.org/patch/610820/
> > 
> > This is restoring the inlines for the size calculation functions, but
> > I think that's ok. They are rather small and they're called from the
> > event notification path (ie. packet path), so the compiler just place
> > them out of the way when not needed and we calm down the gcc warning.
> 
> Looks good. I'll put this in my randconfig builder to replace my own
> patch and will let you know if you missed something.

Thanks, will wait for your ack then.

[toc] | [prev] | [next] | [standalone]

#1382026

On Monday 18 April 2016 20:43:36 Pablo Neira Ayuso wrote:
> On Mon, Apr 18, 2016 at 08:33:15PM +0200, Arnd Bergmann wrote:
> > On Monday 18 April 2016 20:16:59 Pablo Neira Ayuso wrote:
> > > On Sat, Apr 16, 2016 at 10:17:43PM +0200, Arnd Bergmann wrote:
> > > > A recent patch removed many 'inline' annotations for static
> > > > functions in this file, which has caused warnings for functions
> > > > that are not used in a given configuration, in particular when
> > > > CONFIG_NF_CONNTRACK_EVENTS is disabled:
> > > > 
> > > > nf_conntrack_netlink.c:572:15: 'ctnetlink_timestamp_size' defined but not used
> > > > nf_conntrack_netlink.c:546:15: 'ctnetlink_acct_size' defined but not used
> > > > nf_conntrack_netlink.c:339:12: 'ctnetlink_label_size' defined but not used
> > > 
> > > Arnd, thanks for the fix.
> > > 
> > > I'm planning to push this though:
> > > 
> > > http://patchwork.ozlabs.org/patch/610820/
> > > 
> > > This is restoring the inlines for the size calculation functions, but
> > > I think that's ok. They are rather small and they're called from the
> > > event notification path (ie. packet path), so the compiler just place
> > > them out of the way when not needed and we calm down the gcc warning.
> > 
> > Looks good. I'll put this in my randconfig builder to replace my own
> > patch and will let you know if you missed something.
> 
> Thanks, will wait for your ack then.

100 clean randconfig builds later, looks good to me.

Acked-by: Arnd Bergmann <arnd@arndb.de>

[toc] | [prev] | [next] | [standalone]

#1382030

On Mon, Apr 18, 2016 at 10:04:43PM +0200, Arnd Bergmann wrote:
> On Monday 18 April 2016 20:43:36 Pablo Neira Ayuso wrote:
> > On Mon, Apr 18, 2016 at 08:33:15PM +0200, Arnd Bergmann wrote:
> > > On Monday 18 April 2016 20:16:59 Pablo Neira Ayuso wrote:
> > > > On Sat, Apr 16, 2016 at 10:17:43PM +0200, Arnd Bergmann wrote:
> > > > > A recent patch removed many 'inline' annotations for static
> > > > > functions in this file, which has caused warnings for functions
> > > > > that are not used in a given configuration, in particular when
> > > > > CONFIG_NF_CONNTRACK_EVENTS is disabled:
> > > > > 
> > > > > nf_conntrack_netlink.c:572:15: 'ctnetlink_timestamp_size' defined but not used
> > > > > nf_conntrack_netlink.c:546:15: 'ctnetlink_acct_size' defined but not used
> > > > > nf_conntrack_netlink.c:339:12: 'ctnetlink_label_size' defined but not used
> > > > 
> > > > Arnd, thanks for the fix.
> > > > 
> > > > I'm planning to push this though:
> > > > 
> > > > http://patchwork.ozlabs.org/patch/610820/
> > > > 
> > > > This is restoring the inlines for the size calculation functions, but
> > > > I think that's ok. They are rather small and they're called from the
> > > > event notification path (ie. packet path), so the compiler just place
> > > > them out of the way when not needed and we calm down the gcc warning.
> > > 
> > > Looks good. I'll put this in my randconfig builder to replace my own
> > > patch and will let you know if you missed something.
> > 
> > Thanks, will wait for your ack then.
> 
> 100 clean randconfig builds later, looks good to me.
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Thanks! I have applied this to nf-next.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web