lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-Id: <1286451360.6992.32.camel@firesoul.comx.local> Date: Thu, 07 Oct 2010 13:36:00 +0200 From: Jesper Dangaard Brouer <hawk@...x.dk> To: Eric Dumazet <eric.dumazet@...il.com> Cc: David Miller <davem@...emloft.net>, netdev@...r.kernel.org, jeffrey.t.kirsher@...el.com Subject: Re: [net-next PATCH] igb: update adapter stats when reading /proc/net/dev. On Thu, 2010-10-07 at 12:24 +0200, Eric Dumazet wrote: > Le jeudi 07 octobre 2010 à 12:06 +0200, Jesper Dangaard Brouer a écrit : > > On Wed, 2010-10-06 at 23:46 -0700, David Miller wrote: > > > From: Eric Dumazet <eric.dumazet@...il.com> > > > Date: Thu, 07 Oct 2010 08:44:08 +0200 > > > > > > > Le mercredi 06 octobre 2010 à 23:36 -0700, David Miller a écrit : > > > >> From: Jesper Dangaard Brouer <hawk@...x.dk> > > > >> Date: Tue, 05 Oct 2010 16:18:33 +0200 > > > >> > > > >> > Network driver igb: Improve the accuracy of stats in /proc/net/dev, by > > > >> > updating the adapter stats when reading /proc/net/dev. Currently the > > > >> > stats are updated by the watchdog timer every 2 sec, or when getting > > > >> > stats via ethtool -S. > > > >> > > > > >> > A number of userspace apps read these /proc/net/dev stats every second, > > > >> > e.g. ifstat, which then gives a perceived very bursty traffic pattern, > > > >> > which is actually false. > > > >> > > > > >> > Signed-off-by: Jesper Dangaard Brouer <hawk@...x.dk> > > > >> > > > >> I assume that the Intel folks will take care of integrating this > > > >> now that the locking is fixed. > > > > > > > > I integrated Jesper patch into my cumulative patch. > > > > > > Ok. > > > > I'm fine with this, as long as the commit message describe this change > > of accuracy of stats in /proc/net/dev. > > > > Something like: > > > > This patch also increase the accuracy of stats in /proc/net/dev, by > > updating the adapter stats when reading /proc/net/dev. Previously > > the stats were only updated by the watchdog timer every 2 sec, which > > resulted in false observations from userspace. > > > > > > Well, its not exactly true :) > > Previously, igb stats were updated : > - By watchdog timer, every 2 secs > - Every time an "ethtool -S ethX" was done > > There is no general guarantee netdev stats are immediately available to > user. I'm not talking general, just for this driver. And I'm not giving any guarantees, that is why I choose the wording "increase the accuracy" and not "provide accurate stats". > ndo_get_stats() is not allowed to sleep, (because of bonding...), so > drivers can not always provide accurate stats, if they need to make a > long transaction with hardware. > > Other drivers do the same (provide hardware statistics), with about one > second resolution. Yes, a lot of drivers don't provide accurate states. And one second resolution, as most drivers use, will usually be good enough for most admins. Our usage pattern is a bit different, as we are very interested in measuring bursty traffic. I'm explain you why during the Netfilter Workshop, if I get my talk about IPTV accepted ;-) I also use the increased accuracy, when running my pktgen testing scripts. > So the "resulted in false observations from userspace." is something > that might upset admins, but is not a hard requirement of netdev stats. Its definitly not a hard requirement, but lets fix it where we can. Then change the text: "resulted in false observations from userspace" to: "resulted in inaccurate observations from userspace" -- Med venlig hilsen / Best regards Jesper Brouer ComX Networks A/S Linux Network Kernel Developer Cand. Scient Datalog / MSc.CS Author of http://adsl-optimizer.dk LinkedIn: http://www.linkedin.com/in/brouer -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists