netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: [PATCH 08/23] e1000: add multicast stats counters
@ 2006-09-20 23:50 cramerj
  2006-09-21  0:47 ` Jeff Garzik
  0 siblings, 1 reply; 7+ messages in thread
From: cramerj @ 2006-09-20 23:50 UTC (permalink / raw)
  To: Jeff Garzik, Williams, Mitch A
  Cc: netdev, Kok, Auke-jan H, Brandeburg, Jesse, Ronciak, John

> Williams, Mitch A wrote:
> >>> +	{ "rx_broadcast", E1000_STAT(stats.bprc) },
> >>> +	{ "tx_broadcast", E1000_STAT(stats.bptc) },
> >>> +	{ "rx_multicast", E1000_STAT(stats.mprc) },
> >>> +	{ "tx_multicast", E1000_STAT(stats.mptc) },
> >>>  	{ "rx_errors", E1000_STAT(net_stats.rx_errors) },
> >>>  	{ "tx_errors", E1000_STAT(net_stats.tx_errors) },
> >>>  	{ "tx_dropped", E1000_STAT(net_stats.tx_dropped) },
> >> NAK -- you also need to remove the standard net stats, which are
> >> exported elsewhere
> >
> > Jeff, can you please explain the reason for this NAK a little more?
> > Neither Auke nor I understand why you rejected the patch.
> >
> > This patch just adds the display of a few more stats in Ethtool.  It
> > doesn't affect any other counters, and is really just a convenience
> > feature.  I added this to the driver because of a customer request.
> 
> Adding those stats is fine.  You guys just need to remove the existing
> mess first.
> 
> 	Jeff
> 

Since we have 1-to-1 mapping of some of our statistics registers to the
net_stats, we could s/net_stats/stats/.  However, there are a few
net_stats (e.g. net_stats.rx_errors) that encapsulate more than one
e1000 statistic register of which we don't have a private stat member
defined.

For those statistics, is it really necessary to add another stat
structure just to rm "net_stats" from that list we pass to ethtool?  At
best, it would look something like this...

  { "foo_count", E1000_STAT(stats.foo) },
- { "rx_errors", E1000_STAT(net_stats.rx_errors) },
+ { "rx_errors", E1000_STAT(eth_stats.rx_errors) },
  { "bar_count", E1000_STAT(stats.bar) },

If so, well, OK.  I'm just scratching my head as to why it's a "mess"
as-is.

I've missed obvious alternatives before; care to enlighten?

-Jeb

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 08/23] e1000: add multicast stats counters
@ 2006-09-20 16:38 Williams, Mitch A
  2006-09-20 19:22 ` Jeff Garzik
  0 siblings, 1 reply; 7+ messages in thread
From: Williams, Mitch A @ 2006-09-20 16:38 UTC (permalink / raw)
  To: netdev, Jeff Garzik; +Cc: Kok, Auke-jan H, Brandeburg, Jesse, Ronciak, John

>> +	{ "rx_broadcast", E1000_STAT(stats.bprc) },
>> +	{ "tx_broadcast", E1000_STAT(stats.bptc) },
>> +	{ "rx_multicast", E1000_STAT(stats.mprc) },
>> +	{ "tx_multicast", E1000_STAT(stats.mptc) },
>>  	{ "rx_errors", E1000_STAT(net_stats.rx_errors) },
>>  	{ "tx_errors", E1000_STAT(net_stats.tx_errors) },
>>  	{ "tx_dropped", E1000_STAT(net_stats.tx_dropped) },
>
>NAK -- you also need to remove the standard net stats, which are 
>exported elsewhere

Jeff, can you please explain the reason for this NAK a little more?
Neither Auke nor I understand why you rejected the patch.

This patch just adds the display of a few more stats in Ethtool.  It
doesn't affect any other counters, and is really just a convenience
feature.  I added this to the driver because of a customer request.

Thank you in advance for edifying us.

-Mitch


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH 00/23] e100, e1000, ixgb updates
@ 2006-09-19 17:26 Kok, Auke
  2006-09-19 17:28 ` [PATCH 08/23] e1000: add multicast stats counters Kok, Auke
  0 siblings, 1 reply; 7+ messages in thread
From: Kok, Auke @ 2006-09-19 17:26 UTC (permalink / raw)
  To: Garzik, Jeff
  Cc: netdev, Brandeburg, Jesse, Kok, Auke, Kok, Auke, Ronciak, John

Hi,

The following updates are available through git:

git pull git://lost.foo-projects.org/~ahkok/git/netdev-2.6 upstream

And apply against c233289c29369dba7177ca873e9b8ed457af2a78 (Jeff Garzik:
"drivers/net/phy/fixed: #if 0 some incomplete code")

Cheers,

Auke


---

Auke Kok <auke-jan.h.kok@intel.com>:
     e100, e1000, ixgb: update copyright header and remove LICENSE
     e100, e1000, ixgb: Fix an impossible memory overwrite bug
     e100: rework WoL and shutdown handling
     e1000: driver state fixes (race fix)
     e1000: revert 'e1000: Remove 0x1000 as supported device'
     ixgb: convert to netdev_priv(netdev)
     e100, e1000, ixgb: increment version numbers

Bruce Allan <bruce.w.allan@intel.com>:
     e1000: Jumbo frames fixes for 82573
     e1000: handle manageability for pci-e adapters at PHY powerdown
     e1000: don't strip vlan ID if 8021q claims it

Jeff Kirsher <jeffrey.t.kirsher@intel.com>:
     e1000: rename flow control symbols
     e1000: add enums for several link properties
     e1000: allow ethtool to pass arbitrary speed advertisment
     e1000: Fix MANC detection for PCIE adapters
     e1000: gather hardware bit tweaks.
     e1000: add PCI-E capability detection code
     e1000: reduce RAR entries available for ICH8
     e1000: rework polarity, NVM, eeprom code and fixes.

Jesse Brandeburg <jesse.brandeburg@intel.com>:
     e100: Add debugging code for cb cleaning and csum failures.
     e1000: Maybe stop TX if not enough free descriptors
     ixgb: combine more rx descriptors to improve performance

Mitch Williams <mitch.a.williams@intel.com>:
     e1000: add multicast stats counters

Nicholas Nunley <nicholas.d.nunley@intel.com>:
     e1000: remove unused code and make symbols static

---

 drivers/net/e100.c                |  110 +
 drivers/net/e1000/LICENSE         |  339 ++++++
 drivers/net/e1000/Makefile        |   35 
 drivers/net/e1000/e1000.h         |   65 -
 drivers/net/e1000/e1000_ethtool.c |  340 ++----
 drivers/net/e1000/e1000_hw.c      | 2100 ++++++++++++++++++++------------------
 drivers/net/e1000/e1000_hw.h      |  116 --
 drivers/net/e1000/e1000_main.c    |  407 ++-----
 drivers/net/e1000/e1000_osdep.h   |   35 
 drivers/net/e1000/e1000_param.c   |  208 +--
 drivers/net/ixgb/Makefile         |   38 
 drivers/net/ixgb/ixgb.h           |   43 
 drivers/net/ixgb/ixgb_ee.c        |   36 
 drivers/net/ixgb/ixgb_ee.h        |   36 
 drivers/net/ixgb/ixgb_ethtool.c   |   42 
 drivers/net/ixgb/ixgb_hw.c        |   47 
 drivers/net/ixgb/ixgb_hw.h        |   36 
 drivers/net/ixgb/ixgb_ids.h       |   37 
 drivers/net/ixgb/ixgb_main.c      |  190 ---
 drivers/net/ixgb/ixgb_osdep.h     |   36 
 drivers/net/ixgb/ixgb_param.c     |   36 
 21 files changed, 2249 insertions(+), 2083 deletions(-)


---
Auke Kok <auke-jan.h.kok@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2006-09-27 20:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-20 23:50 [PATCH 08/23] e1000: add multicast stats counters cramerj
2006-09-21  0:47 ` Jeff Garzik
2006-09-27 20:09   ` Auke Kok
  -- strict thread matches above, loose matches on Subject: below --
2006-09-20 16:38 Williams, Mitch A
2006-09-20 19:22 ` Jeff Garzik
2006-09-19 17:26 [PATCH 00/23] e100, e1000, ixgb updates Kok, Auke
2006-09-19 17:28 ` [PATCH 08/23] e1000: add multicast stats counters Kok, Auke
2006-09-19 19:28   ` Jeff Garzik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).