netfilter: conntrack: Guard possoble unused functions

PATCH Superseded Andy Shevchenko 2 participants 11 messages Sep 5, 2024 – Sep 16, 2024

Subsystems (3)

Networking Network drivers netfilter

lore

Share & tools

lore

Andy Shevchenko

author

2024-09-05 20:36 UTC #0 lore raw

PATCH 1/1 netfilter: conntrack: Guard possoble unused functions

Some of the functions may be unused, it prevents kernel builds with clang, make W=1 and CONFIG_WERROR=y:

net/netfilter/nf_conntrack_netlink.c:657:22: error: unused function 'ctnetlink_acct_size' [-Werror,-Wunused-function]
657 | static inline size_t ctnetlink_acct_size(const struct nf_conn *ct)

      |                      ^~~~~~~~~~~~~~~~~~~
net/netfilter/nf_conntrack_netlink.c:667:19: error: unused function 'ctnetlink_secctx_size' [-Werror,-Wunused-function]
  667 | static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
      |                   ^~~~~~~~~~~~~~~~~~~~~
net/netfilter/nf_conntrack_netlink.c:683:22: error: unused function 'ctnetlink_timestamp_size' [-Werror,-Wunused-function]
  683 | static inline size_t ctnetlink_timestamp_size(const struct nf_conn *ct)
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~

Fix this by guarding possible unused functions with ifdeffery.

See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static inline functions for W=1 build").

Fixes 4a96300cec88 ("netfilter: ctnetlink: restore inlining for netlink message size calculation")

Signed-off-by Andy Shevchenko <[email protected]>

---
 net/netfilter/nf_conntrack_netlink.c | 2 ++
 1 file changed, 2 insertions(+)

net/netfilter/nf_conntrack_netlink.c +2 -0

index 4cbf71d0786b..7ab7dc7569e7 100644
@@ -654,6 +654,7 @@ static size_t ctnetlink_proto_size(const struct nf_conn *ct)
 }
 #endif
 
+#if defined(CONFIG_NF_CONNTRACK_EVENTS) || defined(CONFIG_NETFILTER_NETLINK_GLUE_CT)
 static inline size_t ctnetlink_acct_size(const struct nf_conn *ct)
 {
 	if (!nf_ct_ext_exist(ct, NF_CT_EXT_ACCT))

@@ -690,6 +691,7 @@ static inline size_t ctnetlink_timestamp_size(const struct nf_conn *ct)
 	return 0;
 #endif
 }
+#endif
 
 #ifdef CONFIG_NF_CONNTRACK_EVENTS
 static size_t ctnetlink_nlmsg_size(const struct nf_conn *ct)

Signature · 1 line

2.43.0.rc1.1336.g36b5255a03ac

Simon Horman

reviewer

2024-09-06 16:29 UTC #1 lore raw

Quoted 5 lines · Andy Shevchenko

Some of the functions may be unused, it prevents kernel builds with clang, make W=1 and CONFIG_WERROR=y:
net/netfilter/nf_conntrack_netlink.c:657:22: error: unused function 'ctnetlink_acct_size' [-Werror,-Wunused-function]
657 | static inline size_t ctnetlink_acct_size(const struct nf_conn *ct)

Quoted 7 lines

      |                      ^~~~~~~~~~~~~~~~~~~
net/netfilter/nf_conntrack_netlink.c:667:19: error: unused function 'ctnetlink_secctx_size' [-Werror,-Wunused-function]
  667 | static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
      |                   ^~~~~~~~~~~~~~~~~~~~~
net/netfilter/nf_conntrack_netlink.c:683:22: error: unused function 'ctnetlink_timestamp_size' [-Werror,-Wunused-function]
  683 | static inline size_t ctnetlink_timestamp_size(const struct nf_conn *ct)
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~

Hi Andy,

Local testing seems to show that the warning is still emitted for ctnetlink_label_size if CONFIG_NETFILTER_NETLINK_GLUE_CT is enabled but CONFIG_NF_CONNTRACK_EVENTS is not.

Quoted 7 lines

Fix this by guarding possible unused functions with ifdeffery.
See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static inline functions for W=1 build").
Fixes 4a96300cec88 ("netfilter: ctnetlink: restore inlining for netlink message size calculation")

I'm not sure that this qualifies as a fix, rather I think it should be targeted at net-next without a Fixes tag.

Quoted 1 line

Signed-off-by Andy Shevchenko <[email protected]>

...

Simon Horman

reviewer

2024-09-06 16:31 UTC #2 lore raw

Quoted 1 line · Simon Horman

On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote:

...

Quoted 12 lines

Hi Andy,
Local testing seems to show that the warning is still emitted for ctnetlink_label_size if CONFIG_NETFILTER_NETLINK_GLUE_CT is enabled but CONFIG_NF_CONNTRACK_EVENTS is not.
Quoted 6 lines
Fix this by guarding possible unused functions with ifdeffery.
See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static inline functions for W=1 build").

Quoted 8 lines

Quoted 1 line
Fixes 4a96300cec88 ("netfilter: ctnetlink: restore inlining for netlink message size calculation")
I'm not sure that this qualifies as a fix, rather I think it should be targeted at net-next without a Fixes tag.
Quoted 1 line
Signed-off-by Andy Shevchenko <[email protected]>
...

Sorry, one more minor thing: possible is misspelt in the subject.

Andy Shevchenko

author

2024-09-09 09:26 UTC #3 lore raw

In reply to Simon Horman

Quoted 14 lines · Simon Horman

Quoted 5 lines · Andy Shevchenko
Some of the functions may be unused, it prevents kernel builds with clang, make W=1 and CONFIG_WERROR=y:
net/netfilter/nf_conntrack_netlink.c:657:22: error: unused function 'ctnetlink_acct_size' [-Werror,-Wunused-function]
657 | static inline size_t ctnetlink_acct_size(const struct nf_conn *ct)
Quoted 7 lines
      |                      ^~~~~~~~~~~~~~~~~~~
net/netfilter/nf_conntrack_netlink.c:667:19: error: unused function 'ctnetlink_secctx_size' [-Werror,-Wunused-function]
  667 | static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
      |                   ^~~~~~~~~~~~~~~~~~~~~
net/netfilter/nf_conntrack_netlink.c:683:22: error: unused function 'ctnetlink_timestamp_size' [-Werror,-Wunused-function]
  683 | static inline size_t ctnetlink_timestamp_size(const struct nf_conn *ct)
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~

Quoted 5 lines

Hi Andy,
Local testing seems to show that the warning is still emitted for ctnetlink_label_size if CONFIG_NETFILTER_NETLINK_GLUE_CT is enabled but CONFIG_NF_CONNTRACK_EVENTS is not.

Hmm... Let me try that. I am using mostly x86_64_defconfig for the testing. The idea is to have once x86_64_defconfig to be clean with W=1 and since then it will be easier to enable it unconditionally for CIs for _that_ particular configuration(s) ("s" in case of i386_defconfig to be at the same level).

Quoted 9 lines

Quoted 6 lines
Fix this by guarding possible unused functions with ifdeffery.
See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static inline functions for W=1 build").
Fixes 4a96300cec88 ("netfilter: ctnetlink: restore inlining for netlink message size calculation")
I'm not sure that this qualifies as a fix, rather I think it should be targeted at net-next without a Fixes tag.

Okay.

Thank you for the review!

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2024-09-09 09:41 UTC #4 lore raw

In reply to Simon Horman

Quoted 5 lines · Simon Horman

On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote:
Local testing seems to show that the warning is still emitted for ctnetlink_label_size if CONFIG_NETFILTER_NETLINK_GLUE_CT is enabled but CONFIG_NF_CONNTRACK_EVENTS is not.

Can you elaborate on this, please?
I can not reproduce.

#
# Core Netfilter Configuration
#
CONFIG_NETFILTER_INGRESS=y
CONFIG_NETFILTER_EGRESS=y
CONFIG_NETFILTER_SKIP_EGRESS=y
CONFIG_NETFILTER_NETLINK=y
CONFIG_NETFILTER_NETLINK_LOG=y
CONFIG_NF_CONNTRACK=y
CONFIG_NF_LOG_SYSLOG=m
CONFIG_NF_CONNTRACK_SECMARK=y
# CONFIG_NF_CONNTRACK_PROCFS is not set
# CONFIG_NF_CONNTRACK_LABELS is not set
CONFIG_NF_CONNTRACK_FTP=y
CONFIG_NF_CONNTRACK_IRC=y
# CONFIG_NF_CONNTRACK_NETBIOS_NS is not set
CONFIG_NF_CONNTRACK_SIP=y
CONFIG_NF_CT_NETLINK=y
CONFIG_NETFILTER_NETLINK_GLUE_CT=y
CONFIG_NF_NAT=y
CONFIG_NF_NAT_FTP=y
CONFIG_NF_NAT_IRC=y
CONFIG_NF_NAT_SIP=y
CONFIG_NF_NAT_MASQUERADE=y
# CONFIG_NF_TABLES is not set
CONFIG_NETFILTER_XTABLES=y
# CONFIG_NETFILTER_XTABLES_COMPAT is not set

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Simon Horman

reviewer

2024-09-09 15:17 UTC #5 lore raw

Quoted 9 lines · Andy Shevchenko

Quoted 5 lines · Simon Horman
On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote:
Local testing seems to show that the warning is still emitted for ctnetlink_label_size if CONFIG_NETFILTER_NETLINK_GLUE_CT is enabled but CONFIG_NF_CONNTRACK_EVENTS is not.
Can you elaborate on this, please?
I can not reproduce.

Sure, let me retest and get back to you.

Andy Shevchenko

author

2024-09-09 15:38 UTC #6 lore raw

Quoted 6 lines · Simon Horman

Quoted 5 lines · Andy Shevchenko
Quoted 4 lines · Simon Horman
On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote:
Local testing seems to show that the warning is still emitted for ctnetlink_label_size if CONFIG_NETFILTER_NETLINK_GLUE_CT is enabled

Hold on, this is not related to the patch.
It might be another issue.

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
but CONFIG_NF_CONNTRACK_EVENTS is not.
Can you elaborate on this, please?
I can not reproduce.
Sure, let me retest and get back to you.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Simon Horman

reviewer

2024-09-09 18:30 UTC #7 lore raw

Quoted 10 lines · Andy Shevchenko

Quoted 6 lines · Simon Horman
Quoted 5 lines · Andy Shevchenko
Quoted 4 lines · Simon Horman
On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote:
Local testing seems to show that the warning is still emitted for ctnetlink_label_size if CONFIG_NETFILTER_NETLINK_GLUE_CT is enabled
Hold on, this is not related to the patch.
It might be another issue.

Yes, sorry, I see that now too.

Perhaps it can be fixed separately, something like this:

net/netfilter/nf_conntrack_netlink.c +2 -0

index 8fd2b9e392a7..fcd1b800b2c1 100644
@@ -383,6 +383,7 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
 #endif
 
 #ifdef CONFIG_NF_CONNTRACK_LABELS
+#ifdef CONFIG_NETFILTER_NETLINK_GLUE_CT
 static inline int ctnetlink_label_size(const struct nf_conn *ct)
 {
 	struct nf_conn_labels *labels = nf_ct_labels_find(ct);

@@ -391,6 +392,7 @@ static inline int ctnetlink_label_size(const struct nf_conn *ct)
 		return 0;
 	return nla_total_size(sizeof(labels->bits));
 }
+#endif
 
 static int
 ctnetlink_dump_labels(struct sk_buff *skb, const struct nf_conn *ct)

Quoted 13 lines

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
but CONFIG_NF_CONNTRACK_EVENTS is not.
Can you elaborate on this, please?
I can not reproduce.
Sure, let me retest and get back to you.
Signature · 2 lines
With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2024-09-10 08:12 UTC #8 lore raw

Quoted 9 lines · Simon Horman

Quoted 8 lines · Andy Shevchenko
Quoted 6 lines · Simon Horman
Quoted 5 lines · Andy Shevchenko
Quoted 4 lines · Simon Horman
On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote:
Local testing seems to show that the warning is still emitted for ctnetlink_label_size if CONFIG_NETFILTER_NETLINK_GLUE_CT is enabled

Quoted 6 lines

Quoted 2 lines
Hold on, this is not related to the patch.
It might be another issue.
Yes, sorry, I see that now too.
Perhaps it can be fixed separately, something like this:

If you make a patch, it will help somebody who has that in their configuration files enabled (with the other one being disabled). Note, I use x86_64_defconfig which doesn't have this specific issue to be occurred.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Simon Horman

reviewer

2024-09-10 09:45 UTC #9 lore raw

Quoted 13 lines · Andy Shevchenko

Quoted 12 lines · Simon Horman
Quoted 10 lines · Andy Shevchenko
Quoted 6 lines · Simon Horman
Quoted 5 lines · Andy Shevchenko
Quoted 4 lines · Simon Horman
On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote:
Local testing seems to show that the warning is still emitted for ctnetlink_label_size if CONFIG_NETFILTER_NETLINK_GLUE_CT is enabled
Hold on, this is not related to the patch.
It might be another issue.

Quoted 7 lines

Quoted 3 lines
Yes, sorry, I see that now too.
Perhaps it can be fixed separately, something like this:
If you make a patch, it will help somebody who has that in their configuration files enabled (with the other one being disabled). Note, I use x86_64_defconfig which doesn't have this specific issue to be occurred.

Thanks, I'll plan to submit a patch.

Simon Horman

reviewer

2024-09-16 15:17 UTC #10 lore raw

Quoted 16 lines · Simon Horman

Quoted 15 lines · Andy Shevchenko
Quoted 14 lines · Simon Horman
Quoted 10 lines · Andy Shevchenko
Quoted 6 lines · Simon Horman
Quoted 5 lines · Andy Shevchenko
Quoted 4 lines · Simon Horman
On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote:
Local testing seems to show that the warning is still emitted for ctnetlink_label_size if CONFIG_NETFILTER_NETLINK_GLUE_CT is enabled
Hold on, this is not related to the patch.
It might be another issue.
Yes, sorry, I see that now too.

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Perhaps it can be fixed separately, something like this:
If you make a patch, it will help somebody who has that in their configuration files enabled (with the other one being disabled). Note, I use x86_64_defconfig which doesn't have this specific issue to be occurred.
Thanks, I'll plan to submit a patch.

The patch grew into two, I've posted them here:

- [PATCH nf-next 0/2] netfilter: conntrack: label helpers conditional compilation updates
https://lore.kernel.org/netfilter-devel/[email protected]/