* [net-2.6 PATCH 1/3] ixgbevf: Fix VF Stats accounting after reset
@ 2010-03-19 12:59 Jeff Kirsher
2010-03-19 13:00 ` [net-2.6 PATCH 2/3] ixgbevf: Shorten up delay timer for watchdog task Jeff Kirsher
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jeff Kirsher @ 2010-03-19 12:59 UTC (permalink / raw
To: davem; +Cc: netdev, gospo, Greg Rose, Jeff Kirsher
From: Greg Rose <gregory.v.rose@intel.com>
The counters in the 82599 Virtual Function are not clear on read. They
accumulate to the maximum value and then roll over. They are also not
cleared when the VF executes a soft reset, so it is possible they are
non-zero when the driver loads and starts. This has all been accounted
for in the code that keeps the stats up to date but there is one case
that is not. When the PF driver is reset the counters in the VF are
all reset to zero. This adds an additional accounting overhead into
the VF driver when the PF is reset under its feet. This patch adds
additional counters that are used by the VF driver to accumulate and
save stats after a PF reset has been detected. Prior to this patch
displaying the stats in the VF after the PF has reset would show
bogus data.
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbevf/ethtool.c | 42 +++++++++++++++-------
drivers/net/ixgbevf/ixgbevf_main.c | 68 +++++++++++++++++++++++-------------
drivers/net/ixgbevf/vf.h | 6 +++
3 files changed, 78 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ixgbevf/ethtool.c b/drivers/net/ixgbevf/ethtool.c
index 399be0c..6fdd651 100644
--- a/drivers/net/ixgbevf/ethtool.c
+++ b/drivers/net/ixgbevf/ethtool.c
@@ -46,22 +46,32 @@ struct ixgbe_stats {
int sizeof_stat;
int stat_offset;
int base_stat_offset;
+ int saved_reset_offset;
};
-#define IXGBEVF_STAT(m, b) sizeof(((struct ixgbevf_adapter *)0)->m), \
- offsetof(struct ixgbevf_adapter, m), \
- offsetof(struct ixgbevf_adapter, b)
+#define IXGBEVF_STAT(m, b, r) sizeof(((struct ixgbevf_adapter *)0)->m), \
+ offsetof(struct ixgbevf_adapter, m), \
+ offsetof(struct ixgbevf_adapter, b), \
+ offsetof(struct ixgbevf_adapter, r)
static struct ixgbe_stats ixgbe_gstrings_stats[] = {
- {"rx_packets", IXGBEVF_STAT(stats.vfgprc, stats.base_vfgprc)},
- {"tx_packets", IXGBEVF_STAT(stats.vfgptc, stats.base_vfgptc)},
- {"rx_bytes", IXGBEVF_STAT(stats.vfgorc, stats.base_vfgorc)},
- {"tx_bytes", IXGBEVF_STAT(stats.vfgotc, stats.base_vfgotc)},
- {"tx_busy", IXGBEVF_STAT(tx_busy, zero_base)},
- {"multicast", IXGBEVF_STAT(stats.vfmprc, stats.base_vfmprc)},
- {"rx_csum_offload_good", IXGBEVF_STAT(hw_csum_rx_good, zero_base)},
- {"rx_csum_offload_errors", IXGBEVF_STAT(hw_csum_rx_error, zero_base)},
- {"tx_csum_offload_ctxt", IXGBEVF_STAT(hw_csum_tx_good, zero_base)},
- {"rx_header_split", IXGBEVF_STAT(rx_hdr_split, zero_base)},
+ {"rx_packets", IXGBEVF_STAT(stats.vfgprc, stats.base_vfgprc,
+ stats.saved_reset_vfgprc)},
+ {"tx_packets", IXGBEVF_STAT(stats.vfgptc, stats.base_vfgptc,
+ stats.saved_reset_vfgptc)},
+ {"rx_bytes", IXGBEVF_STAT(stats.vfgorc, stats.base_vfgorc,
+ stats.saved_reset_vfgorc)},
+ {"tx_bytes", IXGBEVF_STAT(stats.vfgotc, stats.base_vfgotc,
+ stats.saved_reset_vfgotc)},
+ {"tx_busy", IXGBEVF_STAT(tx_busy, zero_base, zero_base)},
+ {"multicast", IXGBEVF_STAT(stats.vfmprc, stats.base_vfmprc,
+ stats.saved_reset_vfmprc)},
+ {"rx_csum_offload_good", IXGBEVF_STAT(hw_csum_rx_good, zero_base,
+ zero_base)},
+ {"rx_csum_offload_errors", IXGBEVF_STAT(hw_csum_rx_error, zero_base,
+ zero_base)},
+ {"tx_csum_offload_ctxt", IXGBEVF_STAT(hw_csum_tx_good, zero_base,
+ zero_base)},
+ {"rx_header_split", IXGBEVF_STAT(rx_hdr_split, zero_base, zero_base)},
};
#define IXGBE_QUEUE_STATS_LEN 0
@@ -455,10 +465,14 @@ static void ixgbevf_get_ethtool_stats(struct net_device *netdev,
ixgbe_gstrings_stats[i].stat_offset;
char *b = (char *)adapter +
ixgbe_gstrings_stats[i].base_stat_offset;
+ char *r = (char *)adapter +
+ ixgbe_gstrings_stats[i].saved_reset_offset;
data[i] = ((ixgbe_gstrings_stats[i].sizeof_stat ==
sizeof(u64)) ? *(u64 *)p : *(u32 *)p) -
((ixgbe_gstrings_stats[i].sizeof_stat ==
- sizeof(u64)) ? *(u64 *)b : *(u32 *)b);
+ sizeof(u64)) ? *(u64 *)b : *(u32 *)b) +
+ ((ixgbe_gstrings_stats[i].sizeof_stat ==
+ sizeof(u64)) ? *(u64 *)r : *(u32 *)r);
}
}
diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
index ca653c4..43927e1 100644
--- a/drivers/net/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ixgbevf/ixgbevf_main.c
@@ -1610,6 +1610,44 @@ static inline void ixgbevf_rx_desc_queue_enable(struct ixgbevf_adapter *adapter,
(adapter->rx_ring[rxr].count - 1));
}
+static void ixgbevf_save_reset_stats(struct ixgbevf_adapter *adapter)
+{
+ /* Only save pre-reset stats if there are some */
+ if (adapter->stats.vfgprc || adapter->stats.vfgptc) {
+ adapter->stats.saved_reset_vfgprc += adapter->stats.vfgprc -
+ adapter->stats.base_vfgprc;
+ adapter->stats.saved_reset_vfgptc += adapter->stats.vfgptc -
+ adapter->stats.base_vfgptc;
+ adapter->stats.saved_reset_vfgorc += adapter->stats.vfgorc -
+ adapter->stats.base_vfgorc;
+ adapter->stats.saved_reset_vfgotc += adapter->stats.vfgotc -
+ adapter->stats.base_vfgotc;
+ adapter->stats.saved_reset_vfmprc += adapter->stats.vfmprc -
+ adapter->stats.base_vfmprc;
+ }
+}
+
+static void ixgbevf_init_last_counter_stats(struct ixgbevf_adapter *adapter)
+{
+ struct ixgbe_hw *hw = &adapter->hw;
+
+ adapter->stats.last_vfgprc = IXGBE_READ_REG(hw, IXGBE_VFGPRC);
+ adapter->stats.last_vfgorc = IXGBE_READ_REG(hw, IXGBE_VFGORC_LSB);
+ adapter->stats.last_vfgorc |=
+ (((u64)(IXGBE_READ_REG(hw, IXGBE_VFGORC_MSB))) << 32);
+ adapter->stats.last_vfgptc = IXGBE_READ_REG(hw, IXGBE_VFGPTC);
+ adapter->stats.last_vfgotc = IXGBE_READ_REG(hw, IXGBE_VFGOTC_LSB);
+ adapter->stats.last_vfgotc |=
+ (((u64)(IXGBE_READ_REG(hw, IXGBE_VFGOTC_MSB))) << 32);
+ adapter->stats.last_vfmprc = IXGBE_READ_REG(hw, IXGBE_VFMPRC);
+
+ adapter->stats.base_vfgprc = adapter->stats.last_vfgprc;
+ adapter->stats.base_vfgorc = adapter->stats.last_vfgorc;
+ adapter->stats.base_vfgptc = adapter->stats.last_vfgptc;
+ adapter->stats.base_vfgotc = adapter->stats.last_vfgotc;
+ adapter->stats.base_vfmprc = adapter->stats.last_vfmprc;
+}
+
static int ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
{
struct net_device *netdev = adapter->netdev;
@@ -1656,6 +1694,9 @@ static int ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
/* enable transmits */
netif_tx_start_all_queues(netdev);
+ ixgbevf_save_reset_stats(adapter);
+ ixgbevf_init_last_counter_stats(adapter);
+
/* bring the link up in the watchdog, this could race with our first
* link up interrupt but shouldn't be a problem */
adapter->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
@@ -2228,27 +2269,6 @@ out:
return err;
}
-static void ixgbevf_init_last_counter_stats(struct ixgbevf_adapter *adapter)
-{
- struct ixgbe_hw *hw = &adapter->hw;
-
- adapter->stats.last_vfgprc = IXGBE_READ_REG(hw, IXGBE_VFGPRC);
- adapter->stats.last_vfgorc = IXGBE_READ_REG(hw, IXGBE_VFGORC_LSB);
- adapter->stats.last_vfgorc |=
- (((u64)(IXGBE_READ_REG(hw, IXGBE_VFGORC_MSB))) << 32);
- adapter->stats.last_vfgptc = IXGBE_READ_REG(hw, IXGBE_VFGPTC);
- adapter->stats.last_vfgotc = IXGBE_READ_REG(hw, IXGBE_VFGOTC_LSB);
- adapter->stats.last_vfgotc |=
- (((u64)(IXGBE_READ_REG(hw, IXGBE_VFGOTC_MSB))) << 32);
- adapter->stats.last_vfmprc = IXGBE_READ_REG(hw, IXGBE_VFMPRC);
-
- adapter->stats.base_vfgprc = adapter->stats.last_vfgprc;
- adapter->stats.base_vfgorc = adapter->stats.last_vfgorc;
- adapter->stats.base_vfgptc = adapter->stats.last_vfgptc;
- adapter->stats.base_vfgotc = adapter->stats.last_vfgotc;
- adapter->stats.base_vfmprc = adapter->stats.last_vfmprc;
-}
-
#define UPDATE_VF_COUNTER_32bit(reg, last_counter, counter) \
{ \
u32 current_counter = IXGBE_READ_REG(hw, reg); \
@@ -2416,9 +2436,9 @@ static void ixgbevf_watchdog_task(struct work_struct *work)
}
}
-pf_has_reset:
ixgbevf_update_stats(adapter);
+pf_has_reset:
/* Force detection of hung controller every watchdog period */
adapter->detect_tx_hung = true;
@@ -3390,8 +3410,6 @@ static int __devinit ixgbevf_probe(struct pci_dev *pdev,
/* setup the private structure */
err = ixgbevf_sw_init(adapter);
- ixgbevf_init_last_counter_stats(adapter);
-
#ifdef MAX_SKB_FRAGS
netdev->features = NETIF_F_SG |
NETIF_F_IP_CSUM |
@@ -3449,6 +3467,8 @@ static int __devinit ixgbevf_probe(struct pci_dev *pdev,
adapter->netdev_registered = true;
+ ixgbevf_init_last_counter_stats(adapter);
+
/* print the MAC address */
hw_dbg(hw, "%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x\n",
netdev->dev_addr[0],
diff --git a/drivers/net/ixgbevf/vf.h b/drivers/net/ixgbevf/vf.h
index 799600e..1f31b05 100644
--- a/drivers/net/ixgbevf/vf.h
+++ b/drivers/net/ixgbevf/vf.h
@@ -157,6 +157,12 @@ struct ixgbevf_hw_stats {
u64 vfgorc;
u64 vfgotc;
u64 vfmprc;
+
+ u64 saved_reset_vfgprc;
+ u64 saved_reset_vfgptc;
+ u64 saved_reset_vfgorc;
+ u64 saved_reset_vfgotc;
+ u64 saved_reset_vfmprc;
};
struct ixgbevf_info {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [net-2.6 PATCH 2/3] ixgbevf: Shorten up delay timer for watchdog task
2010-03-19 12:59 [net-2.6 PATCH 1/3] ixgbevf: Fix VF Stats accounting after reset Jeff Kirsher
@ 2010-03-19 13:00 ` Jeff Kirsher
2010-03-20 4:06 ` David Miller
2010-03-19 13:00 ` [net-2.6 PATCH 3/3] ixgbevf: Message formatting cleanups Jeff Kirsher
2010-03-20 4:06 ` [net-2.6 PATCH 1/3] ixgbevf: Fix VF Stats accounting after reset David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Jeff Kirsher @ 2010-03-19 13:00 UTC (permalink / raw
To: davem; +Cc: netdev, gospo, Greg Rose, Jeff Kirsher
From: Greg Rose <gregory.v.rose@intel.com>
The recovery from PF reset works better when you shorten up the delay
until the watchdog task executes.
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbevf/ixgbevf_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
index 43927e1..3de93ae 100644
--- a/drivers/net/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ixgbevf/ixgbevf_main.c
@@ -965,7 +965,7 @@ static irqreturn_t ixgbevf_msix_mbx(int irq, void *data)
if ((msg & IXGBE_MBVFICR_VFREQ_MASK) == IXGBE_PF_CONTROL_MSG)
mod_timer(&adapter->watchdog_timer,
- round_jiffies(jiffies + 10));
+ round_jiffies(jiffies + 1));
return IRQ_HANDLED;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [net-2.6 PATCH 3/3] ixgbevf: Message formatting cleanups
2010-03-19 12:59 [net-2.6 PATCH 1/3] ixgbevf: Fix VF Stats accounting after reset Jeff Kirsher
2010-03-19 13:00 ` [net-2.6 PATCH 2/3] ixgbevf: Shorten up delay timer for watchdog task Jeff Kirsher
@ 2010-03-19 13:00 ` Jeff Kirsher
2010-03-20 4:06 ` David Miller
2010-03-20 4:06 ` [net-2.6 PATCH 1/3] ixgbevf: Fix VF Stats accounting after reset David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Jeff Kirsher @ 2010-03-19 13:00 UTC (permalink / raw
To: davem; +Cc: netdev, gospo, Greg Rose, Jeff Kirsher
From: Greg Rose <gregory.v.rose@intel.com>
Clean up some text output formatting.
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbevf/ixgbevf_main.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
index 3de93ae..d6cbd94 100644
--- a/drivers/net/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ixgbevf/ixgbevf_main.c
@@ -2419,7 +2419,7 @@ static void ixgbevf_watchdog_task(struct work_struct *work)
if (!netif_carrier_ok(netdev)) {
hw_dbg(&adapter->hw, "NIC Link is Up %s, ",
((link_speed == IXGBE_LINK_SPEED_10GB_FULL) ?
- "10 Gbps" : "1 Gbps"));
+ "10 Gbps\n" : "1 Gbps\n"));
netif_carrier_on(netdev);
netif_tx_wake_all_queues(netdev);
} else {
@@ -2695,7 +2695,7 @@ static int ixgbevf_open(struct net_device *netdev)
if (hw->adapter_stopped) {
err = IXGBE_ERR_MBX;
printk(KERN_ERR "Unable to start - perhaps the PF"
- "Driver isn't up yet\n");
+ " Driver isn't up yet\n");
goto err_setup_reset;
}
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [net-2.6 PATCH 1/3] ixgbevf: Fix VF Stats accounting after reset
2010-03-19 12:59 [net-2.6 PATCH 1/3] ixgbevf: Fix VF Stats accounting after reset Jeff Kirsher
2010-03-19 13:00 ` [net-2.6 PATCH 2/3] ixgbevf: Shorten up delay timer for watchdog task Jeff Kirsher
2010-03-19 13:00 ` [net-2.6 PATCH 3/3] ixgbevf: Message formatting cleanups Jeff Kirsher
@ 2010-03-20 4:06 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-03-20 4:06 UTC (permalink / raw
To: jeffrey.t.kirsher; +Cc: netdev, gospo, gregory.v.rose
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 19 Mar 2010 05:59:52 -0700
> From: Greg Rose <gregory.v.rose@intel.com>
>
> The counters in the 82599 Virtual Function are not clear on read. They
> accumulate to the maximum value and then roll over. They are also not
> cleared when the VF executes a soft reset, so it is possible they are
> non-zero when the driver loads and starts. This has all been accounted
> for in the code that keeps the stats up to date but there is one case
> that is not. When the PF driver is reset the counters in the VF are
> all reset to zero. This adds an additional accounting overhead into
> the VF driver when the PF is reset under its feet. This patch adds
> additional counters that are used by the VF driver to accumulate and
> save stats after a PF reset has been detected. Prior to this patch
> displaying the stats in the VF after the PF has reset would show
> bogus data.
>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-2.6 PATCH 2/3] ixgbevf: Shorten up delay timer for watchdog task
2010-03-19 13:00 ` [net-2.6 PATCH 2/3] ixgbevf: Shorten up delay timer for watchdog task Jeff Kirsher
@ 2010-03-20 4:06 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-03-20 4:06 UTC (permalink / raw
To: jeffrey.t.kirsher; +Cc: netdev, gospo, gregory.v.rose
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 19 Mar 2010 06:00:12 -0700
> From: Greg Rose <gregory.v.rose@intel.com>
>
> The recovery from PF reset works better when you shorten up the delay
> until the watchdog task executes.
>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-2.6 PATCH 3/3] ixgbevf: Message formatting cleanups
2010-03-19 13:00 ` [net-2.6 PATCH 3/3] ixgbevf: Message formatting cleanups Jeff Kirsher
@ 2010-03-20 4:06 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-03-20 4:06 UTC (permalink / raw
To: jeffrey.t.kirsher; +Cc: netdev, gospo, gregory.v.rose
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 19 Mar 2010 06:00:31 -0700
> From: Greg Rose <gregory.v.rose@intel.com>
>
> Clean up some text output formatting.
>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-03-20 4:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-19 12:59 [net-2.6 PATCH 1/3] ixgbevf: Fix VF Stats accounting after reset Jeff Kirsher
2010-03-19 13:00 ` [net-2.6 PATCH 2/3] ixgbevf: Shorten up delay timer for watchdog task Jeff Kirsher
2010-03-20 4:06 ` David Miller
2010-03-19 13:00 ` [net-2.6 PATCH 3/3] ixgbevf: Message formatting cleanups Jeff Kirsher
2010-03-20 4:06 ` David Miller
2010-03-20 4:06 ` [net-2.6 PATCH 1/3] ixgbevf: Fix VF Stats accounting after reset David Miller
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 read-only IMAP folder(s) and NNTP newsgroup(s).