diff mbox

[net-next,12/14] igb: Add mechanism for detecting latched hardware Rx timestamp

Message ID 1358422519-20981-13-git-send-email-jeffrey.t.kirsher@intel.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Kirsher, Jeffrey T Jan. 17, 2013, 11:35 a.m. UTC 
From: Matthew Vick <matthew.vick@intel.com>

Add a check against possible Rx timestamp freezing in the hardware via
watchdog mechanism. This situation can occur when an Rx timestamp has been
latched, but the packet has been dropped because the Rx ring is full.

Whenever a packet comes in that should be timestamped, the Rx timestamp
gets latched into the hardware registers and we will store the jiffy value
in the rx_ring. The watchdog will keep track of his own jiffy timer
whenever there is no valid timestamp in the registers.

If the watchdog detects a valid timestamp in the registers, meaning that no
Rx packet has consumed it yet, it will check which time is most recent: the
last time in the watchdog or any time in the rx_rings. If the most recent
"event" was more than 5 seconds ago, it will flush the Rx timestamp and
print a warning message to the syslog.

Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Acked-by: Jacob Keller <Jacob.e.keller@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb.h         |  4 +++
 drivers/net/ethernet/intel/igb/igb_ethtool.c |  1 +
 drivers/net/ethernet/intel/igb/igb_main.c    |  1 +
 drivers/net/ethernet/intel/igb/igb_ptp.c     | 45 ++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+)

Comments

Richard Cochran Jan. 17, 2013, 3:38 p.m. UTC | #1 
On Thu, Jan 17, 2013 at 03:35:17AM -0800, Jeff Kirsher wrote:
> From: Matthew Vick <matthew.vick@intel.com>
> 
> Add a check against possible Rx timestamp freezing in the hardware via
> watchdog mechanism. This situation can occur when an Rx timestamp has been
> latched, but the packet has been dropped because the Rx ring is full.

Does this also need fixing in stable?

The igb patches look okay to me.

Acked-by: Richard Cochran <richardcochran@gmail.com>


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vick, Matthew Jan. 17, 2013, 4:51 p.m. UTC | #2 
On 1/17/13 7:38 AM, "Richard Cochran" <richardcochran@gmail.com> wrote:

>On Thu, Jan 17, 2013 at 03:35:17AM -0800, Jeff Kirsher wrote:
>> From: Matthew Vick <matthew.vick@intel.com>
>> 
>> Add a check against possible Rx timestamp freezing in the hardware via
>> watchdog mechanism. This situation can occur when an Rx timestamp has
>>been
>> latched, but the packet has been dropped because the Rx ring is full.
>
>Does this also need fixing in stable?
>
>The igb patches look okay to me.
>
>Acked-by: Richard Cochran <richardcochran@gmail.com>

Thanks for the review, Richard!

I'm not too particularly worried about getting this one into stable. This
is to prevent it from theoretically occurring rather than a bug fix for an
issue someone has reported. Plus, it only really affects 82576, as for
82580 and newer we put the timestamp in the packet buffer instead of the
RXSTMP registers. I think it's good to have this fix in going forward,
though.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 8c6f59d..ad317ab 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -221,6 +221,7 @@  struct igb_ring {
 		struct igb_tx_buffer *tx_buffer_info;
 		struct igb_rx_buffer *rx_buffer_info;
 	};
+	unsigned long last_rx_timestamp;
 	void *desc;			/* descriptor ring memory */
 	unsigned long flags;		/* ring specific flags */
 	void __iomem *tail;		/* pointer to ring tail register */
@@ -415,10 +416,12 @@  struct igb_adapter {
 	struct work_struct ptp_tx_work;
 	struct sk_buff *ptp_tx_skb;
 	unsigned long ptp_tx_start;
+	unsigned long last_rx_ptp_check;
 	spinlock_t tmreg_lock;
 	struct cyclecounter cc;
 	struct timecounter tc;
 	u32 tx_hwtstamp_timeouts;
+	u32 rx_hwtstamp_cleared;
 
 	char fw_version[32];
 #ifdef CONFIG_IGB_HWMON
@@ -486,6 +489,7 @@  extern void igb_ptp_init(struct igb_adapter *adapter);
 extern void igb_ptp_stop(struct igb_adapter *adapter);
 extern void igb_ptp_reset(struct igb_adapter *adapter);
 extern void igb_ptp_tx_work(struct work_struct *work);
+extern void igb_ptp_rx_hang(struct igb_adapter *adapter);
 extern void igb_ptp_tx_hwtstamp(struct igb_adapter *adapter);
 extern void igb_ptp_rx_rgtstamp(struct igb_q_vector *q_vector,
 				struct sk_buff *skb);
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 6f2579c..3ff3794 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -93,6 +93,7 @@  static const struct igb_stats igb_gstrings_stats[] = {
 	IGB_STAT("os2bmc_tx_by_host", stats.o2bspc),
 	IGB_STAT("os2bmc_rx_by_host", stats.b2ogprc),
 	IGB_STAT("tx_hwtstamp_timeouts", tx_hwtstamp_timeouts),
+	IGB_STAT("rx_hwtstamp_cleared", rx_hwtstamp_cleared),
 };
 
 #define IGB_NETDEV_STAT(_net_stat) { \
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 360b991..603fa4b 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3991,6 +3991,7 @@  static void igb_watchdog_task(struct work_struct *work)
 	}
 
 	igb_spoof_check(adapter);
+	igb_ptp_rx_hang(adapter);
 
 	/* Reset the timer */
 	if (!test_bit(__IGB_DOWN, &adapter->state))
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 086af46..2ec53d7 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -429,6 +429,51 @@  static void igb_ptp_overflow_check(struct work_struct *work)
 }
 
 /**
+ * igb_ptp_rx_hang - detect error case when Rx timestamp registers latched
+ * @adapter: private network adapter structure
+ *
+ * This watchdog task is scheduled to detect error case where hardware has
+ * dropped an Rx packet that was timestamped when the ring is full. The
+ * particular error is rare but leaves the device in a state unable to timestamp
+ * any future packets.
+ */
+void igb_ptp_rx_hang(struct igb_adapter *adapter)
+{
+	struct e1000_hw *hw = &adapter->hw;
+	struct igb_ring *rx_ring;
+	u32 tsyncrxctl = rd32(E1000_TSYNCRXCTL);
+	unsigned long rx_event;
+	int n;
+
+	if (hw->mac.type != e1000_82576)
+		return;
+
+	/* If we don't have a valid timestamp in the registers, just update the
+	 * timeout counter and exit
+	 */
+	if (!(tsyncrxctl & E1000_TSYNCRXCTL_VALID)) {
+		adapter->last_rx_ptp_check = jiffies;
+		return;
+	}
+
+	/* Determine the most recent watchdog or rx_timestamp event */
+	rx_event = adapter->last_rx_ptp_check;
+	for (n = 0; n < adapter->num_rx_queues; n++) {
+		rx_ring = adapter->rx_ring[n];
+		if (time_after(rx_ring->last_rx_timestamp, rx_event))
+			rx_event = rx_ring->last_rx_timestamp;
+	}
+
+	/* Only need to read the high RXSTMP register to clear the lock */
+	if (time_is_before_jiffies(rx_event + 5 * HZ)) {
+		rd32(E1000_RXSTMPH);
+		adapter->last_rx_ptp_check = jiffies;
+		adapter->rx_hwtstamp_cleared++;
+		dev_warn(&adapter->pdev->dev, "clearing Rx timestamp hang");
+	}
+}
+
+/**
  * igb_ptp_tx_hwtstamp - utility function which checks for TX time stamp
  * @adapter: Board private structure.
  *