[PATCH] pktgen: avoid unused-const-variable warning

[PATCH] pktgen: avoid unused-const-variable warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

When extra warnings are enable, there are configurations that build
pktgen without CONFIG_XFRM, which leaves a static const variable unused:

net/core/pktgen.c:213:1: error: unused variable 'F_IPSEC' [-Werror,-Wunused-const-variable]
  213 | PKT_FLAGS
      | ^~~~~~~~~
net/core/pktgen.c:197:2: note: expanded from macro 'PKT_FLAGS'
  197 |         pf(IPSEC)               /* ipsec on for flows */                \
      |         ^~~~~~~~~

This could be marked as __maybe_unused, or by making the one use visible
to the compiler by slightly rearranging the #ifdef blocks. The second
variant looks slightly nicer here, so use that.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/core/pktgen.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 55064713223e..402e01a2ce19 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -158,9 +158,7 @@
 #include <net/udp.h>
 #include <net/ip6_checksum.h>
 #include <net/addrconf.h>
-#ifdef CONFIG_XFRM
 #include <net/xfrm.h>
-#endif
 #include <net/netns/generic.h>
 #include <asm/byteorder.h>
 #include <linux/rcupdate.h>
@@ -2363,13 +2361,13 @@ static inline int f_pick(struct pktgen_dev *pkt_dev)
 }
 
 
-#ifdef CONFIG_XFRM
 /* If there was already an IPSEC SA, we keep it as is, else
  * we go look for it ...
 */
 #define DUMMY_MARK 0
 static void get_ipsec_sa(struct pktgen_dev *pkt_dev, int flow)
 {
+#ifdef CONFIG_XFRM
 	struct xfrm_state *x = pkt_dev->flows[flow].x;
 	struct pktgen_net *pn = net_generic(dev_net(pkt_dev->odev), pg_net_id);
 	if (!x) {
@@ -2395,11 +2393,10 @@ static void get_ipsec_sa(struct pktgen_dev *pkt_dev, int flow)
 		}
 
 	}
-}
 #endif
+}
 static void set_cur_queue_map(struct pktgen_dev *pkt_dev)
 {
-
 	if (pkt_dev->flags & F_QUEUE_MAP_CPU)
 		pkt_dev->cur_queue_map = smp_processor_id();
 
@@ -2574,10 +2571,8 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev)
 				pkt_dev->flows[flow].flags |= F_INIT;
 				pkt_dev->flows[flow].cur_daddr =
 				    pkt_dev->cur_daddr;
-#ifdef CONFIG_XFRM
 				if (pkt_dev->flags & F_IPSEC)
 					get_ipsec_sa(pkt_dev, flow);
-#endif
 				pkt_dev->nflows++;
 			}
 		}
-- 
2.39.5

Re: [PATCH] pktgen: avoid unused-const-variable warning

[Peter Seiderer]

Hello Arnd,

On Tue, 25 Feb 2025 09:57:14 +0100, Arnd Bergmann <arnd@kernel.org> wrote:

> From: Arnd Bergmann <arnd@arndb.de>
>
> When extra warnings are enable, there are configurations that build
> pktgen without CONFIG_XFRM, which leaves a static const variable unused:
>
> net/core/pktgen.c:213:1: error: unused variable 'F_IPSEC' [-Werror,-Wunused-const-variable]
>   213 | PKT_FLAGS
>       | ^~~~~~~~~
> net/core/pktgen.c:197:2: note: expanded from macro 'PKT_FLAGS'
>   197 |         pf(IPSEC)               /* ipsec on for flows */                \
>       |         ^~~~~~~~~
>
> This could be marked as __maybe_unused, or by making the one use visible
> to the compiler by slightly rearranging the #ifdef blocks. The second
> variant looks slightly nicer here, so use that.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  net/core/pktgen.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index 55064713223e..402e01a2ce19 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -158,9 +158,7 @@
>  #include <net/udp.h>
>  #include <net/ip6_checksum.h>
>  #include <net/addrconf.h>
> -#ifdef CONFIG_XFRM
>  #include <net/xfrm.h>
> -#endif

This ifdef/endif can be kept (as the xfrm stuff is still not used)...

>  #include <net/netns/generic.h>
>  #include <asm/byteorder.h>
>  #include <linux/rcupdate.h>
> @@ -2363,13 +2361,13 @@ static inline int f_pick(struct pktgen_dev *pkt_dev)
>  }
>
>
> -#ifdef CONFIG_XFRM
>  /* If there was already an IPSEC SA, we keep it as is, else
>   * we go look for it ...
>  */
>  #define DUMMY_MARK 0

A now unused define...

>  static void get_ipsec_sa(struct pktgen_dev *pkt_dev, int flow)
>  {
> +#ifdef CONFIG_XFRM
>  	struct xfrm_state *x = pkt_dev->flows[flow].x;
>  	struct pktgen_net *pn = net_generic(dev_net(pkt_dev->odev), pg_net_id);

Maybe better this way here?

	const u32 dummy_mark = 0;

>  	if (!x) {
> @@ -2395,11 +2393,10 @@ static void get_ipsec_sa(struct pktgen_dev *pkt_dev, int flow)
>  		}
>
>  	}
> -}
>  #endif
> +}
>  static void set_cur_queue_map(struct pktgen_dev *pkt_dev)
>  {
> -
>  	if (pkt_dev->flags & F_QUEUE_MAP_CPU)
>  		pkt_dev->cur_queue_map = smp_processor_id();
>
> @@ -2574,10 +2571,8 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev)
>  				pkt_dev->flows[flow].flags |= F_INIT;
>  				pkt_dev->flows[flow].cur_daddr =
>  				    pkt_dev->cur_daddr;
> -#ifdef CONFIG_XFRM
>  				if (pkt_dev->flags & F_IPSEC)
>  					get_ipsec_sa(pkt_dev, flow);
> -#endif
>  				pkt_dev->nflows++;
>  			}
>  		}

Otherwise works as expected, you can add my (with or without the suggested
changes)

Reviewed-by: Peter Seiderer <ps.report@gmx.net>

Regards,
Peter

Re: [PATCH] pktgen: avoid unused-const-variable warning

[Paolo Abeni]

On 2/26/25 7:17 PM, Peter Seiderer wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>> When extra warnings are enable, there are configurations that build
>> pktgen without CONFIG_XFRM, which leaves a static const variable unused:
>>
>> net/core/pktgen.c:213:1: error: unused variable 'F_IPSEC' [-Werror,-Wunused-const-variable]
>>   213 | PKT_FLAGS
>>       | ^~~~~~~~~
>> net/core/pktgen.c:197:2: note: expanded from macro 'PKT_FLAGS'
>>   197 |         pf(IPSEC)               /* ipsec on for flows */                \
>>       |         ^~~~~~~~~
>>
>> This could be marked as __maybe_unused, or by making the one use visible
>> to the compiler by slightly rearranging the #ifdef blocks. The second
>> variant looks slightly nicer here, so use that.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>  net/core/pktgen.c | 9 ++-------
>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
>> index 55064713223e..402e01a2ce19 100644
>> --- a/net/core/pktgen.c
>> +++ b/net/core/pktgen.c
>> @@ -158,9 +158,7 @@
>>  #include <net/udp.h>
>>  #include <net/ip6_checksum.h>
>>  #include <net/addrconf.h>
>> -#ifdef CONFIG_XFRM
>>  #include <net/xfrm.h>
>> -#endif
> 
> This ifdef/endif can be kept (as the xfrm stuff is still not used)...

FTR, I think dropping unneeded #ifdef is preferable in c files: only
such file build time is affected, and the code is more readable.

> 
>>  #include <net/netns/generic.h>
>>  #include <asm/byteorder.h>
>>  #include <linux/rcupdate.h>
>> @@ -2363,13 +2361,13 @@ static inline int f_pick(struct pktgen_dev *pkt_dev)
>>  }
>>
>>
>> -#ifdef CONFIG_XFRM
>>  /* If there was already an IPSEC SA, we keep it as is, else
>>   * we go look for it ...
>>  */
>>  #define DUMMY_MARK 0
> 
> A now unused define...
> 
>>  static void get_ipsec_sa(struct pktgen_dev *pkt_dev, int flow)
>>  {
>> +#ifdef CONFIG_XFRM
>>  	struct xfrm_state *x = pkt_dev->flows[flow].x;
>>  	struct pktgen_net *pn = net_generic(dev_net(pkt_dev->odev), pg_net_id);
> 
> Maybe better this way here?

I think the unused define is preferable; I think pre-processor defines
are cheaper than static const.

Thanks,

Paolo

Re: [PATCH] pktgen: avoid unused-const-variable warning

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 25 Feb 2025 09:57:14 +0100 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> When extra warnings are enable, there are configurations that build
> pktgen without CONFIG_XFRM, which leaves a static const variable unused:
> 
> net/core/pktgen.c:213:1: error: unused variable 'F_IPSEC' [-Werror,-Wunused-const-variable]
>   213 | PKT_FLAGS
>       | ^~~~~~~~~
> net/core/pktgen.c:197:2: note: expanded from macro 'PKT_FLAGS'
>   197 |         pf(IPSEC)               /* ipsec on for flows */                \
>       |         ^~~~~~~~~
> 
> [...]

Here is the summary with links:
  - pktgen: avoid unused-const-variable warning
    https://git.kernel.org/netdev/net-next/c/af4a5da8ed54

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] pktgen: avoid unused-const-variable warning

[Peter Seiderer]

Hello Paolo,

On Thu, 27 Feb 2025 12:35:45 +0100, Paolo Abeni <pabeni@redhat.com> wrote:

> On 2/26/25 7:17 PM, Peter Seiderer wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >> When extra warnings are enable, there are configurations that build
> >> pktgen without CONFIG_XFRM, which leaves a static const variable unused:
> >>
> >> net/core/pktgen.c:213:1: error: unused variable 'F_IPSEC' [-Werror,-Wunused-const-variable]
> >>   213 | PKT_FLAGS
> >>       | ^~~~~~~~~
> >> net/core/pktgen.c:197:2: note: expanded from macro 'PKT_FLAGS'
> >>   197 |         pf(IPSEC)               /* ipsec on for flows */                \
> >>       |         ^~~~~~~~~
> >>
> >> This could be marked as __maybe_unused, or by making the one use visible
> >> to the compiler by slightly rearranging the #ifdef blocks. The second
> >> variant looks slightly nicer here, so use that.
> >>
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >>  net/core/pktgen.c | 9 ++-------
> >>  1 file changed, 2 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> >> index 55064713223e..402e01a2ce19 100644
> >> --- a/net/core/pktgen.c
> >> +++ b/net/core/pktgen.c
> >> @@ -158,9 +158,7 @@
> >>  #include <net/udp.h>
> >>  #include <net/ip6_checksum.h>
> >>  #include <net/addrconf.h>
> >> -#ifdef CONFIG_XFRM
> >>  #include <net/xfrm.h>
> >> -#endif
> >
> > This ifdef/endif can be kept (as the xfrm stuff is still not used)...
>
> FTR, I think dropping unneeded #ifdef is preferable in c files: only
> such file build time is affected, and the code is more readable.

The ifdef/endif emphasizes no xfrm usage (even by mistake) in case CONFIG_XFRM
is not defined, but in the end a matter of taste ;-)

>
> >
> >>  #include <net/netns/generic.h>
> >>  #include <asm/byteorder.h>
> >>  #include <linux/rcupdate.h>
> >> @@ -2363,13 +2361,13 @@ static inline int f_pick(struct pktgen_dev *pkt_dev)
> >>  }
> >>
> >>
> >> -#ifdef CONFIG_XFRM
> >>  /* If there was already an IPSEC SA, we keep it as is, else
> >>   * we go look for it ...
> >>  */
> >>  #define DUMMY_MARK 0
> >
> > A now unused define...
> >
> >>  static void get_ipsec_sa(struct pktgen_dev *pkt_dev, int flow)
> >>  {
> >> +#ifdef CONFIG_XFRM
> >>  	struct xfrm_state *x = pkt_dev->flows[flow].x;
> >>  	struct pktgen_net *pn = net_generic(dev_net(pkt_dev->odev), pg_net_id);
> >
> > Maybe better this way here?
> >
> > 	const u32 dummy_mark = 0;
>
> I think the unused define is preferable; I think pre-processor defines
> are cheaper than static const.

In which regards cheaper (out of interest)?

Both (with and without static) produce the same code see e.g.

	https://godbolt.org/z/Tsr1jM45r
	https://godbolt.org/z/6sr1o8da3

Regards,
Peter

>
> Thanks,
>
> Paolo
>

Re: [PATCH] pktgen: avoid unused-const-variable warning

[Paolo Abeni]

On 2/27/25 3:21 PM, Peter Seiderer wrote:
> On Thu, 27 Feb 2025 12:35:45 +0100, Paolo Abeni <pabeni@redhat.com> wrote:
>> I think the unused define is preferable; I think pre-processor defines
>> are cheaper than static const.
> 
> In which regards cheaper (out of interest)?
> 
> Both (with and without static) produce the same code see e.g.
> 
> 	https://godbolt.org/z/Tsr1jM45r
> 	https://godbolt.org/z/6sr1o8da3

I must admit I was unsure the compiler would always optimize out the
constant.

I guess the macro could still produce shorter build time (by a few clock
cycles, nothing measurable ;), so in the end it boils down again to a
personal preference.

Cheers,

Paolo