| Message ID | 20240905203612.333421-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v1,1/1] netfilter: conntrack: Guard possoble unused functions | expand |
On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote: > 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) > | ^~~~~~~~~~~~~~~~~~~~~~~~ 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. > > 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. > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> ...
On Fri, Sep 06, 2024 at 05:29:38PM +0100, Simon Horman wrote: > On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote: ... > 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. > > > > > 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. > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > ... Sorry, one more minor thing: possible is misspelt in the subject.
On Fri, Sep 06, 2024 at 05:29:38PM +0100, Simon Horman wrote: > On Thu, Sep 05, 2024 at 11:36:12PM +0300, Andy Shevchenko wrote: > > 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) > > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > 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). > > 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!
On Fri, Sep 06, 2024 at 05:29:38PM +0100, Simon Horman wrote: > 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
On Mon, Sep 09, 2024 at 12:37:51PM +0300, Andy Shevchenko wrote: > On Fri, Sep 06, 2024 at 05:29:38PM +0100, Simon Horman wrote: > > 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.
On Mon, Sep 09, 2024 at 04:17:12PM +0100, Simon Horman wrote: > On Mon, Sep 09, 2024 at 12:37:51PM +0300, Andy Shevchenko wrote: > > On Fri, Sep 06, 2024 at 05:29:38PM +0100, Simon Horman wrote: > > > 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. > > > 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.
On Mon, Sep 09, 2024 at 06:36:07PM +0300, Andy Shevchenko wrote: > On Mon, Sep 09, 2024 at 04:17:12PM +0100, Simon Horman wrote: > > On Mon, Sep 09, 2024 at 12:37:51PM +0300, Andy Shevchenko wrote: > > > On Fri, Sep 06, 2024 at 05:29:38PM +0100, Simon Horman wrote: > > > > 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: diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index 8fd2b9e392a7..fcd1b800b2c1 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -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) > > > > > 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. > > -- > With Best Regards, > Andy Shevchenko > >
On Mon, Sep 09, 2024 at 07:30:43PM +0100, Simon Horman wrote: > On Mon, Sep 09, 2024 at 06:36:07PM +0300, Andy Shevchenko wrote: > > On Mon, Sep 09, 2024 at 04:17:12PM +0100, Simon Horman wrote: > > > On Mon, Sep 09, 2024 at 12:37:51PM +0300, Andy Shevchenko wrote: > > > > On Fri, Sep 06, 2024 at 05:29:38PM +0100, Simon Horman wrote: > > > > > 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: 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.
On Tue, Sep 10, 2024 at 11:12:21AM +0300, Andy Shevchenko wrote: > On Mon, Sep 09, 2024 at 07:30:43PM +0100, Simon Horman wrote: > > On Mon, Sep 09, 2024 at 06:36:07PM +0300, Andy Shevchenko wrote: > > > On Mon, Sep 09, 2024 at 04:17:12PM +0100, Simon Horman wrote: > > > > On Mon, Sep 09, 2024 at 12:37:51PM +0300, Andy Shevchenko wrote: > > > > > On Fri, Sep 06, 2024 at 05:29:38PM +0100, Simon Horman wrote: > > > > > > 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: > > 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.
On Tue, Sep 10, 2024 at 10:45:20AM +0100, Simon Horman wrote: > On Tue, Sep 10, 2024 at 11:12:21AM +0300, Andy Shevchenko wrote: > > On Mon, Sep 09, 2024 at 07:30:43PM +0100, Simon Horman wrote: > > > On Mon, Sep 09, 2024 at 06:36:07PM +0300, Andy Shevchenko wrote: > > > > On Mon, Sep 09, 2024 at 04:17:12PM +0100, Simon Horman wrote: > > > > > On Mon, Sep 09, 2024 at 12:37:51PM +0300, Andy Shevchenko wrote: > > > > > > On Fri, Sep 06, 2024 at 05:29:38PM +0100, Simon Horman wrote: > > > > > > > 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: > > > > 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/20240916-ct-ifdef-v1-0-81ef1798143b@kernel.org/
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index 4cbf71d0786b..7ab7dc7569e7 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -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)
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 <andriy.shevchenko@linux.intel.com> --- net/netfilter/nf_conntrack_netlink.c | 2 ++ 1 file changed, 2 insertions(+)