[net-2.6 PATCH v2 1/2] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters

[net-2.6 PATCH v2 1/2] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters

[Jeff Kirsher]

From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>

Fix the restart_queue and non_eop_desc counters from being
double-counted.  They are cumulative in each ring, so we don't want to
add them to the cumulative result in the adapter's master counter.
Otherwise, the stats will be inaccurate

Signed-off-by:  Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_main.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 35ea8c9..22519fa 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -4511,6 +4511,7 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
 	struct ixgbe_hw *hw = &adapter->hw;
 	u64 total_mpc = 0;
 	u32 i, missed_rx = 0, mpc, bprc, lxon, lxoff, xon_off_tot;
+	u64 non_eop_descs = 0, restart_queue = 0;
 
 	if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) {
 		u64 rsc_count = 0;
@@ -4528,10 +4529,12 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
 
 	/* gather some stats to the adapter struct that are per queue */
 	for (i = 0; i < adapter->num_tx_queues; i++)
-		adapter->restart_queue += adapter->tx_ring[i].restart_queue;
+		restart_queue += adapter->tx_ring[i]->restart_queue;
+	adapter->restart_queue = restart_queue;
 
 	for (i = 0; i < adapter->num_rx_queues; i++)
-		adapter->non_eop_descs += adapter->tx_ring[i].non_eop_descs;
+		non_eop_descs += adapter->rx_ring[i]->non_eop_descs;
+	adapter->non_eop_descs = non_eop_descs;
 
 	adapter->stats.crcerrs += IXGBE_READ_REG(hw, IXGBE_CRCERRS);
 	for (i = 0; i < 8; i++) {

[net-2.6 PATCH v2 2/2] ixgbe: Fix 82598 premature copper PHY link indicatation

[Jeff Kirsher]

From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>

Modified patch with Dave's comments to replace mdelay with proper msleep.
Fix 82598 copper link issue, where the phy prematurely indicates link
before it is ready to process packets. The new function looks for phy
link and indicates that, when it is available. If phy is not ready
within few seconds of MAC indicating link, the function will return
failure which translates to link down indication.

Signed-off-by:  Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_82598.c |   38 ++++++++++++++++++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_type.h  |    2 ++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_82598.c b/drivers/net/ixgbe/ixgbe_82598.c
index e2d5343..204177d 100644
--- a/drivers/net/ixgbe/ixgbe_82598.c
+++ b/drivers/net/ixgbe/ixgbe_82598.c
@@ -510,6 +510,40 @@ static s32 ixgbe_start_mac_link_82598(struct ixgbe_hw *hw,
 }
 
 /**
+ *  ixgbe_validate_link_ready - Function looks for phy link
+ *  @hw: pointer to hardware structure
+ *
+ *  Function indicates success when phy link is available. If phy is not ready
+ *  within 5 seconds of MAC indicating link, the function returns error.
+ **/
+static s32 ixgbe_validate_link_ready(struct ixgbe_hw *hw)
+{
+	u32 timeout;
+	u16 an_reg;
+
+	if (hw->device_id != IXGBE_DEV_ID_82598AT2)
+		return 0;
+
+	for (timeout = 0;
+	     timeout < IXGBE_VALIDATE_LINK_READY_TIMEOUT; timeout++) {
+		hw->phy.ops.read_reg(hw, MDIO_STAT1, MDIO_MMD_AN, &an_reg);
+
+		if ((an_reg & MDIO_AN_STAT1_COMPLETE) &&
+		    (an_reg & MDIO_STAT1_LSTATUS))
+			break;
+
+		msleep(100);
+	}
+
+	if (timeout == IXGBE_VALIDATE_LINK_READY_TIMEOUT) {
+		hw_dbg(hw, "Link was indicated but link is down\n");
+		return IXGBE_ERR_LINK_SETUP;
+	}
+
+	return 0;
+}
+
+/**
  *  ixgbe_check_mac_link_82598 - Get link/speed status
  *  @hw: pointer to hardware structure
  *  @speed: pointer to link speed
@@ -589,6 +623,10 @@ static s32 ixgbe_check_mac_link_82598(struct ixgbe_hw *hw,
 	else
 		*speed = IXGBE_LINK_SPEED_1GB_FULL;
 
+	if ((hw->device_id == IXGBE_DEV_ID_82598AT2) && (*link_up == true) &&
+	    (ixgbe_validate_link_ready(hw) != 0))
+		*link_up = false;
+
 	/* if link is down, zero out the current_mode */
 	if (*link_up == false) {
 		hw->fc.current_mode = ixgbe_fc_none;
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index f3e8d52..84650c6 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -841,6 +841,8 @@
 #define IXGBE_MPVC      0x04318
 #define IXGBE_SGMIIC    0x04314
 
+#define IXGBE_VALIDATE_LINK_READY_TIMEOUT 50
+
 /* Omer CORECTL */
 #define IXGBE_CORECTL           0x014F00
 /* BARCTRL */

Re: [net-2.6 PATCH v2 1/2] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 15 Dec 2009 13:56:59 -0800

> From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
> 
> Fix the restart_queue and non_eop_desc counters from being
> double-counted.  They are cumulative in each ring, so we don't want to
> add them to the cumulative result in the adapter's master counter.
> Otherwise, the stats will be inaccurate
> 
> Signed-off-by:  Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
> Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

Re: [net-2.6 PATCH v2 2/2] ixgbe: Fix 82598 premature copper PHY link indicatation

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 15 Dec 2009 13:57:20 -0800

> From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
> 
> Modified patch with Dave's comments to replace mdelay with proper msleep.
> Fix 82598 copper link issue, where the phy prematurely indicates link
> before it is ready to process packets. The new function looks for phy
> link and indicates that, when it is available. If phy is not ready
> within few seconds of MAC indicating link, the function will return
> failure which translates to link down indication.
> 
> Signed-off-by:  Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
> Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

Re: [net-2.6 PATCH v2 1/2] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 15 Dec 2009 21:13:54 -0800 (PST)

> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Tue, 15 Dec 2009 13:56:59 -0800
> 
>> From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
>> 
>> Fix the restart_queue and non_eop_desc counters from being
>> double-counted.  They are cumulative in each ring, so we don't want to
>> add them to the cumulative result in the adapter's master counter.
>> Otherwise, the stats will be inaccurate
>> 
>> Signed-off-by:  Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
>> Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> Applied.

So how did you build test this?

drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_update_stats’:
drivers/net/ixgbe/ixgbe_main.c:4532: error: invalid type argument of ‘->’ (have ‘struct ixgbe_ring’)
drivers/net/ixgbe/ixgbe_main.c:4536: error: invalid type argument of ‘->’ (have ‘struct ixgbe_ring’)

Re: [net-2.6 PATCH v2 1/2] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 15 Dec 2009 23:16:51 -0800 (PST)

> From: David Miller <davem@davemloft.net>
> Date: Tue, 15 Dec 2009 21:13:54 -0800 (PST)
> 
>> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> Date: Tue, 15 Dec 2009 13:56:59 -0800
>> 
>>> From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
>>> 
>>> Fix the restart_queue and non_eop_desc counters from being
>>> double-counted.  They are cumulative in each ring, so we don't want to
>>> add them to the cumulative result in the adapter's master counter.
>>> Otherwise, the stats will be inaccurate
>>> 
>>> Signed-off-by:  Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
>>> Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> 
>> Applied.
> 
> So how did you build test this?
> 
> drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_update_stats’:
> drivers/net/ixgbe/ixgbe_main.c:4532: error: invalid type argument of ‘->’ (have ‘struct ixgbe_ring’)
> drivers/net/ixgbe/ixgbe_main.c:4536: error: invalid type argument of ‘->’ (have ‘struct ixgbe_ring’)

I'm making the obvious fix "s/->/./", but really....

RE: [net-2.6 PATCH v2 1/2] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters

[Chilakala, Mallikarjuna]

>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>Sent: Tuesday, December 15, 2009 11:19 PM
>To: Kirsher, Jeffrey T
>Cc: netdev@vger.kernel.org; gospo@redhat.com; Chilakala, Mallikarjuna;
>Waskiewicz Jr, Peter P
>Subject: Re: [net-2.6 PATCH v2 1/2] ixgbe: Fix tx_restart_queue/non_eop_desc
>statistics counters
>
>From: David Miller <davem@davemloft.net>
>Date: Tue, 15 Dec 2009 23:16:51 -0800 (PST)
>
>> From: David Miller <davem@davemloft.net>
>> Date: Tue, 15 Dec 2009 21:13:54 -0800 (PST)
>>
>>> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> Date: Tue, 15 Dec 2009 13:56:59 -0800
>>>
>>>> From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
>>>>
>>>> Fix the restart_queue and non_eop_desc counters from being
>>>> double-counted.  They are cumulative in each ring, so we don't want to
>>>> add them to the cumulative result in the adapter's master counter.
>>>> Otherwise, the stats will be inaccurate
>>>>
>>>> Signed-off-by:  Mallikarjuna R Chilakala
><mallikarjuna.chilakala@intel.com>
>>>> Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
>>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>>
>>> Applied.
>>
>> So how did you build test this?
>>
>> drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_update_stats’:
>> drivers/net/ixgbe/ixgbe_main.c:4532: error: invalid type argument of ‘->’
>(have ‘struct ixgbe_ring’)
>> drivers/net/ixgbe/ixgbe_main.c:4536: error: invalid type argument of ‘->’
>(have ‘struct ixgbe_ring’)
>
>I'm making the obvious fix "s/->/./", but really....

Thanks Dave for fixing the issue, some changes in Jeff's GIT tree
has created this merge issue. Most likely the order of patches that were
send out might have created this problem.