diff mbox series

net: netcp: MAX_SKB_FRAGS is now 'int'

Message ID	20230331074919.1299425-1-arnd@kernel.org
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Arnd Bergmann <arnd@kernel.org> To: Jakub Kicinski <kuba@kernel.org> Cc: Arnd Bergmann <arnd@arndb.de>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Paolo Abeni <pabeni@redhat.com>, Nikolay Aleksandrov <razor@blackwall.org>, Jason Xing <kerneljasonxing@gmail.com>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] net: netcp: MAX_SKB_FRAGS is now 'int' Date: Fri, 31 Mar 2023 09:48:56 +0200 Message-Id: <20230331074919.1299425-1-arnd@kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	net: netcp: MAX_SKB_FRAGS is now 'int' \| net: netcp: MAX_SKB_FRAGS is now 'int'

Commit Message

Arnd Bergmann March 31, 2023, 7:48 a.m. UTC

  From: Arnd Bergmann <arnd@arndb.de>

The type of MAX_SKB_FRAGS has changed recently, so the debug printk
needs to be updated:

drivers/net/ethernet/ti/netcp_core.c: In function 'netcp_create_interface':
drivers/net/ethernet/ti/netcp_core.c:2084:30: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Werror=format=]
 2084 |                 dev_err(dev, "tx-pool size too small, must be at least %ld\n",
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 3948b05950fd ("net: introduce a config option to tweak MAX_SKB_FRAGS")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/ti/netcp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

patchwork-bot+netdevbpf@kernel.org March 31, 2023, 8:40 a.m. UTC | #1

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 31 Mar 2023 09:48:56 +0200 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The type of MAX_SKB_FRAGS has changed recently, so the debug printk
> needs to be updated:
> 
> drivers/net/ethernet/ti/netcp_core.c: In function 'netcp_create_interface':
> drivers/net/ethernet/ti/netcp_core.c:2084:30: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Werror=format=]
>  2084 |                 dev_err(dev, "tx-pool size too small, must be at least %ld\n",
>       |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> [...]

Here is the summary with links:
  - net: netcp: MAX_SKB_FRAGS is now 'int'
    https://git.kernel.org/netdev/net/c/c5b959eeb7f9

You are awesome, thank you!

Alexander Lobakin March 31, 2023, 3:08 p.m. UTC | #2

From: Arnd Bergmann <arnd@kernel.org>
Date: Fri, 31 Mar 2023 09:48:56 +0200

> From: Arnd Bergmann <arnd@arndb.de>
> 
> The type of MAX_SKB_FRAGS has changed recently, so the debug printk
> needs to be updated:
> 
> drivers/net/ethernet/ti/netcp_core.c: In function 'netcp_create_interface':
> drivers/net/ethernet/ti/netcp_core.c:2084:30: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Werror=format=]
>  2084 |                 dev_err(dev, "tx-pool size too small, must be at least %ld\n",
>       |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fixes: 3948b05950fd ("net: introduce a config option to tweak MAX_SKB_FRAGS")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ethernet/ti/netcp_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
> index 1bb596a9d8a2..dfdbcdeb991f 100644
> --- a/drivers/net/ethernet/ti/netcp_core.c
> +++ b/drivers/net/ethernet/ti/netcp_core.c
> @@ -2081,7 +2081,7 @@ static int netcp_create_interface(struct netcp_device *netcp_device,
>  	netcp->tx_pool_region_id = temp[1];
>  
>  	if (netcp->tx_pool_size < MAX_SKB_FRAGS) {
> -		dev_err(dev, "tx-pool size too small, must be at least %ld\n",
> +		dev_err(dev, "tx-pool size too small, must be at least %d\n",
>  			MAX_SKB_FRAGS);
>  		ret = -ENODEV;
>  		goto quit;

(not related to the actual fix)

I'd personally define %MAX_SKB_FRAGS as `(u32)CONFIG_MAX_SKB_FRAGS`.
It can't be below zero or above %U32_MAX and we have to define it
manually anyway, so why not cast to the type expected from it :D

Thanks,
Olek

Eric Dumazet March 31, 2023, 4:58 p.m. UTC | #3

On Fri, Mar 31, 2023 at 5:08 PM Alexander Lobakin
<aleksander.lobakin@intel.com> wrote:
>
> From: Arnd Bergmann <arnd@kernel.org>
> Date: Fri, 31 Mar 2023 09:48:56 +0200
>
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > The type of MAX_SKB_FRAGS has changed recently, so the debug printk
> > needs to be updated:
> >
> > drivers/net/ethernet/ti/netcp_core.c: In function 'netcp_create_interface':
> > drivers/net/ethernet/ti/netcp_core.c:2084:30: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Werror=format=]
> >  2084 |                 dev_err(dev, "tx-pool size too small, must be at least %ld\n",
> >       |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Fixes: 3948b05950fd ("net: introduce a config option to tweak MAX_SKB_FRAGS")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/net/ethernet/ti/netcp_core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
> > index 1bb596a9d8a2..dfdbcdeb991f 100644
> > --- a/drivers/net/ethernet/ti/netcp_core.c
> > +++ b/drivers/net/ethernet/ti/netcp_core.c
> > @@ -2081,7 +2081,7 @@ static int netcp_create_interface(struct netcp_device *netcp_device,
> >       netcp->tx_pool_region_id = temp[1];
> >
> >       if (netcp->tx_pool_size < MAX_SKB_FRAGS) {
> > -             dev_err(dev, "tx-pool size too small, must be at least %ld\n",
> > +             dev_err(dev, "tx-pool size too small, must be at least %d\n",
> >                       MAX_SKB_FRAGS);
> >               ret = -ENODEV;
> >               goto quit;
>
> (not related to the actual fix)
>
> I'd personally define %MAX_SKB_FRAGS as `(u32)CONFIG_MAX_SKB_FRAGS`.
> It can't be below zero or above %U32_MAX and we have to define it
> manually anyway, so why not cast to the type expected from it :D
>

Some files have the assumption MAX_SKB_FRAGS can be understood by the
C preprocessor.

#if MAX_SKB_FRAGS > 32
...

Kconfig does not allow to define unsigned int.
That would be the easiest way really...

Nathan Chancellor March 31, 2023, 9:44 p.m. UTC | #4

On Fri, Mar 31, 2023 at 08:40:18AM +0000, patchwork-bot+netdevbpf@kernel.org wrote:
> Hello:
> 
> This patch was applied to netdev/net.git (main)
> by David S. Miller <davem@davemloft.net>:
> 
> On Fri, 31 Mar 2023 09:48:56 +0200 you wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > The type of MAX_SKB_FRAGS has changed recently, so the debug printk
> > needs to be updated:
> > 
> > drivers/net/ethernet/ti/netcp_core.c: In function 'netcp_create_interface':
> > drivers/net/ethernet/ti/netcp_core.c:2084:30: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Werror=format=]
> >  2084 |                 dev_err(dev, "tx-pool size too small, must be at least %ld\n",
> >       |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > [...]
> 
> Here is the summary with links:
>   - net: netcp: MAX_SKB_FRAGS is now 'int'
>     https://git.kernel.org/netdev/net/c/c5b959eeb7f9

net now warns:

  In file included from include/linux/device.h:15,
                   from include/linux/dma-mapping.h:7,
                   from include/linux/skbuff.h:28,
                   from include/linux/if_ether.h:19,
                   from include/linux/ethtool.h:18,
                   from include/linux/phy.h:16,
                   from include/linux/of_net.h:9,
                   from drivers/net/ethernet/ti/netcp_core.c:16:
  drivers/net/ethernet/ti/netcp_core.c: In function 'netcp_create_interface':
  drivers/net/ethernet/ti/netcp_core.c:2084:30: error: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Werror=format=]
   2084 |                 dev_err(dev, "tx-pool size too small, must be at least %d\n",
        |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
    110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
        |                              ^~~
  include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
    144 |         dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
        |                                                        ^~~~~~~
  drivers/net/ethernet/ti/netcp_core.c:2084:17: note: in expansion of macro 'dev_err'
   2084 |                 dev_err(dev, "tx-pool size too small, must be at least %d\n",
        |                 ^~~~~~~
  drivers/net/ethernet/ti/netcp_core.c:2084:73: note: format string is defined here
   2084 |                 dev_err(dev, "tx-pool size too small, must be at least %d\n",
        |                                                                        ~^
        |                                                                         |
        |                                                                         int
        |                                                                        %ld
  cc1: all warnings being treated as errors

The commit this patch is fixing is only in net-next and my patch to fix
this warning is already applied:

https://git.kernel.org/netdev/net-next/c/3292004c90c8

c5b959eeb7f9 should be reverted in net (I am running out of time today
otherwise I would just send a patch).

Cheers,
Nathan

Jakub Kicinski April 1, 2023, 4:20 a.m. UTC | #5

On Fri, 31 Mar 2023 14:44:44 -0700 Nathan Chancellor wrote:
> The commit this patch is fixing is only in net-next and my patch to fix
> this warning is already applied:
> 
> https://git.kernel.org/netdev/net-next/c/3292004c90c8
> 
> c5b959eeb7f9 should be reverted in net (I am running out of time today
> otherwise I would just send a patch).

Sorry for the mix-up, cleaned up now.

David Laight April 1, 2023, 6:20 p.m. UTC | #6

From: Eric Dumazet
> Sent: 31 March 2023 17:58
....
> > I'd personally define %MAX_SKB_FRAGS as `(u32)CONFIG_MAX_SKB_FRAGS`.
> > It can't be below zero or above %U32_MAX and we have to define it
> > manually anyway, so why not cast to the type expected from it :D
> >
> 
> Some files have the assumption MAX_SKB_FRAGS can be understood by the
> C preprocessor.
> 
> #if MAX_SKB_FRAGS > 32

...

You could use:
#define MAX_SKB_FRAGS (CONFIG_MAX_SKB_FRAGS + 0u)
to force the type to be 'unsigned int' while still leaving
a value that cpp groks.
(Or add 0ul to get the type back? to 'unsigned long'.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

diff mbox series

Patch

diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 1bb596a9d8a2..dfdbcdeb991f 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -2081,7 +2081,7 @@  static int netcp_create_interface(struct netcp_device *netcp_device,
 	netcp->tx_pool_region_id = temp[1];
 
 	if (netcp->tx_pool_size < MAX_SKB_FRAGS) {
-		dev_err(dev, "tx-pool size too small, must be at least %ld\n",
+		dev_err(dev, "tx-pool size too small, must be at least %d\n",
 			MAX_SKB_FRAGS);
 		ret = -ENODEV;
 		goto quit;