Sasha, please use reply-all to keep everyone in cc (including me...). On 2017/04/24 11:17, Neftin, Sasha wrote: > On 4/23/2017 15:53, Neftin, Sasha wrote: > > -----Original Message----- > > From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On > > Behalf Of Benjamin Poirier > > Sent: Saturday, April 22, 2017 00:20 > > To: Kirsher, Jeffrey T <jeffrey.t.kirs...@intel.com> > > Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; Stefan Priebe > > <s.pri...@profihost.ag> > > Subject: [Intel-wired-lan] [PATCH 1/2] e1000e: Don't return uninitialized > > stats > > > > Some statistics passed to ethtool are garbage because e1000e_get_stats64() > > doesn't write them, for example: tx_heartbeat_errors. This leaks kernel > > memory to userspace and confuses users. > > > > Do like ixgbe and use dev_get_stats() which first zeroes out > > rtnl_link_stats64. > > > > Reported-by: Stefan Priebe <s.pri...@profihost.ag> > > Signed-off-by: Benjamin Poirier <bpoir...@suse.com> > > --- > > drivers/net/ethernet/intel/e1000e/ethtool.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c > > b/drivers/net/ethernet/intel/e1000e/ethtool.c > > index 7aff68a4a4df..f117b90cdc2f 100644 > > --- a/drivers/net/ethernet/intel/e1000e/ethtool.c > > +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c > > @@ -2063,7 +2063,7 @@ static void e1000_get_ethtool_stats(struct net_device > > *netdev, > > pm_runtime_get_sync(netdev->dev.parent); > > - e1000e_get_stats64(netdev, &net_stats); > > + dev_get_stats(netdev, &net_stats); > > pm_runtime_put_sync(netdev->dev.parent); > > -- > > 2.12.2 > > > > _______________________________________________ > > Intel-wired-lan mailing list > > intel-wired-...@lists.osuosl.org > > http://lists.osuosl.org/mailman/listinfo/intel-wired-lan > > Hello, > > We would like to not accept this patch. Suggested generic method > '*dev_get_stats' (net/core/dev.c) calls 'ops->ndo_get_stats64' method which > eventually calls e1000e_get_stats64 (netdev.c) - so there is same > functionality. Also, see that 'e1000e_get_stats64' method in netdev.c (line
No, it's not the same functionality because dev_get_stats() does a memset on the rtnl_link_stats64 struct. > 5928) calls 'memset' with 0's before update statistics. Local sanity check I don't see any memset in e1000e_get_stats64(). What kernel version are you looking at? > in our lab shows 'tx_heartbeat_errors' counter reported as 0. > Please see the mail I just sent to Paul Menzel <pmen...@molgen.mpg.de> for more information about the issue and how to reproduce it.
