RE: Issue with driver i40e stat strings count mismatch

2018-07-31 Thread Keller, Jacob E


> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
> On Behalf Of Stefan Assmann
> Sent: Tuesday, July 31, 2018 12:06 AM
> To: Jesper Dangaard Brouer ; Kirsher, Jeffrey T
> 
> Cc: Topel, Bjorn ; Duyck, Alexander H
> ; intel-wired-lan  l...@lists.osuosl.org>; netdev@vger.kernel.org
> Subject: Re: Issue with driver i40e stat strings count mismatch
> 
> On 10.07.2018 13:17, Jesper Dangaard Brouer wrote:
> > Hi Intel-fokes,
> >
> > Your i40e driver have issues with it's ethtool stats.  A warning
> > triggers at drivers/net/ethernet/intel/i40e/i40e_ethtool.c line 1907
> > (see splash below) in func i40e_get_stat_strings().
> 
> Hi Jesper,
> 
> I ran into the same issue. Here's my proposed fix.
> 
> From 46c74c25496bab06712641c7b2b6b34e365397a2 Mon Sep 17 00:00:00 2001
> From: Stefan Assmann 
> Date: Mon, 30 Jul 2018 21:38:43 +0200
> Subject: [PATCH] i40e: fix i40e_get_stat_strings strings count warning
> 
> The current code calculates p - data, which results in a negative value.
> Therefore the WARN_ONCE condition will always be true.
> Fix this by calculating data - p instead.
> 
> Fixes: 9b10df596bd4 ("i40e: use WARN_ONCE to replace the commented
> BUG_ON size check")
> 
> Signed-off-by: Stefan Assmann 
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index 6947a2a571cb..5d670f4ce5ac 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -1903,7 +1903,7 @@ static void i40e_get_stat_strings(struct net_device
> *netdev, u8 *data)
>   data += ETH_GSTRING_LEN;
>   }
> 
> - WARN_ONCE(p - data != i40e_get_stats_count(netdev) *
> ETH_GSTRING_LEN,
> + WARN_ONCE(data - p != i40e_get_stats_count(netdev) *
> ETH_GSTRING_LEN,
> "stat strings count mismatch!");
>  }
> 


Thanks Stefan. Sorry about this one. I had a fix for this a while back 
internally but I think it somehow got lost in the shuffle. It's now in the 
process of being posted.

It's the same fix though, so I don't feel strongly about which gets applied. 
Thus...

Acked-by: Jacob Keller 

Thanks,
Jake

> --
> 2.17.1
> 
> >
> > [ 5077.779518] [ cut here ]
> > [ 5077.784493] stat strings count mismatch!
> > [ 5077.784529] WARNING: CPU: 0 PID: 2293 at
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:1907
> i40e_get_strings+0x477/0x4b0 [i40e]
> > [ 5077.800941] Modules linked in: act_gact cls_u32 sch_ingress xt_tcpudp
> iptable_raw ip_tables x_tables tun nfnetlink bridge stp llc bpfilter sunrpc
> coretemp kvm_intel kvm irqbypass intel_cstate intel_uncore intel_rapl_perf
> pcspkr i2c_i801 wmi ipmi_si ipmi_devintf ipmi_msghandler acpi_pad
> sch_fq_codel ixgbe mlx5_core mlxfw i40e devlink hid_generic igb mdio
> i2c_algo_bit ptp sd_mod i2c_core pps_core [last unloaded: x_tables]
> > [ 5077.839833] CPU: 0 PID: 2293 Comm: ethtool Not tainted 
> > 4.18.0-rc3-net-next-
> EdwardCree01+ #484
> > [ 5077.848962] Hardware name: Supermicro Super Server/X10SRi-F, BIOS 2.0a
> 08/01/2016
> > [ 5077.857049] RIP: 0010:i40e_get_strings+0x477/0x4b0 [i40e]
> > [ 5077.862776] Code: 98 49 39 c4 0f 84 2e fc ff ff 80 3d b3 da 03 00 00 0f 
> > 85 21 fc ff
> ff 48 c7 c7 24 42 19 a0 c6 05 9f da 03 00 01 e8 e9 9c ee e0 <0f> 0b e9 07 fc 
> ff ff 48 83
> c4 10 48 c7 c1 80 01 19 a0 be 20 00 00
> > [ 5077.882506] RSP: 0018:c90003af3c18 EFLAGS: 00010296
> > [ 5077.888063] RAX: 001c RBX: c90003ce1440 RCX:
> 0006
> > [ 5077.895528] RDX: 0007 RSI: 0096 RDI:
> 88087ca15530
> > [ 5077.902991] RBP: c90003ce1440 R08: 001c R09:
> 0411
> > [ 5077.910453] R10: 000fffe0 R11: 82a4e66d R12: 
> > cbc0
> > [ 5077.917913] R13: 88087c50f000 R14: 0008 R15:
> a0199620
> > [ 5077.925376] FS:  7f7a84bcb740() GS:88087ca0()
> knlGS:
> > [ 5077.934061] CS:  0010 DS:  ES:  CR0: 80050033
> > [ 5077.940133] CR2: 55c7d709b000 CR3: 00081acd0004 CR4:
> 003606f0
> > [ 5077.947609] DR0:  DR1:  DR2:
> 
> > [ 5077.955070] DR3:  DR6: fffe0ff0 DR7:
> 0400
> > [ 5077.962531] Call Trace:
> > [ 5077.965309]  dev_ethtool+0xf4e/0x2430
> > [ 5077.969305]  ? get_p

Re: Issue with driver i40e stat strings count mismatch

2018-07-31 Thread Jesper Dangaard Brouer


On Tue, 31 Jul 2018 09:05:40 +0200 Stefan Assmann  wrote:

> From: Stefan Assmann 
> To: Jesper Dangaard Brouer ,  Jeff Kirsher 
> 
> Cc: Björn Töpel ,  "alexander.h.du...@intel.com" 
> ,  intel-wired-lan 
> ,  "netdev@vger.kernel.org" 
> 
> Subject: Re: Issue with driver i40e stat strings count mismatch
> Date: Tue, 31 Jul 2018 09:05:40 +0200
> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
>  Thunderbird/52.9.1
> Message-ID: <64be8e6a-c285-2864-fd91-356eba645...@kpanic.de>
> 
> On 10.07.2018 13:17, Jesper Dangaard Brouer wrote:
> > Hi Intel-fokes,
> > 
> > Your i40e driver have issues with it's ethtool stats.  A warning
> > triggers at drivers/net/ethernet/intel/i40e/i40e_ethtool.c line 1907
> > (see splash below) in func i40e_get_stat_strings().  
> 
> Hi Jesper,
> 
> I ran into the same issue. Here's my proposed fix.

Thanks for following up Stefan :-)

I'm hoping some Intel people will look at evaluating this fix? ...


> From 46c74c25496bab06712641c7b2b6b34e365397a2 Mon Sep 17 00:00:00 2001
> From: Stefan Assmann 
> Date: Mon, 30 Jul 2018 21:38:43 +0200
> Subject: [PATCH] i40e: fix i40e_get_stat_strings strings count warning
> 
> The current code calculates p - data, which results in a negative value.
> Therefore the WARN_ONCE condition will always be true.
> Fix this by calculating data - p instead.
> 
> Fixes: 9b10df596bd4 ("i40e: use WARN_ONCE to replace the commented BUG_ON 
> size check")
> 
> Signed-off-by: Stefan Assmann 
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c 
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index 6947a2a571cb..5d670f4ce5ac 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -1903,7 +1903,7 @@ static void i40e_get_stat_strings(struct net_device 
> *netdev, u8 *data)
>   data += ETH_GSTRING_LEN;
>   }
> 
> - WARN_ONCE(p - data != i40e_get_stats_count(netdev) * ETH_GSTRING_LEN,
> + WARN_ONCE(data - p != i40e_get_stats_count(netdev) * ETH_GSTRING_LEN,
> "stat strings count mismatch!");
>  }
> 

- - 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

My error report:

[ 5077.779518] [ cut here ]
[ 5077.784493] stat strings count mismatch!
[ 5077.784529] WARNING: CPU: 0 PID: 2293 at 
drivers/net/ethernet/intel/i40e/i40e_ethtool.c:1907 
i40e_get_strings+0x477/0x4b0 [i40e]
[ 5077.800941] Modules linked in: act_gact cls_u32 sch_ingress xt_tcpudp 
iptable_raw ip_tables x_tables tun nfnetlink bridge stp llc bpfilter sunrpc 
coretemp kvm_intel kvm irqbypass intel_cstate intel_uncore intel_rapl_perf 
pcspkr i2c_i801 wmi ipmi_si ipmi_devintf ipmi_msghandler acpi_pad sch_fq_codel 
ixgbe mlx5_core mlxfw i40e devlink hid_generic igb mdio i2c_algo_bit ptp sd_mod 
i2c_core pps_core [last unloaded: x_tables]
[ 5077.839833] CPU: 0 PID: 2293 Comm: ethtool Not tainted 
4.18.0-rc3-net-next-EdwardCree01+ #484
[ 5077.848962] Hardware name: Supermicro Super Server/X10SRi-F, BIOS 2.0a 
08/01/2016
[ 5077.857049] RIP: 0010:i40e_get_strings+0x477/0x4b0 [i40e]
[ 5077.862776] Code: 98 49 39 c4 0f 84 2e fc ff ff 80 3d b3 da 03 00 00 0f 85 
21 fc ff ff 48 c7 c7 24 42 19 a0 c6 05 9f da 03 00 01 e8 e9 9c ee e0 <0f> 0b e9 
07 fc ff ff 48 83 c4 10 48 c7 c1 80 01 19 a0 be 20 00 00 
[ 5077.882506] RSP: 0018:c90003af3c18 EFLAGS: 00010296
[ 5077.888063] RAX: 001c RBX: c90003ce1440 RCX: 0006
[ 5077.895528] RDX: 0007 RSI: 0096 RDI: 88087ca15530
[ 5077.902991] RBP: c90003ce1440 R08: 001c R09: 0411
[ 5077.910453] R10: 000fffe0 R11: 82a4e66d R12: cbc0
[ 5077.917913] R13: 88087c50f000 R14: 0008 R15: a0199620
[ 5077.925376] FS:  7f7a84bcb740() GS:88087ca0() 
knlGS:
[ 5077.934061] CS:  0010 DS:  ES:  CR0: 80050033
[ 5077.940133] CR2: 55c7d709b000 CR3: 00081acd0004 CR4: 003606f0
[ 5077.947609] DR0:  DR1:  DR2: 
[ 5077.955070] DR3:  DR6: fffe0ff0 DR7: 0400
[ 5077.962531] Call Trace:
[ 5077.965309]  dev_ethtool+0xf4e/0x2430
[ 5077.969305]  ? get_page_from_freelist+0x2bb/0x1240
[ 5077.974428]  ? dev_ioctl+0x1e9/0x3c0
[ 5077.978332]  dev_ioctl+0x1e9/0x3c0
[ 5077.982062]  sock_do_ioctl+0xa8/0x140
[ 5077.986057]  ? sock_ioctl+0x1c0/0x300
[ 5077.990051]  sock_ioctl+0x1c0/0x300
[ 5077.993864]  ? __handle_mm_fault+0xa82/0xfd0
[ 5077.998462

Re: Issue with driver i40e stat strings count mismatch

2018-07-31 Thread Stefan Assmann
On 10.07.2018 13:17, Jesper Dangaard Brouer wrote:
> Hi Intel-fokes,
> 
> Your i40e driver have issues with it's ethtool stats.  A warning
> triggers at drivers/net/ethernet/intel/i40e/i40e_ethtool.c line 1907
> (see splash below) in func i40e_get_stat_strings().

Hi Jesper,

I ran into the same issue. Here's my proposed fix.

>From 46c74c25496bab06712641c7b2b6b34e365397a2 Mon Sep 17 00:00:00 2001
From: Stefan Assmann 
Date: Mon, 30 Jul 2018 21:38:43 +0200
Subject: [PATCH] i40e: fix i40e_get_stat_strings strings count warning

The current code calculates p - data, which results in a negative value.
Therefore the WARN_ONCE condition will always be true.
Fix this by calculating data - p instead.

Fixes: 9b10df596bd4 ("i40e: use WARN_ONCE to replace the commented BUG_ON size 
check")

Signed-off-by: Stefan Assmann 
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c 
b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 6947a2a571cb..5d670f4ce5ac 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1903,7 +1903,7 @@ static void i40e_get_stat_strings(struct net_device 
*netdev, u8 *data)
data += ETH_GSTRING_LEN;
}

-   WARN_ONCE(p - data != i40e_get_stats_count(netdev) * ETH_GSTRING_LEN,
+   WARN_ONCE(data - p != i40e_get_stats_count(netdev) * ETH_GSTRING_LEN,
  "stat strings count mismatch!");
 }

-- 
2.17.1

> 
> [ 5077.779518] [ cut here ]
> [ 5077.784493] stat strings count mismatch!
> [ 5077.784529] WARNING: CPU: 0 PID: 2293 at 
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:1907 
> i40e_get_strings+0x477/0x4b0 [i40e]
> [ 5077.800941] Modules linked in: act_gact cls_u32 sch_ingress xt_tcpudp 
> iptable_raw ip_tables x_tables tun nfnetlink bridge stp llc bpfilter sunrpc 
> coretemp kvm_intel kvm irqbypass intel_cstate intel_uncore intel_rapl_perf 
> pcspkr i2c_i801 wmi ipmi_si ipmi_devintf ipmi_msghandler acpi_pad 
> sch_fq_codel ixgbe mlx5_core mlxfw i40e devlink hid_generic igb mdio 
> i2c_algo_bit ptp sd_mod i2c_core pps_core [last unloaded: x_tables]
> [ 5077.839833] CPU: 0 PID: 2293 Comm: ethtool Not tainted 
> 4.18.0-rc3-net-next-EdwardCree01+ #484
> [ 5077.848962] Hardware name: Supermicro Super Server/X10SRi-F, BIOS 2.0a 
> 08/01/2016
> [ 5077.857049] RIP: 0010:i40e_get_strings+0x477/0x4b0 [i40e]
> [ 5077.862776] Code: 98 49 39 c4 0f 84 2e fc ff ff 80 3d b3 da 03 00 00 0f 85 
> 21 fc ff ff 48 c7 c7 24 42 19 a0 c6 05 9f da 03 00 01 e8 e9 9c ee e0 <0f> 0b 
> e9 07 fc ff ff 48 83 c4 10 48 c7 c1 80 01 19 a0 be 20 00 00 
> [ 5077.882506] RSP: 0018:c90003af3c18 EFLAGS: 00010296
> [ 5077.888063] RAX: 001c RBX: c90003ce1440 RCX: 
> 0006
> [ 5077.895528] RDX: 0007 RSI: 0096 RDI: 
> 88087ca15530
> [ 5077.902991] RBP: c90003ce1440 R08: 001c R09: 
> 0411
> [ 5077.910453] R10: 000fffe0 R11: 82a4e66d R12: 
> cbc0
> [ 5077.917913] R13: 88087c50f000 R14: 0008 R15: 
> a0199620
> [ 5077.925376] FS:  7f7a84bcb740() GS:88087ca0() 
> knlGS:
> [ 5077.934061] CS:  0010 DS:  ES:  CR0: 80050033
> [ 5077.940133] CR2: 55c7d709b000 CR3: 00081acd0004 CR4: 
> 003606f0
> [ 5077.947609] DR0:  DR1:  DR2: 
> 
> [ 5077.955070] DR3:  DR6: fffe0ff0 DR7: 
> 0400
> [ 5077.962531] Call Trace:
> [ 5077.965309]  dev_ethtool+0xf4e/0x2430
> [ 5077.969305]  ? get_page_from_freelist+0x2bb/0x1240
> [ 5077.974428]  ? dev_ioctl+0x1e9/0x3c0
> [ 5077.978332]  dev_ioctl+0x1e9/0x3c0
> [ 5077.982062]  sock_do_ioctl+0xa8/0x140
> [ 5077.986057]  ? sock_ioctl+0x1c0/0x300
> [ 5077.990051]  sock_ioctl+0x1c0/0x300
> [ 5077.993864]  ? __handle_mm_fault+0xa82/0xfd0
> [ 5077.998462]  ? do_vfs_ioctl+0x8d/0x5e0
> [ 5078.002550]  do_vfs_ioctl+0x8d/0x5e0
> [ 5078.006456]  ? handle_mm_fault+0xd0/0x210
> [ 5078.010790]  ksys_ioctl+0x70/0x80
> [ 5078.014429]  __x64_sys_ioctl+0x16/0x20
> [ 5078.018505]  do_syscall_64+0x42/0xf0
> [ 5078.022411]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [ 5078.027789] RIP: 0033:0x7f7a84394dc7
> [ 5078.031703] Code: b3 66 90 48 8b 05 d9 00 2d 00 64 c7 00 26 00 00 00 48 c7 
> c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 
> 01 f0 ff ff 73 01 c3 48 8b 0d a9 00 2d 00 f7 d8 64 89 01 48 
> [ 5078.051437] RSP: 002b:7ffd64d68338 EFLAGS: 0246 ORIG_RAX: 
> 0010
> [ 5078.059596] RAX: ffda RBX: 55c7d7099260 RCX: 
> 7f7a84394dc7
> [ 5078.067062] RDX: 7ffd64d684e0 RSI: 8946 RDI: 
> 0003
> [ 5078.074524] RBP: 7ffd64d684d0 R08: ffb0 R09: 
> 55c7d7099260
> [ 5078.081986] R10:  R11: